fix(api): unwrap error envelope to document root (DEBT-034)#113
Conversation
…T-034)
FastAPI serializes `raise HTTPException(detail=err.to_envelope())` as
`{"detail": {"error": {...}}}`, so the envelope sat one level below the
top-level `{"error": {...}}` the docs and the uncaught-500 handler use.
A shared `StarletteHTTPException` handler (`vektra_shared.http_errors`)
unwraps the envelope to the root when `exc.detail` is already an envelope
dict, and delegates to FastAPI's default handler otherwise, preserving
`exc.headers`. Registered by `create_app()` and by the package test apps
so both emit the same wire shape.
- widget: drop the now-dead `detail.error` fallback (bundle rebuilt from
source by the Docker widget-builder stage; it is gitignored)
- tests: flip 23 HTTP-wire assertions from `body["detail"]["error"]` to
`body["error"]`; the exception-object assertions are left unchanged
- docs: note the envelope is at the root in api.md / error-codes.md
Proven on the wire: /api/v1/providers with no token returned
{"detail":{"error":{code:ERR-AUTH-001}}} before and
{"error":{code:ERR-AUTH-001}} after; a 404 still returns {"detail":"Not Found"}.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Warning Review limit reached
Next review available in: 36 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe PR adds shared FastAPI error-handler registration that unwraps envelope-shaped HTTP errors to ChangesRoot-level error envelope
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant APIClient
participant FastAPIApp
participant ErrorHandler
participant JSONResponse
APIClient->>FastAPIApp: send request
FastAPIApp->>ErrorHandler: handle HTTPException
ErrorHandler->>JSONResponse: return {"error": {...}}
JSONResponse-->>APIClient: root-level error response
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Code Review
This pull request resolves DEBT-034 by unwrapping the REQ-010 error envelope from under the detail key to the JSON document root ({"error": {...}}) across all error paths. A centralized register_error_handlers function was introduced in vektra_shared and integrated across all components and test applications. Consequently, documentation, the frontend widget, and test assertions were updated to check body["error"] instead of body["detail"]["error"]. Note that this change directly contradicts rule 29 of the current REPO_STYLE_GUIDE, which must be updated to reflect this new root-level error envelope contract.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
…view)
The Gemini styleguide still mandated the pre-DEBT-034 nested contract
(`body["detail"]["error"]["code"]`), which this PR inverts. Update it to the
root-level `{"error": {...}}` shape and note that router-only test apps must
call `register_error_handlers(app)` to match the production wire shape.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Addressed the styleguide point in 3029db4: |
What & why
Closes DEBT-034. FastAPI serializes
raise HTTPException(detail=err.to_envelope())as{"detail": {"error": {...}}}, so the REQ-010 envelope sat one level below the top-level{"error": {...}}thatdocs/reference/api.md,error-codes.md, and the uncaught-500 handler all use. A client following the docs looked forbody.error.codeand found nothing; the real path for almost every error wasbody.detail.error.code. Two shapes, one of them an accidental FastAPI artifact.This unwraps the envelope to the JSON document root for every error path — explicit raises and uncaught 500s alike — so
error.codeis a single, documented path.How
A shared
StarletteHTTPExceptionhandler invektra_shared/http_errors.py:exc.detailat the root viaJSONResponse(status_code=exc.status_code, content=exc.detail, headers=exc.headers)whenexc.detailis already an envelope dict (has anerrorkey);HTTPExceptions and FastAPI's own 404/405 keep{"detail": ...}unchanged;exc.headers, so the rate-limit raise (auth.py) keeps itsX-RateLimit-*headers.It lives in
vektra_sharedand is registered both bycreate_app()and by each package's test_make_app()fixtures, so the assembled app and the minimal router apps in tests emit the identical wire shape (otherwise the tests would assert a shape production never emits).Consumer audit
vektra-learn/widget/src/api-client.js): already read both shapes (errData?.error?.message || errData?.detail?.error?.message) — the now-deaddetail.errorbranch is removed. The bundle (vektra-learn/static/vektra-chat.js) is gitignored and rebuilt from source by the Dockerwidget-builderstage; no committed bundle to update.body["detail"]["error"]tobody["error"]across 7 files. The 16exc_info.value.detail["error"]assertions inspect the raised object (never hit the handler) and are unchanged.api.md/error-codes.mdalready showed the root form; each gained a one-line note that the envelope is at the root.parse_error_enveloperead onlydetail.errorand would have degraded toHTTP {code}. Updated there in a companion PR to read the rooterrorfirst withdetail.erroras a legacy fallback (order-independent, works against backends on either side).Verification
make lintclean (ruff, mypy, import-linter 8/8),make test783 passed.POST/GET /api/v1/providerswith no token — before:{"detail":{"error":{"code":"ERR-AUTH-001",...}}}; after:{"error":{"code":"ERR-AUTH-001",...}}.GET /api/v1/does-not-exist(404) still returns{"detail":"Not Found"}, confirming the conditional fallthrough.Notes
Summary by CodeRabbit
Bug Fixes
errorobject, including unexpected server errors.error.codeanderror.messagewithout adetailwrapper.Documentation