fix(db): bound pool disconnect so teardown can't eat CLI output (#1972)

pool.end() against PgBouncer transaction-mode never drained, so disconnect
blocked until the CLI's 10s force-exit fired and process.exit()'d mid-write,
truncating stdout (e.g. #1959's relational query returned empty). Add a
gbrain-owned endPoolBounded(pool): Promise.race of pool.end({timeout}) against
a hard timer, so teardown is bounded regardless of what postgres.js does and is
testable. connection-manager ends its direct + read pools concurrently so the
per-pool bounds don't stack. PGLite disconnect is unaffected.
This commit is contained in:
Garry Tan
2026-06-09 08:35:49 -07:00
parent 2f9e2136da
commit 752e2d2d0d
5 changed files with 98 additions and 6 deletions
+40
View File
@@ -0,0 +1,40 @@
/**
* #1972 — gbrain-owned hard bound on pool teardown.
*
* The bug: `pool.end()` against PgBouncer transaction-mode never drains, so
* disconnect blocked until the CLI's 10s force-exit fired and truncated stdout.
* postgres.js's own `{ timeout }` is internal (a stub ignores it; it's not a
* guarantee we own), so `endPoolBounded` wraps every end in a Promise.race we
* control. These tests assert the bound is real (resolves even when `.end()`
* never settles) and that we still pass `{ timeout }` so a healthy drain is fast.
*/
import { describe, test, expect } from 'bun:test';
import { endPoolBounded, POOL_END_TIMEOUT_SECONDS } from '../src/core/db.ts';
describe('endPoolBounded', () => {
test('resolves fast when .end() settles quickly, forwarding { timeout }', async () => {
let calledWith: unknown;
const pool = { end: async (opts?: { timeout?: number }) => { calledWith = opts; } };
const t0 = Date.now();
await endPoolBounded(pool);
expect(Date.now() - t0).toBeLessThan(500);
expect(calledWith).toEqual({ timeout: POOL_END_TIMEOUT_SECONDS });
});
test('resolves within the gbrain bound even when .end() NEVER settles', async () => {
// This is the PgBouncer hang: .end() returns a promise that never resolves.
// The bare `await pool.end()` would hang until the CLI's 10s force-exit.
const pool = { end: () => new Promise<void>(() => { /* never resolves */ }) };
const t0 = Date.now();
await endPoolBounded(pool);
const elapsed = Date.now() - t0;
expect(elapsed).toBeGreaterThanOrEqual(POOL_END_TIMEOUT_SECONDS * 1000);
expect(elapsed).toBeLessThan(5000); // well under the CLI's 10s force-exit deadline
});
test('never throws when .end() rejects (teardown must not propagate)', async () => {
const pool = { end: async () => { throw new Error('pool boom'); } };
await expect(endPoolBounded(pool)).resolves.toBeUndefined();
});
});
@@ -81,7 +81,10 @@ describe('postgres-engine / module-singleton ownership (#1471)', () => {
const disconnect = stripComments(extractFn(DB_SRC, 'disconnect'));
const snapshotIdx = disconnect.search(/const\s+s\s*=\s*sql/);
const nullIdx = disconnect.search(/\bsql\s*=\s*null/);
const endIdx = disconnect.search(/\bawait\s+s\.end\s*\(/);
// #1972: the pool end is now wrapped in the gbrain-owned hard bound
// `endPoolBounded(s)` instead of a bare `s.end()`. The ordering contract is
// unchanged: snapshot + null the singleton BEFORE awaiting the end.
const endIdx = disconnect.search(/\bawait\s+(?:s\.end\s*\(|endPoolBounded\s*\(\s*s\b)/);
expect(snapshotIdx).toBeGreaterThanOrEqual(0);
expect(nullIdx).toBeGreaterThanOrEqual(0);
expect(endIdx).toBeGreaterThanOrEqual(0);