margelo / margelo/react-native-nitro-sqlite
loadFile masks statement errors by rolling back twice
Nobody has claimed this yet.
- Dominant language
- C
- Stars
- 565
- Forks
- 53
- Avg merge
- 1d 21h
- Merged PRs (30d)
- 18
Description
Summary
loadFile() rolls back twice after a statement failure. The second ROLLBACK fails with cannot rollback - no transaction is active, masking the actual import failure and the library's intended CouldNotLoadFile error.
Verified on main at ad8b835 (v9.7.0).
Code evidence
- A per-line
NitroSQLiteExceptioncatch executesROLLBACKat line 28. - It throws
CouldNotLoadFileat line 30. - The outer
catch (...)catches that exception and executesROLLBACKagain at line 40. - SQLite reports
cannot rollback - no transaction is active; that newSqlExecutionErrorescapes before the intendedUnknownErrorat line 41 can be thrown.
The same outer catch also replaces useful BEGIN/COMMIT failures with a generic message, and a rollback failure can replace any primary error.
Smallest reproducer
Create a SQL file with one valid command followed by invalid SQL:
CREATE TABLE imported(id INTEGER PRIMARY KEY);
THIS IS INVALID SQL;
Then:
const db = open({ name: 'load-file.sqlite' })
await expect(db.loadFileAsync(path)).rejects.toThrow()
Expected:
- The transaction is rolled back exactly once.
- The error identifies
loadFile, the source file, and ideally the failing command/line. importeddoes not exist.
Observed from the current control flow:
- The transaction is rolled back.
- The intended
CouldNotLoadFile(..., "Transaction was rolled back")is caught internally. - A second rollback fails and surfaces
cannot rollback - no transaction is active, masking the import error.
The rollback behavior itself is reproducible with BEGIN; ROLLBACK; ROLLBACK;, where SQLite rejects the second rollback.
Impact
Import failures lose their actionable error and source context. Applications cannot reliably distinguish malformed import files from transaction-state errors, making migrations and recovery diagnostics substantially harder.
Acceptance criteria
- Give one scope sole ownership of transaction finalization; execute at most one rollback per failed import transaction.
- Track whether
BEGINsucceeded and whether the transaction is still active before attempting rollback. - Preserve the primary statement/BEGIN/COMMIT error, including file and failing line/command context.
- If rollback also fails, retain both errors without masking the primary cause.
- Close the input file through RAII rather than repeated manual close paths.
Regression-test target
A native or Harness test that imports a file containing a valid command followed by invalid SQL and asserts:
- The promise rejects with the original import/SQL context, not
cannot rollback - no transaction is active. - The valid command was rolled back.
- The same connection remains usable afterward.
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start in packages/react-native-nitro-sqlite/cpp/importSqlFile.cpp, especially the lines and catch blocks identified in the issue. Reproduce the failure with a valid SQL command followed by invalid SQL, then add a native or Harness regression test. Done means one rollback, preserved import/SQL context, no imported table, and a connection that remains usable.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- cpp, react-native, sqlite
- Domain
- databases, mobile
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Quiet
- Clarity
- Clearly specified
- Newbie friendliness
- 58/100