Align API error envelopes
This commit is contained in:
@@ -660,3 +660,11 @@ Added:
|
||||
- Desktop screenshots are always captured; mobile screenshots are captured by default and can be disabled with `CAPTURE_MOBILE=0`.
|
||||
- Added readiness syntax coverage and regression checks for the non-mutating screenshot capture contract.
|
||||
- No API contracts, migrations, product capabilities, live provider fetching or AI/model dependency changes were introduced.
|
||||
|
||||
## Sprint 60 API error-envelope contract hardening (2026-06-18)
|
||||
|
||||
- Aligned backend error responses with the documented `ApiError` contract: top-level `error`, `message`, `details` and `request_id`.
|
||||
- Preserved frontend compatibility with both the canonical top-level error payload and the older nested error-object shape.
|
||||
- Added regression coverage for AppError, HTTPException and validation-error envelopes.
|
||||
- Added static frontend parser coverage so API client error parsing does not drift silently.
|
||||
- No migrations, product capabilities, live provider fetching or AI/model dependency changes were introduced.
|
||||
|
||||
+42
-10
@@ -11,13 +11,17 @@ from app.core.errors import AppError
|
||||
from app.core.logging import configure_logging
|
||||
|
||||
|
||||
def _to_error_payload(code: str, message: str, details: dict | list | None = None) -> dict:
|
||||
def _to_error_payload(
|
||||
code: str,
|
||||
message: str,
|
||||
details: dict | list | None = None,
|
||||
request_id: str | None = None,
|
||||
) -> dict:
|
||||
return {
|
||||
"error": {
|
||||
"code": code,
|
||||
"message": message,
|
||||
"details": details or {},
|
||||
},
|
||||
"error": code,
|
||||
"message": message,
|
||||
"details": details or {},
|
||||
"request_id": request_id,
|
||||
}
|
||||
|
||||
|
||||
@@ -58,28 +62,56 @@ def create_app() -> FastAPI:
|
||||
async def app_error(request: Request, exc: AppError): # noqa: ARG001
|
||||
return JSONResponse(
|
||||
status_code=exc.status_code,
|
||||
content=_to_error_payload(exc.code, exc.message, exc.details),
|
||||
content=_to_error_payload(
|
||||
exc.code,
|
||||
exc.message,
|
||||
exc.details,
|
||||
request_id=request.headers.get("x-request-id"),
|
||||
),
|
||||
)
|
||||
|
||||
@app.exception_handler(HTTPException)
|
||||
async def http_error(request: Request, exc: HTTPException): # noqa: ARG001
|
||||
code = "HTTP_ERROR"
|
||||
message = str(exc.detail)
|
||||
details = {}
|
||||
if isinstance(exc.detail, dict):
|
||||
code = str(exc.detail.get("error") or exc.detail.get("code") or code)
|
||||
message = str(exc.detail.get("message") or message)
|
||||
raw_details = exc.detail.get("details")
|
||||
details = raw_details if isinstance(raw_details, (dict, list)) else {}
|
||||
return JSONResponse(
|
||||
status_code=exc.status_code,
|
||||
content=_to_error_payload("HTTP_ERROR", str(exc.detail), {}),
|
||||
content=_to_error_payload(
|
||||
code,
|
||||
message,
|
||||
details,
|
||||
request_id=request.headers.get("x-request-id"),
|
||||
),
|
||||
)
|
||||
|
||||
@app.exception_handler(RequestValidationError)
|
||||
async def validation_error(request: Request, exc: RequestValidationError): # noqa: ARG001
|
||||
return JSONResponse(
|
||||
status_code=422,
|
||||
content=_to_error_payload("VALIDATION_ERROR", "Validation failed", exc.errors()),
|
||||
content=_to_error_payload(
|
||||
"VALIDATION_ERROR",
|
||||
"Validation failed",
|
||||
exc.errors(),
|
||||
request_id=request.headers.get("x-request-id"),
|
||||
),
|
||||
)
|
||||
|
||||
@app.exception_handler(Exception)
|
||||
async def unexpected_error(request: Request, exc: Exception): # noqa: ARG001
|
||||
return JSONResponse(
|
||||
status_code=500,
|
||||
content=_to_error_payload("INTERNAL_ERROR", "Unexpected server error", {"type": exc.__class__.__name__}),
|
||||
content=_to_error_payload(
|
||||
"INTERNAL_ERROR",
|
||||
"Unexpected server error",
|
||||
{"type": exc.__class__.__name__},
|
||||
request_id=request.headers.get("x-request-id"),
|
||||
),
|
||||
)
|
||||
|
||||
return app
|
||||
|
||||
@@ -0,0 +1,49 @@
|
||||
from __future__ import annotations
|
||||
|
||||
from fastapi import HTTPException
|
||||
from fastapi.testclient import TestClient
|
||||
|
||||
from app.main import app, create_app
|
||||
|
||||
|
||||
def assert_contract_error(payload: dict, code: str) -> None:
|
||||
assert payload["error"] == code
|
||||
assert isinstance(payload["message"], str)
|
||||
assert "details" in payload
|
||||
assert "request_id" in payload
|
||||
|
||||
|
||||
def test_app_error_uses_top_level_api_error_contract() -> None:
|
||||
client = TestClient(app)
|
||||
|
||||
response = client.get("/api/v1/external/providers/unknown")
|
||||
|
||||
assert response.status_code == 404
|
||||
assert_contract_error(response.json(), "PROVIDER_NOT_FOUND")
|
||||
assert response.json()["message"] == "Provider not found"
|
||||
|
||||
|
||||
def test_http_exception_uses_top_level_api_error_contract() -> None:
|
||||
test_app = create_app()
|
||||
|
||||
@test_app.get("/__test__/http-error")
|
||||
def raise_http_error() -> None:
|
||||
raise HTTPException(status_code=404, detail="Project not found")
|
||||
|
||||
client = TestClient(test_app)
|
||||
response = client.get("/__test__/http-error")
|
||||
|
||||
assert response.status_code == 404
|
||||
assert_contract_error(response.json(), "HTTP_ERROR")
|
||||
assert response.json()["message"] == "Project not found"
|
||||
|
||||
|
||||
def test_validation_error_uses_top_level_api_error_contract() -> None:
|
||||
client = TestClient(app)
|
||||
|
||||
response = client.get("/api/v1/projects/not-a-uuid")
|
||||
|
||||
assert response.status_code == 422
|
||||
assert_contract_error(response.json(), "VALIDATION_ERROR")
|
||||
assert response.json()["message"] == "Validation failed"
|
||||
assert isinstance(response.json()["details"], list)
|
||||
@@ -0,0 +1,22 @@
|
||||
from __future__ import annotations
|
||||
|
||||
from pathlib import Path
|
||||
|
||||
|
||||
ROOT = Path(__file__).resolve().parents[2]
|
||||
|
||||
|
||||
def test_frontend_api_client_accepts_top_level_error_contract() -> None:
|
||||
client = (ROOT / "frontend" / "src" / "services" / "api" / "client.ts").read_text(encoding="utf-8")
|
||||
|
||||
assert 'typeof payload?.error === "string"' in client
|
||||
assert "payload?.message" in client
|
||||
assert "payload?.details" in client
|
||||
|
||||
|
||||
def test_frontend_api_client_remains_legacy_error_tolerant() -> None:
|
||||
client = (ROOT / "frontend" / "src" / "services" / "api" / "client.ts").read_text(encoding="utf-8")
|
||||
|
||||
assert "legacyError?.code" in client
|
||||
assert "legacyError?.message" in client
|
||||
assert "legacyError?.details" in client
|
||||
@@ -103,7 +103,8 @@ def test_provider_api_envelopes_and_invalid_provider() -> None:
|
||||
|
||||
invalid_response = client.get("/api/v1/external/providers/unknown")
|
||||
assert invalid_response.status_code == 404
|
||||
assert invalid_response.json()["error"]["code"] == "PROVIDER_NOT_FOUND"
|
||||
assert invalid_response.json()["error"] == "PROVIDER_NOT_FOUND"
|
||||
assert invalid_response.json()["message"] == "Provider not found"
|
||||
|
||||
|
||||
def test_provider_import_api_returns_clear_not_configured_response() -> None:
|
||||
|
||||
@@ -2349,3 +2349,33 @@ Limitations:
|
||||
|
||||
Next recommended pass:
|
||||
- Run a backend error-envelope audit for expected user-error paths.
|
||||
|
||||
## Sprint 60 API error-envelope contract hardening (2026-06-18)
|
||||
|
||||
Changed:
|
||||
- Audited the backend error payload shape against `docs/API_CONTRACTS.md`.
|
||||
- Changed central FastAPI error serialization to return top-level `error`, `message`, `details` and `request_id` fields.
|
||||
- Preserved HTTPException detail dict support for future explicit error codes.
|
||||
- Updated the frontend API client to parse the canonical top-level error contract while remaining tolerant of the older nested `error.code/message/details` shape.
|
||||
- Added regression tests for AppError, HTTPException and validation-error envelopes.
|
||||
- Updated provider-registry error assertions to the canonical top-level schema.
|
||||
- Added static frontend parser coverage for canonical and legacy error payloads.
|
||||
- Updated `docs/TODO.md` and `CHANGELOG.md`.
|
||||
|
||||
Tested:
|
||||
- `cd backend && python -m pytest tests/test_error_envelope_contract.py tests/test_sprint7b_provider_registry.py::test_provider_api_envelopes_and_invalid_provider -q` (`4 passed`)
|
||||
- `python -m compileall backend/app`
|
||||
- `cd frontend && npm run typecheck`
|
||||
- `cd backend && python -m pytest -q` (`224 passed`)
|
||||
- `cd frontend && npm run build`
|
||||
- `cd backend && python -m pytest tests/test_error_envelope_contract.py tests/test_frontend_api_client_error_parser.py tests/test_sprint7b_provider_registry.py::test_provider_api_envelopes_and_invalid_provider -q` (`6 passed`)
|
||||
- `bash scripts/run_readiness_check.sh` (`226 passed`)
|
||||
|
||||
Open:
|
||||
- Deploy Tower and verify a live expected-error response on the browser-facing API.
|
||||
|
||||
Limitations:
|
||||
- This pass changes only the centralized response envelope and frontend parser. It does not rename route-level error codes or change product behavior.
|
||||
|
||||
Next recommended pass:
|
||||
- Expand golden datasets beyond the current building QA fixtures.
|
||||
|
||||
+3
-2
@@ -59,7 +59,7 @@ This file now starts with the current implementation status. Older preparation/b
|
||||
- [x] Demo workflow orchestration hook decomposition.
|
||||
- [x] Final `App.tsx` import/encoding cleanup and size audit.
|
||||
- [x] Optional final bootstrap-effect extraction.
|
||||
- [ ] Decide next V1 stabilization focus: backend error-envelope audit or golden dataset expansion.
|
||||
- [ ] Decide next V1 stabilization focus: golden dataset expansion or frontend visual polish backlog.
|
||||
|
||||
## Sprint 8 status
|
||||
|
||||
@@ -329,4 +329,5 @@ This file now starts with the current implementation status. Older preparation/b
|
||||
- [x] Add a safe export retention/cleanup command for demo environments.
|
||||
- [x] Add a live dry-run maintenance smoke for demo export cleanup.
|
||||
- [x] Add browser screenshot artifact automation for visual regression handoff.
|
||||
- [ ] Add backend error-envelope audit for expected user-error paths.
|
||||
- [x] Add backend error-envelope audit for expected user-error paths.
|
||||
- [ ] Expand golden datasets beyond building QA fixtures.
|
||||
|
||||
@@ -19,9 +19,10 @@ export class ApiHttpError extends Error {
|
||||
async function parseResponse<T>(response: Response): Promise<T> {
|
||||
const payload = await response.json().catch(() => ({}));
|
||||
if (!response.ok) {
|
||||
const code = payload?.error?.code ?? "REQUEST_ERROR";
|
||||
const message = payload?.error?.message ?? `Request failed (${response.status})`;
|
||||
const details = payload?.error?.details;
|
||||
const legacyError = typeof payload?.error === "object" && payload?.error !== null ? payload.error : null;
|
||||
const code = typeof payload?.error === "string" ? payload.error : legacyError?.code ?? "REQUEST_ERROR";
|
||||
const message = payload?.message ?? legacyError?.message ?? `Request failed (${response.status})`;
|
||||
const details = payload?.details ?? legacyError?.details;
|
||||
throw new ApiHttpError(message, code, details);
|
||||
}
|
||||
return payload.data as T;
|
||||
|
||||
Reference in New Issue
Block a user