search

dusk-network / rusk

The reference Dusk platform implementation and tools
Mozilla Public License 2.0
158 stars 59 forks source link

Humanize error handling #1321

Open nortonandreev opened 7 months ago

nortonandreev commented 7 months ago

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

kieranhall commented 6 months ago

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 commented 6 months ago

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 commented 6 months ago

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.