Skip to content

A query accepted between a socket's error and close never settles, and its connection wedges after the next reconnect (ECONNRESET on Node) #1246

Open

Description

@grant-unwin

Summary

On Node, a TCP reset (RST) on a pooled connection makes the socket emit error first and close later, in the event loop's close phase. Promise callbacks run in between. Anything that reaches Connection.execute() in that gap is pushed onto sent and written to the destroyed socket. Nothing ever rejects it:

  • errored() rejects the in-flight query but leaves it in query (connection.js:392), so execute() keeps queueing behind it (169–170).
  • closed(hadError = true) only rejects query/sent when !hadError (453), so after an RST whatever is in sent stays there.

The connection then goes to closed with the orphan still in sent. The next time the pool reuses it, the reconnecting query (initial) succeeds. After that, ReadyForQuery takes the orphan from sent as the active query (573) and waits for a reply that will never come (576–577). The connection never calls onopen again and stays in connecting:

  • With max: 1, every later query hangs.
  • With max > 1, the pool silently loses one connection. Nothing recovers it: idle_timeout only runs in open, max_lifetime calls end(), which waits on the orphan (416–418), and the socket itself is healthy. It was still held after 4.7 × a 10 s max_lifetime.

sql.end() without a timeout also never resolves after any of these RSTs, because the stale query reference fails the guard at 416 (this part is #1097 / #1130 / #1142).

Tested with postgres 3.4.8 and 3.4.9 (src/connection.js and src/index.js are byte-identical between them, and master differs only in 411429e), Node v24.16.0, and PostgreSQL 16.15. On Bun 1.3.13 an RST surfaces as CONNECTION_CLOSED (no error-before-close gap), so the pool case does not reproduce there.

How a query gets into the gap

  1. sql.begin(): the scope's catch sends rollback on the same connection (index.js:267–271 via the scope handler at 291–296). It reaches c.execute() whenever the connection is not in full, i.e. when the statement that was reset had no bind parameters. begin() itself still rejects correctly (via connection.onclose, 245), so the leaked connection is invisible to the caller.
  2. reserve() + a transaction on the reserved handle: the ROLLBACK goes through the reserve handler (227–231). It never settles in either case: it is orphaned in sent, or, if the connection is in full, left in the reserved queue that onclose abandons. So a begin written on a reserved connection never returns.
  3. Pool pipelining: during the gap the dead connection is still in busy if its in-flight query had no parameters. handler() pipelines new queries onto it when there are no open/closed connections (index.js:339–341). In a burst of 50 queries issued during the gap at max: 5, 16 never settled when the burst queries had no parameters, and 1 when they had parameters.
  4. A rejection handler that immediately queries again with max: 1 (Part A of the repro).

The same handler with a FIN (socket.end()) instead of an RST settles everything, because closed(false) rejects query and sent.

Reproduction (only postgres and node:net)

repro.mjs puts a TCP proxy in front of Postgres and resets the sockets with resetAndDestroy().

import net from "node:net";
import postgres from "postgres";

const target = new URL(process.env.DATABASE_URL ?? "postgres://postgres:postgres@127.0.0.1:5432/postgres");
const pairs = [];
const proxy = net.createServer(client => {
	const upstream = net.connect(Number(target.port || 5432), target.hostname);
	client.pipe(upstream).pipe(client);
	client.on("error", () => {});
	upstream.on("error", () => {});
	pairs.push({ client, upstream });
});
await new Promise(resolve => proxy.listen(0, "127.0.0.1", resolve));
const via = new URL(target);
via.hostname = "127.0.0.1";
via.port = String(proxy.address().port);

const reset = () => {
	for (const { client, upstream } of pairs.splice(0)) {
		upstream.destroy();
		client.resetAndDestroy();
	}
};
const settle = (promise, ms = 3000) =>
	Promise.race([
		Promise.resolve(promise).then(v => `resolved ${JSON.stringify(v)}`, e => `rejected ${e.code}`),
		new Promise(resolve => setTimeout(resolve, ms, `STILL PENDING after ${ms} ms`)),
	]);

console.log("A. max: 1");
{
	const sql = postgres(via.toString(), { max: 1 });
	await sql`select 1`;
	let gap;
	setTimeout(reset, 300);
	const first = await sql`select pg_sleep(2)`.catch(error => {
		gap = sql`select 1 as x`.execute(); // after 'error', before 'close'
		return `rejected ${error.code}`;
	});
	console.log("  reset query            :", first);
	console.log("  query issued on reject :", await settle(gap));
	console.log("  next query             :", await settle(sql`select 2 as x`));
	console.log("  query after that       :", await settle(sql`select 3 as x`));
	console.log("  sql.end()              :", await settle(sql.end().then(() => "ok")));
}

console.log("B. max: 10, sql.begin()");
{
	const sql = postgres(via.toString(), { max: 10 });
	const round = () => Promise.all(Array.from({ length: 10 }, (_, i) => sql`select ${i}::int as i, pg_sleep(0.5)`));
	setTimeout(reset, 300); // the transaction holds this pool's only open socket
	console.log("  begin() with a reset statement :", await settle(sql.begin(tx => tx`select pg_sleep(2)`)));
	for (let k = 1; k <= 3; k++) {
		const started = performance.now();
		const r = await settle(round(), 5000);
		console.log(`  round ${k}: 10 concurrent 0.5 s queries:`, r.startsWith("resolved") ? `ok in ${Math.round(performance.now() - started)} ms` : r);
	}
	console.log("  sql.end()                      :", await settle(sql.end().then(() => "ok")));
}
process.exit(0);

Output with 3.4.9 on Node v24.16.0 (3.4.8 is identical):

A. max: 1
  reset query            : rejected ECONNRESET
  query issued on reject : STILL PENDING after 3000 ms
  next query             : resolved [{"x":2}]
  query after that       : STILL PENDING after 3000 ms
  sql.end()              : STILL PENDING after 3000 ms

B. max: 10, sql.begin()
  begin() with a reset statement : rejected CONNECTION_CLOSED
  round 1: 10 concurrent 0.5 s queries: ok in 525 ms
  round 2: 10 concurrent 0.5 s queries: ok in 1010 ms     <- one of the ten connections is gone
  round 3: 10 concurrent 0.5 s queries: ok in 1008 ms
  sql.end()                      : STILL PENDING after 3000 ms

Suggested fix

Two lines in src/connection.js. Together they settle the orphan, keep the connection usable and let end() resolve:

   function error(err) {
     if (connection.queue === queues.connecting && options.host[retries + 1])
       return

     errored(err)
+    query = null
     while (sent.length)
       queryError(sent.shift(), err)
   }
@@
-    !hadError && (query || sent.length) && error(Errors.connection('CONNECTION_CLOSED', options, socket))
+    if (query || sent.length)
+      error(Errors.connection('CONNECTION_CLOSED', options, socket))

Output of the same repro with the patch:

A. max: 1
  reset query            : rejected ECONNRESET
  query issued on reject : rejected CONNECTION_CLOSED
  next query             : resolved [{"x":2}]
  query after that       : resolved [{"x":3}]
  sql.end()              : resolved "ok"

B. max: 10, sql.begin()
  begin() with a reset statement : rejected CONNECTION_CLOSED
  round 1: 10 concurrent 0.5 s queries: ok in 542 ms
  round 2: 10 concurrent 0.5 s queries: ok in 506 ms
  round 3: 10 concurrent 0.5 s queries: ok in 506 ms
  sql.end()                      : resolved "ok"

Notes on the two lines:

  • The closed() change on its own fixes Parts A and B. It does not fix end() after an RST that is followed by no query. That needs query = null.
  • query = null goes in error(), not errored(). errored() is also reached from execute()'s catch when build() throws on a live connection. Clearing there makes the next pipelined query resolve with an empty result (comment on #1142). With the placement above, select ${undefined} followed by select 42 still gives rejected UNDEFINED_VALUE and [{"value":42}].
  • if rather than a leading (: the file has no semicolons, and the line follows return reconnect().
  • What it does not cover: a query left in a reserved connection's own queue (index.js:228–229, used when the connection is in full). onclose abandons that queue, so a ROLLBACK sent through a reserved handle after a parameterised statement was reset still never settles. This is the reserve() counterpart of Transaction-local queued queries never settle when the reserved connection closes #1186 and needs its own fix.

Related

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions