Skip to content

Conversation

@tnederlof
Copy link
Collaborator

@tnederlof tnederlof commented Oct 31, 2025

  • Add custom error handling module (backend/app/errors/) with:

    • ErrorCode enum for consistent error codes
    • AppError base class and specific error types (NotFoundError, BadRequestError, ConflictError)
    • ErrorResponse schema for structured error responses
    • Global exception handlers for AppError, validation errors, and unexpected errors
  • Register exception handlers in FastAPI app

  • Migrate all HTTPException raises to typed custom errors

  • Update error responses to include code, error_code, message, and details fields

  • Fix validation error handler to properly serialize exception context

  • Add comprehensive error handling tests (test_error_handling.py)

  • Update existing tests to expect 409 for conflicts instead of 400

  • Update existing tests to check 'message' field instead of 'detail'

All tests pass (57/57), linting clean, type checking clean, E2E tests pass (34/34)

Amp thread: https://ampcode.com/threads/T-99ac3974-7980-4570-a919-7317aa90678d

- Add custom error handling module (backend/app/errors/) with:
  - ErrorCode enum for consistent error codes
  - AppError base class and specific error types (NotFoundError, BadRequestError, ConflictError)
  - ErrorResponse schema for structured error responses
  - Global exception handlers for AppError, validation errors, and unexpected errors

- Register exception handlers in FastAPI app
- Migrate all HTTPException raises to typed custom errors
- Update error responses to include code, error_code, message, and details fields
- Fix validation error handler to properly serialize exception context

- Add comprehensive error handling tests (test_error_handling.py)
- Update existing tests to expect 409 for conflicts instead of 400
- Update existing tests to check 'message' field instead of 'detail'

All tests pass (57/57), linting clean, type checking clean, E2E tests pass (34/34)
Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Summary

This PR implements a structured error handling system for the FastAPI application, replacing generic HTTPException usage with custom exception classes (AppError, NotFoundError, BadRequestError, ConflictError) and standardized error responses via ErrorResponse. The changes affect error handling throughout the API endpoints and update tests to validate the new error response format.

Key changes:

  • New error handling module with exception classes, error codes enum, response schema, and exception handlers
  • All API endpoints now raise typed exceptions instead of generic HTTPException
  • Error responses now include both HTTP status code and application-level error codes
  • Tests updated to verify structured error responses (404, 409, 400, 422)

Considerations:

  • The ErrorResponse.code field name could be clarified as status_code to avoid confusion with error_code, though the current naming is functional
  • Consider whether all validation errors should include the full Pydantic error details in production, as this may expose internal structure

View this review on Amp

message=str(exc.detail) if exc.detail else "Error",
)
return JSONResponse(status_code=exc.status_code, content=body.model_dump())

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The generic Exception handler silently swallows all unexpected errors without logging. This makes debugging production issues difficult. Consider adding logging before returning the generic error response:

import logging
logger = logging.getLogger(__name__)

@app.exception_handler(Exception)
async def handle_unexpected(request: Request, exc: Exception) -> JSONResponse:
    logger.exception("Unexpected error occurred", exc_info=exc)
    # ... rest of handler

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants