fix(core): give sqlx pool-exhaustion errors a distinct, actionable message (PSA-40) #8

Merged

nrupard merged 2 commits from feat/psa-40-pool-error-message into main

2026-06-03 20:14:17 +02:00

nrupard commented

2026-06-03 20:06:25 +02:00

Owner

Problem

From<sqlx::Error> for AppError (errors/mod.rs) mapped every sqlx error, including PoolTimedOut and PoolClosed, to the generic "A database error occurred". When a bounded pool is exhausted the log gave no hint that the cause was pool exhaustion rather than a query or connection fault. Diagnosing the bunyip BUNYIP-41 blob-cache pool starvation required reading dunite source because the surfaced message was generic.

Change

Special-case the two connection-availability variants before the catch-all:

PoolTimedOut -> "database connection pool exhausted (no connection available within the acquire timeout)"
PoolClosed -> "database connection pool is closed"

Observability only: both remain DatabaseError (status 500, code DATABASE_ERROR), so existing consumers compile unchanged and no new variant is added. The catch-all is kept for genuinely unexpected variants.

Acceptance criteria

A PoolTimedOut surfaces a message that names pool exhaustion, distinguishable in logs from a generic query error.
Existing AppError consumers compile unchanged (no new variant; status/code unchanged).

Tests

sqlx::Error is #[non_exhaustive], so its variants cannot be constructed in a test. The two regression tests provoke real values from a lazily-built pool, needing no live database or reachable host:

pool_closed_maps_to_distinct_message: connect_lazy then close() then acquire() yields PoolClosed deterministically (no network).
pool_timed_out_maps_to_distinct_pool_exhaustion_message: a one-connection pool with a 150ms acquire timeout pointed at a refused loopback port (127.0.0.1:1) yields PoolTimedOut.

fmt + clippy -D warnings clean; full cargo test --workspace green (130 tests); the timed-out test verified non-flaky across 15 runs.

#PSA-40

## Problem `From<sqlx::Error> for AppError` (errors/mod.rs) mapped every sqlx error, including `PoolTimedOut` and `PoolClosed`, to the generic "A database error occurred". When a bounded pool is exhausted the log gave no hint that the cause was pool exhaustion rather than a query or connection fault. Diagnosing the bunyip BUNYIP-41 blob-cache pool starvation required reading dunite source because the surfaced message was generic. ## Change Special-case the two connection-availability variants before the catch-all: - `PoolTimedOut` -> "database connection pool exhausted (no connection available within the acquire timeout)" - `PoolClosed` -> "database connection pool is closed" Observability only: both remain `DatabaseError` (status 500, code `DATABASE_ERROR`), so existing consumers compile unchanged and no new variant is added. The catch-all is kept for genuinely unexpected variants. ## Acceptance criteria - [x] A PoolTimedOut surfaces a message that names pool exhaustion, distinguishable in logs from a generic query error. - [x] Existing AppError consumers compile unchanged (no new variant; status/code unchanged). ## Tests `sqlx::Error` is `#[non_exhaustive]`, so its variants cannot be constructed in a test. The two regression tests provoke real values from a lazily-built pool, needing no live database or reachable host: - `pool_closed_maps_to_distinct_message`: `connect_lazy` then `close()` then `acquire()` yields `PoolClosed` deterministically (no network). - `pool_timed_out_maps_to_distinct_pool_exhaustion_message`: a one-connection pool with a 150ms acquire timeout pointed at a refused loopback port (`127.0.0.1:1`) yields `PoolTimedOut`. fmt + clippy -D warnings clean; full `cargo test --workspace` green (130 tests); the timed-out test verified non-flaky across 15 runs. #PSA-40

nrupard added 1 commit

2026-06-03 20:06:25 +02:00

fix(core): give sqlx pool-exhaustion errors a distinct, actionable message (PSA-40)

Checks / fmt + clippy + test (pull_request) Successful in 18s

Details

76432db5ac

`From<sqlx::Error> for AppError` mapped every sqlx error, including `PoolTimedOut` and `PoolClosed`, to the generic "A database error occurred". When a bounded pool is exhausted the log gave no hint that the cause was pool exhaustion rather than a query or connection fault; diagnosing the bunyip BUNYIP-41 blob-cache pool starvation required reading dunite source because the surfaced message was generic.

Special-case the two connection-availability variants before the catch-all so they carry a distinct message: PoolTimedOut -> "database connection pool exhausted (no connection available within the acquire timeout)", PoolClosed -> "database connection pool is closed". This is observability only: both stay `DatabaseError` (status 500, code DATABASE_ERROR), so existing consumers compile unchanged and no new variant is added. The catch-all is kept for genuinely unexpected variants.

`sqlx::Error` is `#[non_exhaustive]`, so its variants cannot be constructed in a test. The two regression tests provoke real values from a lazily-built pool (no live database or reachable host): close-then-acquire yields PoolClosed deterministically, and a one-connection pool with a short acquire timeout pointed at a refused loopback port yields PoolTimedOut. Both assert the distinct message; verified non-flaky across 15 runs.

#PSA-40

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

nrupard added 1 commit

2026-06-03 20:11:47 +02:00

test(core): harden pool_timed_out test (close pool, wider timeout margin) (PSA-40 review)

Checks / fmt + clippy + test (pull_request) Successful in 17s

Details

create-release / create-release (pull_request) Has been skipped

Details

05f84d174e

Code review hygiene on the PSA-40 tests: close the pool at the end of the PoolTimedOut test so its background reaper task does not linger past the test (mirrors the PoolClosed test, which already closes). Widen the acquire timeout from 150ms to 500ms so a slow CI scheduler still lets the acquire window elapse into PoolTimedOut rather than racing the timer, and document that PoolTimedOut arises whenever no connection becomes available in the window regardless of whether the loopback dial is reachable, so the test depends on neither a live database nor the socket attempt.

#PSA-40

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>