In-flight query killed by a Postgres restart returns 500, not the retryable 503

[beardthelion]

What happens

When Postgres terminates a backend that has a query in flight (a restart, a failover, or pg_terminate_backend), the request returns 500 db_error rather than the retryable 503 db_unavailable that every other database-outage path returns.

Observed against a real Postgres 16.14 through a PgPool, driving the actual sqlx::Error -> anyhow -> AppError -> IntoResponse chain the handlers use:

in-flight query, backend terminated
  -> sqlx variant = Database, SQLSTATE = "57P01"
  -> db_unavailable() = false
  -> HTTP 500 {"error":"db_error","message":"error returned from database: terminating connection due to administrator command"}

Why

crates/gitlawb-node/src/error.rs:65 classifies by sqlx variant:

fn db_unavailable(e: &sqlx::Error) -> bool {
    matches!(e, sqlx::Error::PoolTimedOut | sqlx::Error::PoolClosed
                | sqlx::Error::Io(_) | sqlx::Error::Tls(_))
}

sqlx turns every Postgres ErrorResponse into Error::Database without inspecting severity (sqlx-postgres-0.8.6/src/connection/stream.rs:128-131), so a FATAL 57P01 arrives as Database, misses this predicate, and falls to the 500 db_error arm at error.rs:150.

The sharpest part is that the classification depends on timing rather than on the condition. If the socket closes before the FATAL is read, sqlx yields Io(UnexpectedEof) (sqlx-core-0.8.6/src/net/socket/buffered.rs:286-294) and the identical real-world event returns 503. So a fast-mode restart and an immediate-mode restart produce different status codes for the same outage.

What is NOT affected

Worth stating so this does not get "fixed" in the wrong place: 53300 too_many_connections and 57P03 cannot_connect_now are already handled. sqlx's pool retries exactly those two during the connect phase (sqlx-core-0.8.6/src/pool/inner.rs:376 with sqlx-postgres-0.8.6/src/error.rs:189-201) and the elapsed acquire deadline becomes PoolTimedOut (sqlx-core-0.8.6/src/pool/mod.rs:652), which db_unavailable() already matches. Those paths return 503 today.

A backend killed while its connection sits idle in the pool also needs no fix: the pool opens a fresh connection and the caller never sees an error. I confirmed both.

The exposure is therefore narrow: queries in flight at the moment of termination.

Fix direction

Classify on SQLSTATE in addition to variant, mirroring the code()-prefix downcast main.rs:685 already does for its own purpose:

fn db_unavailable(e: &sqlx::Error) -> bool {
    if let sqlx::Error::Database(db) = e {
        if let Some(code) = db.code() {
            return code.starts_with("57") || code.starts_with("08");
        }
    }
    matches!(e, sqlx::Error::PoolTimedOut | sqlx::Error::PoolClosed
                | sqlx::Error::Io(_) | sqlx::Error::Tls(_))
}

Class 57 is operator intervention, class 08 is connection exception. This compiles and flips the reproduction above from 500 to 503; with it reverted the same probe returns 500 again. I have not benchmarked which other class-08 codes occur in practice, and the class-57/08 choice is a judgement call rather than something I verified exhaustively, so treat the exact code set as open.

Two things worth deciding alongside it:

sqlx::Error::Database is also how genuine query bugs arrive (a bad column, a constraint violation), and error.rs:63 documents the current split as deliberate: connection-level failures are retryable, server-reported query errors are not. A SQLSTATE test is a narrow exception to that rule rather than a reversal of it, and the doc comment should say so if this lands.

There is no supported sqlx predicate to lean on. DatabaseError::is_transient_in_connect_phase() exists but is #[doc(hidden)], covers only the two codes above, and is scoped to the connect phase, which is the phase already handled. Hand-matching the SQLSTATE class is the available option.

Scope

Pre-existing, and not introduced by #254 (that PR routed three handlers into this classifier; it did not change the classifier). Nothing in this repo currently branches on 503 vs 500 for these routes, so the impact today is on external clients, proxies, and retry policies rather than on anything in-tree.