Implement API response contract and custom exception handling#35
Merged
saudzahirr merged 7 commits intomainfrom Feb 24, 2026
Merged
Implement API response contract and custom exception handling#35saudzahirr merged 7 commits intomainfrom
saudzahirr merged 7 commits intomainfrom
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review infoConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds request-ID middleware, a hierarchical API exception hierarchy and handlers that return structured JSON error envelopes (including request_id), new response schemas, registers middleware and handlers in the FastAPI app, and replaces a deps HTTPException with ServiceUnavailableException; plus .gitignore expansions. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Middleware as add_request_id
participant Router as FastAPI Handler
participant DB as deps.get_db
participant ExceptionHandler as Global Exception Handlers
participant Response as HTTP Response
Client->>Middleware: HTTP Request (maybe X-Request-ID)
Middleware->>Middleware: extract or generate request_id (UUID4)
Middleware->>Router: call_next(request) with request.state.request_id
alt Handler calls DB
Router->>DB: deps.get_db()
DB-->>Router: connection or raise ServiceUnavailableException
end
alt Success
Router-->>Response: JSONResponse (success) + X-Request-ID
else APIException
Router->>ExceptionHandler: APIException
ExceptionHandler->>ExceptionHandler: build error envelope with request_id
ExceptionHandler-->>Response: JSONResponse (API status) + X-Request-ID
else RequestValidationError
Router->>ExceptionHandler: RequestValidationError
ExceptionHandler->>ExceptionHandler: parse validation errors, build envelope
ExceptionHandler-->>Response: JSONResponse (422) + X-Request-ID
else Unhandled Exception
Router->>ExceptionHandler: Exception
ExceptionHandler->>ExceptionHandler: log traceback, build 500 envelope
ExceptionHandler-->>Response: JSONResponse (500) + X-Request-ID
end
Response->>Client: HTTP Response
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@backend/API_RESPONSE_CONTRACT.md`:
- Around line 145-165: The fenced code blocks under the "Example
Request/Response with Request ID" section (and the other flagged ranges) are
missing surrounding blank lines and language tags; update each request/response
fence to have a blank line before and after the triple-backticks and add an
appropriate language tag (e.g., ```http for the request/response examples and
```json for JSON bodies) so that all examples follow markdownlint rules and
render correctly.
In `@backend/app/core/exceptions.py`:
- Around line 26-35: The ValidationException currently constructs the base error
with http_status=400 while using ErrorCode.VALIDATION_FAILED; change the
http_status passed in ValidationException.__init__ to 422 so the class aligns
with the API contract and existing Pydantic handler; also update the example
entry in the response schema (in backend/app/schemas/response.py) that documents
VALIDATION_FAILED to show http_status: 422 instead of 400 to keep docs
consistent with the exception class.
In `@backend/app/main.py`:
- Around line 91-94: Replace eager f-string interpolation in logging calls with
lazy %-style formatting: change the logger.warning call that currently uses
f"API Exception: {exc.error_code} - {exc.message}" to use a format string and
pass exc.error_code and exc.message as separate positional args (keeping the
extra={"request_id": request_id, "error_code": exc.error_code} argument). Do the
same refactor for the other logging statements mentioned (the similar logger.*
calls at the other occurrences around the API exception handling blocks) so all
logging uses format strings like "API Exception: %s - %s" with parameters
instead of f-strings.
- Line 24: The code passes settings.DATABASE_DSN (a PostgresDsn object) directly
into ConnectionPool; convert it to a plain string before use. Update the
instantiation of ConnectionPool so it receives str(settings.DATABASE_DSN) (same
fix applied in pre_start.py), ensuring any code that constructs the pool (the
ConnectionPool call in main.py) receives a string DSN rather than the
PostgresDsn object.
In `@build/lib/backend/app/api/deps.py`:
- Around line 11-12: get_db currently raises a plain HTTPException which
bypasses the new standardized error handlers; replace the HTTPException raise in
get_db with raising APIException(status_code=500, detail="Database pool not
initialized") (or equivalent constructor used by your APIException), and
add/import APIException into deps.py so the custom exception middleware will
format the response consistently.
In `@build/lib/backend/app/main.py`:
- Line 3: The imports typing.Any and http_exception_handler are unused in
main.py; remove the unused import statement "from typing import Any" and the
import of http_exception_handler (where it's imported) so the file only imports
symbols that are actually used (e.g., delete or trim the import lines
referencing Any and http_exception_handler and run the linter/type-check to
confirm no references remain).
- Line 26: settings.DATABASE_DSN is a PostgresDsn object, not a plain string, so
passing it directly to ConnectionPool causes a type/serialization issue; update
the ConnectionPool instantiation in main.py to pass str(settings.DATABASE_DSN)
(i.e., convert the PostgresDsn to a string) so
ConnectionPool(settings.DATABASE_DSN) becomes
ConnectionPool(str(settings.DATABASE_DSN)), ensuring ConnectionPool receives a
proper DSN string.
- Around line 88-97: Remove the dead stub function get_request_id_from_context()
from the module: it only imports fastapi/Headers inside itself, catches all
exceptions, and always returns a new UUID, so delete the entire function
definition (def get_request_id_from_context(...)) to eliminate non-functional
code; ensure no remaining references to get_request_id_from_context() exist in
the file before committing.
---
Duplicate comments:
In `@backend/app/schemas/response.py`:
- Around line 56-67: The example in Config.json_schema_extra is inconsistent:
the "error.code" ("VALIDATION_FAILED") doesn't match the canonical HTTP status
chosen elsewhere; update this example in backend/app/schemas/response.py (Config
-> json_schema_extra -> example) so the "error.code", "error.message" and
"error.http_status" reflect the canonical status decided in the exception
hierarchy (e.g., if the canonical status for validation is 422 change
http_status to 422 and keep code "VALIDATION_FAILED" and a validation message;
if the canonical status is 400 change code to "BAD_REQUEST" and adjust message
accordingly), and ensure "details" and "request_id" remain representative.
In `@build/lib/backend/app/core/exceptions.py`:
- Around line 26-35: The ValidationException currently sets http_status=400 in
__init__ (class ValidationException) which conflicts with the API contract;
update ValidationException to use the contract-approved status (e.g.,
http_status=422) and ensure the change is applied both in the source
ValidationException implementation and in this built artifact
(build/lib/backend/app/core/exceptions.py) so they stay in sync, then regenerate
the build artifacts so the compiled module reflects the corrected http_status.
In `@build/lib/backend/app/schemas/response.py`:
- Around line 57-67: The json_schema_extra example in response.py currently
shows an error object with code "VALIDATION_FAILED" but the http_status value is
inconsistent with the canonical status chosen; update the example inside
json_schema_extra so the "http_status" matches the officially chosen status for
VALIDATION_FAILED (e.g., set to 400 if validation errors are 400, or change the
"code" to the status-aligned code), and ensure the "code" and "http_status"
inside the example error object are consistent with the canonical exception
handling used elsewhere.
Member
Author
|
@laraibg786 Please take a look! |
Member
Author
|
Resolves #5 |
laraibg786
requested changes
Feb 21, 2026
Collaborator
laraibg786
left a comment
laraibg786
left a comment
There was a problem hiding this comment.
There is a build dir in the diff of this PR. I think you forgot to remove this. Please also check if this is something we want to be included in .gitignore
12 tasks
…d API response management
…rom main application
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@backend/app/main.py`:
- Around line 77-103: The API exception handlers (api_exception_handler,
validation_exception_handler, general_exception_handler) currently log a
request_id and include it in the JSON body but don't set the X-Request-ID
response header; update each handler to read/derive request_id the same way and
include it in the JSONResponse by passing headers={"X-Request-ID": request_id}
so clients receive the header consistently when exceptions bypass middleware.
ℹ️ Review info
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (6)
.gitignorebackend/app/api/deps.pybackend/app/api/middlewares.pybackend/app/core/exceptions.pybackend/app/main.pybackend/app/schemas/response.py
There was a problem hiding this comment.
♻️ Duplicate comments (1)
backend/app/core/exception_handlers.py (1)
15-107:⚠️ Potential issue | 🟠 MajorAdd
X-Request-IDheader on error responses.When exceptions occur, the request ID middleware won’t always set the header; clients will miss trace IDs even though the body includes
request_id. Add the header directly in each handler’s JSONResponse.🐛 Proposed fix (apply same pattern to all three handlers)
- return JSONResponse( - status_code=exc.http_status, - content=error_response, - ) + return JSONResponse( + status_code=exc.http_status, + content=error_response, + headers={"X-Request-ID": request_id}, + )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/app/core/exception_handlers.py` around lines 15 - 107, The error handlers api_exception_handler, validation_exception_handler, and general_exception_handler should include the trace ID in the response headers; after you compute request_id in each handler add the header "X-Request-ID" to the JSONResponse (e.g., return JSONResponse(..., headers={"X-Request-ID": request_id} ) ) so clients receive the same request_id shown in the response body; update each handler (api_exception_handler, validation_exception_handler, general_exception_handler) to pass the headers dict to JSONResponse.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@backend/app/core/exception_handlers.py`:
- Around line 15-107: The error handlers api_exception_handler,
validation_exception_handler, and general_exception_handler should include the
trace ID in the response headers; after you compute request_id in each handler
add the header "X-Request-ID" to the JSONResponse (e.g., return
JSONResponse(..., headers={"X-Request-ID": request_id} ) ) so clients receive
the same request_id shown in the response body; update each handler
(api_exception_handler, validation_exception_handler, general_exception_handler)
to pass the headers dict to JSONResponse.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@backend/app/api/middlewares.py`:
- Around line 6-9: The middleware add_request_id currently uses
request.headers.get("X-Request-ID", str(uuid.uuid4())) which will propagate an
empty string if the header exists but is blank; change the logic in
add_request_id to check the header value explicitly (e.g., read raw_header =
request.headers.get("X-Request-ID") and set request_id = raw_header if
raw_header and raw_header.strip() else str(uuid.uuid4())) and assign that to
request.state.request_id so an empty header falls back to a generated UUID.
In `@backend/app/core/exception_handlers.py`:
- Around line 49-56: When building validation details in the exception handler
loop (for error in exc.errors()) avoid leaving the "field" as an empty string
for root-level errors: compute the joined locator string from error["loc"][1:],
and if that join yields "" set the field to None before appending to details;
update the block that constructs the dict (the "field" entry) in the exception
handler module so root-level validation errors use None instead of "".
ℹ️ Review info
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (8)
.gitignorebackend/app/api/deps.pybackend/app/api/middlewares.pybackend/app/core/exception_handlers.pybackend/app/core/exceptions.pybackend/app/main.pybackend/app/schemas/__init__.pybackend/app/schemas/response.py
…or response structure
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@backend/app/api/middlewares.py`:
- Around line 6-10: The middleware add_request_id currently uses the raw
X-Request-ID header value (raw_header) when present, which preserves surrounding
whitespace; change it to trim the header before using it: compute a stripped
value (raw_header.strip()) and set request.state.request_id to that stripped
value only if it's non-empty, otherwise generate a new uuid with
str(uuid.uuid4()); update the logic around request.headers.get("X-Request-ID")
and request.state.request_id in add_request_id accordingly.
ℹ️ Review info
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (3)
backend/app/api/middlewares.pybackend/app/core/exception_handlers.pybackend/app/main.py
This was referenced Mar 5, 2026
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
2 participants
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 by CodeRabbit
New Features
Bug Fixes
Chores