Enhance CI workflow and implement global exception handling#4
Enhance CI workflow and implement global exception handling#4WillianSilva51 merged 3 commits intomainfrom
Conversation
…factor security and quotes API
…error response schema
Removed the Check Sync step from the workflow.
There was a problem hiding this comment.
Pull request overview
This PR introduces centralized, structured exception handling for the FastAPI app, standardizing error responses via a new ErrorResponse schema, and updates CI/dependencies and a couple of API parameter/type annotations.
Changes:
- Added
ErrorResponseschema plus custom domain exceptions and centralized exception handlers returning standardized JSON. - Registered exception handlers globally in
api/main.pyand adjusted a couple of API typings/validations (verify_api_keyoptional input; quoteslimitallows 0). - Updated dependency export/checking (
requirements.txtregeneration + CI “Check Sync” step) and core package exports.
Reviewed changes
Copilot reviewed 10 out of 11 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| requirements.txt | Regenerated exported dependency set with uv export annotations/markers. |
| api/schemas/error_schema.py | Introduces a standard error response model + helper constructor. |
| api/schemas/init.py | Exposes error_schema from the schemas package. |
| api/exceptions/custom_exceptions.py | Adds two custom exception types for domain/not-found cases. |
| api/exceptions/handlers.py | Adds FastAPI exception handlers that map errors to ErrorResponse JSON. |
| api/exceptions/init.py | Exposes the new exceptions modules. |
| api/main.py | Registers global exception handlers and refactors lifespan signature. |
| api/core/security.py | Makes verify_api_key accept None from APIKeyHeader(auto_error=False). |
| api/core/init.py | Updates exported core modules. |
| api/routers/quotes.py | Updates dependency typing and allows limit=0 via query validation. |
| .github/workflows/python-tests.yml | Adds “Check Sync” step to enforce requirements.txt matches lock export. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| app.add_exception_handler(HTTPException, http_handler) # type: ignore | ||
| app.add_exception_handler(DomainValidationException, domain_validation_handler) # type: ignore | ||
| app.add_exception_handler(ResourceNotFoundException, resource_not_found_handler) # type: ignore | ||
| app.add_exception_handler(RequestValidationError, request_validation_handler) # type: ignore | ||
| app.add_exception_handler(Exception, global_exception_handler) |
There was a problem hiding this comment.
Global exception handler registration introduces an API-level contract (standardized error shape), but there are no automated tests asserting the JSON schema/status codes for common cases (HTTPException, validation errors, uncaught exceptions). Adding a small FastAPI TestClient suite around these handlers would prevent regressions and ensure clients can rely on the new error response format.
| return _json_error_response( | ||
| ErrorResponse.from_http_status( | ||
| HTTPStatus(exc.status_code), | ||
| message=exc.detail, |
There was a problem hiding this comment.
http_handler passes exc.detail into ErrorResponse.message, but HTTPException.detail can be non-string (e.g., dict/list). That will raise a Pydantic validation error inside the exception handler and can prevent the standardized error response from being returned. Consider ensuring message is always a string (e.g., str(exc.detail) when not a string) and optionally put the original detail payload into details.
| return _json_error_response( | |
| ErrorResponse.from_http_status( | |
| HTTPStatus(exc.status_code), | |
| message=exc.detail, | |
| detail = exc.detail | |
| message = detail if isinstance(detail, str) else str(detail) | |
| details = None if isinstance(detail, str) else detail | |
| return _json_error_response( | |
| ErrorResponse.from_http_status( | |
| HTTPStatus(exc.status_code), | |
| message=message, | |
| details=details, |
| return _json_error_response( | ||
| ErrorResponse.from_http_status( | ||
| HTTPStatus(exc.status_code), |
There was a problem hiding this comment.
HTTPStatus(exc.status_code) will raise ValueError for non-standard/custom HTTP status codes (FastAPI allows any int). If that happens, the handler itself will error and you’ll lose the intended response formatting. Consider falling back to HTTPStatus.INTERNAL_SERVER_ERROR or using the raw exc.status_code int when it’s not a valid HTTPStatus member.
| return _json_error_response( | |
| ErrorResponse.from_http_status( | |
| HTTPStatus(exc.status_code), | |
| status = ( | |
| HTTPStatus(exc.status_code) | |
| if exc.status_code in HTTPStatus._value2member_map_ | |
| else HTTPStatus.INTERNAL_SERVER_ERROR | |
| ) | |
| return _json_error_response( | |
| ErrorResponse.from_http_status( | |
| status, |
| @classmethod | ||
| def from_http_status( | ||
| cls, | ||
| status_code: HTTPStatus, | ||
| message: str, | ||
| details: Sequence[Any] | dict[str, Any] | list[Any] | None = None, | ||
| error: str | None = None, | ||
| ) -> "ErrorResponse": | ||
| return ErrorResponse( | ||
| status_code=status_code, | ||
| error=error or status_code.phrase, | ||
| message=message, | ||
| details=details, | ||
| ) |
There was a problem hiding this comment.
from_http_status is a @classmethod but it constructs ErrorResponse(...) directly instead of using cls(...). Using cls avoids hard-coding the concrete type and makes the method safe to reuse if this model is subclassed or customized later.
This pull request introduces a structured and consistent error handling system to the API, along with some minor improvements to dependency management and parameter validation. The changes add custom exception classes, centralized exception handlers, and a standardized error response schema, ensuring that errors are handled gracefully and returned in a predictable format.
Error handling improvements:
ResourceNotFoundExceptionandDomainValidationExceptionincustom_exceptions.pyto represent specific error scenarios.handlers.pyfor custom exceptions, request validation errors, HTTP errors, and uncaught exceptions, all returning standardized JSON responses using the newErrorResponseschema.main.pyso that all relevant errors are caught and processed consistently across the API. [1] [2]ErrorResponseschema inerror_schema.pyto standardize the structure of error responses, including status code, error message, and optional details.__all__lists inexceptionsandschemaspackages to include the new modules and ensure proper exposure. [1] [2]Other improvements:
verify_api_keydependency and its usage in routers acceptNoneas a valid input, improving type safety and clarity. [1] [2]limitquery parameter inget_all_quotesfrom 1 to 0, allowing clients to explicitly request zero results.core/__init__.pyto reflect recent module changes, removing unused imports and adding new ones.requirements.txtis in sync with the lock file.lifespanfunction signature inmain.pyfor consistency.