Transaction rollback returns `conn closed` instead of `ErrTxDone` when the pgconn is closed

[zepatrik]

Describe the bug

I spend quite some time further debugging #2551, and think that I now found the actual issue I was running into.

When the context is canceled while a transaction query is executed, the low-level pgconn.PgConn is closed due to an i/o timeout at

pgx/pgconn/pgconn.go

Lines 1533 to 1539 in 82b212c

	msg, err := mrr.pgConn.receiveMessage()
	if err != nil {
	mrr.pgConn.contextWatcher.Unwatch()
	mrr.err = normalizeTimeoutError(mrr.ctx, err)
	mrr.closed = true
	mrr.pgConn.asyncClose()
	return nil, mrr.err

But the outer pgx.dbTx state is not set to "closed".

To Reproduce

Steps to reproduce the behavior:

package main

import (
	"context"
	"fmt"
	"os"
	"time"

	"github.com/jackc/pgx/v5"
)

func main() {
	dsn := os.Getenv("TEST_DATABASE_POSTGRESQL")

	conn, err := pgx.Connect(context.Background(), dsn)
	if err != nil {
		fmt.Printf("unexpected error: %v\n", err)
		os.Exit(1)
	}

	ctx, cancel := context.WithTimeout(context.Background(), 100*time.Millisecond)
	defer cancel()

	tx, err := conn.BeginTx(ctx, pgx.TxOptions{})
	if err != nil {
		fmt.Printf("unexpected error: %v\n", err)
		os.Exit(1)
	}

	// Sleep for longer than the context timeout to trigger a cancellation
	_, _ = tx.Exec(ctx, "SELECT pg_sleep(1)")
	err = tx.Rollback(context.Background())
	fmt.Printf("error on first rollback: %v\n", err)

	err = tx.Rollback(context.Background())
	fmt.Printf("error on second rollback: %v\n", err)
}

// outputs:
//   error on first rollback: conn closed
//   error on second rollback: tx is closed

Expected behavior

The transaction state should match the underlying connection state, or rather the error returned from tx.Rollback should be pgx.ErrTxClosed when the connection is closed.
It generally seems weird to me that the low-level connection would close itself without notifying the higher-level code that created the connection.

Actual behavior

An unexpected, non-assertable, low-level error is returned.

Version

• Go: $ go version -> go version go1.26.2 darwin/arm64
• PostgreSQL: $ psql --no-psqlrc --tuples-only -c 'select version()' -> PostgreSQL 16.13 (Debian 16.13-1.pgdg13+1) on aarch64-unknown-linux-gnu, compiled by gcc (Debian 14.2.0-19) 14.2.0, 64-bit
• pgx: $ grep 'github.com/jackc/pgx/v[0-9]' go.mod -> v5.9.2

Additional context

Potentially related: #1145 #2100

[sean-]

I've spent time in this space for other databases and think the current behavior is correct, but the ergonomics are poor because the error you get back is not errors.Is-matchable.

Why Rollback does not return ErrTxClosed here

ErrTxClosed means "this Tx handle has already been finalized by a prior Commit or Rollback". It is a statement about client-side handle state, not about what happened on the server or on the wire.

When a context is cancelled (or any fatal error occurs) mid-query, pgConn.asyncClose() marks the underlying connection dead. At that point the Tx handle has not been finalized: tx.closed is still false. The first Rollback() then tries to send ROLLBACK, pgConn.lock() sees the connection is closed, and returns conn closed. That is the truthful answer: the ROLLBACK was never delivered. Rollback then marks the handle closed and kills the conn, so a second call returns ErrTxClosed. There is no leak, and pooled connections are still destroyed via the IsClosed() check in Release().

Server errors vs transport errors

For durable systems, there is a meaningful difference between these two semantics and it's worth the distinction:

• A server error is PostgreSQL telling you something over a live connection: a *pgconn.PgError with a SQLSTATE, or a ROLLBACK command tag in response to COMMIT (surfaced as ErrTxCommitRollback). You know exactly what state the server is in because it told you.
• A transport error means the connection is gone: context cancellation, socket close, network failure. The client never received confirmation of anything. PostgreSQL will abort the in-flight transaction when it notices the socket dropped, but the client cannot observe that; from the client's side the outcome is unknown.

Folding the transport case into ErrTxClosed would let defer tx.Rollback() callers, who routinely suppress ErrTxClosed, silently swallow a lost connection. Some callers want that; some want to log it. The library should NOT decide for them.

The actual gap

The dead-connection error today is an unexported connLockError with no errors.Is target (i.e. no sentinel error), so callers are pushed toward side-channel checks (tx.Conn().IsClosed()) or pgconn.SafeToRetry(err), which is broader than "conn closed" (it is also true for conn busy, an application bug you would not want to suppress).

Proposed additive fix

Export a sentinel at the pgconn layer and have the existing error unwrap to it. This is v5-safe (no return values change, existing error strings are unchanged) and useful everywhere a dead connection is touched, not just Rollback:

--- a/pgconn/errors.go
+++ b/pgconn/errors.go
+// ErrConnClosed is returned (possibly wrapped) when an operation is attempted on a
+// connection that has already been closed. Use errors.Is to test for it.
+var ErrConnClosed = errors.New("conn closed")
+
 type connLockError struct {
      status string
 }

 func (e *connLockError) SafeToRetry() bool {
      return true // a lock failure by definition happens before the connection is used.
 }

 func (e *connLockError) Error() string {
      return e.status
 }
+
+func (e *connLockError) Unwrap() error {
+     if e.status == "conn closed" {
+             return ErrConnClosed
+     }
+     return nil
+}

Caller code then becomes:

defer func() {
    err := tx.Rollback(ctx)
    if err == nil || errors.Is(err, pgx.ErrTxClosed) || errors.Is(err, pgconn.ErrConnClosed) {
        return
    }
    log.Printf("rollback failed: %v", err)
}()

Two sentinels, two distinct meanings: ErrTxClosed = handle already finalized, ErrConnClosed = transport gone before the command could be sent. Happy to take a PR along these lines (with a regression test that cancels mid-Exec and asserts the first Rollback() satisfies errors.Is(err, pgconn.ErrConnClosed)), or I can put one up.

Hypothetically the ergonomics could be tightened further, but it would be a breaking change. The ideal shape is probably an outcome-oriented sentinel such as ErrNotCommitted that Commit/Rollback always return on any non-success path, wrapping whichever underlying cause applies: ErrTxClosed, ErrConnClosed, a *pgconn.PgError with a SQLSTATE, context.Canceled, and so on. Callers who only care about the outcome test errors.Is(err, pgx.ErrNotCommitted); callers who care why unwrap or errors.As for the specific layer.

In systems like this, imo, it is generally desirable to have transport-layer and application-layer failures funnel through the same top-level error interface, with the detail expressed as distinct wrapped causes rather than as disjoint top-level types the caller has to enumerate. I do not have a fully baked design for that right now, and it is firmly v6 territory since it changes what errors.Is matches for existing callers, but I think that is the crux of the problem you are stubbing your toe on. The ErrConnClosed addition above is the v5-safe step toward it.

[zepatrik]

Thanks for the great write-up, I fully agree on all your points. Having high-level errors would be great, but I also feel like this is kind of a go language issue in the broader picture.

One idea that comes to my mind is to not alter any errors that are returned, but introduce the high-level errors as something that is only implementing interface{ Is(error) bool }:

type ErrNotCommitted struct {}

func (e ErrNotCommitted) Is(other error) bool {
	if pgErr, isPgErr := errors.AsType[*pgconn.PgError](other); isPgErr && pgErr.Code == someValue {
		return true
	}
	return errors.Is(err, pgconn.ErrConnClosed) || // other alternatives
}

That way the change is fully backwards-compatible as nothing changed but only this error was added, there is no overhead in constructing all the potential higher-level errors, and they can still later be added to the error chain directly if desired. It is like adding an error assertion helper func IsErrNotCommited(error) bool, but keeping compatibility with the stdlib errors interfaces.

[sean-]

The hard part is you don't know if a transaction was actually committed or not if a client has an error at the transport layer (i.e. the server may have committed the transaction, but the response message to the client ends up on the other end of a partition). I don't have an answer to this. Looking back at my suggestion, ErrNotCommitted would only work for server messages that make it through to the client. The reality is any transport error means the connection, including any outstanding transaction on the connection, is in an ambiguous state with undefined behavior.