From b346fc6a9029808b82c3ab553256a07182a59b56 Mon Sep 17 00:00:00 2001 From: CTO Date: Fri, 7 Aug 2026 11:46:08 +0000 Subject: [PATCH] =?UTF-8?q?fix(db):=20retry=20on=20ECONNREFUSED=20?= =?UTF-8?q?=E2=80=94=20stop=20exiting=201=20on=20a=20Postgres=20endpoint?= =?UTF-8?q?=20gap=20(BLO-21615)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `gbrain serve` has had connect retry with exponential backoff since v0.21, but it never fired for the most common transient failure in a container environment, because that error was classified as permanent. `connect ECONNREFUSED :` is the kernel's connect(2) refusal. It is a different string from Postgres's own `connection refused` prose and was in neither retry list, so `isRetryableDbConnectError` returned false, `connectWithRetry` rethrew on attempt 1, and the process exited 1. On Kubernetes that is what kube-proxy REJECTs with for the entire duration of any database pod restart, so every DB blip became a container restart: 125 restarts over 17 days on the gbrain-mcp `admin-ui` container, each a four-second life ending in the single line Cannot connect to database: connect ECONNREFUSED 10.99.216.174:5432. Fix: Check your connection URL in ~/.gbrain/config.json The retry machinery was always correct and always wired in. Only the classification was wrong. Two changes: - Add /ECONNREFUSED/i to CONN_PATTERNS in retry-matcher.ts. - Delegate db.ts:isRetryableDbConnectError to retry-matcher.ts. db.ts kept its own inline copy of the pattern list — the exact drift retry-matcher.ts was written to end — and it had fallen five patterns behind: no ECONNREFUSED, no 08xxx SQLSTATE class, no CONNECTION_ENDED, no 53300. So connect-time retry recovered from strictly fewer conditions than every other retry site. There is now one list, not two. Tests pin the exact production error string at both levels: the matcher, and connectWithRetry surviving a two-attempt refusal window. Both fail on the pre-change source (connectWithRetry throws on attempt 1) and pass after. Permanent errors — missing extension, missing relation, syntax error — are still not retried. Co-Authored-By: Claude --- CHANGELOG.md | 12 ++++++++++++ VERSION | 2 +- package.json | 2 +- src/core/db.ts | 26 +++++++++++++------------- src/core/retry-matcher.ts | 8 ++++++++ test/minions.test.ts | 29 +++++++++++++++++++++++++++++ test/retry-matcher.test.ts | 20 ++++++++++++++++++++ 7 files changed, 84 insertions(+), 15 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 565627272c..3f5d1ec1da 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,18 @@ All notable changes to GBrain will be documented in this file. +## [0.42.51.1] - 2026-08-07 + +**A server starting up while Postgres is briefly unreachable now waits for it instead of dying.** `gbrain serve` has had connect retry with exponential backoff since v0.21 — but it never fired for the single most common transient failure in a container environment, because that error was classified as permanent. + +`connect ECONNREFUSED :` is what the kernel returns when nothing is accepting on the port, and on Kubernetes it is what kube-proxy returns for the entire duration of any database pod restart. It is a different string from Postgres's own `connection refused` prose, and it was not in the retry list — so `connectWithRetry` rethrew on the first attempt and the process exited 1. Observed in production as 125 restarts over 17 days of a `gbrain serve --http` container, each one a four-second life ending in `Cannot connect to database: connect ECONNREFUSED …`. + +The retry machinery was always correct and always wired in; only the classification was wrong. + +### Fixed +- **`ECONNREFUSED` is treated as a transient connect error.** A server starting during a database restart, failover, or endpoint gap now retries with backoff and comes up, instead of exiting 1 and relying on a supervisor to restart it. Genuinely permanent failures (missing extension, missing relation, syntax error) are still not retried. +- **`db.ts` no longer keeps a second, drifted copy of the retry-pattern list.** `isRetryableDbConnectError` delegates to `retry-matcher.ts`, the module introduced precisely to stop this drift. The private copy had fallen five patterns behind — it was missing `ECONNREFUSED`, the `08xxx` SQLSTATE class, `CONNECTION_ENDED`, and `53300` — so connect-time retry silently recovered from strictly fewer conditions than every other retry site in the codebase. + ## [0.42.51.0] - 2026-06-17 **`gbrain sync` stops bottlenecking all its workers on a single database row, a malformed checkpoint can no longer wedge a source, and `gbrain doctor` tells an actively-running sync apart from a stuck one.** A slow source that fell behind HEAD could read as permanently stale even while it imported every cycle: sync was single-core-bound at the database layer, so handing it more workers didn't help, and the freshness check couldn't see that a sync was in fact running. diff --git a/VERSION b/VERSION index 6a6c1a02ae..6779db8ac4 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -0.42.51.0 +0.42.51.1 diff --git a/package.json b/package.json index 82b43c3bc8..408db3c650 100644 --- a/package.json +++ b/package.json @@ -143,5 +143,5 @@ "bun": ">=1.3.10" }, "license": "MIT", - "version": "0.42.51.0" + "version": "0.42.51.1" } diff --git a/src/core/db.ts b/src/core/db.ts index 99dae2136a..bb50760427 100644 --- a/src/core/db.ts +++ b/src/core/db.ts @@ -322,19 +322,19 @@ export async function withTransaction(fn: (tx: ReturnType) = }) as Promise; } -const RETRYABLE_DB_CONNECT_PATTERNS = [ - /password authentication failed/i, - /connection refused/i, - /the database system is starting up/i, - /Connection terminated unexpectedly/i, - /ECONNRESET/i, -]; - -export function isRetryableDbConnectError(err: unknown): boolean { - const msg = err instanceof Error ? err.message : String(err); - if (!msg) return false; - return RETRYABLE_DB_CONNECT_PATTERNS.some(p => p.test(msg)); -} +// BLO-21615: this predicate used to carry its own inline 5-pattern list — the +// exact drift `retry-matcher.ts` was created to end ("Before this module these +// predicates lived inline at each site and drifted over time"). db.ts was never +// migrated, so it silently stayed a subset: it lacked ECONNREFUSED, the 08xxx +// SQLSTATE class, CONNECTION_ENDED and 53300. Consequence in production: a +// `serve --http` starting during a DB-pod restart got `connect ECONNREFUSED +// :5432`, connectWithRetry classified it NON-retryable, rethrew on +// attempt 1, and the container exited 1 — 125 restarts over 17 days on +// gbrain-mcp/admin-ui. The retry+backoff was always wired; only the +// classification was wrong. Delegate so there is one list, not two. +// Imported (not just re-exported) so connectWithRetry below binds it locally. +import { isRetryableConnError as isRetryableDbConnectError } from './retry-matcher.ts'; +export { isRetryableDbConnectError }; export interface ConnectWithRetryOpts { attempts?: number; diff --git a/src/core/retry-matcher.ts b/src/core/retry-matcher.ts index c25605e8d8..c28a1c7b0f 100644 --- a/src/core/retry-matcher.ts +++ b/src/core/retry-matcher.ts @@ -17,6 +17,14 @@ const CONN_PATTERNS = [ /the database system is starting up/i, /Connection terminated unexpectedly/i, /ECONNRESET/i, + // BLO-21615: the kernel's connect(2) refusal, as surfaced by Node/Bun — + // "connect ECONNREFUSED :". Distinct from ECONNRESET (peer reset + // an ESTABLISHED socket) and NOT matched by /connection refused/i above, + // which only catches Postgres's own prose form. On Kubernetes this is what + // kube-proxy REJECTs with while a Service has zero ready endpoints — i.e. + // for the whole of any DB pod restart — so it is the single most common + // transient connect failure in-cluster, and it was the one pattern missing. + /ECONNREFUSED/i, /connection.*closed/i, /server closed the connection/i, /could not connect to server/i, diff --git a/test/minions.test.ts b/test/minions.test.ts index ec990406a8..75bd1be1f6 100644 --- a/test/minions.test.ts +++ b/test/minions.test.ts @@ -2177,6 +2177,35 @@ describe('connectWithRetry / isRetryableDbConnectError', () => { expect(isRetryableDbConnectError(new Error('something happened: ECONNRESET'))).toBe(true); }); + // BLO-21615: db.ts kept its own inline pattern list that lacked ECONNREFUSED, + // so connectWithRetry rethrew on attempt 1 and `serve --http` exited 1 every + // time it started during a DB-pod restart. Now delegates to retry-matcher. + test('isRetryableDbConnectError matches ECONNREFUSED (BLO-21615)', async () => { + const { isRetryableDbConnectError } = await import('../src/core/db.ts'); + expect(isRetryableDbConnectError(new Error('connect ECONNREFUSED 10.99.216.174:5432'))).toBe(true); + }); + + test('connectWithRetry survives an ECONNREFUSED window (BLO-21615)', async () => { + const { connectWithRetry } = await import('../src/core/db.ts'); + let attempts = 0; + const fakeEngine = { + connect: async () => { + attempts++; + // Two refusals — a DB pod restart leaving the Service with zero ready + // endpoints — then the endpoint comes back. + if (attempts <= 2) { + throw new Error( + 'Cannot connect to database: connect ECONNREFUSED 10.99.216.174:5432. ' + + 'Fix: Check your connection URL in ~/.gbrain/config.json', + ); + } + }, + } as unknown as Parameters[0]; + + await connectWithRetry(fakeEngine, { database_url: 'postgres://x' }, { baseDelayMs: 1, log: () => {} }); + expect(attempts).toBe(3); + }); + test('isRetryableDbConnectError rejects permanent errors', async () => { const { isRetryableDbConnectError } = await import('../src/core/db.ts'); expect(isRetryableDbConnectError(new Error('extension "vector" does not exist'))).toBe(false); diff --git a/test/retry-matcher.test.ts b/test/retry-matcher.test.ts index da066ef828..a383a8841c 100644 --- a/test/retry-matcher.test.ts +++ b/test/retry-matcher.test.ts @@ -58,6 +58,26 @@ describe('isRetryableConnError', () => { expect(isRetryableConnError(new Error('ECONNRESET'))).toBe(true); }); + // BLO-21615 regression. ECONNREFUSED is a DIFFERENT failure from ECONNRESET + // (refused connect(2) vs. reset of an established socket) and is not matched + // by /connection refused/i, so it needs its own pattern and its own guard. + test('matches ECONNREFUSED', () => { + expect(isRetryableConnError(new Error('connect ECONNREFUSED 10.0.0.1:5432'))).toBe(true); + }); + + test('matches the exact GBrainError text that crash-looped gbrain-mcp/admin-ui', () => { + // Byte-for-byte the message GBrainError composes as + // `${problem}: ${cause_description}. Fix: ${fix}` from db.ts connect(), + // and byte-for-byte the single line `kubectl logs --previous` showed for + // each of the 125 exit-1 restarts. If this stops matching, the container + // goes back to dying on any Postgres endpoint gap. + const err = new Error( + 'Cannot connect to database: connect ECONNREFUSED 10.99.216.174:5432. ' + + 'Fix: Check your connection URL in ~/.gbrain/config.json', + ); + expect(isRetryableConnError(err)).toBe(true); + }); + test('matches database-starting-up', () => { expect( isRetryableConnError(new Error('the database system is starting up'))