fix(postgres): module-mode reconnect preserves the shared singleton (#1745)

reconnect() branches on connection style. Module-singleton engines re-establish
idempotently via db.connect() (no-op when alive) + refresh the ConnectionManager
read pool, never db.disconnect() — so a transient blip no longer nulls the shared
sql out from under concurrent ops (which threw 'connect() has not been called').
Fail-loud on real connect failure. Instance pools keep teardown+recreate.
This commit is contained in:
Garry Tan
2026-06-03 01:29:34 -07:00
parent b5abf5beda
commit 46f944bb7f
+35 -4
View File
@@ -4775,16 +4775,47 @@ export class PostgresEngine implements BrainEngine {
}
/**
* Reconnect the engine by tearing down the current pool and creating a fresh one.
* No-ops if no saved config (module-singleton mode) or if already reconnecting.
* Reconnect the engine after a transient connection blip.
*
* v0.42.11.0 (#1745): branch on connection style.
*
* - INSTANCE pools (worker engines, `poolSize` set) own their `_sql` — tearing
* it down and rebuilding is correct and isolated; nobody else shares it.
*
* - MODULE-singleton engines SHARE `db.ts`'s `sql`. Calling `db.disconnect()`
* here (via `this.disconnect()`) nulls it out from under EVERY concurrent op
* (other dream-cycle phases, minion-queue `promoteDelayed`), which then throw
* "connect() has not been called" in the disconnect→connect window. postgres.js
* already auto-replaces dead sockets inside its pool, so a transient blip
* recovers WITHOUT a teardown. Recover idempotently instead: `db.connect()`
* is a no-op when the singleton is alive (the common case) and re-establishes
* it only if some other path nulled it — never introducing a null window.
*
* Scope: this fixes the singleton-NULL-window bug specifically. It does NOT
* rebuild a genuinely WEDGED-but-live pool (db.connect() no-ops there); that is
* a different failure mode postgres.js owns.
*/
async reconnect(): Promise<void> {
if (!this._savedConfig || this._reconnecting) return;
if (this._connectionStyle !== 'instance') {
// Module-singleton: never tear down the shared pool. db.connect() is
// idempotent (no-op when the singleton is alive — the common #1745 path).
// FAIL-LOUD (codex): do NOT swallow a real connect failure — a swallowed
// error would make reconnect() resolve "successfully" and let the
// supervisor reset its health-failure counter / emit db_reconnected when
// the DB is actually down. A throw propagates as the real cause (matches
// the withRetry+reconnect contract and the instance path's posture).
await db.connect(this._savedConfig);
// If db.connect() RE-CREATED the singleton (another path nulled it), the
// ConnectionManager set at connect-time still points at the ended old
// pool. Refresh it. Idempotent no-op when the singleton was already alive.
this.connectionManager?.setReadPool(db.getConnection());
return;
}
this._reconnecting = true;
try {
// Tear down old pool (best-effort — it may already be dead)
// Instance pool: tear down old pool (best-effort — it may already be dead).
try { await this.disconnect(); } catch { /* swallow */ }
// Create fresh pool
await this.connect(this._savedConfig);
} finally {
this._reconnecting = false;