diff --git a/CHANGELOG.md b/CHANGELOG.md index cc337eca..dec9c2a2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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. diff --git a/backend/app/main.py b/backend/app/main.py index 705b4b18..666f23f3 100644 --- a/backend/app/main.py +++ b/backend/app/main.py @@ -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 diff --git a/backend/tests/test_error_envelope_contract.py b/backend/tests/test_error_envelope_contract.py new file mode 100644 index 00000000..e5577e0e --- /dev/null +++ b/backend/tests/test_error_envelope_contract.py @@ -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) diff --git a/backend/tests/test_frontend_api_client_error_parser.py b/backend/tests/test_frontend_api_client_error_parser.py new file mode 100644 index 00000000..8ca8fe68 --- /dev/null +++ b/backend/tests/test_frontend_api_client_error_parser.py @@ -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 diff --git a/backend/tests/test_sprint7b_provider_registry.py b/backend/tests/test_sprint7b_provider_registry.py index 3e5f87ec..ed9869df 100644 --- a/backend/tests/test_sprint7b_provider_registry.py +++ b/backend/tests/test_sprint7b_provider_registry.py @@ -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: diff --git a/docs/CODEX_EXECUTION_LOG.md b/docs/CODEX_EXECUTION_LOG.md index 20dd4cf3..f4099a06 100644 --- a/docs/CODEX_EXECUTION_LOG.md +++ b/docs/CODEX_EXECUTION_LOG.md @@ -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. diff --git a/docs/TODO.md b/docs/TODO.md index 0978aa46..2ac0e5a8 100644 --- a/docs/TODO.md +++ b/docs/TODO.md @@ -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. diff --git a/frontend/src/services/api/client.ts b/frontend/src/services/api/client.ts index 14af4212..98760c99 100644 --- a/frontend/src/services/api/client.ts +++ b/frontend/src/services/api/client.ts @@ -19,9 +19,10 @@ export class ApiHttpError extends Error { async function parseResponse(response: Response): Promise { 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;