diff --git a/.gemini/styleguide.md b/.gemini/styleguide.md index 69bb2a7..9e6257e 100644 --- a/.gemini/styleguide.md +++ b/.gemini/styleguide.md @@ -25,8 +25,12 @@ interface instead. - All route handlers must be `async def`. - Authentication is dependency-injected via `Depends()`. Never inline auth logic in handlers. -- HTTP error responses use `ErrorResponse.to_envelope()` for the `detail` field. - Tests must check `body["detail"]["error"]["code"]`, not `body["error"]["code"]`. +- HTTP error responses raise `HTTPException(detail=ErrorResponse.to_envelope())`. + The `register_error_handlers` handler (`vektra_shared.http_errors`) unwraps the + envelope to the JSON document root, so on the wire it is `{"error": {...}}` + (DEBT-034). Tests must check `body["error"]["code"]`, not + `body["detail"]["error"]["code"]`. Test apps that mount a router directly must + call `register_error_handlers(app)` to get the same wire shape as production. - Use `app.dependency_overrides` in tests, not `patch()` for FastAPI dependencies. ## Error handling diff --git a/.s2s/BACKLOG.md b/.s2s/BACKLOG.md index 0141245..fbf8fc3 100644 --- a/.s2s/BACKLOG.md +++ b/.s2s/BACKLOG.md @@ -993,7 +993,7 @@ A client cannot branch on an error code for these endpoints, and the operator-fa ### DEBT-034: the error envelope is nested under `detail` on the wire, but documented as top-level -**Status**: planned | **Priority**: low | **Created**: 2026-07-15 +**Status**: completed | **Priority**: low | **Created**: 2026-07-15 | **Completed**: 2026-07-15 **Origin**: DEBT-033 (2026-07-15). **Context**: FastAPI serializes `raise HTTPException(status_code=..., detail=err.to_envelope())` as `{"detail": {"error": {...}}}` — the REQ-010 envelope sits one level down, under `detail`. This is how **every** enveloped endpoint behaves (auth, ingest, index, analytics, learn, admin, and the endpoints fixed in DEBT-033), and the whole test suite already asserts `resp.json()["detail"]["error"]["code"]`, so the nesting is the de-facto contract. The one exception is the global unhandled-exception handler in `vektra-app/main.py`, which returns top-level `{"error": {...}}` via `JSONResponse`. So there are two shapes after all — not "envelope vs bare" (DEBT-033 closed that), but "envelope under `detail`" (explicit raises) vs "envelope at top level" (uncaught 500s). `docs/reference/api.md` and `docs/reference/error-codes.md` show only the top-level form, so a client that follows the docs looks for `body.error.code` and finds nothing; the real path for almost every error is `body.detail.error.code`. @@ -1004,8 +1004,10 @@ A client cannot branch on an error code for these endpoints, and the operator-fa 3. Leave as-is. Rejected: the docs actively mislead about where the code lives. **Acceptance criteria**: -- [ ] The REQ-010 envelope is reachable at a single, documented JSON path across all error responses (explicit raises and uncaught 500s alike) -- [ ] `docs/reference/api.md` and `docs/reference/error-codes.md` match the actual wire shape +- [x] The REQ-010 envelope is reachable at a single, documented JSON path across all error responses (explicit raises and uncaught 500s alike) +- [x] `docs/reference/api.md` and `docs/reference/error-codes.md` match the actual wire shape + +**Resolution** (2026-07-15): chose **option 1 (unwrap)**. A single `@app.exception_handler(StarletteHTTPException)` in `vektra-app/main.py` returns `exc.detail` at the document root via `JSONResponse(status_code=exc.status_code, content=exc.detail, headers=exc.headers)` when `exc.detail` is already an envelope dict (has an `error` key), and delegates to FastAPI's default `http_exception_handler` otherwise — so the admin-UI string-detail raises and FastAPI's own 404/405 keep the `{"detail": ...}` shape, and the rate-limit raise (`auth.py`) keeps its `X-RateLimit-*` headers. Both error paths (explicit raises and uncaught 500s) now converge on top-level `{"error": {...}}`, which the docs already showed; api.md and error-codes.md gained a one-line clarification that the envelope is at the root. **Consumer audit**: the widget already read both shapes (`errData?.error?.message || errData?.detail?.error?.message`) — dead branch removed, gitignored bundle rebuilt from source by the Docker `widget-builder` stage. 23 HTTP-wire test assertions flipped from `body["detail"]["error"]` to `body["error"]` across 7 files; the 16 `exc_info.value.detail["error"]` assertions (raised-object, never hit the handler) were left untouched. **Cross-repo**: `vektra-moodle`'s `parse_error_envelope` read only `detail.error` and would have degraded to `HTTP {code}`; updated in that repo (separate PR) to read the root `error` first with `detail.error` as fallback. Proven on the live wire with a real `curl` before/after. --- diff --git a/CHANGELOG.md b/CHANGELOG.md index 6a554c4..823892d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -27,6 +27,7 @@ Convention (Keep a Changelog 1.1.0): ### Fixed +- **api**: the REQ-010 error envelope now sits at the JSON document root — `{"error": {...}}` — for every error path, matching `docs/reference/api.md` and `error-codes.md`, which have always shown it there (DEBT-034). It was the follow-up DEBT-033 left standing: FastAPI serializes `raise HTTPException(detail=err.to_envelope())` as `{"detail": {"error": {...}}}`, so the envelope sat one level down for every explicit raise (auth, ingest, index, analytics, learn, admin), while only the global unhandled-exception handler returned it at top level — two shapes, and a client following the docs looked for `body.error.code` and found nothing. A single `StarletteHTTPException` handler in `vektra-app/main.py` unwraps the envelope to the root (`JSONResponse(status_code=exc.status_code, content=exc.detail, headers=exc.headers)`) when `exc.detail` is already an envelope dict, and delegates to FastAPI's default handler otherwise — so the admin UI's string-detail `HTTPException`s and FastAPI's own 404/405 are unchanged, and the rate-limit raise keeps its `X-RateLimit-*` headers. No per-endpoint change, no HTTP status change; the whole suite's wire assertions move from `body["detail"]["error"]` to `body["error"]`. The chatbot widget already read both shapes defensively (`errData?.error?.message || errData?.detail?.error?.message`); the dead branch is removed. The `vektra-moodle` plugin's error parser reads `detail.error` and is updated in that repo to read the root `error` first (its display degrades to `HTTP {code}` otherwise, not a hard break). Proven on the wire: `POST /api/v1/query` with an invalid key returned `{"detail":{"error":{"code":"ERR-AUTH-001",...}}}` before and `{"error":{"code":"ERR-AUTH-001",...}}` after. - **api**: the REQ-010 error envelope (`{"error": {category, code, message, remediation, request_id}}`) is now returned by every API error path, not most of them (DEBT-033). `docs/reference/api.md` first presented it as universal, then (DOCS-009) walked that back with a caveat naming four endpoints that raised a bare FastAPI `{"detail": "..."}` instead — no code to branch on, no remediation. The caveat itself undercounted: grepping the handlers turned up the same gap on `POST /api/v1/query` (registry / pipeline-not-configured), the admin authentication dependency and the API-key creation path (registry / key-store-not-configured), and `GET /api/v1/admin/health?detail=full` (service-initializing) — none of which it listed, so removing the caveat as written would have made the doc claim a universality four more endpoints still broke. All of them now raise `ErrorResponse.to_envelope()` with a machine-readable code, at the same HTTP status they returned before (the change is shape-only, and no test asserted the bare shape): `ERR-CONFIG-001` is reused for an uninitialised provider registry and `ERR-CONFIG-002` for a missing key store — the codes the global unhandled-exception handler already assigns to internal faults — and `ERR-CONV-001/002/003` (conversation not found / store unavailable / store cannot decrypt turns), `ERR-QUERY-005` (query pipeline unavailable), `ERR-ADMIN-008` (deep-health auth not ready) and `ERR-INDEX-003/004` (reindex target equals the active version / reindex job not found) are new, each registered in `vektra_shared/errors.py`, `_CODE_STATUS_OVERRIDE` and `docs/reference/error-codes.md`. The api.md caveat is removed. One representation gap is left standing on purpose: the wire shape is `{"detail": {"error": {...}}}`, because FastAPI wraps an `HTTPException` detail under `detail`, so the envelope sits one level down for every enveloped endpoint (auth included), while the top-level `{"error": {...}}` only ever comes from the global 500 handler — the api.md example shows the logical envelope, and reconciling that is a separate follow-up. - **config**: `VEKTRA_LLM_PROVIDER=""` was accepted and the stack **booted and served with an empty LLM provider**, deferring the misconfiguration to the first query — which is the one thing the ARCH-057 startup sequence exists to prevent (REQ-011, NFR-009). The field is required, but typed as a bare `str`, and `""` is a perfectly valid `str`, so pydantic never raised and step 1 never fired. It is now `min_length=1`, so an empty value fails at startup with the same structured, remediable error an absent one already produced. Found by running `tests/test_startup.py` for the first time (DEBT-031): the test had asserted this exact behaviour since it was written, believing `docker run -e VAR=` *unset* the variable when it in fact sets it to the empty string — so it had been asserting against a value the product happily accepted. Nobody found out, because nothing ever ran it. This is the third time a suite in this repo turned out to be broken the moment someone executed it. The same test also caught **BUG-025** (filed, not fixed here): the structured error is emitted and then `raise SystemExit(1)` travels out of the ASGI lifespan into uvicorn, which logs a `Traceback` after it — so a typo'd env var yields the good message *and* a stack dump, where NFR-009 asks for the former instead of the latter. Fixing that means validating before `uvicorn.run()` rather than inside the lifespan, i.e. touching the boot path BUG-024 broke, so it is tracked on its own; the assertion is `xfail(strict=True)`, which turns the suite red the moment the fix lands and the marker is left behind. - **startup**: a failed ARCH-057 startup step printed a raw Python `Traceback` next to the structured `[STARTUP ERROR]`, where NFR-009 asks for the remediable message *instead of* the stack dump (BUG-025, REQ-011). The 11-step sequence ran inside the ASGI lifespan, so an aborting step's `raise SystemExit(1)` travelled out into uvicorn, which logged the exception before "Application startup failed." — and it applied to every step, not just config validation. The sequence now lives in a loop-agnostic `_run_startup()` that raises `StartupValidationError` (never `SystemExit`), and the container serves through a new `main()` entrypoint (`python -m vektra_app.main`, wired in `docker/entrypoint.sh`) that runs the validation in the very event loop that will serve, then starts uvicorn with `lifespan="off"`. A failed step logs its `[STARTUP ERROR]` and exits non-zero with no ASGI and no traceback; sharing one loop keeps the async resources built during validation (DB engine, HTTP clients) bound to the loop that serves, and `config.get_loop_factory()` preserves uvloop (the CLI default that uvicorn 0.36 stopped exposing through `setup_event_loop()`). The lifespan keeps validating and raising `SystemExit` for the in-process/TestClient path, so `test_missing_llm_provider_aborts_startup` is untouched; the `xfail(strict=True)` on `test_no_raw_traceback_on_startup_failure` is removed. Proven on the container for a config-validation failure and a database_connectivity failure (structured error, exit 1, no traceback) and a valid boot (all 11 steps logged, `startup_complete`, `Uvicorn running`). The required check names (`CI gate`, `Integration tests + NFR gates`) are unchanged. diff --git a/docs/reference/api.md b/docs/reference/api.md index b20b962..d00e673 100644 --- a/docs/reference/api.md +++ b/docs/reference/api.md @@ -1332,5 +1332,7 @@ Every endpoint returns errors in the same envelope (REQ-010): } ``` -Clients can branch on `error.code`, which is stable, rather than parsing -`message`. See [error codes reference](error-codes.md) for the complete list. +The `error` object is at the JSON document root for every error response, so +`error.code` is read at the top level with no `detail` wrapper. Clients can +branch on `error.code`, which is stable, rather than parsing `message`. See +[error codes reference](error-codes.md) for the complete list. diff --git a/docs/reference/error-codes.md b/docs/reference/error-codes.md index df2f727..7e0af70 100644 --- a/docs/reference/error-codes.md +++ b/docs/reference/error-codes.md @@ -19,6 +19,8 @@ Every error response uses this shape: } ``` +The `error` object is at the JSON document root for **every** error response — both explicit errors and uncaught internal faults. Clients read `error.code` at the top level; there is no `detail` wrapper. + **Fields**: - `category`: one of `TRANSIENT`, `PERMANENT`, `CONFIGURATION`, `UPSTREAM` (BR-001) - `code`: stable identifier in the format `ERR-{COMPONENT}-{NUMBER}` diff --git a/vektra-admin/tests/test_integration.py b/vektra-admin/tests/test_integration.py index 86df7ae..304000f 100644 --- a/vektra-admin/tests/test_integration.py +++ b/vektra-admin/tests/test_integration.py @@ -32,6 +32,7 @@ from sqlalchemy.ext.asyncio import AsyncSession, async_sessionmaker, create_async_engine from starlette.middleware.base import BaseHTTPMiddleware +from vektra_shared.http_errors import register_error_handlers from vektra_shared.registry import ProviderRegistry # --------------------------------------------------------------------------- @@ -171,6 +172,7 @@ async def dispatch(self, request: Request, call_next): _app.mount("/admin/static", get_static_files(), name="admin-static") _app.include_router(ui_router) register_ui_exception_handlers(_app) + register_error_handlers(_app) _app.include_router(router) return _app @@ -408,9 +410,8 @@ async def test_no_auth_returns_error_envelope(client): resp = await client.get("/api/v1/api-keys") assert resp.status_code == 401 body = resp.json() - detail = body.get("detail", {}) - assert "error" in detail - assert detail["error"]["code"] == "ERR-AUTH-001" + assert "error" in body + assert body["error"]["code"] == "ERR-AUTH-001" # --------------------------------------------------------------------------- @@ -608,7 +609,7 @@ async def test_namespace_config_patch_rejects_unknown_key( headers={"Authorization": f"Bearer {admin_key}"}, ) assert resp.status_code == 400, resp.text - err = resp.json()["detail"]["error"] + err = resp.json()["error"] assert err["code"] == "ERR-ADMIN-006" assert "not_a_real_key" in err["message"] @@ -627,7 +628,7 @@ async def test_namespace_config_patch_rejects_invalid_value( headers={"Authorization": f"Bearer {admin_key}"}, ) assert resp.status_code == 400, resp.text - err = resp.json()["detail"]["error"] + err = resp.json()["error"] assert err["code"] == "ERR-ADMIN-007" assert "banana" in err["message"] @@ -651,7 +652,7 @@ async def test_namespace_config_patch_rejects_non_string_value( headers={"Authorization": f"Bearer {admin_key}"}, ) assert resp.status_code == 400, resp.text - err = resp.json()["detail"]["error"] + err = resp.json()["error"] assert err["code"] == "ERR-ADMIN-007" @@ -718,7 +719,7 @@ async def test_namespace_config_patch_not_found(client, bootstrap_key): headers={"Authorization": f"Bearer {admin_key}"}, ) assert resp.status_code == 404, resp.text - assert resp.json()["detail"]["error"]["code"] == "ERR-ADMIN-005" + assert resp.json()["error"]["code"] == "ERR-ADMIN-005" async def test_namespace_config_patch_requires_admin_scope( @@ -829,7 +830,7 @@ async def test_namespace_config_patch_rejects_show_sources_non_bool( headers={"Authorization": f"Bearer {admin_key}"}, ) assert resp.status_code == 400, (bad_value, resp.text) - err = resp.json()["detail"]["error"] + err = resp.json()["error"] assert err["code"] == "ERR-ADMIN-007" @@ -960,7 +961,7 @@ async def test_namespace_config_get_404_on_missing(client, bootstrap_key): headers={"Authorization": f"Bearer {admin_key}"}, ) assert resp.status_code == 404, resp.text - err = resp.json()["detail"]["error"] + err = resp.json()["error"] assert err["code"] == "ERR-ADMIN-005" @@ -1028,5 +1029,5 @@ async def test_namespace_config_patch_rejects_citations_enabled_non_bool( headers={"Authorization": f"Bearer {admin_key}"}, ) assert resp.status_code == 400, resp.text - err = resp.json()["detail"]["error"] + err = resp.json()["error"] assert err["code"] == "ERR-ADMIN-007" diff --git a/vektra-analytics/tests/test_api.py b/vektra-analytics/tests/test_api.py index 878aa73..bd6cb7b 100644 --- a/vektra-analytics/tests/test_api.py +++ b/vektra-analytics/tests/test_api.py @@ -12,6 +12,7 @@ from vektra_analytics.api import _get_session, router from vektra_analytics.service import AnalyticsService, MetricsResponse from vektra_shared.auth import ApiKeyInfo +from vektra_shared.http_errors import register_error_handlers from vektra_shared.registry import ProviderRegistry from vektra_shared.types import ChunkRef, QueryTrace, StepTrace @@ -57,6 +58,7 @@ async def _mock_session(): app.dependency_overrides[_get_session] = _mock_session + register_error_handlers(app) return app @@ -189,8 +191,8 @@ def test_list_traces_service_unavailable(self): resp = client.get("/api/v1/traces", headers=_auth_headers()) assert resp.status_code == 503 data = resp.json() - assert "error" in data["detail"] - assert data["detail"]["error"]["code"] == "ERR-ANALYTICS-001" + assert "error" in data + assert data["error"]["code"] == "ERR-ANALYTICS-001" # --------------------------------------------------------------------------- @@ -224,8 +226,8 @@ def test_get_trace_not_found(self): resp = client.get(f"/api/v1/traces/{uuid4()}", headers=_auth_headers()) assert resp.status_code == 404 data = resp.json() - assert "error" in data["detail"] - assert data["detail"]["error"]["code"] == "ERR-ANALYTICS-002" + assert "error" in data + assert data["error"]["code"] == "ERR-ANALYTICS-002" def test_get_trace_invalid_uuid(self): svc = _mock_service() diff --git a/vektra-app/src/vektra_app/main.py b/vektra-app/src/vektra_app/main.py index 72acabf..0726f55 100644 --- a/vektra-app/src/vektra_app/main.py +++ b/vektra-app/src/vektra_app/main.py @@ -33,6 +33,7 @@ from vektra_shared.config import QueryPipelineConfig, VektraSettings from vektra_shared.db import init_db from vektra_shared.errors import ERR_CONFIG_001, ErrorCategory, ErrorResponse +from vektra_shared.http_errors import register_error_handlers from vektra_shared.protocols import EmbeddingProvider from vektra_shared.registry import ProviderRegistry from vektra_shared.startup import ( @@ -768,7 +769,12 @@ def create_app() -> FastAPI: # 5. Correlation ID (outermost - sets request_id before anything else) app.add_middleware(CorrelationIdMiddleware) - # --- Global exception handler --- + # --- Global exception handlers --- + # The REQ-010 envelope is unwrapped to the document root for every + # HTTPException whose detail is already an envelope (DEBT-034). Shared with + # the test apps via vektra_shared so both register identical behavior. + register_error_handlers(app) + @app.exception_handler(Exception) async def _unhandled_exception(request: Request, exc: Exception) -> JSONResponse: request_id = getattr(request.state, "request_id", None) diff --git a/vektra-app/tests/test_app_integration.py b/vektra-app/tests/test_app_integration.py index acf2618..fc24a29 100644 --- a/vektra-app/tests/test_app_integration.py +++ b/vektra-app/tests/test_app_integration.py @@ -157,5 +157,5 @@ def test_smoke_startup_health_and_auth(db_url: str) -> None: resp = client.get("/api/v1/providers") assert resp.status_code == 401 err_body = resp.json() - # FastAPI wraps HTTPException detail as {"detail": ...} - assert "detail" in err_body + # REQ-010 envelope at the document root (DEBT-034) + assert "error" in err_body diff --git a/vektra-app/tests/test_error_codes.py b/vektra-app/tests/test_error_codes.py index d39b271..2927718 100644 --- a/vektra-app/tests/test_error_codes.py +++ b/vektra-app/tests/test_error_codes.py @@ -148,10 +148,11 @@ def query_key(running_app: Any, admin_key: str) -> str: def _assert_error_envelope(body: dict, expected_code: str) -> None: """Assert the response matches REQ-010 envelope with NFR-009 remediation.""" - # FastAPI wraps HTTPException detail as {"detail": {...}} - envelope = body.get("detail", body) - assert "error" in envelope, f"Missing 'error' key in envelope: {envelope}" - error = envelope["error"] + # The REQ-010 envelope sits at the document root for every error path — + # explicit HTTPException raises (unwrapped by _envelope_http_exception) and + # uncaught 500s alike (DEBT-034). + assert "error" in body, f"Missing 'error' key at envelope root: {body}" + error = body["error"] assert error["code"] == expected_code, ( f"Expected {expected_code}, got {error['code']}" ) diff --git a/vektra-core/tests/test_api.py b/vektra-core/tests/test_api.py index 56c15b3..2838eae 100644 --- a/vektra-core/tests/test_api.py +++ b/vektra-core/tests/test_api.py @@ -15,6 +15,7 @@ from vektra_core.api import router from vektra_shared.auth import ApiKeyInfo +from vektra_shared.http_errors import register_error_handlers from vektra_shared.registry import ProviderRegistry from vektra_shared.types import ( QueryChunk, @@ -104,6 +105,7 @@ def _make_app(registry: ProviderRegistry) -> FastAPI: app = FastAPI() app.state.registry = registry app.include_router(router) + register_error_handlers(app) return app @@ -149,7 +151,7 @@ async def test_query_requires_auth(): ) assert resp.status_code == 401 body = resp.json() - assert body["detail"]["error"]["code"] == "ERR-AUTH-001" + assert body["error"]["code"] == "ERR-AUTH-001" async def test_query_wrong_key_returns_401(): diff --git a/vektra-core/tests/test_feedback_api.py b/vektra-core/tests/test_feedback_api.py index ce95806..6dcab3a 100644 --- a/vektra-core/tests/test_feedback_api.py +++ b/vektra-core/tests/test_feedback_api.py @@ -15,6 +15,7 @@ from vektra_core.api import router from vektra_core.conversation import PersistentConversationStore from vektra_shared.auth import ApiKeyInfo +from vektra_shared.http_errors import register_error_handlers from vektra_shared.registry import ProviderRegistry from vektra_shared.types import ( QueryChunk, @@ -100,6 +101,7 @@ async def _override_session(): app.dependency_overrides[get_session] = _override_session + register_error_handlers(app) return app @@ -295,7 +297,7 @@ async def test_get_conversation_not_found(): ) assert resp.status_code == 404 - assert resp.json()["detail"]["error"]["code"] == "ERR-CONV-001" + assert resp.json()["error"]["code"] == "ERR-CONV-001" async def test_get_conversation_soft_deleted_returns_404(): @@ -365,7 +367,7 @@ async def test_delete_conversation_not_found(): ) assert resp.status_code == 404 - assert resp.json()["detail"]["error"]["code"] == "ERR-CONV-001" + assert resp.json()["error"]["code"] == "ERR-CONV-001" async def test_conversation_no_persistent_store_returns_503(): @@ -383,7 +385,7 @@ async def test_conversation_no_persistent_store_returns_503(): ) assert resp.status_code == 503 - assert resp.json()["detail"]["error"]["code"] == "ERR-CONV-002" + assert resp.json()["error"]["code"] == "ERR-CONV-002" # --------------------------------------------------------------------------- diff --git a/vektra-index/tests/test_index_version_cleanup.py b/vektra-index/tests/test_index_version_cleanup.py index bd2607c..a78dd67 100644 --- a/vektra-index/tests/test_index_version_cleanup.py +++ b/vektra-index/tests/test_index_version_cleanup.py @@ -30,6 +30,7 @@ from vektra_shared.auth import ApiKeyInfo from vektra_shared.errors import ActiveIndexVersionError +from vektra_shared.http_errors import register_error_handlers from vektra_shared.registry import ProviderRegistry # --------------------------------------------------------------------------- @@ -228,6 +229,7 @@ def _make_app( ), ) app.include_router(router) + register_error_handlers(app) return app @@ -278,7 +280,7 @@ async def test_endpoint_refuses_to_delete_the_version_being_served() -> None: # REQ-010 envelope with a code: an operator tool has to tell "wrong version, # nothing happened" apart from "the store is down", and cannot do that from # a bare string. - err = body["detail"]["error"] + err = body["error"] assert err["code"] == "ERR-INDEX-001" assert err["details"] == {"namespace": "default", "index_version": 1} assert "currently being served" in err["message"] @@ -293,7 +295,7 @@ async def test_endpoint_rejects_version_zero() -> None: status, body = await _delete_version(app, 0) assert status == 400 - assert body["detail"]["error"]["code"] == "ERR-INDEX-002" + assert body["error"]["code"] == "ERR-INDEX-002" client.delete.assert_not_awaited() diff --git a/vektra-ingest/tests/test_api.py b/vektra-ingest/tests/test_api.py index 7a5b12d..798a69b 100644 --- a/vektra-ingest/tests/test_api.py +++ b/vektra-ingest/tests/test_api.py @@ -11,6 +11,7 @@ from starlette.middleware.base import BaseHTTPMiddleware from vektra_shared.auth import ApiKeyInfo +from vektra_shared.http_errors import register_error_handlers from vektra_shared.registry import ProviderRegistry # --------------------------------------------------------------------------- @@ -62,6 +63,7 @@ async def _refresh(obj): yield session app.dependency_overrides[get_session] = _mock_get_session + register_error_handlers(app) return app @@ -92,7 +94,7 @@ async def test_ingest_no_token_returns_401(): ) as c: resp = await c.post("/api/v1/ingest", files={"file": ("test.pdf", b"data")}) assert resp.status_code == 401 - assert resp.json()["detail"]["error"]["code"] == "ERR-AUTH-001" + assert resp.json()["error"]["code"] == "ERR-AUTH-001" @pytest.mark.asyncio @@ -109,7 +111,7 @@ async def test_ingest_query_scope_returns_403(): headers={"Authorization": "Bearer anytoken"}, ) assert resp.status_code == 403 - assert resp.json()["detail"]["error"]["code"] == "ERR-AUTH-003" + assert resp.json()["error"]["code"] == "ERR-AUTH-003" @pytest.mark.asyncio @@ -164,7 +166,7 @@ async def test_file_too_large_returns_413(): del os.environ["VEKTRA_MAX_FILE_SIZE_MB"] assert resp.status_code == 413 - assert resp.json()["detail"]["error"]["code"] == "ERR-INGEST-002" + assert resp.json()["error"]["code"] == "ERR-INGEST-002" # --------------------------------------------------------------------------- @@ -272,7 +274,7 @@ async def test_sync_ingest_scanned_pdf_returns_422(): ) assert resp.status_code == 422 - assert resp.json()["detail"]["error"]["code"] == "ERR-INGEST-003" + assert resp.json()["error"]["code"] == "ERR-INGEST-003" # --------------------------------------------------------------------------- diff --git a/vektra-learn/widget/src/api-client.js b/vektra-learn/widget/src/api-client.js index e0fecb8..4978752 100644 --- a/vektra-learn/widget/src/api-client.js +++ b/vektra-learn/widget/src/api-client.js @@ -132,8 +132,7 @@ export class ApiClient { } } const errData = await response.json().catch(() => ({})); - const serverMessage = - errData?.error?.message || errData?.detail?.error?.message; + const serverMessage = errData?.error?.message; const msg = response.status === 401 ? `HTTP 401${serverMessage ? `: ${serverMessage}` : ""}` diff --git a/vektra-shared/src/vektra_shared/http_errors.py b/vektra-shared/src/vektra_shared/http_errors.py new file mode 100644 index 0000000..3de617c --- /dev/null +++ b/vektra-shared/src/vektra_shared/http_errors.py @@ -0,0 +1,45 @@ +"""FastAPI exception handlers for the REQ-010 error envelope (DEBT-034). + +Kept separate from errors.py (the pure envelope model) so the wire-shape +wiring lives in one place and every app registers identical behavior: the +assembled application in vektra-app and the minimal router apps built in each +package's tests. Without a single registrar the two drift — the real app +unwraps the envelope to the document root while a bare test app leaves it +nested under ``detail``, and the tests then assert a shape production never +emits. +""" + +from __future__ import annotations + +from fastapi import FastAPI, Request +from fastapi.exception_handlers import http_exception_handler +from fastapi.responses import JSONResponse +from starlette.exceptions import HTTPException as StarletteHTTPException +from starlette.responses import Response + + +def register_error_handlers(app: FastAPI) -> None: + """Register the REQ-010 envelope exception handlers on a FastAPI app. + + HTTPException-based errors carry the REQ-010 envelope in ``exc.detail`` + (raised as ``detail=err.to_envelope()``). FastAPI's default handler would + serialize that as ``{"detail": {"error": {...}}}``, one level below where + the docs and the uncaught-500 handler put it. This unwraps it so every + error path carries the envelope at the document root ``{"error": {...}}``. + Non-envelope details (the admin UI's string messages, FastAPI's own + 404/405) fall through to the default handler unchanged. ``exc.headers`` is + preserved so the rate-limit raise keeps its ``X-RateLimit-*`` headers. + """ + + @app.exception_handler(StarletteHTTPException) + async def _envelope_http_exception( + request: Request, exc: StarletteHTTPException + ) -> Response: + detail = exc.detail + if isinstance(detail, dict) and "error" in detail: + return JSONResponse( + status_code=exc.status_code, + content=detail, + headers=exc.headers, + ) + return await http_exception_handler(request, exc) diff --git a/vektra-shared/tests/test_auth.py b/vektra-shared/tests/test_auth.py index 370cf8a..9670129 100644 --- a/vektra-shared/tests/test_auth.py +++ b/vektra-shared/tests/test_auth.py @@ -7,6 +7,7 @@ from vektra_shared.auth import ApiKeyInfo, require_scope from vektra_shared.errors import ERR_AUTH_001, ERR_AUTH_003 +from vektra_shared.http_errors import register_error_handlers from vektra_shared.registry import ProviderRegistry @@ -26,6 +27,7 @@ async def admin_only(key: ApiKeyInfo = Depends(require_scope("admin"))): async def query_only(key: ApiKeyInfo = Depends(require_scope("query"))): return {"key_id": str(key.key_id)} + register_error_handlers(app) return app @@ -57,9 +59,9 @@ def test_missing_authorization_header_returns_401(self): client = TestClient(_make_app(store)) response = client.get("/admin-only") assert response.status_code == 401 - # FastAPI wraps HTTPException detail as {"detail": } + # REQ-010 envelope at the document root (DEBT-034) body = response.json() - assert body["detail"]["error"]["code"] == ERR_AUTH_001 + assert body["error"]["code"] == ERR_AUTH_001 def test_invalid_token_returns_401(self): store = MockKeyStore({}) @@ -69,7 +71,7 @@ def test_invalid_token_returns_401(self): ) assert response.status_code == 401 body = response.json() - assert body["detail"]["error"]["code"] == ERR_AUTH_001 + assert body["error"]["code"] == ERR_AUTH_001 def test_revoked_token_returns_401(self): # A revoked token would not be in the key store, same as invalid @@ -91,8 +93,8 @@ def test_wrong_scope_returns_403(self): ) assert response.status_code == 403 body = response.json() - assert body["detail"]["error"]["code"] == ERR_AUTH_003 - assert "admin" in body["detail"]["error"]["message"] + assert body["error"]["code"] == ERR_AUTH_003 + assert "admin" in body["error"]["message"] def test_correct_scope_on_query_endpoint(self): store = MockKeyStore({"q-token": ApiKeyInfo(key_id=uuid4(), scopes=["query"])})