Implement custom exception handling and global error management #22
Conversation
WalkthroughThe changes refactor error handling and report generation execution across three core files. Custom exception classes are introduced for standardized error responses, API routes are updated to execute report generation in the background with custom exception handling, and centralized exception handlers are registered at the application level to ensure consistent logging and error responses. Changes
Sequence DiagramsequenceDiagram
participant Client
participant API as generate_report_endpoint
participant TaskQueue as Background<br/>Task Queue
participant Runner as _run_agents_in_background
participant Orchestrator
participant ExceptionHandler as Exception Handler
participant Logger as api_logger
Client->>API: POST /report
API->>TaskQueue: Queue _run_agents_in_background
API-->>Client: 202 Accepted
TaskQueue->>Runner: Execute background task
alt Execution Success
Runner->>Orchestrator: execute_agents_concurrently
Orchestrator-->>Runner: Complete
Runner->>Logger: Log success
else Execution Fails
Runner->>Logger: Log failure details
Runner->>Runner: Record in in_memory_reports
Runner->>ExceptionHandler: Raise AgentExecutionException
ExceptionHandler->>Logger: Log error
ExceptionHandler-->>Client: 500 Error Response
end
Client->>API: GET /report/{id}
alt Report Found
API-->>Client: Report data
else Report Not Found
API->>ExceptionHandler: Raise ReportNotFoundException
ExceptionHandler->>Logger: Log 404
ExceptionHandler-->>Client: 404 Error Response
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ 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: 1
🧹 Nitpick comments (1)
backend/main.py (1)
14-56: Exception handlers look good, but note response format consistency.The exception handlers are well-structured with proper logging. The static analysis warnings about unused
requestparameters are false positives—FastAPI requires this signature for exception handlers.However, there's a minor inconsistency: in the
HTTPExceptionhandler (lines 36-45), both"message"and"detail"are set toexc.detail, which is redundant. Consider whether this should follow the pattern of the custom exception handlers (where"message"provides a generic error type and"detail"provides specifics).For consistency with other handlers, consider this adjustment:
@app.exception_handler(HTTPException) async def http_exception_handler(request: Request, exc: HTTPException): api_logger.error(f"HTTPException: {exc.detail} (Status Code: {exc.status_code})") return JSONResponse( status_code=exc.status_code, content={ - "message": exc.detail, + "message": "HTTP Exception", "detail": exc.detail }, )
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
backend/app/api/v1/routes.py(4 hunks)backend/app/core/exceptions.py(1 hunks)backend/main.py(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
backend/app/api/v1/routes.py (4)
backend/app/models/report_models.py (2)
ReportRequest(4-6)ReportResponse(8-10)backend/app/services/report_service.py (3)
generate_report(9-19)get_report_status_from_memory(29-31)get_report_data(33-46)backend/app/core/orchestrator.py (2)
create_orchestrator(103-116)execute_agents_concurrently(80-101)backend/app/core/exceptions.py (2)
ReportNotFoundException(4-6)AgentExecutionException(8-10)
backend/main.py (1)
backend/app/core/exceptions.py (2)
ReportNotFoundException(4-6)AgentExecutionException(8-10)
🪛 Ruff (0.14.2)
backend/app/api/v1/routes.py
35-35: Do not catch blind exception: Exception
(BLE001)
backend/main.py
15-15: Unused function argument: request
(ARG001)
26-26: Unused function argument: request
(ARG001)
37-37: Unused function argument: request
(ARG001)
48-48: Unused function argument: request
(ARG001)
🔇 Additional comments (4)
backend/app/core/exceptions.py (1)
4-10: LGTM! Clean exception class design.The custom exception classes follow FastAPI conventions properly. Both classes appropriately extend
HTTPException, use suitable status codes (404 for not found, 500 for execution failures), and provide sensible default messages with the flexibility to override via thedetailparameter.backend/app/api/v1/routes.py (3)
32-41: Background task error handling is appropriate.The broad exception catch in this background task is intentional and correct. Since background tasks run asynchronously and cannot propagate exceptions to the caller, catching all exceptions prevents crashes and allows the system to gracefully mark the report as "failed" for client polling.
The static analysis hint about catching blind
Exceptioncan be safely ignored in this context.
57-57: Good refactor to custom exception.Correctly replaced generic
HTTPExceptionwith the more specificReportNotFoundException, which improves error handling clarity and consistency across the API.
88-88: Good refactor to custom exception.Correctly replaced generic
HTTPExceptionwith the more specificReportNotFoundException, improving error handling consistency.
| from backend.app.services.report_service import generate_report, in_memory_reports, get_report_status_from_memory, get_report_data | ||
| from backend.app.core.orchestrator import create_orchestrator | ||
| from backend.app.core.logger import api_logger | ||
| from backend.app.core.exceptions import ReportNotFoundException, AgentExecutionException |
There was a problem hiding this comment.
Remove unused import.
AgentExecutionException is imported but never used in this file. While ReportNotFoundException is correctly used on lines 57 and 88, AgentExecutionException appears to be unused.
Apply this diff:
-from backend.app.core.exceptions import ReportNotFoundException, AgentExecutionException
+from backend.app.core.exceptions import ReportNotFoundException📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| from backend.app.core.exceptions import ReportNotFoundException, AgentExecutionException | |
| from backend.app.core.exceptions import ReportNotFoundException |
🤖 Prompt for AI Agents
In backend/app/api/v1/routes.py around line 7, the import statement includes
AgentExecutionException which is not used; edit the import to remove
AgentExecutionException so only ReportNotFoundException is imported (i.e.,
change the line to import only ReportNotFoundException from
backend.app.core.exceptions).
This PR introduces custom exception classes and a global exception handler for consistent error management across the API.
Changes
ReportNotFoundExceptionandAgentExecutionExceptioninapp/core/exceptions.py.routes.pyto catch specific exceptions and return appropriate HTTP status codes.main.pyto log errors and standardize response formats.Summary by CodeRabbit
New Features
Improvements