Skip to content

Commit 98645c8

Browse files
dmealingclaude
andauthored
fix(integration-tests): gate shared-sidecar mode on a real connection (kills the ts-integration flake) (#167)
The ts integration lane intermittently hung to the scenarios' 60s test timeout on the main push path (twice: the #163 and #165 merges), always in the sidecar-backed generated-routes lane (create-201 / jsonb-open-bag-roundtrip). Root cause: shared-sidecar mode (startOnSharedPostgres) did `admin.connect()` immediately with NO readiness gate and NO client-side connect timeout — unlike the per-container path, which deliberately gates on a real `canConnect()` because `pg_isready` (what the CI `services: postgres` health-check uses) can report success during postgres' first-boot init window. Under CI host contention the sidecar is also transiently slow to accept connections. With no connect timeout, pg's first connection to a not-yet-answering sidecar hangs indefinitely → the scenario burns its full 60s timeout instead of retrying. Fix: extend the file's existing readiness pattern to shared mode — `waitForSharedReady()` polls a real short-timeout probe connection until the sidecar answers (or a 45s budget, kept under the 60s test timeout so the retry loop can actually succeed), and every Client now carries connectionTimeoutMillis so a stalled attempt fails fast and is retried rather than hanging. When the sidecar is healthy the gate returns on the first probe (~ms); when it never comes up, it throws a clear error instead of an opaque timeout. Verified: the two previously-flaky lanes (api-contract-generated, api-contract-jsonb) run 23/23 green against a real local sidecar in shared mode. The change can only prevent hangs — no behavior change on the healthy path. Claude-Session: https://claude.ai/code/session_01Ew1XfYSbEAezxjs9opynAe Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 53fcfcb commit 98645c8

1 file changed

Lines changed: 34 additions & 3 deletions

File tree

server/typescript/packages/integration-tests/src/postgres-container.ts

Lines changed: 34 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,14 @@ import { Client } from "pg";
3131
/** Env var naming the shared CI Postgres sidecar (admin URL). Unset = per-container fallback. */
3232
const SHARED_PG_URL_ENV = "METAOBJECTS_TEST_PG_URL";
3333

34+
/** Per-attempt connect timeout so a stalled connection fails fast instead of
35+
* hanging (pg's default has no client-side connect timeout). */
36+
const CONNECT_TIMEOUT_MS = 10_000;
37+
/** Total budget for the shared sidecar to start answering real connections. Kept
38+
* under the scenarios' 60s test timeout so the retry loop has headroom to succeed
39+
* (and to leave time for CREATE DATABASE + server boot) rather than racing it. */
40+
const SHARED_READY_DEADLINE_MS = 45_000;
41+
3442
export interface RunningPg {
3543
readonly connectionUri: string;
3644
stop(): Promise<void>;
@@ -62,8 +70,17 @@ export async function startPostgres(image = "postgres:16-alpine"): Promise<Runni
6270
// on stop. A dedicated database per scenario preserves the per-container path's
6371
// "pristine empty DB" isolation.
6472
async function startOnSharedPostgres(adminUri: string): Promise<RunningPg> {
73+
// Gate on a REAL connection before doing anything. The CI `services: postgres`
74+
// health-check uses `pg_isready`, which — like the per-container path documents
75+
// (waitForPgReady) — can report success during postgres' first-boot init window;
76+
// and under CI host contention the server can be transiently slow to accept
77+
// connections. Without this gate the first admin.connect() below hangs until the
78+
// scenario's 60s test timeout (the intermittent ts-integration flake). Retrying a
79+
// short-timeout probe waits the sidecar out gracefully instead.
80+
await waitForSharedReady(adminUri);
81+
6582
const dbName = `mo_test_${randomUUID().replace(/-/g, "")}`;
66-
const admin = new Client({ connectionString: adminUri });
83+
const admin = new Client({ connectionString: adminUri, connectionTimeoutMillis: CONNECT_TIMEOUT_MS });
6784
await admin.connect();
6885
try {
6986
// Identifier is a fixed-shape generated name (no user input) — safe to inline.
@@ -75,7 +92,7 @@ async function startOnSharedPostgres(adminUri: string): Promise<RunningPg> {
7592
return {
7693
connectionUri,
7794
stop: async () => {
78-
const drop = new Client({ connectionString: adminUri });
95+
const drop = new Client({ connectionString: adminUri, connectionTimeoutMillis: CONNECT_TIMEOUT_MS });
7996
await drop.connect();
8097
try {
8198
await drop.query(`DROP DATABASE IF EXISTS "${dbName}" WITH (FORCE)`);
@@ -86,6 +103,20 @@ async function startOnSharedPostgres(adminUri: string): Promise<RunningPg> {
86103
};
87104
}
88105

106+
// Poll a real client connection to the shared sidecar until it answers or the
107+
// deadline elapses — the shared-mode analogue of waitForPgReady (mode 2). Cheap
108+
// (~ms) when the sidecar is already healthy; only retries when it is genuinely slow.
109+
async function waitForSharedReady(adminUri: string): Promise<void> {
110+
const deadline = Date.now() + SHARED_READY_DEADLINE_MS;
111+
while (Date.now() < deadline) {
112+
if (await canConnect(adminUri)) return;
113+
await new Promise((res) => setTimeout(res, 250));
114+
}
115+
throw new Error(
116+
`shared Postgres sidecar (${SHARED_PG_URL_ENV}) did not accept a connection within ${SHARED_READY_DEADLINE_MS / 1000}s`,
117+
);
118+
}
119+
89120
/** Replace the database (path) component of a postgres:// URL. */
90121
function withDatabase(uri: string, dbName: string): string {
91122
const u = new URL(uri);
@@ -113,7 +144,7 @@ async function waitForPgReady(name: string, uri: string): Promise<void> {
113144
}
114145

115146
async function canConnect(uri: string): Promise<boolean> {
116-
const client = new Client({ connectionString: uri });
147+
const client = new Client({ connectionString: uri, connectionTimeoutMillis: CONNECT_TIMEOUT_MS });
117148
try {
118149
await client.connect();
119150
await client.query("SELECT 1");

0 commit comments

Comments
 (0)