Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor(backend): throw exception instead of returning null #1133

Merged
merged 7 commits into from
Aug 20, 2021

Conversation

davidyuk
Copy link
Member

@davidyuk davidyuk commented Jul 7, 2021

Known exceptions:

[Vue warn]: Error in mounted hook (Promise/async): "Error: Request failed with 400"

found in

---> <WordDetail> at src/views/WordDetail.vue
       <App> at src/App.vue
         <Root>

when opening details of word with a space in the name (aeternity/tipping-community-backend#368)


GET https://testnet.superhero.aeternity.art/profile/image/ak_2UGL6qArnbWx9fEkJyszssMkCjpu3VeLFBPA6MXYV3APfDSz1k 404

Chrome always report 404 even if they handled, we can't avoid it


I'm proposing a way how errors should be handled:

  • don't throw an error if it is somehow indicated in ui (red color or a separate modal window)
  • if an error indicated somehow, it may be printed to console (to be able to debug it in production)

Later all not caught exceptions will show modal asking the user to report it. So we shouldn't have unhandled exceptions, at the same time we shouldn't handle them in a too general fashion (like I'm removing in this PR .catch(() => null)).

How to test this PR:

  • try all features
  • ensure that all of them works correctly
  • report about all exceptions in console (red-colored) that weren't indicated in UI and not mentioned in the list above

fixes #707

@davidyuk davidyuk force-pushed the feature/exception-throwing branch from 75675e9 to 1a442e6 Compare July 7, 2021 07:47
@github-actions
Copy link

github-actions bot commented Jul 7, 2021

@davidyuk davidyuk force-pushed the feature/exception-throwing branch from 1a442e6 to 911e6d5 Compare July 7, 2021 07:57
this.sendingTip = false;
if (e.code && e.code === 4) {
} catch (error) {
if (error.code && error.code === 4) {
return;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why return without anything?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The author wanted to omit this kind of exceptions. It was introduced in 0b5506d I can't get what is supposed to fix, proposing to remove

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok :D good question

@@ -48,8 +48,7 @@ export default {
},
},
async mounted() {
const profile = await Backend.getProfile(this.address);
this.name = profile ? profile.preferredChainName : null;
this.name = (await Backend.getProfile(this.address)).preferredChainName;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we can get the names from the names list in the store, the stored name for an account is also the preferredChainName, as the backend sorts them that way when they are globally fetched. This reduces the number of requests.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have created #1134 to address this

@@ -108,7 +88,7 @@ export default class Backend {
return backendFetch(`tips${query}`);
};

static addToken = async (address) => backendFetchNoTimeout('tokenCache/addToken', {
static addToken = async (address) => backendFetch('tokenCache/addToken', {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this request can take long, when it times out the user gets to see a non working version of word-bazaar, as his token is not indexed and cached properly yet

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

actually, timeout in backendFetch was removed in 22f0ee4

@davidyuk davidyuk force-pushed the feature/exception-throwing branch from 911e6d5 to d77ddfd Compare July 7, 2021 08:43
@davidyuk davidyuk marked this pull request as ready for review July 7, 2021 09:48
@davidyuk davidyuk force-pushed the feature/exception-throwing branch from 4dfa640 to 74a4785 Compare July 7, 2021 10:41
Copy link
Collaborator

@Liubov-crypto Liubov-crypto left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Trending - pass
Search - pass
Change profile pic- pass, but have to refresh a page to see results
Change back pic - pass
Change profile info - pass
Cookies - pass
Send Tip on the Feed - pass (with some issue)
Retip on the feed - pass (with some issue)
Post without tipping - pass
Add a comment - pass
Tip comment on the feed - pass (with some issue)
Pin/Unpin - pass
Claim - pass
Report post - pass
Voting - pass
Vote - pass
Revoke Vote - pass
Delegate voting power - pass
Create a new poll - pass
Search - pass
Meet - pass
Send a tip to user icon/comment - failed, the same on the product.
Updates - pass
Faq- pass

Change profile pic- pass, but have to refresh a page to see results:
FF1
FF2

Got this error when tried to send a tip on the feed (tip was sent):
tip on the feed error
tip on the F
Tip on the feed 2
tip on the feed 3
after tip

Got this error when tried to retip content on the feed (retip was sent):
error when retip
error on FF retip content
errr retip 2
retip 3

Got this error when tried to tip comment on the feed (tip was sent):
tip comment on the feed
tip comments on the feed 1
tip comment 2

Voting:
voting

I accidentally put a space before a wallet address and got this:
мще 2

Meet:
Meet

The error sent tip in the Meet (the same on the product):
error send tip in meet

Remain:
Go back button affect the iframe wallet
Different names in Meet and wallet

@davidyuk
Copy link
Member Author

but have to refresh a page to see results

This one is fixed

@Liubov-crypto
Copy link
Collaborator

but have to refresh a page to see results

This one is fixed

Cool! Please let me know if there is anything else to check.

@davidyuk
Copy link
Member Author

@Liubov-crypto I've added a workaround for v1 tips/retips, please recheck errors from the last review.

@Liubov-crypto
Copy link
Collaborator

@Liubov-crypto I've added a workaround for v1 tips/retips, please recheck errors from the last review.

Retested, LGTM.

Trending - pass
Search - pass

Change profile pic- pass

Change back pic - pass
Change profile info - pass
Cookies - pass

Send Tip on the Feed - pass

Retip on the feed - pass

Post without tipping - pass
Add a comment - pass
Tip comment on the feed - pass
Pin/Unpin - pass
Claim - pass

Report post - pass

Voting - pass
Vote - pass

Revoke Vote - pass
Delegate voting power - pass

Search - pass

Meet - pass

Send a tip to user icon/comment - pass 

Updates - pass

Faq- pass

@stale stale bot added the stale label Jul 28, 2021
@stale stale bot closed this Jul 31, 2021
@aeternity aeternity deleted a comment from stale bot Aug 20, 2021
@CedrikNikita CedrikNikita reopened this Aug 20, 2021
@stale stale bot removed the stale label Aug 20, 2021
@CedrikNikita CedrikNikita force-pushed the feature/exception-throwing branch from adc5574 to b95e11e Compare August 20, 2021 03:35
@github-actions
Copy link

SSR will be deployed to superhero.aeternity.io, bundle report

@CedrikNikita CedrikNikita merged commit 95e91a5 into develop Aug 20, 2021
@CedrikNikita CedrikNikita deleted the feature/exception-throwing branch August 20, 2021 08:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Don't omit exceptions thrown from backend fetch
5 participants