fix(postgres): roll back a transaction cancelled during BEGIN

[rubenfiszel]

Fixes #4393.

The problem

PgTransactionManager::begin raises transaction_depth only after the BEGIN round trip:

let rollback = Rollback::new(conn);
rollback.conn.queue_simple_query(statement.as_str())?;
rollback.conn.wait_until_ready().await?;      // <-- cancellation here
if !rollback.conn.in_transaction() { return Err(Error::BeginFailed); }
rollback.conn.inner.transaction_depth += 1;   // <-- only now

start_rollback is a no-op while that depth is zero, and it is what both guards call — the Rollback guard added in #2819 and the Transaction-level one added in #3980. So a future cancelled in that window queues nothing and the session stays inside a transaction.

Floating::return_to_pool then validates the connection with raw.ping(), which for Postgres is a bare wait_until_ready: it drains the ReadyForQuery but never inspects its transaction-status byte. The connection is judged healthy and handed to the next borrower, whose statements run inside the stale transaction and hold its locks. The first error turns the session into idle in transaction (aborted), after which every unrelated query on that connection fails with 25P02 — and it never self-heals, because the depth is still zero, so no later drop queues a rollback either. Only max_lifetime clears it.

In production this turned a transient database slowdown into a ~20 minute outage: one poisoned connection, reused hundreds of times a second, surfacing the same 25P02 at a dozen unrelated call sites at once.

#2054 covers this and was closed by #2057, which fixed SQLite. #3980 restructures Transaction::begin so a cancelled caller triggers the drop guard, but for Postgres that still funnels into the same depth-gated start_rollback, so it does not reach this case. Reproduced on both 0.8.6 and 0.9.0.

The change

Claim the depth before the round trip and unwind it if the BEGIN did not take, so the guards have something to act on.

The queued ROLLBACK goes into the same write buffer as the BEGIN, and is flushed after it — so the statement it rolls back has always been sent first. That also holds for the savepoint case: a cancelled nested begin now queues ROLLBACK TO SAVEPOINT _sqlx_savepoint_{depth} for the savepoint it just queued, rather than the enclosing one.

Test

it_rolls_back_a_transaction_cancelled_during_begin in tests/postgres/postgres.rs. It cancels a begin_with("BEGIN; SELECT pg_sleep(2);") — a plain BEGIN answers too quickly to cancel reliably, and the sleep widens the same round trip — then asserts the session is back to idle rather than idle in transaction.

Verified against a real server: the test fails on main with left: Some("idle in transaction"), and passes with the change. I also ran the whole --test postgres suite before and after: the failure set is identical apart from this test (4 pre-existing failures in my environment, from fixtures my database lacks), so nothing else changes behaviour — including it_can_work_with_failed_transactions, it_can_work_with_nested_transactions and it_can_fail_and_recover, which all pass.

Known trade-off

A BEGIN that is rejected by the server now queues a ROLLBACK on a session that never entered a transaction, which Postgres answers with WARNING: there is no transaction in progress. That seemed clearly preferable to leaking the transaction, but if you would rather not change that path, the alternative is to track "BEGIN sent, outcome unknown" in a separate flag instead of reusing the depth.

A cheaper fix you may prefer

Postgres reports transaction status in every ReadyForQuery, and it is already decoded into PgConnection::transaction_status. in_transaction() is pub(crate), and the public Connection::is_in_transaction() returns the client-side depth — precisely the value that is wrong here. Having return_to_pool consult the server-reported status would catch this and any other cancellation that leaves a connection dirty, at no extra round trip. Happy to take the PR in that direction instead if you prefer it.