Skip to content

fix(db): ensure transaction depth resets when rollback fails - #1725

Open
xiaobaZeo wants to merge 1 commit into
colbymchenry:mainfrom
xiaobaZeo:fix/sqlite-transaction-depth-reset
Open

fix(db): ensure transaction depth resets when rollback fails#1725
xiaobaZeo wants to merge 1 commit into
colbymchenry:mainfrom
xiaobaZeo:fix/sqlite-transaction-depth-reset

Conversation

@xiaobaZeo

Copy link
Copy Markdown

Summary

In NodeSqliteAdapter.transaction() (src/db/sqlite-adapter.ts), if a transaction encounters an error and SQLite automatically aborts the transaction before a rollback is executed, calling ROLLBACK throws an error (cannot rollback - no transaction is active). Previously, this._txDepth = 0 was placed after this._db.exec('ROLLBACK') without a finally block, causing _txDepth to remain greater than 0 on error. As a consequence, subsequent transaction() calls on the same connection falsely assumed they were inside a nested transaction and omitted BEGIN/COMMIT, degrading operations to auto-commit and violating ACID isolation.

This PR wraps the rollback and depth reset in a finally block with a committed flag, ensuring:

  1. _txDepth is guaranteed to reset to 0 regardless of closure errors, commit failures, or rollback failures.
  2. Any rollback error when SQLite has already aborted the transaction is safely tolerated.
  3. Added unit tests in __tests__/sqlite-backend.test.ts covering both standard rollback and pre-aborted transaction scenarios.
  4. Added user-facing entry in CHANGELOG.md under ## [Unreleased].

Validation

  • npx vitest run __tests__/sqlite-backend.test.ts passes (4 tests passed).
  • npm run build succeeds cleanly.

🤖 Generated with Claude Code

Guarantee that NodeSqliteAdapter._txDepth always resets to 0 via try/finally,
preventing transaction isolation loss on subsequent operations if SQLite
aborts the transaction before rollback completes.

Co-Authored-By: Claude Code <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant