Skip to content

fix(api): unwrap error envelope to document root (DEBT-034)#113

Merged
fvadicamo merged 2 commits into
developfrom
fix/debt-034-error-envelope-unwrap
Jul 15, 2026
Merged

fix(api): unwrap error envelope to document root (DEBT-034)#113
fvadicamo merged 2 commits into
developfrom
fix/debt-034-error-envelope-unwrap

Conversation

@fvadicamo

@fvadicamo fvadicamo commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

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": {...}} that docs/reference/api.md, error-codes.md, and the uncaught-500 handler all use. A client following the docs looked for body.error.code and found nothing; the real path for almost every error was body.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.code is a single, documented path.

How

A shared StarletteHTTPException handler in vektra_shared/http_errors.py:

  • returns exc.detail at the 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);
  • delegates to FastAPI's default handler otherwise, so the admin UI's string-detail HTTPExceptions and FastAPI's own 404/405 keep {"detail": ...} unchanged;
  • preserves exc.headers, so the rate-limit raise (auth.py) keeps its X-RateLimit-* headers.

It lives in vektra_shared and is registered both by create_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

  • Widget (vektra-learn/widget/src/api-client.js): already read both shapes (errData?.error?.message || errData?.detail?.error?.message) — the now-dead detail.error branch is removed. The bundle (vektra-learn/static/vektra-chat.js) is gitignored and rebuilt from source by the Docker widget-builder stage; no committed bundle to update.
  • Tests: 23 HTTP-wire assertions flipped from body["detail"]["error"] to body["error"] across 7 files. The 16 exc_info.value.detail["error"] assertions inspect the raised object (never hit the handler) and are unchanged.
  • Docs: api.md / error-codes.md already showed the root form; each gained a one-line note that the envelope is at the root.
  • vektra-moodle (separate repo): parse_error_envelope read only detail.error and would have degraded to HTTP {code}. Updated there in a companion PR to read the root error first with detail.error as a legacy fallback (order-independent, works against backends on either side).

Verification

  • make lint clean (ruff, mypy, import-linter 8/8), make test 783 passed.
  • Proven on the wire against a container built from this branch:
    • POST/GET /api/v1/providers with 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

  • No production releases exist, so the wire change is not treated as a compatibility constraint.
  • No HTTP status codes change; the change is shape-only.

Summary by CodeRabbit

  • Bug Fixes

    • Standardized all API error responses with a top-level error object, including unexpected server errors.
    • Clients can now read stable error codes and messages from error.code and error.message without a detail wrapper.
    • Updated client-side error handling to use the new response format.
  • Documentation

    • Clarified the error-response structure and linked the complete error-code reference.

…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>
@fvadicamo fvadicamo added bug Something isn't working documentation Improvements or additions to documentation component:shared vektra-shared component component:learn vektra-learn component javascript Pull requests that update javascript code labels Jul 15, 2026
@github-actions github-actions Bot added component:core vektra-core component component:ingest vektra-ingest component component:index vektra-index component component:admin vektra-admin component labels Jul 15, 2026
@coderabbitai

coderabbitai Bot commented Jul 15, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@fvadicamo, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 36 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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 configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: cc740950-badb-465e-a5eb-b044793f369b

📥 Commits

Reviewing files that changed from the base of the PR and between 38c15da and 3029db4.

📒 Files selected for processing (1)
  • .gemini/styleguide.md
📝 Walkthrough

Walkthrough

The PR adds shared FastAPI error-handler registration that unwraps envelope-shaped HTTP errors to {"error": {...}} at the JSON root. Application and integration tests, API documentation, changelog text, and the widget client now use the root-level contract.

Changes

Root-level error envelope

Layer / File(s) Summary
Shared handler and application wiring
vektra-shared/src/vektra_shared/http_errors.py, vektra-app/src/vektra_app/main.py
Adds register_error_handlers() and registers it during application creation to preserve root-level envelope responses.
Endpoint test contract migration
vektra-admin/tests/*, vektra-analytics/tests/*, vektra-app/tests/*, vektra-core/tests/*, vektra-index/tests/*, vektra-ingest/tests/*, vektra-shared/tests/*
Registers shared handlers in test applications and updates assertions from detail.error to error.
Client and API contract updates
CHANGELOG.md, docs/reference/*, vektra-learn/widget/src/api-client.js
Documents the root-level error object and updates client message extraction to use error.message.

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
Loading

Possibly related PRs

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 40.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main change: unwrapping the API error envelope to the JSON document root.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/debt-034-error-envelope-unwrap

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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>
@fvadicamo

Copy link
Copy Markdown
Contributor Author

Addressed the styleguide point in 3029db4: .gemini/styleguide.md rules 28-29 now describe the root-level {"error": {...}} contract (body["error"]["code"]) and note that router-only test apps must call register_error_handlers(app) to match the production wire shape. The guide had encoded the pre-DEBT-034 nested convention, which is exactly what this PR inverts, so it was the right place to fix. No CodeRabbit inline findings to address.

@fvadicamo fvadicamo merged commit b5ec449 into develop Jul 15, 2026
24 checks passed
@fvadicamo fvadicamo deleted the fix/debt-034-error-envelope-unwrap branch July 15, 2026 11:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working component:admin vektra-admin component component:core vektra-core component component:index vektra-index component component:ingest vektra-ingest component component:learn vektra-learn component component:shared vektra-shared component documentation Improvements or additions to documentation javascript Pull requests that update javascript code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant