test(testcontainers): add a DB connection-blip harness - #11
Conversation
Source PR: triggerdotdev#4862 Source head: a0da053
|
![shipwright-agent[bot]](https://avatars.githubusercontent.com/u/140162738?s=60&v=4){kind=small}
| import { PrismaClient } from "@trigger.dev/database"; | ||
| import { postgresBlipTest } from "./index"; | ||
|
|
||
| // A minimal infra retry, standing in for the shared read-retry util so this |
There was a problem hiding this comment.
Shipwright · HIGH
'withRetry' is a local reimplementation of a shared retry utility, with a comment admitting it is a stand-in.
Impact: 'withRetry' is a local reimplementation of a shared retry utility, with a comment admitting it is a stand-in. This duplicates behavior and will drift from the real utility; a new hire cannot tell whether the retry semantics here match production.
Suggested fix: Review the cited evidence, fix the risk if confirmed, and rerun Shipwright.
| } | ||
| ); | ||
|
|
||
| export type PostgresBlipTestContext = PostgresTestContext & { blip: DbBlipController }; |
There was a problem hiding this comment.
Shipwright · HIGH
The 'blipFromContainer' helper is exported indirectly through 'postgresBlipTest' but its type signature mixes 'StartedPostgreSqlContainer' and 'TestContext' in a way that obscures
Impact: The 'blipFromContainer' helper is exported indirectly through 'postgresBlipTest' but its type signature mixes 'StartedPostgreSqlContainer' and 'TestContext' in a way that obscures which fields are actually available. The 'TestContext' intersection appears unused and will confuse readers about the contract.
Suggested fix: Review the cited evidence, fix the risk if confirmed, and rerun Shipwright.
| // wildcard `_`) would wrongly match and terminate it. The literal matcher must not, | ||
| // so the sever times out instead of killing the wrong statement. | ||
| postgresBlipTest( | ||
| "severDuringNextStatement matches queryContains literally, not as an ILIKE pattern", |
There was a problem hiding this comment.
Shipwright · HIGH
The literal-match regression test is racy: the slow query is started before the sever call, but the test only awaits the sever rejection and then disposes the client before awaitin
Impact: The literal-match regression test is racy: the slow query is started before the sever call, but the test only awaits the sever rejection and then disposes the client before awaiting 'slow'. If the query is still running when 'dispose()' runs, 'pool.end()' may hang or the catch may never settle, causing a flaky timeout or unhandled rejection.
Suggested fix: Review the cited evidence, fix the risk if confirmed, and rerun Shipwright.
| let nonIdempotentAttempts = 0; | ||
| const nonIdempotentWrite = async () => { | ||
| nonIdempotentAttempts++; | ||
| await client.$executeRawUnsafe( |
There was a problem hiding this comment.
Shipwright · HIGH
The non-idempotent write test asserts 'countTag(...) === 2', which depends on the retry actually re-executing the INSERT after the severed 'pg_sleep'.
Impact: The non-idempotent write test asserts 'countTag(...) === 2', which depends on the retry actually re-executing the INSERT after the severed 'pg_sleep'. If the sever lands before the INSERT commits, the retry produces 1 row and the test fails; if the sever lands after the sleep completes, the first attempt succeeds and the test also fails. This makes the test timing-dependent and likely flaky in CI.
Suggested fix: Review the cited evidence, fix the risk if confirmed, and rerun Shipwright.
| `SELECT pg_terminate_backend(pid) AS terminated | ||
| FROM pg_stat_activity | ||
| WHERE datname = current_database() | ||
| AND pid <> pg_backend_pid() |
There was a problem hiding this comment.
Shipwright · HIGH
'severIdle' and 'severDuringNextStatement' use 'pg_terminate_backend' against all client backends except the admin connection.
Impact: 'severIdle' and 'severDuringNextStatement' use 'pg_terminate_backend' against all client backends except the admin connection. If a test database is shared or a non-test client connects with a different 'application_name', this harness will terminate unrelated backends, potentially disrupting other tests or local development sessions.
Suggested fix: Review the cited evidence, fix the risk if confirmed, and rerun Shipwright.
| await admin.connect(); | ||
| // Swallow async connection errors so a consumer that severs a DB the admin | ||
| // isn't excluded from (or drops it while open) can't crash the test worker. | ||
| admin.on("error", () => {}); |
There was a problem hiding this comment.
Shipwright · HIGH
The admin connection swallows all 'error' events with an empty handler.
Impact: The admin connection swallows all 'error' events with an empty handler. This can hide authentication failures, connection drops, or protocol errors during setup, causing the harness to fail later with misleading 'no active statement' errors instead of surfacing the real cause.
Suggested fix: Review the cited evidence, fix the risk if confirmed, and rerun Shipwright.
1 participant
Summary
Adds a test-only harness for simulating a Postgres connection blip, so tests can prove their database code survives a dropped connection. It exports
createDbBlipControllerand apostgresBlipTestfixture from@internal/testcontainers.How it works
The harness severs connections from a separate admin connection using
pg_terminate_backend, scoped to the test's own database, so it composes with any Prisma client under test and stays isolated across parallel tests. Two modes:severIdle()kills idle backends. A pooled (driver-adapter) client absorbs this transparently: the pool evicts the dead connection and the next query just works.severDuringNextStatement()kills a statement mid-flight, surfacing a connection error to the caller; the client then recovers on the next retry.The bundled tests demonstrate both, plus the correctness property that matters before adding retries anywhere: a non-idempotent write double-applies when retried after a post-commit blip, while an idempotent write (deterministic id plus
ON CONFLICT) stays at exactly one row.Source merge-base:
43ecf15f80277c0eb931c08ddd89de55179cc795Source head:
a0da053d88d3f3fe9d221f0b950c94075fccabc1