Serialize Error.cause when degrading storage - #824
Conversation
Avoid interpolating a raw cause object into the fallback-to-cache alert so logs stay readable.
Avoid a nested ternary in degradePerformance and assert the Cause field is a readable string, not [object Object].
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 698a3d963a
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| if (error.cause instanceof Error) { | ||
| causeMessage = error.cause.message; | ||
| } else if (typeof error.cause === 'string') { | ||
| causeMessage = error.cause; |
There was a problem hiding this comment.
Preserve non-Error cause details
When Error.cause is a valid non-string value that is not an Error instance—such as {message: 'quota denied'} or an error from another JavaScript realm—this leaves causeMessage empty, so the resulting alert contains only Cause: and loses the diagnostic information this change is intended to make readable. Safely serialize these remaining cause values or extract their message rather than discarding them.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Same class of nit as the #826 cross-realm comment.
degradePerformance only runs on our own new Error('IDBKeyVal store could not be created') from this window. We never attach a plain-object cause, and we do not run this path in an iframe that would produce a foreign-realm Error.
The point of this PR is to stop interpolating a raw cause object as [object Object]. Serializing arbitrary leftover values would reintroduce that. Error and string cover every cause we actually set.
Not changing this.
Details
When storage degrades to cache-only, the fallback alert interpolated
error.causedirectly. A non-string cause becomes[object Object]in logs.Serialize
Error.causeto a message string (Error message, or the string itself) before logging.Split out of #802.
Related Issues
For #802
Linked E/App PR
Expensify/App#99225
Automated Tests
No new tests. Logging-only change in
degradePerformance.Manual Tests
[object Object].Author Checklist
### Related Issuessection above### Linked E/App PRsection above, and verified this change against it (E/App CI passed and manual testing completed)TestssectiontoggleReportand notonIconClick)myBool && <MyComponent />.STYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)/** comment above it */thisproperly so there are no scoping issues (i.e. foronClick={this.submit}the methodthis.submitshould be bound tothisin the constructor)thisare necessary to be bound (i.e. avoidthis.submit = this.submit.bind(this);ifthis.submitis never passed to a component event handler likeonClick)Avataris modified, I verified thatAvataris working as expected in all cases)mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.Screenshots/Videos
Android: Native
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari