sql.begin() resolves when COMMIT returns ROLLBACK after a caught savepoint failure

Problem

sql.begin() can fulfill with the callback's return value even though PostgreSQL rolled back the transaction. A caught error from SAVEPOINT creation or ROLLBACK TO itself can leave the transaction aborted; the driver then sends COMMIT, receives a ROLLBACK command-completion tag, and still resolves the outer promise.

This is about the public transaction result. Callers cannot reliably use fulfillment of sql.begin() to distinguish a committed transaction from a rolled-back one in this path.

Expected behavior

For a regular transaction completed by COMMIT, sql.begin() should reject if the server returns ROLLBACK. Catching a savepoint error should continue to permit the outer transaction to commit when rolling back that savepoint actually restored a usable transaction.

Actual behavior

BEGIN
SAVEPOINT fails with a native PostgreSQL statement timeout (57014)
Caller catches the rejected sql.savepoint() promise
Outer callback returns normally
COMMIT -> PostgreSQL returns CommandComplete("ROLLBACK")
sql.begin() fulfills with the callback's result
pg_xact_status(root_xid) = aborted

The connection stays open throughout. This was verified with native statement timeouts on the real SAVEPOINT and ROLLBACK TO commands, using controlled delivery delays. It does not require a pooler or connection reuse.

Related: #1212 already reports the same COMMIT-returning-ROLLBACK false-success symptom through a prepared-COMMIT retry on a transaction pooler. That PR changes the protocol used for transaction control statements. This issue proposes checking the final transaction outcome itself, with caught savepoint-command failures as another trigger.

Reproduction

Tested with Postgres.js 3.4.9, PostgreSQL 17.10, and Node.js 24.0.1 on macOS arm64. The COMMIT-result check is also absent from current master at 411429e.

For a short deterministic reproduction, the fixture below deliberately invalidates a child savepoint so the driver's ROLLBACK TO fails. This is fault injection, not suggested application code. It exercises the same final-outcome bug without depending on timing or a pooler. No application tables or rows are created or modified.

Against a local test PostgreSQL 14+ instance, set the usual PGHOST, PGPORT, PGUSER, PGPASSWORD, and PGDATABASE variables. In a separate directory:

npm install postgres@3.4.9
node repro.mjs

import postgres from 'postgres'

const sql = postgres({ max: 1, ssl: false }) // Standard PG* environment variables.
let xid
let childCode
try {
  const result = await sql.begin(async tx => {
    ;[{ xid }] = await tx`select pg_current_xact_id()::text as xid`
    await tx`savepoint parent_marker`
    try {
      await tx.savepoint(async sp => {
        // Fault fixture: invalidate the driver's child savepoint without disconnecting.
        await sp`rollback to savepoint parent_marker`
        throw new Error('Force the driver to attempt rollback to the removed child')
      })
    } catch (error) {
      childCode = error.code
    }
    return 'success'
  })
  const [{ status }] = await sql`select pg_xact_status(${xid}::xid8) as status`
  console.log({ childCode, result, status })
} catch (error) {
  const [{ status }] = await sql`select pg_xact_status(${xid}::xid8) as status`
  console.log({ childCode, errorCode: error.code, status })
} finally {
  await sql.end({ timeout: 2 })
}

Unpatched output:

{ childCode: '3B001', result: 'success', status: 'aborted' }

With the COMMIT-result check below:

{ childCode: '3B001', errorCode: 'TRANSACTION_ROLLED_BACK', status: 'aborted' }

Separately, the native-timeout reproduction sends a genuine SAVEPOINT Parse message, then holds its remaining extended-query messages for 600 ms with a probe-only statement_timeout = 200ms. PostgreSQL raises 57014; catching that savepoint rejection produces the same false success on unpatched 3.4.9. PostgreSQL documents that statement_timeout spans extended-query messages. These controlled cases establish possible failure paths, not production incidence.

Cause

In v3.4.9's transaction scope:

1. SAVEPOINT executes before the child's work/rollback try block.
2. Each scope has its own uncaughtError. A SAVEPOINT setup error is recorded by the child, and a parent callback can catch its rejection without setting the root's error state.
3. Root completion awaits COMMIT but ignores its returned command field.

The server's ROLLBACK completion is therefore treated as a successful transaction. A failed ROLLBACK TO that is caught by the parent can reach the same final-outcome problem.

Proposed library fix

Inspect the final COMMIT result in the owning transaction scope and reject on the server's ROLLBACK command tag. One small implementation is:

if (!name) {
  if (prepare) {
    await sql`prepare transaction '${ sql.unsafe(prepare) }'`
  } else {
    const committed = await sql`commit`
    if (committed.command === 'ROLLBACK')
      throw Errors.generic(
        'TRANSACTION_ROLLED_BACK',
        'PostgreSQL rolled back the transaction instead of committing it'
      )
  }
}

The error code/name is a suggestion. The required behavior is rejection of a transaction that the server rolled back. The final-outcome error should not pretend to be the previously caught child error if that error is no longer available.

Keep this check outside the work/rollback catch, so it does not issue another ROLLBACK after the server has already completed the transaction. It uses the existing COMMIT response and requires no extra query. This example leaves the separate PREPARE TRANSACTION path intact and does not change raw SQL query-result semantics. Regenerate the CommonJS, Deno, and Cloudflare variants from the source change.

Regression coverage

• A caught SAVEPOINT setup failure followed by COMMIT returning ROLLBACK rejects the outer transaction.
• A caught ROLLBACK TO failure has the same result when the root is aborted.
• A normal savepoint work error followed by successful ROLLBACK TO still allows the parent to catch and commit.
• A nested setup failure recovered by rolling back its enclosing savepoint still permits the root to commit.
• Existing root rollback/error and successful commit behavior stays intact, including the relevant Node entrypoints.

The short standalone reproduction above was run against both unpatched 3.4.9 and the patched package. Additional native SAVEPOINT and ROLLBACK TO timeout checks in both Node entrypoints changed from fulfilled/aborted to rejected/aborted after the patch; ordinary recovery controls still committed successfully.

Related reports

In addition to #1212 above, this differs from #1090, which concerns implicit transactions and a later ErrorResponse. It also differs from the stale-connection mechanism discussed in #1155: this reproduction keeps the original connection open. A COMMIT-result check alone does not address stale transaction callbacks using a reused connection.

Start in src/index.js around the transaction scope referenced in the issue, then inspect the existing transaction coverage and relevant Node entrypoints. Verify the listed caught SAVEPOINT and ROLLBACK TO cases, normal savepoint recovery, and successful commits; regenerate the CommonJS, Deno, and Cloudflare variants when the source behavior is covered.