Skip to content

Commit e73f163

Browse files
authored
fix(selfhost): guard pg-adapter's rollback so it can't mask the original batch error (#6392)
batch()'s catch block called client.query("ROLLBACK") unguarded, so a rollback failure (plausible exactly when the original error was a connection failure) would propagate instead of the real cause. Wrap it in the same defensive try/catch { ignore } the SQLite adapter's batch() already uses. Added an integration test in test/integration/selfhost-pg.test.ts (gated on PG_TEST_URL) that intercepts a real client's ROLLBACK call to force it to throw, confirming the original batch error still surfaces. Verified against a real local Postgres 16 container that the test genuinely fails without the fix and passes with it. Closes #6282
1 parent f2203a2 commit e73f163

2 files changed

Lines changed: 37 additions & 1 deletion

File tree

src/selfhost/pg-adapter.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,11 @@ export function createPgAdapter(pool: Pool): D1Database {
7171
await client.query("COMMIT");
7272
return out;
7373
} catch (error) {
74-
await client.query("ROLLBACK");
74+
try {
75+
await client.query("ROLLBACK");
76+
} catch {
77+
/* ignore */
78+
}
7579
throw error;
7680
} finally {
7781
client.release();

test/integration/selfhost-pg.test.ts

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,38 @@ suite("Postgres backend (#977) — real Postgres", () => {
5959
expect(still?.n).toBe(1); // the DELETE rolled back
6060
});
6161

62+
it("regression: a rollback failure never masks the original batch error (#6282)", async () => {
63+
// Force the ROLLBACK itself to throw, simulating the exact scenario (a connection failure) where the
64+
// original error and a subsequent rollback failure would otherwise race to be the thrown error. Only
65+
// ROLLBACK is intercepted -- every other query still runs for real against the live Postgres instance.
66+
const realClient = await pool.connect();
67+
const originalQuery = realClient.query.bind(realClient);
68+
let rollbackAttempted = false;
69+
try {
70+
// @ts-expect-error -- intentionally narrowing pg's overloaded query() signature for this simulation
71+
realClient.query = async (...args: Parameters<typeof originalQuery>) => {
72+
if (args[0] === "ROLLBACK") {
73+
rollbackAttempted = true;
74+
throw new Error("simulated rollback failure");
75+
}
76+
return originalQuery(...args);
77+
};
78+
const fakePool = { connect: async () => realClient } as unknown as pg.Pool;
79+
const db = createPgAdapter(fakePool);
80+
81+
await expect(
82+
db.batch([
83+
db.prepare("INSERT INTO system_flags (key, value) VALUES (?, ?) , bad-sql").bind("z", "1"), // syntax error
84+
]),
85+
).rejects.toThrow(/bad-sql|syntax/i); // the ORIGINAL error surfaces, not "simulated rollback failure"
86+
expect(rollbackAttempted).toBe(true);
87+
} finally {
88+
// batch()'s own `finally` already released this client back to the pool -- just restore the
89+
// query override so the shared pool's client isn't left poisoned for later tests.
90+
realClient.query = originalQuery;
91+
}
92+
});
93+
6294
it("prunes rows past the retention window and processJob('prune-retention') does not dead-letter (regression for the live self-host incident: job _selfhost_jobs.id=61132 failed with 'column \"rowid\" does not exist')", async () => {
6395
const db = createPgAdapter(pool);
6496
const env = { DB: db } as unknown as Env;

0 commit comments

Comments
 (0)