fix: backend statement timeouts answer 504, not 500 (#353)

[mauripunzueta]

Closes #353.

What was actually wrong

A statement cancelled by a server-side deadline surfaced as HTTP 500 exception — telling clients a transient resource condition was a server defect, and burying it in the same 5xx bucket as genuine panics.

Two corrections to the issue as filed:

1. The "leaked driver message" premise is already stale. f92f04558 (PR #333, 2026-07-21 — two days before this issue was filed) made RestError::InternalError sanitize: it logs the detail server-side and returns a fixed generic string. No raw driver text reaches the OperationOutcome today. What remained was the wrong status, plus the loss of any actionable signal.

2. The proposed fix would not have fixed the reported symptom. The issue asked for classification inside From<tokio_postgres::Error>. The hot query paths never use From. Each backend module has fn internal_error(message: String) and calls:

.map_err(|e| internal_error(format!("Failed to execute search: {e}")))?

which stringifies the error and throws away err.code(). That includes search_impl.rs — the exact #279 path this issue cites. There are 203 such sites in the Postgres backend alone (269 SQLite, 146 MongoDB).

Matching on the message text instead is not viable: PostgreSQL localizes error messages via lc_messages, so "canceling statement due to statement timeout" is not a stable signal on a non-English server. SQLSTATE is, and it must be read while the typed error is still in hand.

Approach

Classification — all backends. New BackendError::Timeout, distinct from Unavailable: the backend is healthy and deliberately stopped one over-long statement. New classify_{postgres,sqlite,mongodb}_error(context, err) preserve the driver code; the three From<DriverError> impls now route through them.

Backend	Condition	Was	Now
PostgreSQL	57014 query_canceled	500	504
PostgreSQL	53300/53400/57P01/57P02/57P03	500	503
SQLite	SQLITE_INTERRUPT	500	504
SQLite	SQLITE_BUSY / SQLITE_LOCKED (after busy_timeout)	500	503
MongoDB	MaxTimeMSExpired (50) / ExceededTimeLimit (262)	500	504
MongoDB	Io / ConnectionPoolCleared / ServerSelection	500	503
S3	already classified throttling correctly	—	unchanged

SQLite lock-wait expiry is deliberately 503, not 504: it is contention, not an over-long query, and a retry genuinely succeeds. 40001/40P01 are deliberately left alone — retryable, but what a FHIR client should see for a write conflict is a separate decision, and silently 503-ing a deadlock could mask a real lock-ordering defect.

Call sites — 145 across three backends. A typed query_error(context, err) per backend replaces internal_error(format!(...)) on the statements whose cost scales with tenant data volume: search, count, aggregate, history scan, purge, tenant-wide index clear. Those are the statements a deadline actually fires on — a primary-key read cannot run 30s, and if it could, pool exhaustion (already 503) fires first.

Two properties make a mechanical diff this size safe to review:

• The unclassified fallback is byte-identical to the message the call site produced before. An error that is not explicitly classified behaves exactly as it did.
• The helper takes the driver error by type, so a site whose closure yields something else (serde, chrono, a parse) is a compile error, not a silent mis-conversion.

REST. New RestError::GatewayTimeout → 504 + FHIR timeout issue code.

• 504 rather than 503 because a 503 is widely read by load balancers and service meshes as "eject this instance from rotation." A statement timeout says nothing about instance health — ejecting it would be wrong and, under the load that provoked the timeout, actively harmful.
• No Retry-After. Per Postgres composite search is I/O-bound at volume — needs a denormalized one-row-per-group storage layout #279 the cancelled shapes run 12s median / 500s worst — deterministically too slow for the budget. Advising a prompt retry just multiplies load. The 503 family keeps its hint, where a retry helps.
• The client message names no backend product and no HFS_* variable. The issue asked the diagnostic to name HFS_PG_STATEMENT_TIMEOUT_MS; declined deliberately. That fingerprints the deployment while being useless to a caller who cannot set a server env var — and it would regress the sanitization fix: a down backend answers 503+Retry-After, and /_readiness actually probes storage (#286) #333 just established. The knob, the SQLSTATE and the driver text go to the log, for the party who can act on them.
• TransactionError::Timeout also moves 500 → 504 — the same mis-classification, two lines away — in both From<TransactionError> and the parallel bundle mapping in batch.rs, so a transaction timeout reports the same status on either path.

Tests

• SQLite classifier unit tests: interrupt → Timeout, busy/locked → Unavailable, passthrough → Internal, and an explicit assertion that the fallback message is byte-identical to the pre-change text.
• PostgreSQL testcontainer regression test: statement_timeout=250ms + SELECT pg_sleep(5), asserting SQLSTATE 57014 reaches the classifier as Timeout with the caller's context preserved. It asserts on the SQLSTATE, not the message text, so it does not become locale-dependent. This has to be an integration test — tokio_postgres::Error has no public constructor.
• REST: 504 + timeout mapping, message sanitization (asserts the diagnostic leaks no driver text, product name, or env var), absence of Retry-After, transaction-timeout mapping, and a new row in the exhaustive BackendError → status spec table from A down backend answers 500, not 503 — and /_readiness hardcodes "storage": "ok" #286.

Deliberately out of scope

The status map is not uniform after this PR, and that is a choice rather than an oversight:

• bulk_export.rs / bulk_submit.rs (79 pg / 116 sqlite / 18 mongo sites) — these errors surface in job status manifests, not HTTP status codes, so converting them changes nothing user-visible.
• schema.rs — startup DDL. A cancelled migration should fail loudly rather than become a client-facing 504.
• 40001 / 40P01 — see above.
• MongoDB maxTimeMS is not currently set on HFS queries, so the Mongo timeout arm only fires when the deadline comes from the server or connection string. The classification is in place either way; wiring an HFS-side query deadline is a separate change.

Verification note

This environment has no C linker, so cargo check/test/clippy cannot run locally — cargo fmt --check passes (which confirms every file parses). CI is the first real compile. I hand-verified the driver APIs against the vendored sources, which caught one bug the compiler would have: SqlState is a newtype over an enum with an Other(Box<str>) variant, making it non-structural-match, so SqlState::QUERY_CANCELED cannot appear in a pattern — the classifier compares with == instead.

[codecov]

Codecov Report

❌ Patch coverage is 94.53925% with 16 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
crates/persistence/src/error.rs	95.16%	9 Missing ⚠️
...tes/persistence/src/backends/sqlite/search_impl.rs	45.45%	6 Missing ⚠️
...s/persistence/src/backends/postgres/search_impl.rs	50.00%	1 Missing ⚠️

📢 Thoughts on this report? Let us know!