Handle `FILE_ERROR_NO_SPACE` error gracefully

adonesky1 commented 2 years ago

We currently see many instances of the error FILE_ERROR_NO_SPACE in Sentry. These errors occur when a user runs out of disk space and state persistence of app related state fails.

Currently these errors are not surfaced to the user meaning that we will not persist state which may result in unexpected or buggy behavior in the extension and fail to warn the user as to why they may be experiencing buggy behavior.

[ ] We should intercept these errors and show the user a legible error message that will either help them resolve the issue by freeing up space or at least clear up confusion about why the extension is failing to behave as expected.
[ ] We should also move the logging of these errors from sentry to segment.

For Design:

The lifecycle of this error notice needs a spec

Should we allow this to be dismissed? Or should we show this until the user does clear space?
If we allow it to be dismissed, do we leave it dismissed forever? What if the user clears space, then it fills up again?

adonesky1 commented 2 years ago

A WIP commit from @Gudahtt with related work: https://github.com/MetaMask/metamask-extension/commit/f00fe875fd1d8c5974b06206b1fd241e80e4154b

adonesky1 commented 2 years ago

Or should we show this until the user does clear space?

Is there someway we could know this? Other than just seeing when the error stops happening? If not, it seems to me we should just make it dismissible and then pop it up again each time we get this error. @Gudahtt

cc @rachelcope

Gudahtt commented 2 years ago

We try to persist state every time it changes, with a 1 second debounce. Effectively the dismiss button would appear to have not worked if we took that approach.

adonesky1 commented 2 years ago

@Gudahtt so then could we dismiss it when a successful state persistence occurs?

adonesky1 commented 2 years ago

Alright @rachelcope so a slightly updated understanding of our options here: Our best options as I understand them are:

Make this alert dismissible and not show it again until we see that the issue has been fixed and then subsequently reoccurs.
Not make it dismissible by the user but dismiss it ourselves when we detect that the issue has been resolved

With 1 the risk is that users dismiss it thoughtlessly and then don't understand why the app isn't working as expected. With 2 the risk is that users are frustrated with an un-dismissible alert.

cc @Gudahtt

adonesky1 commented 2 years ago

ping here @rachelcope

adonesky1 commented 2 years ago

Friendly ping here @rachelcope ☺️

MetaMask / metamask-extension

Handle `FILE_ERROR_NO_SPACE` error gracefully #14525