diff --git a/src/js/bun/sql.ts b/src/js/bun/sql.ts index c3a74582a25..4a6516ee1da 100644 --- a/src/js/bun/sql.ts +++ b/src/js/bun/sql.ts @@ -145,7 +145,7 @@ const SQL: typeof Bun.SQL = function SQL( transactionQueries.delete(query); } - function queryFromTransactionHandler(transactionQueries, query, handle, err) { + function queryFromTransactionHandler(transactionQueries, preRunGuard, query, handle, err) { const pooledConnection = this; if (err) { transactionQueries.delete(query); @@ -158,6 +158,14 @@ const SQL: typeof Bun.SQL = function SQL( return query.reject(pool.queryCancelledError()); } + if (preRunGuard !== null) { + const guardErr = preRunGuard(); + if (guardErr) { + transactionQueries.delete(query); + return query.reject(guardErr); + } + } + query.finally(onTransactionQueryDisconnected.bind(transactionQueries, query)); try { @@ -177,6 +185,7 @@ const SQL: typeof Bun.SQL = function SQL( values: any[], pooledConnection: PooledPostgresConnection, transactionQueries: Set>, + preRunGuard: (() => Error | null) | null = null, ) { try { const query = new Query( @@ -185,7 +194,7 @@ const SQL: typeof Bun.SQL = function SQL( connectionInfo.bigint ? SQLQueryFlags.allowUnsafeTransaction | SQLQueryFlags.bigint : SQLQueryFlags.allowUnsafeTransaction, - queryFromTransactionHandler.bind(pooledConnection, transactionQueries), + queryFromTransactionHandler.bind(pooledConnection, transactionQueries, preRunGuard), pool, ); @@ -201,6 +210,7 @@ const SQL: typeof Bun.SQL = function SQL( values: any[], pooledConnection: PooledPostgresConnection, transactionQueries: Set>, + preRunGuard: (() => Error | null) | null = null, ) { try { let flags = connectionInfo.bigint @@ -214,7 +224,7 @@ const SQL: typeof Bun.SQL = function SQL( strings, values, flags, - queryFromTransactionHandler.bind(pooledConnection, transactionQueries), + queryFromTransactionHandler.bind(pooledConnection, transactionQueries, preRunGuard), pool, ); transactionQueries.add(query); @@ -551,11 +561,29 @@ const SQL: typeof Bun.SQL = function SQL( pool.attachConnectionCloseHandler(pooledConnection, onClose); } - function run_internal_transaction_sql(string) { + // Some databases (SQLite's ON CONFLICT ROLLBACK / OR ROLLBACK / RAISE(ROLLBACK)) can roll the + // transaction back on their own mid-callback. Once that happens the connection is in autocommit + // and any further COMMIT/ROLLBACK would fail with "no transaction is active". + let needs_rollback = false; + const isConnectionInTransaction = pool.isConnectionInTransaction?.bind(pool); + function transaction_still_open() { + return isConnectionInTransaction === undefined || isConnectionInTransaction(pooledConnection); + } + function transaction_aborted_error() { + return pool.invalidTransactionStateError( + "current transaction is aborted (the database already rolled it back), commands ignored until end of transaction", + ); + } + const preRunGuard = + isConnectionInTransaction === undefined + ? null + : () => (needs_rollback && !transaction_still_open() ? transaction_aborted_error() : null); + + function run_internal_transaction_sql(string, guarded = false) { if (state.connectionState & ReservedConnectionState.closed) { return Promise.$reject(pool.connectionClosedError()); } - return unsafeQueryFromTransaction(string, [], pooledConnection, state.queries); + return unsafeQueryFromTransaction(string, [], pooledConnection, state.queries, guarded ? preRunGuard : null); } function transaction_sql( strings: string | TemplateStringsArray | import("internal/sql/shared.ts").SQLHelper | Query, @@ -576,17 +604,14 @@ const SQL: typeof Bun.SQL = function SQL( return new SQLHelper([strings], values); } - return queryFromTransaction(strings, values, pooledConnection, state.queries); + return queryFromTransaction(strings, values, pooledConnection, state.queries, preRunGuard); } transaction_sql.unsafe = (string, args = []) => { - return unsafeQueryFromTransaction(string, args, pooledConnection, state.queries); + return unsafeQueryFromTransaction(string, args, pooledConnection, state.queries, preRunGuard); }; transaction_sql.file = async (path: string, args = []) => { - return await Bun.file(path) - .text() - .then(text => { - return unsafeQueryFromTransaction(text, args, pooledConnection, state.queries); - }); + const text = await Bun.file(path).text(); + return unsafeQueryFromTransaction(text, args, pooledConnection, state.queries, preRunGuard); }; // reserve is allowed to be called inside transaction connection but will return a new reserved connection from the pool and will not be part of the transaction // this matchs the behavior of the postgres package @@ -669,10 +694,13 @@ const SQL: typeof Bun.SQL = function SQL( for (const query of transactionQueries) { (query as Query).cancel(); } - if (BEFORE_COMMIT_OR_ROLLBACK_COMMAND) { - await run_internal_transaction_sql(BEFORE_COMMIT_OR_ROLLBACK_COMMAND); + if (transaction_still_open()) { + if (BEFORE_COMMIT_OR_ROLLBACK_COMMAND) { + await run_internal_transaction_sql(BEFORE_COMMIT_OR_ROLLBACK_COMMAND); + } + await run_internal_transaction_sql(ROLLBACK_COMMAND); } - await run_internal_transaction_sql(ROLLBACK_COMMAND); + needs_rollback = false; state.connectionState |= ReservedConnectionState.closed; resolve(); }, timeout * 1000); @@ -687,10 +715,13 @@ const SQL: typeof Bun.SQL = function SQL( for (const query of transactionQueries) { (query as Query).cancel(); } - if (BEFORE_COMMIT_OR_ROLLBACK_COMMAND) { - await run_internal_transaction_sql(BEFORE_COMMIT_OR_ROLLBACK_COMMAND); + if (transaction_still_open()) { + if (BEFORE_COMMIT_OR_ROLLBACK_COMMAND) { + await run_internal_transaction_sql(BEFORE_COMMIT_OR_ROLLBACK_COMMAND); + } + await run_internal_transaction_sql(ROLLBACK_COMMAND); } - await run_internal_transaction_sql(ROLLBACK_COMMAND); + needs_rollback = false; state.connectionState |= ReservedConnectionState.closed; }; transaction_sql[Symbol.asyncDispose] = () => transaction_sql.close(); @@ -703,21 +734,26 @@ const SQL: typeof Bun.SQL = function SQL( transactionSavepoints.delete(savepoint_promise); } async function run_internal_savepoint(save_point_name: string, savepoint_callback: TransactionCallback) { - await run_internal_transaction_sql(`${SAVEPOINT_COMMAND} ${save_point_name}`); + await run_internal_transaction_sql(`${SAVEPOINT_COMMAND} ${save_point_name}`, true); try { let result = await savepoint_callback(transaction_sql); + if ($isArray(result)) { + result = await Promise.all(result); + } + if (needs_rollback && !transaction_still_open()) { + throw transaction_aborted_error(); + } if (RELEASE_SAVEPOINT_COMMAND) { // mssql dont have release savepoint await run_internal_transaction_sql(`${RELEASE_SAVEPOINT_COMMAND} ${save_point_name}`); } - if ($isArray(result)) { - result = await Promise.all(result); - } return result; } catch (err) { - if (!(state.connectionState & ReservedConnectionState.closed)) { - await run_internal_transaction_sql(`${ROLLBACK_TO_SAVEPOINT_COMMAND} ${save_point_name}`); + if (!(state.connectionState & ReservedConnectionState.closed) && transaction_still_open()) { + try { + await run_internal_transaction_sql(`${ROLLBACK_TO_SAVEPOINT_COMMAND} ${save_point_name}`); + } catch {} } throw err; } @@ -736,6 +772,9 @@ const SQL: typeof Bun.SQL = function SQL( ) { throw pool.connectionClosedError(); } + if (needs_rollback && !transaction_still_open()) { + throw transaction_aborted_error(); + } if ($isCallable(name)) { savepoint_callback = name as unknown as TransactionCallback; @@ -753,7 +792,6 @@ const SQL: typeof Bun.SQL = function SQL( return await promise.finally(onSavepointFinished.bind(null, promise)); }; } - let needs_rollback = false; try { await run_internal_transaction_sql(BEGIN_COMMAND); needs_rollback = true; @@ -761,6 +799,9 @@ const SQL: typeof Bun.SQL = function SQL( if ($isArray(transaction_result)) { transaction_result = await Promise.all(transaction_result); } + if (needs_rollback && !transaction_still_open()) { + throw transaction_aborted_error(); + } // at this point we dont need to rollback anymore needs_rollback = false; if (BEFORE_COMMIT_OR_ROLLBACK_COMMAND) { @@ -770,15 +811,13 @@ const SQL: typeof Bun.SQL = function SQL( return resolve(transaction_result); } catch (err) { try { - if (!(state.connectionState & ReservedConnectionState.closed) && needs_rollback) { + if (!(state.connectionState & ReservedConnectionState.closed) && needs_rollback && transaction_still_open()) { if (BEFORE_COMMIT_OR_ROLLBACK_COMMAND) { await run_internal_transaction_sql(BEFORE_COMMIT_OR_ROLLBACK_COMMAND); } await run_internal_transaction_sql(ROLLBACK_COMMAND); } - } catch (err) { - return reject(err); - } + } catch {} return reject(err); } finally { state.connectionState |= ReservedConnectionState.closed; diff --git a/src/js/internal/sql/shared.ts b/src/js/internal/sql/shared.ts index 21c781812aa..8990c517b89 100644 --- a/src/js/internal/sql/shared.ts +++ b/src/js/internal/sql/shared.ts @@ -2123,6 +2123,7 @@ export interface DatabaseAdapter { supportsReservedConnections?(): boolean; getConnectionForQuery?(pooledConnection: Connection): ConnectionHandle | null; + isConnectionInTransaction?(connection: Connection): boolean; attachConnectionCloseHandler?(connection: Connection, handler: () => void): void; detachConnectionCloseHandler?(connection: Connection, handler: () => void): void; diff --git a/src/js/internal/sql/sqlite.ts b/src/js/internal/sql/sqlite.ts index 674357d7fb3..27b41896530 100644 --- a/src/js/internal/sql/sqlite.ts +++ b/src/js/internal/sql/sqlite.ts @@ -487,6 +487,9 @@ class SQLiteAdapter implements DatabaseAdapter { const accounts = await sql`SELECT * FROM accounts WHERE id = 1`; expect(accounts[0].balance).toBe(1002); }); + + describe("SQLite automatic ROLLBACK (ON CONFLICT ROLLBACK / OR ROLLBACK / RAISE(ROLLBACK))", () => { + beforeEach(async () => { + await sql`CREATE TABLE t (v TEXT PRIMARY KEY ON CONFLICT ROLLBACK)`; + await sql`CREATE TABLE log (v TEXT)`; + await sql`INSERT INTO t VALUES ('dup')`; + }); + + test("uncaught conflict: begin() rejects with the original statement error, not 'cannot rollback'", async () => { + let caught: any; + try { + await sql.begin(async tx => { + await tx`INSERT INTO t VALUES ('dup')`; + }); + } catch (e) { + caught = e; + } + expect({ code: caught?.code, message: String(caught) }).toEqual({ + code: "SQLITE_CONSTRAINT_PRIMARYKEY", + message: "SQLiteError: UNIQUE constraint failed: t.v", + }); + }); + + test("caught conflict: later statements are refused and nothing is committed", async () => { + let beginErr: any; + let stmtErr: any; + try { + await sql.begin(async tx => { + await tx`INSERT INTO log VALUES ('first')`; + try { + await tx`INSERT INTO t VALUES ('dup')`; + } catch {} + try { + await tx`INSERT INTO log VALUES ('after-auto-rollback')`; + } catch (e) { + stmtErr = e; + throw e; + } + }); + } catch (e) { + beginErr = e; + } + const durable = (await sql`SELECT v FROM log`).map((r: any) => r.v); + expect({ + stmtErrCode: stmtErr?.code, + beginErrCode: beginErr?.code, + durable, + }).toEqual({ + stmtErrCode: "ERR_SQLITE_INVALID_TRANSACTION_STATE", + beginErrCode: "ERR_SQLITE_INVALID_TRANSACTION_STATE", + durable: [], + }); + // the connection must remain usable for a fresh transaction afterwards + await sql.begin(async tx => { + await tx`INSERT INTO log VALUES ('next-tx')`; + }); + expect((await sql`SELECT v FROM log`).map((r: any) => r.v)).toEqual(["next-tx"]); + }); + + test("caught conflict where callback swallows everything: begin() still rejects, COMMIT is not attempted", async () => { + let beginErr: any; + try { + await sql.begin(async tx => { + try { + await tx`INSERT INTO t VALUES ('dup')`; + } catch {} + // every subsequent statement keeps failing; swallowing one must not let the next through + try { + await tx`INSERT INTO log VALUES ('a')`; + } catch {} + try { + await tx`INSERT INTO log VALUES ('b')`; + } catch {} + }); + } catch (e) { + beginErr = e; + } + expect({ + code: beginErr?.code, + message: String(beginErr), + durable: (await sql`SELECT v FROM log`).map((r: any) => r.v), + }).toEqual({ + code: "ERR_SQLITE_INVALID_TRANSACTION_STATE", + message: + "SQLiteError: current transaction is aborted (the database already rolled it back), commands ignored until end of transaction", + durable: [], + }); + }); + + test("array-return form: queries created before the conflict are refused at execution time", async () => { + let beginErr: any; + await sql + .begin(tx => [ + tx`INSERT INTO log VALUES ('before')`, + tx`INSERT INTO t VALUES ('dup')`, + tx`INSERT INTO log VALUES ('leaked')`, + ]) + .catch(e => (beginErr = e)); + expect({ + code: beginErr?.code, + durable: (await sql`SELECT v FROM log`).map((r: any) => r.v), + }).toEqual({ + code: "SQLITE_CONSTRAINT_PRIMARYKEY", + durable: [], + }); + }); + + test("INSERT OR ROLLBACK (statement-level conflict clause) behaves the same", async () => { + await sql`CREATE TABLE t2 (v TEXT PRIMARY KEY)`; + await sql`INSERT INTO t2 VALUES ('dup')`; + let caught: any; + try { + await sql.begin(async tx => { + await tx`INSERT OR ROLLBACK INTO t2 VALUES ('dup')`; + }); + } catch (e) { + caught = e; + } + expect(caught?.code).toBe("SQLITE_CONSTRAINT_PRIMARYKEY"); + }); + + test("tx.unsafe refuses to run after auto-rollback", async () => { + let stmtErr: any; + await sql + .begin(async tx => { + try { + await tx`INSERT INTO t VALUES ('dup')`; + } catch {} + await tx.unsafe("INSERT INTO log VALUES ('after-auto-rollback')"); + }) + .catch(e => (stmtErr = e)); + expect({ + code: stmtErr?.code, + durable: (await sql`SELECT v FROM log`).map((r: any) => r.v), + }).toEqual({ + code: "ERR_SQLITE_INVALID_TRANSACTION_STATE", + durable: [], + }); + }); + + test("tx.savepoint after auto-rollback is refused and its body never runs", async () => { + let bodyRan = false; + let conflictErr: any; + let beginErr: any; + try { + await sql.begin(async tx => { + try { + await tx`INSERT INTO t VALUES ('dup')`; + } catch (e) { + conflictErr = e; + } + await tx.savepoint(async sp => { + bodyRan = true; + await sp`INSERT INTO log VALUES ('from-savepoint')`; + }); + }); + } catch (e) { + beginErr = e; + } + expect({ + bodyRan, + conflictErrCode: conflictErr?.code, + beginErrCode: beginErr?.code, + durable: (await sql`SELECT v FROM log`).map((r: any) => r.v), + }).toEqual({ + bodyRan: false, + conflictErrCode: "SQLITE_CONSTRAINT_PRIMARYKEY", + beginErrCode: "ERR_SQLITE_INVALID_TRANSACTION_STATE", + durable: [], + }); + }); + + test("tx.savepoint with a concurrent unawaited conflict is refused at SAVEPOINT execution time", async () => { + let bodyRan = false; + let beginErr: any; + await sql + .begin(async tx => { + // .catch() enqueues this query's execution microtask before SAVEPOINT's, without + // awaiting it; the conflict auto-rolls-back between the savepoint() call-time + // check and the SAVEPOINT command actually running. + tx`INSERT INTO t VALUES ('dup')`.catch(() => {}); + await tx.savepoint(async sp => { + bodyRan = true; + await sp`INSERT INTO log VALUES ('from-savepoint')`; + }); + }) + .catch(e => (beginErr = e)); + expect({ + bodyRan, + code: beginErr?.code, + durable: (await sql`SELECT v FROM log`).map((r: any) => r.v), + }).toEqual({ + bodyRan: false, + code: "ERR_SQLITE_INVALID_TRANSACTION_STATE", + durable: [], + }); + }); + + test("auto-rollback inside a savepoint propagates the original error and aborts the outer transaction", async () => { + let caught: any; + try { + await sql.begin(async tx => { + await tx`INSERT INTO log VALUES ('outer')`; + await tx.savepoint(async sp => { + await sp`INSERT INTO t VALUES ('dup')`; + }); + }); + } catch (e) { + caught = e; + } + expect({ + code: caught?.code, + durable: (await sql`SELECT v FROM log`).map((r: any) => r.v), + }).toEqual({ + code: "SQLITE_CONSTRAINT_PRIMARYKEY", + durable: [], + }); + }); + }); }); describe("SQLite-specific features", () => { @@ -2014,7 +2233,8 @@ describe("Memory and resource management", () => { await sql`CREATE TABLE stmt_test (id INTEGER PRIMARY KEY, value TEXT)`; - const iterations = 10000; + // debug+ASAN runs ~100x slower; 10k INSERTs there exceed the per-test timeout + const iterations = isDebug ? 1000 : 10000; for (let i = 0; i < iterations; i++) { await sql`INSERT INTO stmt_test (id, value) VALUES (${i}, ${"test" + i})`;