Spaces:
Sleeping
Sleeping
| """Regression tests for the `get_db` yield-based dependency in app/db/database.py. | |
| In FastAPI >= 0.106, exceptions raised inside a path operation (including | |
| ``HTTPException``) are propagated to yield-based dependencies. The session | |
| manager must therefore distinguish routine HTTP 4xx flows from genuine | |
| database errors — otherwise every 404 / 409 / 413 / 422 logs a full ERROR-level | |
| stack trace, drowning real failures in noise across this codebase's 60+ | |
| ``raise HTTPException`` sites. | |
| """ | |
| from __future__ import annotations | |
| import logging | |
| from typing import Any | |
| from unittest.mock import AsyncMock, MagicMock | |
| import pytest | |
| from fastapi import HTTPException | |
| from starlette.exceptions import HTTPException as StarletteHTTPException | |
| from app.db import database as db_mod | |
| class _FakeSessionCtx: | |
| """Async context manager that yields a single AsyncMock session. | |
| Records ``commit`` / ``rollback`` calls so tests can verify both behaviours. | |
| """ | |
| def __init__(self) -> None: | |
| self.session = AsyncMock() | |
| self.session.commit = AsyncMock() | |
| self.session.rollback = AsyncMock() | |
| async def __aenter__(self) -> Any: | |
| return self.session | |
| async def __aexit__(self, exc_type: Any, exc: Any, tb: Any) -> bool: | |
| return False | |
| def _patch_session_factory(monkeypatch: pytest.MonkeyPatch) -> _FakeSessionCtx: | |
| """Replace ``get_session_factory`` with a stub that yields a mock session.""" | |
| ctx = _FakeSessionCtx() | |
| factory = MagicMock(return_value=ctx) | |
| monkeypatch.setattr(db_mod, "get_session_factory", lambda: factory) | |
| return ctx | |
| async def _drive(gen: Any, send: Any = None, *, throw: BaseException | None = None) -> None: | |
| """Drive an async generator one step. Optionally throw an exception in.""" | |
| try: | |
| if throw is not None: | |
| await gen.athrow(throw) | |
| else: | |
| await gen.asend(send) | |
| except StopAsyncIteration: | |
| pass | |
| async def test_get_db_does_not_log_error_for_http_exception( | |
| caplog: pytest.LogCaptureFixture, | |
| _patch_session_factory: _FakeSessionCtx, | |
| ) -> None: | |
| """Regression: routine 404/409/413/422 must not produce an ERROR log.""" | |
| caplog.set_level(logging.DEBUG, logger="app.db.database") | |
| gen = db_mod.get_db() | |
| _ = await gen.__anext__() | |
| with pytest.raises(HTTPException) as ei: | |
| await gen.athrow(HTTPException(status_code=404, detail="not found")) | |
| assert ei.value.status_code == 404 | |
| db_records = [r for r in caplog.records if r.name == "app.db.database"] | |
| error_records = [r for r in db_records if r.levelno >= logging.ERROR] | |
| assert error_records == [], ( | |
| "HTTPException must not generate ERROR-level logs from get_db — " | |
| f"got: {[(r.levelname, r.getMessage()) for r in error_records]}" | |
| ) | |
| _patch_session_factory.session.rollback.assert_awaited_once() | |
| _patch_session_factory.session.commit.assert_not_called() | |
| async def test_get_db_does_not_log_error_for_starlette_http_exception( | |
| caplog: pytest.LogCaptureFixture, | |
| _patch_session_factory: _FakeSessionCtx, | |
| ) -> None: | |
| """Filtering must catch the Starlette base class as well as fastapi.HTTPException.""" | |
| caplog.set_level(logging.DEBUG, logger="app.db.database") | |
| gen = db_mod.get_db() | |
| _ = await gen.__anext__() | |
| with pytest.raises(StarletteHTTPException): | |
| await gen.athrow(StarletteHTTPException(status_code=413, detail="too large")) | |
| error_records = [ | |
| r for r in caplog.records | |
| if r.name == "app.db.database" and r.levelno >= logging.ERROR | |
| ] | |
| assert error_records == [] | |
| _patch_session_factory.session.rollback.assert_awaited_once() | |
| async def test_get_db_logs_genuine_database_error_at_error_level( | |
| caplog: pytest.LogCaptureFixture, | |
| _patch_session_factory: _FakeSessionCtx, | |
| ) -> None: | |
| """Real (non-HTTP) exceptions must still surface with full context.""" | |
| caplog.set_level(logging.DEBUG, logger="app.db.database") | |
| gen = db_mod.get_db() | |
| _ = await gen.__anext__() | |
| boom = RuntimeError("connection lost") | |
| with pytest.raises(RuntimeError): | |
| await gen.athrow(boom) | |
| error_records = [ | |
| r for r in caplog.records | |
| if r.name == "app.db.database" and r.levelno >= logging.ERROR | |
| ] | |
| assert any( | |
| "DB session error" in r.getMessage() and r.exc_info is not None | |
| for r in error_records | |
| ), ( | |
| "Genuine DB errors must still log at ERROR level with traceback — " | |
| f"got: {[(r.levelname, r.getMessage(), bool(r.exc_info)) for r in error_records]}" | |
| ) | |
| _patch_session_factory.session.rollback.assert_awaited_once() | |
| async def test_get_db_commits_on_success( | |
| _patch_session_factory: _FakeSessionCtx, | |
| ) -> None: | |
| """Successful path: commit fires, rollback does not.""" | |
| gen = db_mod.get_db() | |
| _ = await gen.__anext__() | |
| with pytest.raises(StopAsyncIteration): | |
| await gen.__anext__() | |
| _patch_session_factory.session.commit.assert_awaited_once() | |
| _patch_session_factory.session.rollback.assert_not_called() | |
| async def test_get_db_quiet_rollback_failure_under_http_exception_still_logs( | |
| caplog: pytest.LogCaptureFixture, | |
| _patch_session_factory: _FakeSessionCtx, | |
| ) -> None: | |
| """If rollback itself fails while propagating an HTTPException, that | |
| secondary failure is genuine and must be visible — the original | |
| HTTPException must still propagate. | |
| """ | |
| caplog.set_level(logging.DEBUG, logger="app.db.database") | |
| _patch_session_factory.session.rollback.side_effect = RuntimeError("rollback boom") | |
| gen = db_mod.get_db() | |
| _ = await gen.__anext__() | |
| with pytest.raises(HTTPException) as ei: | |
| await gen.athrow(HTTPException(status_code=409, detail="conflict")) | |
| assert ei.value.status_code == 409 | |
| rollback_failure_logs = [ | |
| r for r in caplog.records | |
| if r.name == "app.db.database" | |
| and r.levelno >= logging.ERROR | |
| and "Rollback failed" in r.getMessage() | |
| ] | |
| assert rollback_failure_logs, ( | |
| "A failing rollback during HTTPException propagation must surface — " | |
| "we only suppress noise for normal 4xx flows, not for cascading errors." | |
| ) | |