[DO NOT MERGE] fix: IDB backing store corruption healing (onyx PR #780 test)#91085
[DO NOT MERGE] fix: IDB backing store corruption healing (onyx PR #780 test)#91085leshniak wants to merge 9 commits into
Conversation
Pin onyx to Expensify/react-native-onyx#780 (commit beb75d5f) which adds IDB backing store corruption healing. Remove stale 3.0.71 patch (already reverted upstream in 3.0.72 via PR Expensify#785). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
|
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…heal-test # Conflicts: # package-lock.json # package.json # patches/react-native-onyx/details.md # patches/react-native-onyx/react-native-onyx+3.0.73.patch
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Remove initWithStoredValues usage (removed in onyx 3.0.74 via PR Expensify#782) and unused getKeywordQueryWithCurrentSearchContext import. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
|
@ikevin127 Please copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button] |
|
@Julesssss Please tag me if my C+ review is needed in case there wasn't a C+ already assigned to this effort 🙌 |
Please go ahead @ikevin127, for this review there is an associated Onyx PR for review too, combined into one. |
Reviewer Checklist
Screenshots/VideosMacOS: Chrome chrome.movMacOS: Safari safari.mov |
| */ | ||
| function useClearSelectedDomainMembersOnMoveComplete(clearSelectedMembers: () => void) { | ||
| const [selectedMemberAccountIDs] = useOnyx(ONYXKEYS.DOMAIN_MEMBERS_SELECTED_FOR_MOVE, {initWithStoredValues: false}); | ||
| const [selectedMemberAccountIDs] = useOnyx(ONYXKEYS.DOMAIN_MEMBERS_SELECTED_FOR_MOVE); |
There was a problem hiding this comment.
🟡 CRITICAL ISSUE - Unexplained behavioral change
The PR description does not explain why initWithStoredValues: false was removed. This option controls the initial render behavior:
false: Hook starts inloadingstate, value isundefineduntil Onyx connectstrue(default): Hook immediately returns the stored value
Bug this could cause: If this hook is used during app initialization when DOMAIN_MEMBERS_SELECTED_FOR_MOVE may already have a stale value from a previous session,
removing initWithStoredValues: false means the hook will read that stale value immediately.
If there's logic that depends on the transition from "no selection" → "has selection" or vice versa, using the stale initial value could incorrectly trigger clearSelectedMembers() on mount.
Suggested fix: If this change is intentional (e.g., the new onyx version changes how initWithStoredValues works, or the hook was incorrectly using it), we could add a comment or note in the PR explaining why it was removed. If it's not intentional, we should revert it.
| const connectionID = Onyx.connect({ | ||
| key: ONYXKEYS.NVP_TRY_NEW_DOT, | ||
| initWithStoredValues: true, | ||
|
|
There was a problem hiding this comment.
Formatting: There's a blank line left where the property was.
|
🔴 I got an
Is this expected ? test-3-error.mov |
|
@ikevin127 sorry for not mentioning this before - this is not a PR to merge, just an env for testing the Onyx change. If Onyx looks fine, we'll incorporate it to the app via proper Onyx bump. |
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Explanation of Change
THIS IS ONLY FOR TESTING PURPOSES, DO NOT MERGE (do a proper Onyx bump after merging the Onyx feature)
Pins
react-native-onyxto PR #780 HEAD (3fbb6574) which adds an IDB backing store corruption healing mechanism. This is a test-only PR to validate the fix in Expensify/App context.What PR #780 does:
UnknownError: Internal error opening backing store(884K errors/month, 26.3% of all storage errors)indexedDB.open(), budgeted to 3 attempts (reset on success)Logger.logInfofor heal attempts andLogger.logAlertonly for budget exhaustion / non-recoverable errorsdeleteDatabase(), no provider swap, no UI changesAlso removes stale
react-native-onyx+3.0.73.patch— the changes it patched (reverting PR #770) already landed upstream in onyx 3.0.72 via PR #785, and our git pin includes that fix.Fixed Issues
$ #90636
PROPOSAL:
Tests
Each test injects a monkey-patch via the browser console to simulate the error class, then triggers an Onyx write (e.g. navigate to a different chat). No code changes needed.
Test 1 — Backing store error (Chromium path):
/IDB (heal|InvalidStateError|error|connection closed|Store \w+ does not exist|Creating store)/Log.ts):[info] IDB heal: backing store error detected — dropping cached connection and reopening (2 attempts left)[info] IDB heal: successfully recovered after backing store errorTest 2 — Connection lost (Safari path):
[info] IDB heal: connection lost error detected — dropping cached connection and reopening (2 attempts left)[info] IDB heal: successfully recovered after connection lost errorTest 3 — Budget exhaustion (3 consecutive failures, then gives up):
[info] IDB heal: backing store error detected — dropping cached connection and reopening (2 attempts left)[info] IDB heal: backing store error detected — dropping cached connection and reopening (1 attempts left)[info] IDB heal: backing store error detected — dropping cached connection and reopening (0 attempts left)[alrt] IDB heal: backing store error — heal budget exhausted, giving upsetWithRetrytakes over (up to 5 retries). App may degrade but should not crash or white-screenOffline tests
N/A — this change targets IDB connection/corruption errors, not network state. Onyx's existing offline handling is unchanged.
QA Steps
Same as Tests section above.
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectiontoggleReportand notonIconClick)src/languages/*files and using the translation methodSTYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))npm run compress-svg)Avataris modified, I verified thatAvataris working as expected in all cases)Designlabel and/or tagged@Expensify/designso the design team can review the changes.ScrollViewcomponent to make it scrollable when more elements are added to the page.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
N/A — IDB is web-only, no native changes.
Android: mWeb Chrome
iOS: Native
N/A — IDB is web-only, no native changes.
iOS: mWeb Safari
MacOS: Chrome / Safari
Healed (simulated error):
healed.mov
Killed connection (simulated error):
killed.mp4
Exhausted (simulated error):
exhausted.mov
Healed (simulated on Safari):
safari_error.mp4
Killed connection (simulated on Safari):
safari_killed.mp4
Exhausted (simulated on Safari):
safari_exhausted.mp4
Healed offline (simulated on Chrome):
offline.mp4