diff --git a/docs/adr/037-http-exception-status-and-fastapi-title.md b/docs/adr/037-http-exception-status-and-fastapi-title.md new file mode 100644 index 00000000..33c44aab --- /dev/null +++ b/docs/adr/037-http-exception-status-and-fastapi-title.md @@ -0,0 +1,39 @@ +# 037. HTTP exceptions keep their status, and the FastAPI title comes from config + +- Date: 2026-10-25 +- Status: Accepted + +## Problem + +1. Every `HTTPException` raised in a route was rendered as `500 {"message": "Server Error"}`. `FastAPIProvider` routed `HTTPException` through the exception manager to `HTTPExceptionHandler`, whose `render` always built a 500 response. `ModelNotFoundException` (and the other framework exceptions marked `is_http_exception`, such as `AuthorizationException`) fell through to the same 500 path. Because they only matched the bare `Exception` handler, Starlette served them from `ServerErrorMiddleware`, which re-raises after responding. +2. `/docs` showed the title `Jobins AI Agent (LangChain)`. That string was hard-coded in `FastAPIProvider.register`, but the instance was never used: `Application.__init__` reset `_fastapi` to `None` after `register_providers()`, so the lazy getter built a plain `FastAPI()` titled `FastAPI`. That reset also dropped any instance a provider passed to `use_fastapi` in `register`. + +## Alternatives + +- Duck-type on `is_http_exception` inside `HTTPExceptionHandler` only. This fixes the status code, but Starlette still dispatches these exceptions through `ServerErrorMiddleware`, which re-raises them as server errors. +- Have `DatabaseProvider` register a FastAPI handler for `ModelNotFoundException`. That couples the ORM provider to FastAPI and to the order in which providers run, and it leaves the other framework HTTP exceptions unhandled. +- Add a small, dependency-free `HttpException` base in `fastapi_startkit.exceptions` and register it with FastAPI as a dedicated handler (chosen). + +## Decision + +- New `fastapi_startkit.exceptions.HttpException(Exception)` with `is_http_exception = True`, `get_status()`, `get_response()` and `get_headers()` (which defaults to `None`). `RouteNotFoundException`, `MethodNotAllowedException`, `AuthorizationException`, `ThrottleRequestsException` and `masoniteorm.ModelNotFoundException` now subclass it. `ThrottleRequestsException` therefore returns 429 and passes on any headers it carries, such as `Retry-After`. Its `headers` default changes from a shared mutable `{}` to `None`. +- `HTTPExceptionHandler.report` skips client errors: a Starlette `HTTPException` or a framework `HttpException` with a status below 500. Everything else, including 5xx HTTP exceptions, goes to `exception_manager.report_exception`. This check lives in the FastAPI layer, not in the core `ExceptionHandler`. The core handler cannot import Starlette, and matching on a `status_code` attribute alone would also silence outbound client errors that carry one (for example SDK `APIStatusError`s), which are real server-side failures. +- `HTTPExceptionHandler.render` delegates any Starlette `HTTPException` (which covers FastAPI's subclass) to FastAPI's `http_exception_handler`. A framework `HttpException` is converted to a Starlette `HTTPException` built from its status, response and headers. The result keeps FastAPI's response shape: `{"detail": ...}`, the status code, and the headers. Any other exception still renders as the debug trace or `Server Error` with a 500. +- `FastAPIProvider` registers the handler on Starlette's `HTTPException`, not FastAPI's subclass, so routing 404s and 405s use the same path and carry the request-id header. It also registers the handler on `HttpException`. +- `FastAPIConfig.title = env("APP_NAME", "FastAPI Startkit")` sets the FastAPI title. `Application.__init__` now initialises `_fastapi` before `register_providers()`, so the instance a provider creates in `register` is kept. + +## Validation + +- `tests/fastapi/test_http_exception_rendering.py` uses `HttpTestCase` to check that: + - `HTTPException(400, "Email taken")` returns 400 with `json()["detail"] == "Email taken"` and its custom header; + - a Starlette `HTTPException(418)` keeps its status; + - an unknown route returns 404 `Not Found`; + - `ModelNotFoundException` returns 404 with its message; + - `APP_NAME` sets the OpenAPI title; + - the default title is `FastAPI Startkit`. +- `tests/fastapi/test_exceptions.py` covers: + - `MethodNotAllowedException` (405 plus the `Allow` header); + - `AuthorizationException` (403); + - `ThrottleRequestsException` (429 plus `Retry-After`); + - `report()` skipping 4xx Starlette/FastAPI/framework HTTP exceptions while still reporting a 503 `HTTPException` and a plain `RuntimeError`. +- The full suite runs with coverage of at least 80%. Ruff and basedpyright are clean on the changed files. diff --git a/docs/adr/AGENTS.md b/docs/adr/AGENTS.md index 7b657eb1..da6490a6 100644 --- a/docs/adr/AGENTS.md +++ b/docs/adr/AGENTS.md @@ -35,4 +35,5 @@ Read this index first, then open the records relevant to the change. Before impl | [029](029-cache-module.md) | 2026-10-21 | Cache module with remember, memo, locks and tags | New `fastapi_startkit.cache`: async `Cache.get/put/has/forget/flush`, `get` with a callable default, and `remember(key, ttl, factory)` that runs a sync or async factory only on a miss; `remember(key, ttl)` without a factory is a decorator keyed by `key` plus a hash of the call arguments. Stores implement an async contract with a `MISSING` sentinel (cached `None` is a hit): array, file (pickled per-key files), database (masoniteorm table) and redis (named connection from the existing `RedisManager`, `SET PX` TTL, flush by `SCAN` of prefixed keys, never `FLUSHDB`). TTL in seconds or `timedelta`, `None` forever, <=0 forgets. `CacheConfig` via `resolve_config`; `CacheProvider` binds `cache`; facade `remember` resolves lazily. `Cache.memo()` memoizes reads per asyncio task through a `ContextVar` owned by the task, so requests never share memo state. `Cache.lock(name, seconds, owner)` gives atomic, owner-checked, non-re-entrant locks with `get`, `block` (250 ms polling, `LockTimeoutError`), `release`, `force_release` and `async with`, per store: event-loop dict, `fcntl.flock` file, `lock:` row with update-if-expired, Redis `SET NX PX` plus a Lua owner-checked delete. `Cache.tags([...])` uses Laravel's namespaced keys: random tag ids at `:tag::key`, entries at `:sha1(ids):`, flush rewrites the ids; tags are sorted and deduplicated. Shipped in slices into `feature/caches`; the former memo, lock and tags ADRs (030, 031, 034) are folded in. | | [032](032-migrate-seed-option.md) | 2026-10-21 | db:migrate:fresh and db:migrate:refresh --seed run seeders in the same event loop | `--seed` called the unregistered `seed:run` through Cleo `call` (which also drops the first argument) inside a running event loop, where `db:seed` would call `asyncio.run` again. The seeder selection moves into async `DBSeedCommand.run_seeders`, which `db:seed`, `db:migrate:fresh` and `db:migrate:refresh` all await. `db:migrate:refresh` is now registered. | | [033](033-async-seeder-stub.md) | 2026-10-21 | Generated seeders have an async run | `create_seed.stub` generated a synchronous `run` with docstrings, but `Seeder` awaits `run()`, so every generated table seeder raised `TypeError`. The stub now generates `async def run(self) -> None` with no docstrings; `Seeder` is unchanged. | +| [037](037-http-exception-status-and-fastapi-title.md) | 2026-10-25 | HTTP exceptions keep their status; FastAPI title from config | `HTTPExceptionHandler` rendered every `HTTPException` and `ModelNotFoundException` as 500. Starlette `HTTPException` now goes through FastAPI's `http_exception_handler`, which keeps the status, the `detail` and the headers. A new dependency-free `fastapi_startkit.exceptions.HttpException` base (`get_status`/`get_response`/`get_headers`) is subclassed by `ModelNotFoundException`, `RouteNotFoundException`, `MethodNotAllowedException`, `AuthorizationException` and `ThrottleRequestsException` (429 with `Retry-After`), and is registered as its own FastAPI handler so it never reaches the re-raising `ServerErrorMiddleware`. `HTTPExceptionHandler.report` skips 4xx HTTP exceptions and still reports 5xx. `FastAPIConfig.title` reads `APP_NAME` (default `FastAPI Startkit`). `Application.__init__` no longer discards the FastAPI instance a provider creates in `register`. | | [038](038-storage-binary-copy-and-request-scoped-vite-inertia.md) | 2026-10-25 | Storage binary reads and relative copy; request-scoped Vite and Inertia state | `LocalDriver.copy` resolves both paths through `get_path`. Local and S3 `get` read bytes and return `str` for UTF-8 content or raw `bytes` otherwise, and a new `get_bytes` (also on `StorageManager`/`Storage`) always returns bytes. `get` returns `bytes` for any non-UTF-8 content, including latin-1 text. `S3Driver.append` keeps the existing content. Vite `use_*_tag_attributes` accept dicts and `False`; the self-referencing lambda is gone. A pure ASGI `ViteMiddleware` (registered by `ViteProvider`) opens `Vite.request_scope()`, which holds the request's CSP nonce and preloaded assets in a per-instance `ContextVar` shared with child tasks. Outside a scope, the instance values are process-wide defaults, so a nonce set at startup still applies, and middleware inside the scope can read preloaded assets after `call_next`. `Inertia.set_root_view`/`Inertia.version` write to the current `InertiaRequestState` inside a request and to global defaults otherwise. | diff --git a/fastapi_startkit/src/fastapi_startkit/application.py b/fastapi_startkit/src/fastapi_startkit/application.py index f117c16a..e9c93c13 100644 --- a/fastapi_startkit/src/fastapi_startkit/application.py +++ b/fastapi_startkit/src/fastapi_startkit/application.py @@ -61,9 +61,8 @@ def __init__( self.resolve_environment() self.configure_config() self.configure_paths() - self.register_providers() - self._fastapi: Optional["FastAPI"] = None + self.register_providers() self.load_providers() def set_environment(self, env: str): diff --git a/fastapi_startkit/src/fastapi_startkit/exceptions/__init__.py b/fastapi_startkit/src/fastapi_startkit/exceptions/__init__.py index dd052e35..bb3edbf1 100644 --- a/fastapi_startkit/src/fastapi_startkit/exceptions/__init__.py +++ b/fastapi_startkit/src/fastapi_startkit/exceptions/__init__.py @@ -1,6 +1,7 @@ from .handler import ExceptionHandler from .exceptions import ( AuthorizationException, + HttpException, InvalidRouteCompileException, RouteMiddlewareNotFound, ContainerError, @@ -36,6 +37,7 @@ __all__ = [ "ExceptionHandler", "AuthorizationException", + "HttpException", "InvalidRouteCompileException", "RouteMiddlewareNotFound", "ContainerError", diff --git a/fastapi_startkit/src/fastapi_startkit/exceptions/exceptions.py b/fastapi_startkit/src/fastapi_startkit/exceptions/exceptions.py index 2a81c97e..a688a19b 100644 --- a/fastapi_startkit/src/fastapi_startkit/exceptions/exceptions.py +++ b/fastapi_startkit/src/fastapi_startkit/exceptions/exceptions.py @@ -70,9 +70,20 @@ class RouteException(Exception): pass -class RouteNotFoundException(Exception): +class HttpException(Exception): is_http_exception = True + def get_status(self) -> int: + raise NotImplementedError + + def get_response(self) -> str: + raise NotImplementedError + + def get_headers(self) -> dict[str, str] | None: + return None + + +class RouteNotFoundException(HttpException): def __init__(self, message): super().__init__(message) self.message = message or "Route Not Found" @@ -84,9 +95,7 @@ def get_status(self): return 404 -class MethodNotAllowedException(Exception): - is_http_exception = True - +class MethodNotAllowedException(HttpException): def __init__(self, allowed_methods, method): allowed_list = ", ".join(allowed_methods) message = f"{method} method not allowed for this route. Supported methods are: {allowed_list}." @@ -144,9 +153,7 @@ class NotificationException(Exception): pass -class AuthorizationException(Exception): - is_http_exception = True - +class AuthorizationException(HttpException): def __init__(self, message="", status=403): super().__init__(message) self.message = message or "Action not authorized" @@ -159,8 +166,8 @@ def get_status(self): return self.status -class ThrottleRequestsException(Exception): - def __init__(self, message="Too many attempts", status=429, headers={}): +class ThrottleRequestsException(HttpException): + def __init__(self, message="Too many attempts", status=429, headers: dict[str, str] | None = None): super().__init__(message) self.message = message self.status = status diff --git a/fastapi_startkit/src/fastapi_startkit/fastapi/config/fastapi.py b/fastapi_startkit/src/fastapi_startkit/fastapi/config/fastapi.py index e2a9ab47..c9ab2b5d 100644 --- a/fastapi_startkit/src/fastapi_startkit/fastapi/config/fastapi.py +++ b/fastapi_startkit/src/fastapi_startkit/fastapi/config/fastapi.py @@ -7,6 +7,7 @@ @dataclasses.dataclass class FastAPIConfig(Config): + title: str = env("APP_NAME", "FastAPI Startkit") app_url: str = env("APP_URL", "http://127.0.0.1:8000") app: str = "bootstrap.application:app" reload: bool = env("APP_RELOAD", True) diff --git a/fastapi_startkit/src/fastapi_startkit/fastapi/exceptions.py b/fastapi_startkit/src/fastapi_startkit/fastapi/exceptions.py index 6deb7027..3f5ff1cb 100644 --- a/fastapi_startkit/src/fastapi_startkit/fastapi/exceptions.py +++ b/fastapi_startkit/src/fastapi_startkit/fastapi/exceptions.py @@ -9,10 +9,43 @@ class HTTPExceptionHandler: The base exception handler for FastAPI applications. """ + def report(self, exc) -> None: + from fastapi_startkit.container import Container + + status = self.http_status(exc) + if status is not None and status < 500: + return + + cast("Application", Container.instance()).exception_manager.report_exception(exc) + + @staticmethod + def http_status(exc) -> int | None: + from starlette.exceptions import HTTPException + + from fastapi_startkit.exceptions import HttpException + + if isinstance(exc, HTTPException): + return exc.status_code + if isinstance(exc, HttpException): + return exc.get_status() + return None + async def render(self, request, exc): import traceback + from fastapi.exception_handlers import http_exception_handler from fastapi.responses import JSONResponse + from starlette.exceptions import HTTPException + from fastapi_startkit.container import Container + from fastapi_startkit.exceptions import HttpException + + if isinstance(exc, HTTPException): + return await http_exception_handler(request, exc) + + if isinstance(exc, HttpException): + return await http_exception_handler( + request, HTTPException(exc.get_status(), exc.get_response(), headers=exc.get_headers()) + ) app = cast("Application", Container.instance()) if app.is_debug(): diff --git a/fastapi_startkit/src/fastapi_startkit/fastapi/providers/fastapi_provider.py b/fastapi_startkit/src/fastapi_startkit/fastapi/providers/fastapi_provider.py index fc478a2a..de35c996 100644 --- a/fastapi_startkit/src/fastapi_startkit/fastapi/providers/fastapi_provider.py +++ b/fastapi_startkit/src/fastapi_startkit/fastapi/providers/fastapi_provider.py @@ -16,7 +16,7 @@ def register(self) -> None: self.merge_config_from(config, self.provider_key) fastapi = FastAPI( - title="Jobins AI Agent (LangChain)", + title=config["title"], version="1.0.0", ) @@ -34,8 +34,10 @@ def boot(self): def _register_exception_handlers(self): """Wire exception_manager as a catch-all handler for all exceptions.""" - from fastapi import HTTPException from fastapi.exceptions import RequestValidationError + from starlette.exceptions import HTTPException + + from fastapi_startkit.exceptions import HttpException exception_manager = self.app.exception_manager exception_manager.register_handler(Exception, HTTPExceptionHandler()) @@ -58,5 +60,6 @@ async def handler(request, exc): # FastAPI registers its own handlers for these two types internally, # so they must be overridden explicitly self.app.fastapi.add_exception_handler(HTTPException, handler) + self.app.fastapi.add_exception_handler(HttpException, handler) self.app.fastapi.add_exception_handler(RequestValidationError, handler) self.app.fastapi.add_exception_handler(Exception, handler) diff --git a/fastapi_startkit/src/fastapi_startkit/masoniteorm/exceptions.py b/fastapi_startkit/src/fastapi_startkit/masoniteorm/exceptions.py index ff299737..1ec5288e 100644 --- a/fastapi_startkit/src/fastapi_startkit/masoniteorm/exceptions.py +++ b/fastapi_startkit/src/fastapi_startkit/masoniteorm/exceptions.py @@ -1,6 +1,7 @@ -class ModelNotFoundException(Exception): - is_http_exception = True +from fastapi_startkit.exceptions import HttpException + +class ModelNotFoundException(HttpException): def __init__(self, message="Model Not Found"): super().__init__(message) self.message = message diff --git a/fastapi_startkit/tests/fastapi/test_exceptions.py b/fastapi_startkit/tests/fastapi/test_exceptions.py index d696385b..907873ad 100644 --- a/fastapi_startkit/tests/fastapi/test_exceptions.py +++ b/fastapi_startkit/tests/fastapi/test_exceptions.py @@ -8,9 +8,15 @@ import json from types import SimpleNamespace -from unittest.mock import patch +from unittest.mock import MagicMock, patch + +import pytest +from fastapi import HTTPException +from starlette.exceptions import HTTPException as StarletteHTTPException from fastapi_startkit.container import Container +from fastapi_startkit.exceptions import AuthorizationException, MethodNotAllowedException, ThrottleRequestsException +from fastapi_startkit.masoniteorm.exceptions import ModelNotFoundException from fastapi_startkit.fastapi.exceptions import ( HTTPExceptionHandler, ValidationExceptionHandler, @@ -67,6 +73,57 @@ async def test_production_response_hides_details(self): assert response.status_code == 500 assert json.loads(response.body) == {"message": "Server Error"} + async def test_framework_http_exception_keeps_status_detail_and_headers(self): + response = await HTTPExceptionHandler().render(FakeRequest(), MethodNotAllowedException(["GET"], "POST")) + + assert response.status_code == 405 + assert response.headers["allow"] == "GET" + assert json.loads(response.body)["detail"].startswith("POST method not allowed") + + async def test_authorization_exception_returns_403(self): + response = await HTTPExceptionHandler().render(FakeRequest(), AuthorizationException()) + + assert response.status_code == 403 + assert json.loads(response.body) == {"detail": "Action not authorized"} + + async def test_throttle_exception_returns_429_with_retry_after(self): + exc = ThrottleRequestsException(headers={"Retry-After": "60"}) + + response = await HTTPExceptionHandler().render(FakeRequest(), exc) + + assert response.status_code == 429 + assert response.headers["retry-after"] == "60" + assert json.loads(response.body) == {"detail": "Too many attempts"} + + +class TestHTTPExceptionHandlerReport: + @pytest.mark.parametrize( + "exc", + [ + HTTPException(404), + StarletteHTTPException(422), + AuthorizationException(), + ThrottleRequestsException(), + ModelNotFoundException(), + ], + ) + def test_client_errors_are_not_reported(self, exc): + manager = MagicMock() + + with patch.object(Container, "instance", return_value=SimpleNamespace(exception_manager=manager)): + HTTPExceptionHandler().report(exc) + + manager.report_exception.assert_not_called() + + @pytest.mark.parametrize("exc", [HTTPException(503), RuntimeError("boom")]) + def test_server_errors_are_reported(self, exc): + manager = MagicMock() + + with patch.object(Container, "instance", return_value=SimpleNamespace(exception_manager=manager)): + HTTPExceptionHandler().report(exc) + + manager.report_exception.assert_called_once_with(exc) + class TestValidationExceptionHandler: def test_report_is_noop(self): diff --git a/fastapi_startkit/tests/fastapi/test_http_exception_rendering.py b/fastapi_startkit/tests/fastapi/test_http_exception_rendering.py new file mode 100644 index 00000000..2f2842f8 --- /dev/null +++ b/fastapi_startkit/tests/fastapi/test_http_exception_rendering.py @@ -0,0 +1,97 @@ +import os +import tempfile +from pathlib import Path +from unittest.mock import patch + +from fastapi import HTTPException +from starlette.exceptions import HTTPException as StarletteHTTPException + +from fastapi_startkit.application import Application +from fastapi_startkit.container.container import Container +from fastapi_startkit.fastapi import FastAPIProvider +from fastapi_startkit.fastapi.testing import HttpTestCase +from fastapi_startkit.masoniteorm.exceptions import ModelNotFoundException + + +def make_application(base_path: Path) -> Application: + app = Application(base_path=base_path, env="testing", providers=[FastAPIProvider]) + + @app.fastapi.get("/email-taken") + def email_taken(): + raise HTTPException(400, "Email taken", headers={"X-Reason": "duplicate"}) + + @app.fastapi.get("/teapot") + def teapot(): + raise StarletteHTTPException(418, "I'm a teapot") + + @app.fastapi.get("/missing-model") + def missing_model(): + raise ModelNotFoundException("User with primary key 7 not found.") + + return app + + +class HttpExceptionRenderingTest(HttpTestCase): + def get_application(self) -> Application: + if not hasattr(self, "_application"): + self._tmp_dir = tempfile.TemporaryDirectory() + self.addCleanup(self._tmp_dir.cleanup) + original_instance = Container._instance + self.addCleanup(lambda: setattr(Container, "_instance", original_instance)) + self._application = make_application(Path(self._tmp_dir.name)) + return self._application + + async def test_http_exception_keeps_status_detail_and_headers(self): + response = await self.get("/email-taken") + + response.assert_status(400) + self.assertEqual(response.json()["detail"], "Email taken") + self.assertEqual(response.headers["X-Reason"], "duplicate") + + async def test_starlette_http_exception_keeps_status_and_detail(self): + response = await self.get("/teapot") + + response.assert_status(418) + self.assertEqual(response.json()["detail"], "I'm a teapot") + + async def test_unknown_route_returns_404(self): + response = await self.get("/does-not-exist") + + response.assert_status(404) + self.assertEqual(response.json()["detail"], "Not Found") + + async def test_model_not_found_returns_404(self): + response = await self.get("/missing-model") + + response.assert_status(404) + self.assertEqual(response.json()["detail"], "User with primary key 7 not found.") + + +class FastAPITitleTest(HttpTestCase): + def get_application(self) -> Application: + if not hasattr(self, "_application"): + self._tmp_dir = tempfile.TemporaryDirectory() + self.addCleanup(self._tmp_dir.cleanup) + original_instance = Container._instance + self.addCleanup(lambda: setattr(Container, "_instance", original_instance)) + with patch.dict(os.environ, {"APP_NAME": "Acme Portal"}): + self._application = Application( + base_path=Path(self._tmp_dir.name), env="testing", providers=[FastAPIProvider] + ) + return self._application + + async def test_title_comes_from_app_name(self): + response = await self.get("/openapi.json") + + self.assertEqual(response.json()["info"]["title"], "Acme Portal") + + +def test_title_defaults_to_fastapi_startkit(): + original_instance = Container._instance + try: + with tempfile.TemporaryDirectory() as tmp, patch.dict(os.environ, clear=False) as environ: + environ.pop("APP_NAME", None) + app = Application(base_path=Path(tmp), env="testing", providers=[FastAPIProvider]) + assert app.fastapi.title == "FastAPI Startkit" + finally: + Container._instance = original_instance