arbitrary UNSAFE_TRANSACTION errors when using sql.begin

[JuHwon]

The following query arbitrary causes an UNSAVE_TRANSACTION error.

await this.sql.begin(async (tsql) => {
  await tsql`DELETE
            FROM mapping_table
            WHERE a_id = ${aId}`;
  if (items.length === 0) {
    return;
  }
  await tsql`INSERT INTO mapping_table ${
    // eslint-disable-next-line @ts-safeql/check-sql
    tsql(items, 'a_id', 'b_id')
  }`;
});

the error most likely happens when this code is executed very frequently and items is usually an array betwenn 0 and a few thousand items.

error:

Error: UNSAFE_TRANSACTION: Only use sql.begin, sql.reserved or max: 1
    at CommandComplete (/Users/juhwon/Projects/xx/xx/node_modules/postgres/cjs/src/connection.js:580:29)
    at handle (/Users/juhwon/Projects/xx/xx/node_modules/postgres/cjs/src/connection.js:476:6)
    at Socket.data (/Users/juhwon/Projects/xx/xx/node_modules/postgres/cjs/src/connection.js:315:9)
    at Socket.emit (node:events:514:28)
    at addChunk (node:internal/streams/readable:376:12)
    at readableAddChunk (node:internal/streams/readable:349:9)
    at Socket.Readable.push (node:internal/streams/readable:286:10)
    at TCP.onStreamRead (node:internal/stream_base_commons:190:23)
    at TCP.callbackTrampoline (node:internal/async_hooks:130:17)

to my understanding using sql.begin for something like this should be fine. have we found a bug here or is there something wrong with our code?

[JuHwon]

error object of notice callback:

{
   "severity_local":"WARNING",
   "severity":"WARNING",
   "code":"25001",
   "message":"there is already a transaction in progress",
   "file":"xact.c",
   "line":"3799",
   "routine":"BeginTransactionBlock"
}

maybe this is similar to this one: #274

according to the debug logs the client executes a BEGIN statement on connection id e.g. 4, and then a few ms laters it executes a BEGIN statement with the same connection id again, before the first transaction was handled. there were no other statements between with the affected connection id.

[ngsgh]

We hit this in production and traced a mechanism that fits the "arbitrary" part. Sharing in case it helps — we could not reproduce it locally, so treat the root-cause part as analysis of the source, not a verified repro.

Where the lock is skipped

src/connection.js (v3.4.9, and master at time of writing):

build(q)
return write(toBuffer(q))
  && !q.describeFirst
  && !q.cursorFn
  && sent.length < max_pipeline
  && (!q.options.onexecute || q.options.onexecute(connection))

onexecute is what marks the connection as reserved for the transaction. It sits at the end of an && chain, so it is skipped entirely whenever an earlier term is falsy:

• write(toBuffer(q)) returns false under socket backpressure
• sent.length < max_pipeline is false once the pipeline is full (default 100)

If BEGIN goes out without onexecute running, c.reserved is never set, and when the BEGIN response arrives the guard at src/connection.js (errored(Errors.generic('UNSAFE_TRANSACTION', ...))) fires.

Why it looks arbitrary

The chain can only break when BEGIN is pipelined onto an already-busy connection. On an idle connection sent.length is 0 and BEGIN is ~50 bytes, well under the write threshold, so neither term can be false — it is structurally safe there. That matches the reporter's "large variable-size operations very often": the failure needs a connection that is already saturated at the moment BEGIN is queued.

Blast radius is wider than one query

errored() calls stream.destroy(), so other queries pipelined on the same connection are torn down too. A single UNSAFE_TRANSACTION in your logs may correspond to more than one failed request.

Reproduction attempts (all negative)

Against a local Postgres 17, six shapes: TCP proxy with read stalls and throttling, forced socket.write() returning false, max_pipeline: 1, and 1000 pool queries against 200 pipelined transactions. Zero occurrences. Loopback kernel buffers are large enough that real backpressure never materialises, which is consistent with this only showing up over real network paths.

Workaround we are using

Retrying once on error.code === 'UNSAFE_TRANSACTION'. This appears safe because the guard fires at the BEGIN response — nothing in the transaction body has executed — and errored() destroys the socket, so the server rolls back. We verified no partial commits locally (committed rows always equalled the number of successful transactions).

Happy to test a patch if someone wants to move onexecute out of the chain; note its return value currently participates in the busy/full queue decision, so it is not a pure one-liner.

[dominikwagner]

We hit this in production on postgres@3.4.7 and traced it to the same code path @ngsgh described.

Deterministic reproduction against unmodified PostgreSQL on localhost — no proxy, no protocol tampering:

• Pool: max: 3, max_pipeline: 2
• Hold all 3 connections busy with pg_sleep(0.5)
• Fire 6 regular queries + 5 sql.begin() concurrently
• Run this scenario 20 times: 12–16 iterations produce UNSAFE_TRANSACTION
• PostgreSQL itself reports 25001 "there is already a transaction in progress"

The 25001 warning is the smoking gun — PostgreSQL is telling us the driver sent two BEGINs on one connection.

Root cause: Under load, connection.reserved can be cleared or overwritten while BEGIN is in flight. Three code paths in src/connection.js / src/index.js:

1. execute() short-circuits: when sent.length >= max_pipeline or write() returns false, the && chain skips onexecute, so connection.reserved is never set — this is the path @ngsgh described, and it's what our test triggers
2. handler() dispatches queued BEGIN queries onto already-busy connections
3. drain() calls onopen() on a reserved connection, returning it to the pool mid-transaction

Fix (three parts, all needed):

1. execute() fires onexecute unconditionally when write() succeeds, and returns false so no more queries pipeline onto the connection
2. handler() always queues onexecute queries (BEGIN) instead of dispatching to busy connections
3. drain() guards on !connection.reserved before calling onopen()

@porsager: I opened a PR with a fix — #1207