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

Humanize error handling #1321

Open
nortonandreev opened this issue Jan 29, 2024 · 4 comments
Open

Humanize error handling #1321

nortonandreev opened this issue Jan 29, 2024 · 4 comments
Labels
module:web-wallet Issues related to web-wallet module need:feedback Call for participation: feedback is requested to fix this issue need:specs Call for participation: specs are requested to fix this issue type:enhancement Issues concerning code or feature improvement (performance, refactoring, etc)

Comments

@nortonandreev
Copy link
Contributor

We need to think of a better way to handle errors. I have several concerns around this.

Screenshot 2024-01-29 at 19 49 28

First, as it can be seen on the screenshot (which is a simulated error happening on the Transact flow), the appearance is not great. I don't like that the message is centered, also not a fan of the default style of the expander button of the HTML details component (and I think the details shouldn't be hidden, if the copy is actually user-friendly).

Second, currently, the errors we get from the js library are not meant to be displayed to the end user. They can help the developers with debugging, but it won't be useful information for others. I think the errors should let the user know something has gone wrong, in a language they can understand, and provide them with some suggestions on how to attempt to mitigate the issue.

I don't think it will be possible to have customized errors that we display in all cases when an error can occur, but we can have a generic error message that is more user-friendly and perhaps suggest further steps, such as messaging support if the issue persists, etc.

We could also make use of tools such as Sentry which should allow us to log the actual error, while displaying a customized one to the user, and proactively act when we see users start getting errors in the app.

We also need to make sure we are consistent with the way the errors are displayed.

Screenshot 2024-01-29 at 20 05 31

For example, this is an errored condition in the Setting page and this an another example of an inline error on the login page:

Screenshot 2024-01-29 at 20 07 11

It is obvious that the inline ones will have a different appearance compared to the ones which should take a bigger portion of the screen, but I think we should try to be more consistent.

Generally, the unhappy paths are something we haven't put too much thought of while working on the MVP, however, I think if we get some rules and designs around the different types of errors now, it will be easier to incorporate them going forward, when needed.

cc: @ZER0 @kieranhall @laremas

@nortonandreev nortonandreev added need:feedback Call for participation: feedback is requested to fix this issue need:specs Call for participation: specs are requested to fix this issue type:enhancement Issues concerning code or feature improvement (performance, refactoring, etc) labels Jan 29, 2024
@ZER0 ZER0 transferred this issue from another repository Feb 2, 2024
@ZER0 ZER0 added the module:web-wallet Issues related to web-wallet module label Feb 2, 2024
@kieranhall
Copy link
Member

I think we should definitely introduce Sentry to log all errors and we should output a message with steps to resolve the issue. For example, offering a reset button, so that the sync can start again from scratch.

Beyond that, we need to perhaps look at building in an a recovery process when sync errors occur, so that we can at least try that before giving the nuclear option.

Also take a look at what's written here https://dexie.org/docs/The-Main-Limitations-of-IndexedDB#ambivalent-error-handling

@nortonandreev
Copy link
Contributor Author

nortonandreev commented Mar 13, 2024

I think we should definitely introduce Sentry to log all errors and we should output a message with steps to resolve the issue. For example, offering a reset button, so that the sync can start again from scratch.

Beyond that, we need to perhaps look at building in an a recovery process when sync errors occur, so that we can at least try that before giving the nuclear option.

Also take a look at what's written here https://dexie.org/docs/The-Main-Limitations-of-IndexedDB#ambivalent-error-handling

Yeah, the Sentry integration was captured under this issue (check the comments).

I think, based on that, I will rename the issue and reuse it to add Sentry.

Will check the article tomorrow, thanks for sharing! 🔥

@kieranhall
Copy link
Member

As previously discussed, we will need to have users opt in to sharing crash reporting with us. We should also do a deeper dive into Sentry to ensure there is no attack surface area introduced by it.

@nortonandreev
Copy link
Contributor Author

nortonandreev commented Nov 4, 2024

As mentioned by Andrea here, as we now have banners, we could provide users with CTA there to recover from an error state.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
module:web-wallet Issues related to web-wallet module need:feedback Call for participation: feedback is requested to fix this issue need:specs Call for participation: specs are requested to fix this issue type:enhancement Issues concerning code or feature improvement (performance, refactoring, etc)
Projects
None yet
Development

No branches or pull requests

3 participants