Patch react-native-nitro-sqlite so failed ROLLBACKs stop masking real SQLite errors - #97954
Merged
Julesssss merged 1 commit intoAug 10, 2026
Conversation
Contributor
|
|
elirangoshen
marked this pull request as ready for review
August 6, 2026 10:47
melvin-bot
Bot
requested review from
ahmedGaber93
and removed request for
a team
August 6, 2026 10:47
|
@ahmedGaber93 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] |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 93075772f7
ℹ️ 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".
elirangoshen
marked this pull request as draft
August 6, 2026 13:03
elirangoshen
force-pushed
the
eliran/bump-nitro-sqlite-9.7.0
branch
from
August 6, 2026 13:05
9307577 to
f03c23a
Compare
elirangoshen
changed the title
… error When a batch statement fails and SQLite has already auto-rolled the transaction back (disk-full I/O errors do this), the library's own ROLLBACK throws 'cannot rollback - no transaction is active' and replaces the real error (~2.8k masked lines/day on iOS). Swallow the rollback failure and rethrow the original. Patch over 9.6.0 instead of bumping to 9.7.0 because 9.7.0 force-enables SQLITE_THREADSAFE=0 on iOS and breaks double-open. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
elirangoshen
force-pushed
the
eliran/bump-nitro-sqlite-9.7.0
branch
from
August 6, 2026 13:10
f03c23a to
9299910
Compare
elirangoshen
marked this pull request as ready for review
August 6, 2026 13:26
Contributor
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppa.movAndroid: mWeb Chromeaw.moviOS: HybridAppi.moviOS: mWeb Safariiw.movMacOS: Chrome / Safariw.mov |
Julesssss
approved these changes
Aug 10, 2026
Contributor
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
Contributor
|
🚧 Julesssss has triggered a test Expensify/App build. You can view the workflow run here. |
Contributor
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
|
Contributor
|
🚀 Deployed to staging by https://github.com/Julesssss in version: 9.4.52-0 🚀
|
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
4 participants
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
Adds a
patch-packagepatch toreact-native-nitro-sqlite@9.6.0so a failedROLLBACKno longer masks the original SQLite error in the batch-write path (the path react-native-onyx uses for every write).The bug: when a statement inside
executeBatchfails and SQLite has already auto-rolled the transaction back (which disk-full I/O errors do), the library's ownROLLBACKfails withcannot rollback - no transaction is active— and that error is thrown instead of the original one. In production this hides the real failure: we log ~2,800cannot rollback - no transaction is activelines per day (iOS), none of which say what actually broke. After this patch those surface as the true underlying errors (e.g.disk I/O error,database or disk is full), which the disk-pressure classification in Expensify/react-native-onyx#816 then handles with throttled alerts and free-disk telemetry instead of blind retries.The patch mirrors the fix upstream already shipped in 9.7.0 (margelo/react-native-nitro-sqlite#292): wrap the two
ROLLBACKcalls in try/catch and rethrow the original exception.Why a patch instead of bumping to 9.7.0 (this PR originally bumped; deliberately reverted):
performance_mode = 1→-DSQLITE_THREADSAFE=0(all SQLite mutexes compiled out), with no opt-out. On 9.6.0 this never actually applied — the podspec compared the int1against the string'1', so the flag block was dead code and we've been shipping SQLite's default thread-safe build all along. 9.7.0 fixes that comparison, so the bump silently flips production iOS to a mutex-free SQLite.open()throwsDatabase OnyxDB is already open— andsrc/libs/ExportOnyxState/index.native.tsopensOnyxDBa second time (alongside Onyx's own connection) to export state. That call throws on 9.7.0.The patch delivers the one behavior we need now with zero build-flag or API changes. The bump can follow once upstream makes
performance_modeconfigurable (its own TODO says it should be) and our double-open inExportOnyxStateis reworked.Fixed Issues
$ #97908
PROPOSAL: N/A (dependency patch surfacing masked SQLite errors; part of the disk-pressure work in #97908, symptom tracked in #87869)
Automated Tests
None added here — this PR only patches the dependency's native error propagation. The classification behavior that consumes the now-unmasked errors is tested in the paired onyx PR (Expensify/react-native-onyx#816,
tests/unit/onyxUtilsTest.tsandtests/unit/storage/providers/SQLiteProviderTest.ts).Tests
Regression smoke — the patch only changes which error is thrown when a batch write fails; success paths are untouched:
Offline tests
QA Steps
Same as Tests.
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectionAvatar, 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.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
Screen.Recording.2026-08-06.at.12.24.45.mov