From 9d7b7706e3454552cec0b82a10dc6395c4a0f08e Mon Sep 17 00:00:00 2001 From: Mariano Miguel Date: Sun, 9 Aug 2026 02:13:22 -0300 Subject: [PATCH] fix(drizzle): surface transaction commit failures instead of resolving successfully --- .../src/transactions/beginTransaction.ts | 42 +++++++++-------- .../src/transactions/commitTransaction.ts | 10 +++- test/database/int.spec.ts | 47 +++++++++++++++++++ 3 files changed, 78 insertions(+), 21 deletions(-) diff --git a/packages/drizzle/src/transactions/beginTransaction.ts b/packages/drizzle/src/transactions/beginTransaction.ts index c3887590666..f87416fb29c 100644 --- a/packages/drizzle/src/transactions/beginTransaction.ts +++ b/packages/drizzle/src/transactions/beginTransaction.ts @@ -29,26 +29,30 @@ export const beginTransaction: BeginTransaction = async function beginTransactio // over many files and we don't want to pass the `tx` around like that, // so instead, we "lift" up the `resolve` and `reject` methods // and will call them in our respective transaction methods - const done = this.drizzle - .transaction(async (tx) => { - transaction = tx - await new Promise((res, rej) => { - resolve = () => { - res() - return done - } - reject = () => { - // eslint-disable-next-line @typescript-eslint/prefer-promise-reject-errors - rej() - return done - } - transactionReady() - }) - }, options || this.transactionOptions) - .catch((err) => { - // Connection failed before callback ran - reject instead of hanging forever - transactionFailed(err) + const transactionPromise = this.drizzle.transaction(async (tx) => { + transaction = tx + await new Promise((res, rej) => { + resolve = () => { + res() + // return the raw transaction promise so a failed COMMIT rejects in + // commitTransaction instead of being absorbed by the catch below, + // which is a no-op once the transaction is ready + return transactionPromise + } + reject = () => { + // eslint-disable-next-line @typescript-eslint/prefer-promise-reject-errors + rej() + return done + } + transactionReady() }) + }, options || this.transactionOptions) + + // Connection failed before callback ran - reject instead of hanging forever. + // Also keeps rollback-path rejections handled. + const done = transactionPromise.catch((err) => { + transactionFailed(err) + }) // Need to wait until the transaction is ready // before binding its `resolve` and `reject` methods below diff --git a/packages/drizzle/src/transactions/commitTransaction.ts b/packages/drizzle/src/transactions/commitTransaction.ts index d5d0b0a9d36..d7da74f36db 100644 --- a/packages/drizzle/src/transactions/commitTransaction.ts +++ b/packages/drizzle/src/transactions/commitTransaction.ts @@ -18,7 +18,13 @@ export const commitTransaction: CommitTransaction = async function commitTransac try { await session.resolve() - } catch (_) { - await session.reject() + } catch (error) { + try { + await session.reject() + } catch (_) { + // rollback is best effort once the commit has failed — the commit + // error is the one the caller must see + } + throw error } } diff --git a/test/database/int.spec.ts b/test/database/int.spec.ts index b5819beb20f..b98f3e00490 100644 --- a/test/database/int.spec.ts +++ b/test/database/int.spec.ts @@ -15,6 +15,7 @@ import { migrateVersionsV1_V2, } from '@payloadcms/db-mongodb/migration-utils' import { randomUUID } from 'crypto' +import { sql } from 'drizzle-orm' import * as drizzlePg from 'drizzle-orm/pg-core' import * as drizzleSqlite from 'drizzle-orm/sqlite-core' import fs from 'fs' @@ -2339,6 +2340,52 @@ describe('database', () => { }, ) + it( + 'should surface commit failures to the caller instead of resolving (drizzle)', + { db: (adapter) => adapter.startsWith('postgres') || adapter === 'supabase' }, + async () => { + const req = { + payload, + user, + } as unknown as PayloadRequest + + await initTransaction(req) + const transactionID = req.transactionID as number | string + const session = ( + payload.db as unknown as { + sessions: Record< + number | string, + { db: { execute: (query: unknown) => Promise } } + > + } + ).sessions[transactionID] + + const doc = await payload.create({ collection, data: { title }, req }) + + // A constraint checked only at COMMIT time makes the commit itself + // fail while the connection stays healthy. Created inside the + // transaction, so the failed commit also rolls the table away. + await session.db.execute(sql` + CREATE TABLE commit_failure_probe ( + id integer PRIMARY KEY, + parent integer REFERENCES commit_failure_probe (id) DEFERRABLE INITIALLY DEFERRED + ) + `) + await session.db.execute(sql`INSERT INTO commit_failure_probe (id, parent) VALUES (1, 2)`) + + await expect(commitTransaction(req)).rejects.toThrow() + await killTransaction(req) + + // The write must not be reported as persisted when the commit failed + await expect(() => + payload.findByID({ + id: doc.id, + collection, + }), + ).rejects.toThrow('Not Found') + }, + ) + it( 'should throw error when beginTransaction fails to connect (mongo)', {