Prevent attempts to persist native errors#10011
Conversation
app/scripts/lib/local-store.js
Outdated
There was a problem hiding this comment.
I could probably replace this with serialize-error or something similar, but there were a few bugs on the serialize-error GitHub repo that might affect us. This seemed effective enough for now 🤷
If a native Error is present in the state object we're persisting, it will fail to persist on Firefox. This is because the structured clone implementation on Firefox doesn't support native errors [1]. As a result, the state will fail to persist from the point the error is introduced, leading to a loss in state whenever the extension is next reset. This has been fixed by deep cloning the state before persisting, and removing any native error objects present. If a native error is found, it is converted into a plain object. An error is sent to Sentry the first time a native error is encountered in each extension session. [1]: https://bugzilla.mozilla.org/show_bug.cgi?id=1556604
ad6541f to
5868cf9
Compare
Builds ready [5868cf9]
Page Load Metrics (340 ± 34 ms)
|
|
I reproduced this error on
After that, you should see an error in the background dev console, and it should re-appear whenever state changes (e.g. when you add a token, change networks, add a contact, etc.). After reloading the extension, any state changes you made after this error will be erased. Note that until #10012 is merged, the resulting error message may be obfuscated though. Look at the stack trace to determine whether it's the correct one. The same steps can be followed with any MetaMask extension version between v8.0.0 and v8.1.5 without the need to modify the source code (the error won't be obfuscated either!). With this PR, these steps should no longer break state persistence. |
|
So I'm not sure if this is something we should do. This new deep clone introduces a runtime cost that may be substantial for users with a large state. We could instead let this fail, and rely upon Sentry reports for failure 🤔 |
|
I'm going to close this for now. We can log the error in Sentry instead (#10018). We can revisit this idea if we see this error in Sentry and want additional context. |
If a native Error is present in the state object we're persisting, it will fail to persist on Firefox. This is because the structured clone
implementation on Firefox doesn't support native errors. As a result, the state will fail to persist from the point the error is introduced, leading to a loss in state whenever the extension is next reset.
This has been fixed by deep cloning the state before persisting, and removing any native error objects present. If a native error is found, it is converted into a plain object. An error is sent to Sentry the first time a native error is encountered in each extension session.