You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
If a backend disconnects while a sql.begin() callback is awaiting other work, begin() rejects but the callback keeps running. Once the pool reconnects, that callback can issue queries on the replacement connection. Returning or throwing from it can also commit or roll back a different transaction. Queries still queued inside the original transaction can remain pending indefinitely.
Keep the close error on the transaction, reject its queued queries, and reject subsequent queries through that transaction's handler, including automatic commit/rollback. Clear the connection's pending write and query/result/error state before reuse, too. Without that reset, the first query after reconnect can receive the old backend's fatal error.
The four regression tests cover queued queries, a query through the disconnected transaction, and late commit/rollback. The latter three share a fixture that pauses the original callback until a replacement transaction is open. No new dependencies or public API changes.
Related to #1204. This reproduction specifically covers a backend disconnect followed by connection reuse; I haven't verified that it explains the original report.
Reproduce
Save the following as repro.mjs in the repository root. It needs Node 22 and a PostgreSQL login that can terminate its own other sessions.
importassertfrom'node:assert/strict'importpostgresfrom'./src/index.js'constpool=postgres({max: 1})constadmin=postgres({max: 1})letresume,connectedconstgate=newPromise(resolve=>{resume=resolve})constready=newPromise(resolve=>{connected=resolve})constfailed=pool.begin(asyncsql=>{connected((awaitsql`select 1`).state.pid)awaitgate}).catch(error=>error)try{awaitadmin`select pg_terminate_backend(${awaitready}::int)`assert.equal((awaitfailed).code,'CONNECTION_CLOSED')awaitpool.begin(asyncsql=>{const[{id: before}]=awaitsql`select txid_current()::text as id`resume()awaitnewPromise(resolve=>setImmediate(resolve))const[{id: after}]=awaitsql`select txid_current()::text as id`console.log({ before, after })assert.equal(after,before)})}finally{resume()awaitPromise.all([pool.end({timeout: 0}),admin.end({timeout: 0})])}
Wait for pg_isready to report that the server is accepting connections before running the script. On upstream 411429e, the assertion fails because the transaction IDs differ: the old callback has committed the replacement transaction. On this branch they stay equal and the script exits successfully. I verified both outcomes.
Tests
The first commit, 0282301, adds only the regression tests; the second applies the fix. With the PostgreSQL setup from the existing test workflow in place:
The first run times out in Disconnect rejects queued transaction queries and allows reconnect. The fixed branch passes. The full suite needs both PostgreSQL servers from the workflow (ports 5432 and 5433), SSL, logical replication, prepared transactions, the PostgreSQL CLI tools, and Deno 1.x. Use disposable databases: the existing bootstrap changes server settings and recreates its test database and roles.
Validated against PostgreSQL 17.11:
Node 12.22.12 and 22.22.0: all 268 tests pass in both ESM and CommonJS.
Deno 1.46.3: all 268 tests pass.
Bun 1.4.2: the standalone reproduction and focused probes for all four failure cases pass. The full suite cannot register tests because tests/test.js:20 assumes a Node stack format; the same failure occurs on unmodified upstream.
Thanks for this PR. We tested it together with #1209: the two fix every crash case we could reproduce. We found one case that is still broken, and a one-line fix for it.
What we tested
postgres 3.4.9, Node 26, PostgreSQL 18.4 and 18.6 (same results on both)
max: 1, so the same connection is always reused
On 3.4.9 without the PRs
Scenario
What happens
transaction_timeout or pg_terminate_backend during a query in sql.begin()
Crash: the nextWrite TypeError from #1133. If you catch it, the connection never comes back: every reconnect ends in CONNECT_TIMEOUT.
idle_in_transaction_session_timeout, then another begin() reuses the connection
The dead transaction's INSERT and COMMIT run inside the other transaction. That transaction throws, but its inserts stay committed (#1216).
Any kill during a query
The next query fails with the old backend's 57P01 / 25P04 error.
A plain null check in nextWrite (as in #1168) is not enough. It turns the crash into the same endless CONNECT_TIMEOUT.
Symptom: after an ECONNRESET (for example a failover), one query works. After that the connection hangs forever.
Cause:
On a reset, the socket emits 'error' first and 'close' a moment later. Promise callbacks run in between.
In that gap, begin() sends its rollback. The socket is still set and closedError is not set yet, so the rollback is added to sent.
closed(true) only clears sent when !hadError, so the rollback stays there.
After reconnecting, the connection treats that old rollback as in flight and waits for a reply that never comes.
Repro:
importnetfrom"node:net";importpostgresfrom"postgres";// A TCP proxy in front of Postgres, so we can reset the socket like a failover would.constpairs=[];constproxy=net.createServer((client)=>{constupstream=net.connect(5432,"127.0.0.1");client.pipe(upstream).pipe(client);client.on("error",()=>{});upstream.on("error",()=>{});pairs.push([client,upstream]);}).listen(0,"127.0.0.1");awaitnewPromise((r)=>proxy.once("listening",r));constsql=postgres({host: "127.0.0.1",port: proxy.address().port,max: 1/* , user, password */});awaitsql.begin(async(tx)=>{setTimeout(()=>pairs.forEach(([c,u])=>(u.destroy(),c.resetAndDestroy())),500);awaittx`select pg_sleep(3)`;}).catch((e)=>console.log("begin rejected:",e.code));constask=(tag)=>sql`select ${tag}::text as tag`.then(([r])=>console.log(tag,"->",r.tag));awaitask("Q1");awaitPromise.race([ask("Q2"),newPromise((r)=>setTimeout(()=>r(console.log("Q2 still pending after 3s")),3000))]);process.exit(0);
We independently found and patched the same two issues in production (3.4.9): the stale chunk/nextWriteTimer that leaves reconnects ending in CONNECT_TIMEOUT, and the stale query/errorResponse that replays the old backend's 57P01 onto the next query. Our patch makes the same two resets, so +1 to this approach. We also saw sql.end() never resolve while the connection was in that stale state.
One path this PR doesn't reach: closed() returns early via if (initial) return reconnect() before the new reset lines run. So a connection killed during startup (the fetch_types query) still replays its 57P01 onto the first query, plus an unhandled rejection. Repro and a one-line fix in #1223. We confirmed this PR alone doesn't change the output there.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #1216.
If a backend disconnects while a
sql.begin()callback is awaiting other work,begin()rejects but the callback keeps running. Once the pool reconnects, that callback can issue queries on the replacement connection. Returning or throwing from it can also commit or roll back a different transaction. Queries still queued inside the original transaction can remain pending indefinitely.Keep the close error on the transaction, reject its queued queries, and reject subsequent queries through that transaction's handler, including automatic commit/rollback. Clear the connection's pending write and query/result/error state before reuse, too. Without that reset, the first query after reconnect can receive the old backend's fatal error.
The four regression tests cover queued queries, a query through the disconnected transaction, and late commit/rollback. The latter three share a fixture that pauses the original callback until a replacement transaction is open. No new dependencies or public API changes.
Related to #1204. This reproduction specifically covers a backend disconnect followed by connection reuse; I haven't verified that it explains the original report.
Reproduce
Save the following as
repro.mjsin the repository root. It needs Node 22 and a PostgreSQL login that can terminate its own other sessions.For a disposable local server:
docker run --rm -d --name postgres-disconnect-repro \ -p 127.0.0.1:55432:5432 \ -e POSTGRES_HOST_AUTH_METHOD=trust postgres:17 docker exec postgres-disconnect-repro pg_isready -U postgres PGHOST=127.0.0.1 PGPORT=55432 PGUSER=postgres PGDATABASE=postgres node repro.mjs docker stop postgres-disconnect-reproWait for
pg_isreadyto report that the server is accepting connections before running the script. On upstream411429e, the assertion fails because the transaction IDs differ: the old callback has committed the replacement transaction. On this branch they stay equal and the script exits successfully. I verified both outcomes.Tests
The first commit,
0282301, adds only the regression tests; the second applies the fix. With the PostgreSQL setup from the existing test workflow in place:The first run times out in
Disconnect rejects queued transaction queries and allows reconnect. The fixed branch passes. The full suite needs both PostgreSQL servers from the workflow (ports 5432 and 5433), SSL, logical replication, prepared transactions, the PostgreSQL CLI tools, and Deno 1.x. Use disposable databases: the existing bootstrap changes server settings and recreates its test database and roles.Validated against PostgreSQL 17.11:
tests/test.js:20assumes a Node stack format; the same failure occurs on unmodified upstream.