fix: roll back failed transaction commits (#316)

Track transaction finalization states explicitly and preserve commit failures while rolling back deferred constraint errors. Closes #307.

Co-authored-by: Christoph Pader <chris@margelo.com>

Author: huytdps13400

docs/content/docs/guides/transactions.mdx

@@ -27,6 +27,10 @@ const transferId = await db.transaction(async (tx) => {
 
 When the callback resolves, the wrapper commits unless you already called `tx.commit()` or `tx.rollback()`. When it rejects or throws, the wrapper rolls back unless the transaction has already been finalized. An explicit `tx.rollback()` does not itself reject the callback: it ends the transaction, and the callback can still resolve. Calls to `tx.execute`, `tx.executeAsync`, `tx.commit`, or `tx.rollback` after finalization throw.
 
+A failed COMMIT can leave SQLite's transaction active, for example when a deferred foreign-key constraint fails. NitroSQLite marks a commit complete only after SQLite accepts it. A failed manual or automatic commit triggers rollback and rejects with the commit error. This also happens if the callback catches a failed `tx.commit()` and returns without explicitly rolling back. After the failure, only `tx.rollback()` is allowed on that transaction handle.
+
+If rollback also fails, the rejection preserves the original message and appends the rollback error. Its `cause` is an `AggregateError` containing the normalized primary and rollback errors, in that order. A callback that catches a failed manual rollback still causes the transaction promise to reject. A failed BEGIN does not roll back a transaction that the wrapper did not start.
+
 ## Keep operations inside the callback
 
 The connection queue is occupied for the whole callback. Use `tx.execute` or `tx.executeAsync` for **every** statement on this database during that callback, including statements in helpers.

example/tests/unit/specs/operations/transaction.spec.ts

@@ -11,8 +11,29 @@ import type { User } from '@/model/User'
 import { NitroSQLiteError } from 'react-native-nitro-sqlite'
 import { testDb } from '@tests/db'
 
+const FOREIGN_KEY_COMMIT_ERROR = 'FOREIGN KEY constraint failed'
+
 export default function registerTransactionUnitTests() {
   describe('transaction', () => {
+    it('Transaction, rolls back after automatic deferred foreign key commit failure', async () => {
+      await expectDeferredForeignKeyCommitFailure()
+    })
+
+    it('Transaction, rolls back after manual deferred foreign key commit failure', async () => {
+      await expectDeferredForeignKeyCommitFailure((tx) => tx.commit())
+    })
+
+    it('Transaction, rolls back a caught manual commit failure', async () => {
+      await expectDeferredForeignKeyCommitFailure((tx) => {
+        try {
+          tx.commit()
+        } catch (error) {
+          if (!isNitroSQLiteError(error)) throw error
+          expect(error.message).toContain(FOREIGN_KEY_COMMIT_ERROR)
+        }
+      })
+    })
+
     it('Transaction, auto commit', async () => {
       const id = chance.integer()
       const name = chance.name()
@@ -459,3 +480,37 @@ export default function registerTransactionUnitTests() {
     })
   })
 }
+
+async function expectDeferredForeignKeyCommitFailure(
+  finalize?: (
+    tx: Parameters<Parameters<typeof testDb.transaction>[0]>[0],
+  ) => void,
+) {
+  testDb.execute('PRAGMA foreign_keys = ON')
+  testDb.execute('DROP TABLE IF EXISTS Child')
+  testDb.execute('DROP TABLE IF EXISTS Parent')
+  testDb.execute('CREATE TABLE Parent (id INTEGER PRIMARY KEY)')
+  testDb.execute(
+    'CREATE TABLE Child (parentId INTEGER REFERENCES Parent(id) DEFERRABLE INITIALLY DEFERRED)',
+  )
+
+  try {
+    await testDb.transaction(async (tx) => {
+      tx.execute('INSERT INTO Child (parentId) VALUES (1)')
+      finalize?.(tx)
+    })
+    throw new Error(TEST_ERROR_CODES.EXPECT_PROMISE_REJECTION)
+  } catch (error) {
+    if (isNitroSQLiteError(error)) {
+      expect(error.message).toContain(FOREIGN_KEY_COMMIT_ERROR)
+    } else {
+      throw new Error(TEST_ERROR_CODES.EXPECT_NITRO_SQLITE_ERROR)
+    }
+  }
+
+  expect(testDb.execute('SELECT * FROM Child').rows?._array).toEqual([])
+
+  await testDb.transaction(async (tx) => {
+    tx.execute('INSERT INTO Parent (id) VALUES (1)')
+  })
+}

packages/react-native-nitro-sqlite/src/__tests__/transaction.test.ts

@@ -1,5 +1,6 @@
 jest.mock('../nitro')
 
+import Database from 'better-sqlite3'
 import { HybridNitroSQLite } from '../nitro'
 import { closeDatabaseQueue, openDatabaseQueue } from '../DatabaseQueue'
 import { transaction } from '../operations/transaction'
@@ -208,7 +209,8 @@ describe('transaction', () => {
       }),
     ).rejects.toMatchObject({
       name: 'NitroSQLiteError',
-      message: 'rollback failed',
+      message: 'callback failed\nRollback failed: rollback failed',
+      cause: expect.any(AggregateError),
     })
   })
 
@@ -239,4 +241,152 @@ describe('transaction', () => {
     await Promise.all([first, second])
     expect(order).toEqual(['first', 'second'])
   })
+  it.each(['automatic', 'manual', 'caught manual'])(
+    'rolls back a real deferred constraint after a %s commit failure',
+    async (mode) => {
+      const sqlite = new Database(':memory:')
+      sqlite.exec(
+        'PRAGMA foreign_keys = ON; CREATE TABLE Parent (id INTEGER PRIMARY KEY); CREATE TABLE Child (parentId INTEGER REFERENCES Parent(id) DEFERRABLE INITIALLY DEFERRED)',
+      )
+      const execute = (_name: string, query: string) => {
+        try {
+          sqlite.exec(query)
+          return nativeResult()
+        } catch (error) {
+          // The addon creates errors outside Jest's realm. The native bridge
+          // exposes a JavaScript Error, so mirror that boundary here.
+          if (
+            typeof error === 'object' &&
+            error !== null &&
+            'message' in error &&
+            typeof error.message === 'string'
+          ) {
+            throw new Error(error.message, { cause: error })
+          }
+          throw error
+        }
+      }
+      jest.mocked(HybridNitroSQLite.execute).mockImplementation(execute)
+      jest
+        .mocked(HybridNitroSQLite.executeAsync)
+        .mockImplementation(async (name, query) => execute(name, query))
+      try {
+        await expect(
+          transaction(dbName, async (tx) => {
+            tx.execute('INSERT INTO Child VALUES (1)')
+            if (mode === 'manual') tx.commit()
+            if (mode === 'caught manual') {
+              expect(() => tx.commit()).toThrow('FOREIGN KEY constraint failed')
+              expect(() => tx.execute('SELECT 1')).toThrow(
+                'finalized transaction',
+              )
+              expect(() => tx.executeAsync('SELECT 1')).toThrow(
+                'finalized transaction',
+              )
+              expect(() => tx.commit()).toThrow('finalized transaction')
+            }
+          }),
+        ).rejects.toThrow('FOREIGN KEY constraint failed')
+        expect(sqlite.inTransaction).toBe(false)
+        expect(
+          sqlite.prepare('SELECT COUNT(*) AS count FROM Child').get(),
+        ).toEqual({ count: 0 })
+        await transaction(dbName, async (tx) => {
+          tx.execute('INSERT INTO Parent VALUES (1)')
+        })
+        expect(sqlite.inTransaction).toBe(false)
+      } finally {
+        sqlite.close()
+      }
+    },
+  )
+
+  it('allows explicit rollback to recover a caught commit failure', async () => {
+    jest
+      .mocked(HybridNitroSQLite.execute)
+      .mockImplementation((_name, query) => {
+        if (query === 'COMMIT') throw new Error('commit failed')
+        return nativeResult()
+      })
+    await expect(
+      transaction(dbName, async (tx) => {
+        expect(() => tx.commit()).toThrow('commit failed')
+        tx.rollback()
+        return 'recovered'
+      }),
+    ).resolves.toBe('recovered')
+  })
+
+  it.each([false, true])(
+    'rejects a failed manual rollback even when caught: %s',
+    async (caught) => {
+      jest.mocked(HybridNitroSQLite.execute).mockImplementation(() => {
+        throw new Error('rollback failed')
+      })
+      await expect(
+        transaction(dbName, async (tx) => {
+          if (caught) {
+            expect(() => tx.rollback()).toThrow('rollback failed')
+            return
+          }
+          tx.rollback()
+        }),
+      ).rejects.toThrow('rollback failed')
+      expect(HybridNitroSQLite.execute).toHaveBeenCalledTimes(1)
+    },
+  )
+
+  it('preserves commit and rollback errors in order', async () => {
+    jest
+      .mocked(HybridNitroSQLite.execute)
+      .mockImplementation((_name, query) => {
+        throw new Error(
+          query === 'COMMIT' ? 'commit failed' : 'rollback failed',
+        )
+      })
+    await expect(transaction(dbName, async () => {})).rejects.toMatchObject({
+      message: 'commit failed\nRollback failed: rollback failed',
+      cause: {
+        errors: [
+          expect.objectContaining({ message: 'commit failed' }),
+          expect.objectContaining({ message: 'rollback failed' }),
+        ],
+      },
+    })
+  })
+
+  it('preserves both errors when manual rollback after a failed commit also fails', async () => {
+    jest
+      .mocked(HybridNitroSQLite.execute)
+      .mockImplementation((_name, query) => {
+        throw new Error(
+          query === 'COMMIT' ? 'commit failed' : 'rollback failed',
+        )
+      })
+    await expect(
+      transaction(dbName, async (tx) => {
+        expect(() => tx.commit()).toThrow('commit failed')
+        expect(() => tx.rollback()).toThrow('rollback failed')
+      }),
+    ).rejects.toMatchObject({
+      message: 'commit failed\nRollback failed: rollback failed',
+      cause: {
+        errors: [
+          expect.objectContaining({ message: 'commit failed' }),
+          expect.objectContaining({ message: 'rollback failed' }),
+        ],
+      },
+    })
+    expect(HybridNitroSQLite.execute).toHaveBeenCalledTimes(2)
+  })
+
+  it('does not roll back when BEGIN fails', async () => {
+    jest
+      .mocked(HybridNitroSQLite.executeAsync)
+      .mockRejectedValue(new Error('begin failed'))
+    const callback = jest.fn()
+    await expect(transaction(dbName, callback)).rejects.toThrow('begin failed')
+    expect(callback).not.toHaveBeenCalled()
+    expect(HybridNitroSQLite.execute).not.toHaveBeenCalled()
+  })
 })