Repository navigation
Fix HTTPException/ModelNotFound returning 500 and hard-coded FastAPI title - #315
Merged
Merged
Conversation
HTTPException and ModelNotFoundException rendered as 500 because HTTPExceptionHandler always built a 500 response. Starlette HTTP exceptions now go through FastAPI's http_exception_handler, and a new HttpException base for framework exceptions gets its own FastAPI handler. The hard-coded FastAPI title was never used, because Application reset _fastapi after providers registered. The title now comes from FastAPIConfig.title (APP_NAME, default "FastAPI Startkit").
claude-tmgbedu
pushed a commit
that referenced
this pull request
Oct 25, 2026
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SZhGWNwFRzgkFffoHce4Ht
ThrottleRequestsException now subclasses HttpException, so it renders as 429 and passes on headers such as Retry-After. Its headers default changes from a shared mutable dict to None. HTTPExceptionHandler.report now skips client errors: Starlette/FastAPI HTTPException and framework HttpException with a status below 500. 5xx and non-HTTP exceptions are still reported through the exception manager. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018JLUqZGzL9UdCwwax5fT9B
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
claude-tmgbedu
commented
Oct 25, 2026
claude-tmgbedu
left a comment
Author
There was a problem hiding this comment.
Review + QA of PR #315 (head def516f): APPROVE, with non-blocking notes
Tests
uv run pytest --ignore=tests/masoniteorm/postgres --cov: 3310 passed, 10 skipped, coverage 91.65% (gate is 80%).ruffis clean on the changed files.- Core
fastapi_startkit.exceptionsandmasoniteorm.exceptionsimport fine withstarlette/fastapiblocked from import, so there is no import cycle and no Starlette dependency in core.
Scratch QA
I ran 16 repros with HttpTestCase. The app was real: DatabaseProvider on SQLite plus FastAPIProvider, with a user-published FastAPIConfig that has no title field. All 16 pass.
| Scenario | Result |
|---|---|
HTTPException(400, "Email taken") |
400 {"detail": "Email taken"} |
| Unknown route | 404 {"detail": "Not Found"} and x-request-id present |
| Wrong method | 405 with Allow: POST |
Real User.find_or_fail(7) on SQLite |
404 {"detail": "User with primary key 7 not found."} |
ThrottleRequestsException(headers={"Retry-After": "60"}) |
429 with Retry-After: 60 |
AuthorizationException() |
403 |
Depends() raising HTTPException(401) |
401 with detail |
JSON RequestValidationError |
422 {"errors": ...}, unchanged |
RuntimeError |
500 {"message": "Server Error"}, report_exception called once |
| 400, 404, 429 | not reported |
HTTPException(503) |
reported |
APP_DEBUG=true + RuntimeError |
500 with trace |
APP_DEBUG=true + 4xx |
still {"detail": ...} |
/docs |
shows the APP_NAME title. A published config without title still works, because resolve_config merges the defaults (verified against the example/fastapi-app and example/agents config shape). |
Custom exception_manager.register_handler(ModelNotFoundException, ...) |
still takes precedence |
fastapi.add_exception_handler(404, ...) status-code handler |
still takes precedence |
Notes (non-blocking)
HttpExceptionraised fromBaseHTTPMiddlewareis still re-raised (fastapi/providers/fastapi_provider.py:62-65). Repro: a middleware that raisesThrottleRequestsException. The client now correctly gets 429 (before this PR it got 500). The exception is handled by the bare-Exceptionhandler onServerErrorMiddleware, though, and that middleware re-raises it. Uvicorn therefore logs a traceback for a 4xx. This behaviour existed before the PR and is outside its scope. It matters once a throttle middleware lands.ThrottleRequestsException.headersis nowNoneby default, not{}(exceptions/exceptions.py:169-174). Fixing the shared mutable default is right. Code that mutatesexc.headers[...]on a default-constructed instance would now fail, so consider a changelog line.HttpException.get_status/get_responseraiseNotImplementedError(exceptions/exceptions.py:76-80). A user subclass that forgets to override them raises a second exception inside the handler, and the response becomes a re-raised 500. A defaultstatus = 500/messagepair, orabc.abstractmethod, would fail earlier and more clearly. This is a nit.- 4xx bodies use FastAPI's
{"detail": ...}, while 500 uses{"message": ...}. Neither existing 4xx clients nor 422 regress, since these responses were all 500 before. Browser/Inertia requests that hit a 404/403 still get JSON rather than an HTML error page. That could be a follow-up. - The
Application.__init__reorder (application.py:64-65) now keeps instances created inregister(). For example,example/inertia-pingcrm-appnow really getstitle="Vite Example", which it previously dropped. I found no provider that touchesapp.fastapiduringregister()beforeFastAPIProviderruns (broadcasting, redis, storage and vite all do it inboot()), so I see no side effect.
The ADR 037 is present and indexed in docs/adr/AGENTS.md, and no new explanatory comments or docstrings were added.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
HTTPExceptionHandlerbuilt a 500 response for every exception it received. Starlette and FastAPIHTTPExceptions now go through FastAPI'shttp_exception_handler, so they keep their status,detailand headers. A new dependency-freefastapi_startkit.exceptions.HttpExceptionbase is subclassed byModelNotFoundException(404),RouteNotFoundException,MethodNotAllowedExceptionandAuthorizationException. It is registered as its own FastAPI handler, so these exceptions no longer reach Starlette'sServerErrorMiddleware, which re-raises.FastAPIConfig.titlereadsAPP_NAME(defaultFastAPI Startkit). The hard-coded title had never taken effect:Application.__init__reset_fastapi = Noneafterregister_providers(), which threw away the provider's instance._fastapiis now set up before providers register.Test plan
tests/fastapi/test_http_exception_rendering.py(HttpTestCase) checks:HTTPException(400, "Email taken")returns 400 withdetailand custom headersHTTPExceptionreturns 418ModelNotFoundExceptionreturns 404APP_NAMEsets the title, and the default title is used without itAllowheader and 403.uv run pytest --ignore=tests/masoniteorm/postgres --cov: 3302 passed, 91.59% coverage🤖 Generated with Claude Code
https://claude.ai/code/session_01QgHs6PRFJehnb1BbCNtLww