fix: return 503 service_unavailable on database outage

server/core/errors.py

@@ -9,6 +9,7 @@
 from fastapi.exceptions import RequestValidationError
 from fastapi.responses import JSONResponse
 from pydantic import BaseModel
+from sqlalchemy.exc import OperationalError
 from starlette.exceptions import HTTPException as StarletteHTTPException
 
 logger = structlog.stdlib.get_logger()
@@ -39,7 +40,14 @@ def _request_id(request: Request) -> str | None:
     return getattr(request.state, "request_id", None)
 
 
-def _error_json(code: str, message: str, request: Request, details: Any = None, status: int = 500):
+def _error_json(
+    code: str,
+    message: str,
+    request: Request,
+    details: Any = None,
+    status: int = 500,
+    headers: dict[str, str] | None = None,
+):
     body = ErrorResponse(
         error=ErrorDetail(
             code=code,
@@ -48,7 +56,9 @@ def _error_json(code: str, message: str, request: Request, details: Any = None,
             request_id=_request_id(request),
         )
     )
-    return JSONResponse(status_code=status, content=body.model_dump(exclude_none=True))
+    return JSONResponse(
+        status_code=status, content=body.model_dump(exclude_none=True), headers=headers
+    )
 
 
 # ---------------------------------------------------------------------------
@@ -99,6 +109,29 @@ async def http_exception_handler(request: Request, exc: StarletteHTTPException):
             status=exc.status_code,
         )
 
+    @app.exception_handler(OperationalError)
+    async def operational_error_handler(request: Request, exc: OperationalError):
+        # A DB outage is not a bug in the handling endpoint; /readyz already
+        # reports 503 for the identical condition (ReadinessResult.http_status),
+        # so this keeps both surfaces agreeing on what a DB outage means.
+        # Starlette picks the most specific handler by exception class (MRO),
+        # so this wins over the Exception catch-all regardless of registration
+        # order. server/db/engine.py normalizes the asyncpg outage shapes that
+        # would otherwise bypass this handler (raw OSError at connect time,
+        # bare DBAPIError on a mid-query disconnect) into OperationalError.
+        logger.error(
+            "database_unavailable",
+            request_id=_request_id(request),
+            exc_msg=str(exc)[:200],
+        )
+        return _error_json(
+            code="service_unavailable",
+            message="The database is temporarily unavailable. Please retry shortly.",
+            request=request,
+            status=503,
+            headers={"Retry-After": "5"},
+        )
+
     @app.exception_handler(Exception)
     async def unhandled_exception_handler(request: Request, exc: Exception):
         logger.error(

server/db/engine.py

@@ -2,6 +2,7 @@
 
 from typing import AsyncGenerator
 
+from sqlalchemy import event, exc
 from sqlalchemy.ext.asyncio import AsyncEngine, AsyncSession, async_sessionmaker, create_async_engine
 
 from server.core.config import settings
@@ -11,6 +12,43 @@
 _async_session_factory: async_sessionmaker[AsyncSession] | None = None
 
 
+def _install_outage_normalization(engine: AsyncEngine) -> None:
+    """Surface database outages uniformly as ``OperationalError``.
+
+    The asyncpg dialect leaks two outage shapes that bypass SQLAlchemy's usual
+    error translation, so the app-level ``OperationalError`` handler
+    (``server/core/errors.py``) would never see them:
+
+    - connect-time network failures (DB down/unreachable/DNS) escape as raw
+      ``OSError`` subclasses (``ConnectionRefusedError``, ``socket.gaierror``);
+    - a connection dying mid-query surfaces as a bare ``DBAPIError`` with
+      ``connection_invalidated=True`` instead of ``OperationalError``.
+
+    Both are re-wrapped here so callers see one exception type for "the
+    database is temporarily unavailable". Auth/config mistakes (bad password,
+    unknown database) and genuine SQL bugs take other exception types and are
+    deliberately left alone.
+    """
+
+    @event.listens_for(engine.sync_engine, "do_connect")
+    def _wrap_connect_oserror(dialect, conn_rec, cargs, cparams):
+        try:
+            return dialect.connect(*cargs, **cparams)
+        except OSError as exc_:  # ConnectionRefusedError, gaierror, timeout…
+            raise exc.OperationalError(
+                "database connection failed", None, exc_
+            ) from exc_
+
+    @event.listens_for(engine.sync_engine, "handle_error")
+    def _wrap_disconnect(context):
+        if context.is_disconnect and not isinstance(
+            context.sqlalchemy_exception, exc.OperationalError
+        ):
+            return exc.OperationalError(
+                "database connection lost", None, context.original_exception
+            )
+
+
 def get_engine() -> AsyncEngine:
     """Get or create the async engine (lazy initialization)."""
     global _engine
@@ -24,6 +62,7 @@ def get_engine() -> AsyncEngine:
             pool_timeout=30,
             pool_recycle=300,
         )
+        _install_outage_normalization(_engine)
     return _engine
 
 

tests/test_db_outage_error_handling.py

@@ -0,0 +1,118 @@
+"""Regression test for issue #354.
+
+A database outage (e.g. Postgres unreachable) raises `sqlalchemy.exc.OperationalError`
+from inside a route handler. `register_exception_handlers`'s catch-all `Exception`
+handler caught it and returned 500/`internal_error`, indistinguishable from a genuine
+endpoint bug and not a signal most retry policies back off on. `/readyz` already
+reports 503 for the identical condition (`ReadinessResult.http_status`), so the two
+surfaces disagreed about what a DB outage means.
+"""
+
+from __future__ import annotations
+
+import pytest
+from httpx import ASGITransport, AsyncClient
+from sqlalchemy.exc import OperationalError
+
+from server.app import create_app
+from server.db.engine import dispose_engine, get_engine, get_session
+
+pytestmark = pytest.mark.asyncio
+
+
+async def _raise_operational_error():
+    raise OperationalError("SELECT 1", {}, Exception("connection refused"))
+    yield  # pragma: no cover - unreachable, keeps this an async generator
+
+
+async def test_db_outage_returns_503_not_500():
+    app = create_app()
+    app.dependency_overrides[get_session] = _raise_operational_error
+
+    # No raise_app_exceptions=False here: the OperationalError handler lives in
+    # ExceptionMiddleware, which sends its response WITHOUT re-raising — so this
+    # also pins that a DB outage produces no propagated exception at all.
+    transport = ASGITransport(app=app)
+    async with AsyncClient(transport=transport, base_url="http://test") as ac:
+        resp = await ac.get("/v1/subjects/does-not-matter/health")
+
+    assert resp.status_code == 503
+    body = resp.json()
+    assert body["error"]["code"] == "service_unavailable"
+    assert "Retry-After" in resp.headers
+
+
+async def test_non_db_exceptions_still_return_500():
+    """The catch-all handler must still cover unrelated bugs: only
+    `OperationalError` gets the 503 treatment."""
+    app = create_app()
+
+    async def _raise_value_error():
+        raise ValueError("not a database problem")
+        yield  # pragma: no cover
+
+    app.dependency_overrides[get_session] = _raise_value_error
+
+    # The Exception catch-all lives in ServerErrorMiddleware, which sends its
+    # 500 response and then re-raises the original exception (so real servers
+    # log it); raise_app_exceptions=False lets the test client see the response
+    # instead of the propagated ValueError.
+    transport = ASGITransport(app=app, raise_app_exceptions=False)
+    async with AsyncClient(transport=transport, base_url="http://test") as ac:
+        resp = await ac.get("/v1/subjects/does-not-matter/health")
+
+    assert resp.status_code == 500
+    assert resp.json()["error"]["code"] == "internal_error"
+
+
+# ---------------------------------------------------------------------------
+# The asyncpg dialect does not raise OperationalError for real outages on its
+# own: connect-time failures escape as raw OSError subclasses and a mid-query
+# disconnect surfaces as a bare DBAPIError. server/db/engine.py normalizes
+# both; these tests pin that normalization end to end (issue #354's actual
+# repro is the app-level test below — no dependency override, a real engine
+# pointed at a dead port).
+# ---------------------------------------------------------------------------
+
+
+async def test_engine_wraps_connect_failure_as_operational_error(monkeypatch):
+    from sqlalchemy import text
+    from sqlalchemy.exc import OperationalError
+
+    from server.core.config import settings
+
+    monkeypatch.setattr(
+        settings, "database_url", "postgresql+asyncpg://u:p@127.0.0.1:1/statewave"
+    )
+    await dispose_engine()
+    try:
+        engine = get_engine()
+        with pytest.raises(OperationalError) as excinfo:
+            async with engine.connect() as conn:
+                await conn.execute(text("SELECT 1"))
+        assert isinstance(excinfo.value.orig, OSError)
+    finally:
+        await dispose_engine()
+
+
+async def test_db_outage_returns_503_without_dependency_override(monkeypatch):
+    """Issue #354's reproduction: the database is unreachable, a normal request
+    comes in, and the real get_session/engine path (no override) must yield the
+    503 payload — this fails if OperationalError never surfaces from the engine."""
+    from server.core.config import settings
+
+    monkeypatch.setattr(
+        settings, "database_url", "postgresql+asyncpg://u:p@127.0.0.1:1/statewave"
+    )
+    await dispose_engine()
+    try:
+        app = create_app()
+        transport = ASGITransport(app=app)
+        async with AsyncClient(transport=transport, base_url="http://test") as ac:
+            resp = await ac.get("/v1/subjects/does-not-matter/health")
+
+        assert resp.status_code == 503
+        assert resp.json()["error"]["code"] == "service_unavailable"
+        assert "Retry-After" in resp.headers
+    finally:
+        await dispose_engine()