[Onyx] Fix dead-code async-catch in tryOrDegradePerformance#90768
Draft
elirangoshen wants to merge 2 commits into
Draft
[Onyx] Fix dead-code async-catch in tryOrDegradePerformance#90768elirangoshen wants to merge 2 commits into
elirangoshen wants to merge 2 commits into
Conversation
Contributor
|
|
41 tasks
Picks up the sync-throw handling fix in Expensify/react-native-onyx#784 (comment).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Explanation of Change
This is a draft testing branch that pulls in the react-native-onyx fix for #90632 into New Expensify so reviewers and CI can validate it end-to-end before the onyx PR merges and gets a published release.
What the onyx PR fixes
tryOrDegradePerformanceinlib/storage/index.tswas supposed to detect known IndexedDB failures (e.g.'IDBKeyVal store could not be created') and gracefully fall back to an in-memory provider viadegradePerformance(). It wrapped every storage call in a synchronoustry/catch, but every storage provider method returns aPromiseand the helper didresolve(fn())(noawait). Any rejection fromfn()propagated through the promise chain and never re-entered thecatch. Result: the fallback branch has been dead code and theLogger.logHmmm("Falling back to only using cache…")message has never fired in production.The fix converts the synchronous wrapper into a proper async
.catch(...)so rejections are actually caught:What this PR contains
package.json— bumpsreact-native-onyxto the head commit of Expensify/react-native-onyx#784 via the standard git-hash trick:patches/react-native-onyx/react-native-onyx+3.0.69.patch. That patch'shasMountedRefchange ([Due for payment 2026-04-28] [$250] [Onyx Audit] Fix useOnyx state not resetting on key change in getSnapshot() #85416 — one feweruseOnyxrender on initial mount) is already merged upstream in onyx 3.0.74; the samehasMountedRefref + guarded reset block are present innode_modules/react-native-onyx/dist/useOnyx.js. Keeping a 3.0.69-targeted patch on top of newer onyx source breaksnpm install(patch-package fails on the diverged file), so it has to go here.The proper version-bump PR (pinning to a published onyx release instead of a PR commit) will be opened once Expensify/react-native-onyx#784 lands and a new onyx version is released.
Fixed Issues
$ #90632
PROPOSAL: N/A — task issue assigned directly.
Tests
The fix targets the IndexedDB code path. IndexedDB is web-only — mobile platforms use
SQLiteProviderand never reach the changed branch. So the tests that actually exercise the fix are all web. Mobile testing here is purely a non-regression check.npm installandnpm run web. Confirm the App boots and behaves identically tomainon the happy path: log in, view reports, open a chat, send a message, refresh the page.Logger.logHmmm("Falling back to only using cache and dropping storage")is not logged during normal use.IDBKeyValProvider.setItem(or any other provider method) innode_modules/react-native-onyx/dist/storage/providers/IDBKeyValProvider.js.new Error('IDBKeyVal store could not be created').[Onyx] Error while using IDBKeyValProvider. Falling back to only using cache and dropping storage.plus the error stack.main(without this PR), step 4's log message never appears even though the same underlying IDB error fires — confirming the dead-code bug.useOnyx-heavy screens (Reports list, Money Request flows) on web, iOS, and Android. The deleted patch's effect (one fewer render on initial mount peruseOnyx) is now provided by upstream onyx 3.0.74, so behavior should be identical to currentmain.Offline tests
Onyx's job is offline-first storage, so the offline path is the most important non-regression check:
main— this PR does not change any happy-path storage behavior, only the (previously dead) error-fallback branch.QA Steps
Same as Tests above. The fix only meaningfully changes behavior in a degraded IndexedDB-failure state, which is impractical to reproduce in QA without manual instrumentation. Primary QA value here is verifying no regressions in normal storage flows on web, since this PR routes every Onyx storage call through a rewritten helper.
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectionAvatar, I verified the components usingAvatarare working as expected)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