refactoring-backend #3
@@ -632,14 +632,16 @@ Every non-2xx HTTP response body is a JSON object with this structure:
|
||||
"detail": "Jail 'example' not found",
|
||||
"metadata": {
|
||||
"jail_name": "example"
|
||||
}
|
||||
},
|
||||
"correlation_id": "550e8400-e29b-41d4-a716-446655440000"
|
||||
}
|
||||
```
|
||||
|
||||
**Fields:**
|
||||
- **`code`** (string, required): Machine-readable error code for client-side branching. Examples: `jail_not_found`, `rate_limit_exceeded`, `authentication_required`.
|
||||
- **`code`** (string, required): Machine-readable error code for client-side branching. Examples: `jail_not_found`, `rate_limit_exceeded`, `authentication_required`, `invalid_input`.
|
||||
- **`detail`** (string, required): Human-readable error message. Safe for displaying to users.
|
||||
- **`metadata`** (object, optional): Structured context data relevant to the error. Only includes data safe for client consumption (no sensitive internal state). Examples: offending parameter names, resource identifiers, time windows.
|
||||
- **`metadata`** (object, optional): Structured context data relevant to the error. Only includes data safe for client consumption (no sensitive internal state). Examples: offending parameter names, resource identifiers, field error counts, time windows.
|
||||
- **`correlation_id`** (string | null, optional): Unique request ID for tracing this error across logs and systems. Set by the `CorrelationIdMiddleware`. Use this to correlate client-side errors with server logs for debugging.
|
||||
|
||||
### Exception Hierarchy & Error Codes
|
||||
|
||||
@@ -655,6 +657,8 @@ All domain exceptions inherit from `DomainError` (defined in `backend/app/except
|
||||
| **401** | `AuthenticationError` | `authentication_required` | Authentication or authorization failure, invalid/expired credentials |
|
||||
| **429** | `RateLimitError` | `rate_limit_exceeded` | Rate limit exceeded, too many requests |
|
||||
|
||||
**Note on request validation errors:** Pydantic validation errors (from request body type mismatches, missing required fields, etc.) are automatically caught by the `_request_validation_error_handler` and converted to `ErrorResponse` with `code="invalid_input"`. The `metadata` field includes `field_errors` (count of validation failures) and `first_field` (location of the first error field) to help clients debug malformed requests.
|
||||
|
||||
### Implementing Error Handlers
|
||||
|
||||
Every exception category has a corresponding exception handler registered in `backend/app/main.py`. When a domain exception is raised:
|
||||
|
||||
@@ -1,45 +1,3 @@
|
||||
## [IMPORTANT] API pagination doesn't return metadata
|
||||
|
||||
**Where found**
|
||||
|
||||
- `backend/app/routers/history.py` — returns bare list, no pagination metadata
|
||||
- All paginated routers have same issue
|
||||
|
||||
**Why this is needed**
|
||||
|
||||
Frontend receives bare list, cannot determine: total results, whether more pages exist, last page number. Must guess or re-query.
|
||||
|
||||
**Goal**
|
||||
|
||||
Return pagination metadata with every paginated response.
|
||||
|
||||
**What to do**
|
||||
|
||||
1. Create response wrapper:
|
||||
```python
|
||||
class PaginatedResponse(BaseModel):
|
||||
data: list[Item]
|
||||
pagination: PaginationMetadata
|
||||
```
|
||||
|
||||
2. Update all paginated routers to return this wrapper
|
||||
3. Update frontend to use metadata for UI
|
||||
|
||||
**Possible traps and issues**
|
||||
|
||||
- `SELECT COUNT(*)` is slow on large tables
|
||||
- Response shape change — old frontend may not handle
|
||||
|
||||
**Docs changes needed**
|
||||
|
||||
- Update API documentation § Pagination
|
||||
|
||||
**Doc references**
|
||||
|
||||
- `backend/app/utils/pagination.py`
|
||||
|
||||
---
|
||||
|
||||
## [IMPORTANT] Error response schema inconsistent
|
||||
|
||||
**Where found**
|
||||
|
||||
@@ -24,6 +24,7 @@ if TYPE_CHECKING:
|
||||
|
||||
import structlog
|
||||
from fastapi import FastAPI, HTTPException, Request, status
|
||||
from fastapi.exceptions import RequestValidationError
|
||||
from fastapi.middleware.cors import CORSMiddleware
|
||||
from fastapi.responses import JSONResponse, RedirectResponse
|
||||
from starlette.middleware.base import BaseHTTPMiddleware
|
||||
@@ -621,6 +622,50 @@ async def _http_exception_handler(
|
||||
)
|
||||
|
||||
|
||||
async def _request_validation_error_handler(
|
||||
request: Request,
|
||||
exc: RequestValidationError,
|
||||
) -> JSONResponse:
|
||||
"""Return a standardized error response for Pydantic validation errors.
|
||||
|
||||
Converts FastAPI's RequestValidationError to our unified ErrorResponse format.
|
||||
Aggregates validation errors into metadata for the client to handle.
|
||||
|
||||
Args:
|
||||
request: The incoming FastAPI request.
|
||||
exc: The :class:`fastapi.exceptions.RequestValidationError`.
|
||||
|
||||
Returns:
|
||||
A :class:`fastapi.responses.JSONResponse` with status 400.
|
||||
"""
|
||||
log.warning(
|
||||
"request_validation_error",
|
||||
path=request.url.path,
|
||||
method=request.method,
|
||||
error_count=len(exc.errors()),
|
||||
)
|
||||
|
||||
validation_errors = exc.errors()
|
||||
error_details: dict[str, str | int | float | bool | None] = {}
|
||||
|
||||
if validation_errors:
|
||||
error_details["field_errors"] = len(validation_errors)
|
||||
first_error = validation_errors[0]
|
||||
error_details["first_field"] = ".".join(str(x) for x in first_error["loc"])
|
||||
|
||||
error_response = ErrorResponse(
|
||||
code="invalid_input",
|
||||
detail="Request validation failed.",
|
||||
metadata=error_details,
|
||||
correlation_id=_get_correlation_id(request),
|
||||
)
|
||||
|
||||
return JSONResponse(
|
||||
status_code=status.HTTP_400_BAD_REQUEST,
|
||||
content=error_response.model_dump(),
|
||||
)
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Setup-redirect middleware
|
||||
# ---------------------------------------------------------------------------
|
||||
@@ -854,9 +899,10 @@ def create_app(settings: Settings | None = None) -> FastAPI:
|
||||
# 4. OperationError handler → HTTP 500
|
||||
# 5. ServiceUnavailableError handler → HTTP 503
|
||||
# 6. Generic DomainError handler (catch-all for any unregistered DomainError subclass) → HTTP 500
|
||||
# 7. HTTPException (FastAPI built-ins, validation errors) → HTTP varies
|
||||
# 8. ValueError (Pydantic validation) → HTTP 400
|
||||
# 9. Exception (absolute catch-all for unexpected errors) → HTTP 500
|
||||
# 7. RequestValidationError handler (Pydantic validation errors) → HTTP 400
|
||||
# 8. HTTPException (FastAPI built-ins) → HTTP varies
|
||||
# 9. ValueError (Pydantic validation) → HTTP 400
|
||||
# 10. Exception (absolute catch-all for unexpected errors) → HTTP 500
|
||||
#
|
||||
# This ensures that any new DomainError subclass that inherits from a registered category
|
||||
# is automatically handled with the correct error_code and metadata. If a developer adds
|
||||
@@ -872,6 +918,7 @@ def create_app(settings: Settings | None = None) -> FastAPI:
|
||||
app.add_exception_handler(OperationError, _domain_error_handler) # type: ignore[arg-type]
|
||||
app.add_exception_handler(ServiceUnavailableError, _service_unavailable_handler) # type: ignore[arg-type]
|
||||
app.add_exception_handler(DomainError, _domain_error_handler) # type: ignore[arg-type]
|
||||
app.add_exception_handler(RequestValidationError, _request_validation_error_handler) # type: ignore[arg-type]
|
||||
app.add_exception_handler(HTTPException, _http_exception_handler) # type: ignore[arg-type]
|
||||
app.add_exception_handler(ValueError, _value_error_handler) # type: ignore[arg-type]
|
||||
app.add_exception_handler(Exception, _unhandled_exception_handler)
|
||||
|
||||
186
backend/tests/test_error_response_schema.py
Normal file
186
backend/tests/test_error_response_schema.py
Normal file
@@ -0,0 +1,186 @@
|
||||
"""Test unified error response schema.
|
||||
|
||||
This test suite verifies that all error responses use the unified ErrorResponse
|
||||
format with code, detail, metadata, and correlation_id fields.
|
||||
|
||||
This ensures:
|
||||
1. Frontend can handle all errors with a single schema.
|
||||
2. Error codes are machine-readable.
|
||||
3. Error details include structured metadata.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
from typing import Any
|
||||
|
||||
import pytest
|
||||
from httpx import ASGITransport, AsyncClient
|
||||
from pydantic import BaseModel
|
||||
|
||||
from app.config import Settings
|
||||
from app.main import create_app
|
||||
|
||||
|
||||
class RequestModel(BaseModel):
|
||||
"""Test request model with validation."""
|
||||
|
||||
name: str
|
||||
age: int
|
||||
|
||||
|
||||
class TestErrorResponseSchema:
|
||||
"""Verify all error responses use the unified ErrorResponse schema."""
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_validation_error_returns_unified_schema(
|
||||
self, test_settings: Settings
|
||||
) -> None:
|
||||
"""RequestValidationError returns unified ErrorResponse format."""
|
||||
app = create_app(test_settings)
|
||||
transport = ASGITransport(app=app)
|
||||
async with AsyncClient(transport=transport, base_url="http://test") as client:
|
||||
|
||||
@app.post("/test-validation")
|
||||
async def test_validation(body: RequestModel) -> dict[str, Any]:
|
||||
return {"ok": True}
|
||||
|
||||
# Send invalid data that violates validation
|
||||
response = await client.post(
|
||||
"/test-validation", json={"name": "John", "age": "not_a_number"}
|
||||
)
|
||||
|
||||
assert response.status_code == 400
|
||||
data = response.json()
|
||||
|
||||
# Verify unified schema
|
||||
assert "code" in data, "Missing 'code' field"
|
||||
assert "detail" in data, "Missing 'detail' field"
|
||||
assert "metadata" in data, "Missing 'metadata' field"
|
||||
assert "correlation_id" in data, "Missing 'correlation_id' field"
|
||||
|
||||
# Verify values
|
||||
assert data["code"] == "invalid_input"
|
||||
assert "validation" in data["detail"].lower()
|
||||
assert isinstance(data["metadata"], dict)
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_validation_error_includes_field_details(
|
||||
self, test_settings: Settings
|
||||
) -> None:
|
||||
"""Validation error metadata includes field error count and first field."""
|
||||
app = create_app(test_settings)
|
||||
transport = ASGITransport(app=app)
|
||||
async with AsyncClient(transport=transport, base_url="http://test") as client:
|
||||
|
||||
@app.post("/test-validation")
|
||||
async def test_validation(body: RequestModel) -> dict[str, Any]:
|
||||
return {"ok": True}
|
||||
|
||||
response = await client.post(
|
||||
"/test-validation", json={"name": "John", "age": "invalid"}
|
||||
)
|
||||
|
||||
assert response.status_code == 400
|
||||
data = response.json()
|
||||
|
||||
# Verify metadata contains field error information
|
||||
assert "field_errors" in data["metadata"]
|
||||
assert "first_field" in data["metadata"]
|
||||
assert data["metadata"]["field_errors"] > 0
|
||||
assert "age" in data["metadata"]["first_field"]
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_validation_error_has_correlation_id(
|
||||
self, test_settings: Settings
|
||||
) -> None:
|
||||
"""Validation errors include correlation_id for tracing."""
|
||||
app = create_app(test_settings)
|
||||
transport = ASGITransport(app=app)
|
||||
async with AsyncClient(transport=transport, base_url="http://test") as client:
|
||||
|
||||
@app.post("/test-validation")
|
||||
async def test_validation(body: RequestModel) -> dict[str, Any]:
|
||||
return {"ok": True}
|
||||
|
||||
response = await client.post(
|
||||
"/test-validation", json={"name": 123, "age": "not_a_number"}
|
||||
)
|
||||
|
||||
assert response.status_code == 400
|
||||
data = response.json()
|
||||
|
||||
# correlation_id can be None but field should exist
|
||||
assert "correlation_id" in data
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_missing_required_field_validation(
|
||||
self, test_settings: Settings
|
||||
) -> None:
|
||||
"""Missing required fields return validation error."""
|
||||
app = create_app(test_settings)
|
||||
transport = ASGITransport(app=app)
|
||||
async with AsyncClient(transport=transport, base_url="http://test") as client:
|
||||
|
||||
@app.post("/test-validation")
|
||||
async def test_validation(body: RequestModel) -> dict[str, Any]:
|
||||
return {"ok": True}
|
||||
|
||||
# Send incomplete data
|
||||
response = await client.post("/test-validation", json={"name": "John"})
|
||||
|
||||
assert response.status_code == 400
|
||||
data = response.json()
|
||||
|
||||
assert data["code"] == "invalid_input"
|
||||
assert "validation" in data["detail"].lower()
|
||||
assert "field_errors" in data["metadata"]
|
||||
assert data["metadata"]["field_errors"] > 0
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_validation_error_response_structure(self, test_settings: Settings) -> None:
|
||||
"""All validation errors return consistent ErrorResponse structure."""
|
||||
app = create_app(test_settings)
|
||||
transport = ASGITransport(app=app)
|
||||
async with AsyncClient(transport=transport, base_url="http://test") as client:
|
||||
|
||||
@app.post("/test-validation")
|
||||
async def test_validation(body: RequestModel) -> dict[str, Any]:
|
||||
return {"ok": True}
|
||||
|
||||
# Test with wrong type
|
||||
response = await client.post(
|
||||
"/test-validation",
|
||||
json={"name": "John", "age": []},
|
||||
)
|
||||
|
||||
assert response.status_code == 400
|
||||
data = response.json()
|
||||
|
||||
assert data["code"] == "invalid_input"
|
||||
assert "metadata" in data
|
||||
assert isinstance(data["metadata"], dict)
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_validation_error_does_not_leak_sensitive_data(
|
||||
self, test_settings: Settings
|
||||
) -> None:
|
||||
"""Validation errors do not leak internal implementation details."""
|
||||
app = create_app(test_settings)
|
||||
transport = ASGITransport(app=app)
|
||||
async with AsyncClient(transport=transport, base_url="http://test") as client:
|
||||
|
||||
@app.post("/test-validation")
|
||||
async def test_validation(body: RequestModel) -> dict[str, Any]:
|
||||
return {"ok": True}
|
||||
|
||||
response = await client.post(
|
||||
"/test-validation", json={"name": "John", "age": "not_a_number"}
|
||||
)
|
||||
|
||||
assert response.status_code == 400
|
||||
data = response.json()
|
||||
|
||||
# Verify the response structure is safe
|
||||
assert "stack" not in data
|
||||
assert "traceback" not in data
|
||||
assert "exc_info" not in data
|
||||
Reference in New Issue
Block a user