Skip to content

Commit f94f355

Browse files
Sbussisoclaude
andcommitted
feat(errors): add ApiError envelope + robust frontend parser
Closes the long tail of "[object Object]" toasts that surfaced when a backend route returned a structured error body (e.g. 402 plan_limit_hit) and the frontend's ``new Error(detail)`` stringified the dict. Also normalises Pydantic 422 validation responses through the same envelope so a missing field surfaces as "field required (name)" rather than the raw [{loc, msg, type}] array. Three pieces, one shape: backend/app/core/errors.py — new ApiError(status, code, message, **extras) produces {"detail": {error, message, ...}} on the wire. Drop-in for the existing 402 plan_limit_hit pattern; opt-in for new endpoints. Existing HTTPException(detail="...") sites keep working through the new frontend parser without migration. backend/app/main.py — RequestValidationError exception handler rewrites Pydantic 422 envelope from the default array shape into ApiError's flat shape, with a one-line summary message pointing at the first failing field and the raw error list preserved under detail.errors for clients that need the breakdown. frontend/src/services/api.js — parseErrorBody handles all three current wire shapes (ApiError dict, legacy string, top-level rate-limit envelope) plus the Pydantic array as a defensive fallback for in-flight deploys. The thrown Error always has .message / .code / .status / .detail so every consumer can either show the message in a toast or branch on a stable code without regex-matching text. MCP coexistence: ApiError is REST-only by docstring. ``app.mount("/mcp", mcp_app)`` already isolates FastMCP as an ASGI sub-app, so no exception handler registered on the parent FastAPI app reaches into the MCP layer. MCP tools keep raising ToolError; the JSON-RPC error envelope the protocol mandates stays untouched. Tests: backend/tests/test_errors.py — 6 cases (ApiError minimal/extras/reserved keys/serialization + 422 handler envelope + summary) → 235 backend tests total frontend/tests/services/api.test.js — extended from 5 → 9 cases covering each wire shape → 33 frontend tests total No mass-migration of the existing ~30 raise HTTPException(detail="...") sites. The new parser handles their shape correctly; future endpoints opt into ApiError when they want structured fields. Lower blast radius than a wholesale rewrite, same end-user benefit. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
1 parent f0f6553 commit f94f355

5 files changed

Lines changed: 520 additions & 4 deletions

File tree

backend/app/core/errors.py

Lines changed: 121 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,121 @@
1+
"""Standardized error envelope for the REST surface (`/api/*` routes).
2+
3+
Background
4+
----------
5+
``raise HTTPException(detail="some string")`` was the original pattern across
6+
the codebase, and the frontend parser in ``services/api.js`` evolved around
7+
it. When a few routes started returning structured detail dicts (notably the
8+
402 plan-limit-hit body in ``app.api.hls.push_segment``), ``new Error(detail)``
9+
on the frontend produced ``"[object Object]"`` for any consumer that hadn't
10+
been written to specifically pull a ``message`` field out.
11+
12+
Rather than mass-migrate every existing ``HTTPException`` site at once, this
13+
module adds:
14+
15+
- ``ApiError`` — drop-in replacement that produces a structured envelope.
16+
Use it on new endpoints, and migrate old ones opportunistically when you
17+
touch them.
18+
- The frontend's ``services/api.js`` knows about all three current shapes
19+
(``string``, ``dict``, Pydantic-422 ``list[dict]``) so it never falls
20+
through to the ``[object Object]`` path regardless of which backend
21+
pattern produced the response.
22+
23+
MCP coexistence
24+
---------------
25+
``ApiError`` is for FastAPI REST handlers only. MCP tools at ``POST /mcp``
26+
must keep raising ``fastmcp.exceptions.ToolError`` — the JSON-RPC error
27+
envelope is fixed by the MCP protocol and is what every MCP client (Claude,
28+
Cursor, custom agents) is built to consume. ``app.mount("/mcp", mcp_app)``
29+
mounts FastMCP as an ASGI sub-app, so exception handlers registered on the
30+
parent FastAPI app don't reach into the MCP layer either way — the surfaces
31+
are isolated by construction, this docstring just records the design.
32+
33+
Do NOT call ``raise ApiError`` inside helpers shared between REST handlers
34+
and MCP tool implementations. If you have to share business logic, either
35+
return a result object that each side translates, or raise a plain Python
36+
exception that each side catches and re-wraps. See
37+
``backend/app/mcp/server.py`` lines 503/532/572/595/etc. for the existing
38+
``ToolError`` translation pattern.
39+
"""
40+
41+
from __future__ import annotations
42+
43+
from typing import Any
44+
45+
from fastapi import HTTPException
46+
47+
48+
class ApiError(HTTPException):
49+
"""Structured error response for the REST surface.
50+
51+
Parameters
52+
----------
53+
status_code : int
54+
HTTP status code. 4xx for client errors, 5xx for server.
55+
code : str
56+
Machine-readable error code (snake_case). Frontend consumers can
57+
branch on ``e.code === "plan_limit_hit"`` etc. instead of regex-
58+
matching the message text.
59+
message : str
60+
Human-readable sentence the frontend can show to the user as-is.
61+
Should be operator-friendly, not a stack-trace fragment.
62+
**extra
63+
Optional structured fields to embed in the envelope. Stay flat and
64+
JSON-serializable; the frontend reads them off ``e.detail.<key>``.
65+
66+
Wire format
67+
-----------
68+
::
69+
70+
{
71+
"detail": {
72+
"error": "<code>",
73+
"message": "<message>",
74+
"<extra-key>": "<extra-value>",
75+
...
76+
}
77+
}
78+
79+
The flat shape mirrors the existing 402 plan-limit-hit body, so
80+
``services/api.js``'s parser handles both old call sites and new
81+
``ApiError`` ones identically.
82+
83+
Examples
84+
--------
85+
Simple not-found::
86+
87+
raise ApiError(404, "camera_not_found", "Camera not found")
88+
89+
With structured fields::
90+
91+
raise ApiError(
92+
402,
93+
"plan_limit_hit",
94+
f"Camera over the {plan} plan limit",
95+
plan=plan,
96+
max_cameras=max_cams,
97+
camera_name=camera.name,
98+
)
99+
"""
100+
101+
def __init__(
102+
self,
103+
status_code: int,
104+
code: str,
105+
message: str,
106+
**extra: Any,
107+
) -> None:
108+
detail: dict[str, Any] = {"error": code, "message": message}
109+
# Flatten extras into the envelope rather than nesting them. Matches
110+
# the existing 402 shape so the frontend doesn't need a special case.
111+
for key, value in extra.items():
112+
if key in ("error", "message"):
113+
# These are reserved by the envelope itself. Refuse silently
114+
# rather than overwriting — easier to spot in tests than a
115+
# confusing message swap at runtime.
116+
raise ValueError(
117+
f"ApiError extra key {key!r} collides with reserved "
118+
f"envelope field; rename the kwarg",
119+
)
120+
detail[key] = value
121+
super().__init__(status_code=status_code, detail=detail)

backend/app/main.py

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -140,6 +140,51 @@ async def rate_limit_exceeded_handler(request: Request, exc: RateLimitExceeded)
140140
app.add_exception_handler(RateLimitExceeded, rate_limit_exceeded_handler)
141141
app.add_middleware(SlowAPIMiddleware)
142142

143+
144+
# ── Pydantic validation errors → ApiError envelope ──────────────────
145+
#
146+
# FastAPI's default 422 body for a request that fails Pydantic
147+
# validation is a list of dicts:
148+
#
149+
# {"detail": [{"loc": [...], "msg": "...", "type": "..."}]}
150+
#
151+
# That's machine-friendly but unreadable to a human, and the frontend's
152+
# error parser used to stringify the array into something like
153+
# "[object Object]" before we taught it about the shape. Funnel
154+
# validation failures through the same envelope ApiError uses, so the
155+
# REST surface produces one shape regardless of whether the failure
156+
# came from a hand-raised exception or Pydantic's auto-validation.
157+
#
158+
# Behaviour on the frontend stays consistent: services/api.js looks at
159+
# body.detail.message and shows it as-is. Test code can still inspect
160+
# body.detail.errors for the structured per-field breakdown.
161+
from fastapi.exceptions import RequestValidationError # noqa: E402
162+
163+
@app.exception_handler(RequestValidationError)
164+
async def validation_exception_handler(request: Request, exc: RequestValidationError):
165+
"""Rewrite Pydantic 422 envelope to match ApiError's shape."""
166+
errors = exc.errors()
167+
# Build a one-line summary out of the first failing field — that's
168+
# what gets shown in the toast. The full error list is preserved
169+
# under detail.errors for callers (and tests) that want the breakdown.
170+
if errors:
171+
first = errors[0]
172+
loc = ".".join(str(p) for p in first.get("loc", []) if p != "body")
173+
msg = first.get("msg", "Validation failed")
174+
summary = f"{msg} ({loc})" if loc else msg
175+
else:
176+
summary = "Request validation failed"
177+
return JSONResponse(
178+
status_code=422,
179+
content={
180+
"detail": {
181+
"error": "validation_failed",
182+
"message": summary,
183+
"errors": errors, # full list for clients that want it
184+
},
185+
},
186+
)
187+
143188
# Get frontend URL from environment (set in fly.toml or .env)
144189
frontend_url = os.getenv("FRONTEND_URL", "http://localhost:5173")
145190

backend/tests/test_errors.py

Lines changed: 158 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,158 @@
1+
"""Tests for the ApiError class + the 422 validation exception handler.
2+
3+
The class itself is small enough to test directly; the handler is wired into
4+
``app.main`` so we exercise it through the FastAPI test client by hitting an
5+
existing endpoint with an invalid body and asserting the rewritten envelope.
6+
"""
7+
8+
import pytest
9+
from fastapi import FastAPI, status
10+
from fastapi.testclient import TestClient
11+
from pydantic import BaseModel
12+
13+
from app.core.errors import ApiError
14+
15+
16+
# ── ApiError unit tests ────────────────────────────────────────────
17+
18+
19+
def test_apierror_minimal_envelope():
20+
"""Status, code, and message land in the right slots."""
21+
err = ApiError(404, "camera_not_found", "Camera not found")
22+
assert err.status_code == 404
23+
assert err.detail == {"error": "camera_not_found", "message": "Camera not found"}
24+
25+
26+
def test_apierror_extras_flatten_into_detail():
27+
"""Optional kwargs (plan, max_cameras, etc.) sit alongside error/message
28+
in the same flat dict — matches the existing 402 plan-limit-hit shape so
29+
the frontend's parser handles both old and new sites identically."""
30+
err = ApiError(
31+
402,
32+
"plan_limit_hit",
33+
"Camera over the Free plan limit",
34+
plan="Free",
35+
max_cameras=5,
36+
camera_name="Front Door",
37+
)
38+
assert err.status_code == 402
39+
assert err.detail == {
40+
"error": "plan_limit_hit",
41+
"message": "Camera over the Free plan limit",
42+
"plan": "Free",
43+
"max_cameras": 5,
44+
"camera_name": "Front Door",
45+
}
46+
47+
48+
def test_apierror_rejects_reserved_extra_keys():
49+
"""``error`` is envelope-reserved; collisions raise loudly so a typo
50+
can't silently overwrite the machine-readable code in detail.
51+
52+
(``message`` is also reserved but Python catches that one earlier with
53+
a built-in TypeError because it's the third positional parameter — no
54+
extra check needed for that key.)"""
55+
with pytest.raises(ValueError, match="reserved envelope field"):
56+
ApiError(400, "bad_request", "Body invalid", error="something_else")
57+
58+
59+
def test_apierror_serializes_through_fastapi_handler():
60+
"""End-to-end: ApiError raised from a route handler reaches the wire as
61+
``{detail: {error, message, ...}}`` — the shape services/api.js parses."""
62+
app = FastAPI()
63+
64+
@app.get("/raise")
65+
def _raise():
66+
raise ApiError(
67+
418,
68+
"im_a_teapot",
69+
"Cannot brew coffee",
70+
requested="coffee",
71+
available=["tea"],
72+
)
73+
74+
with TestClient(app) as client:
75+
resp = client.get("/raise")
76+
assert resp.status_code == 418
77+
assert resp.json() == {
78+
"detail": {
79+
"error": "im_a_teapot",
80+
"message": "Cannot brew coffee",
81+
"requested": "coffee",
82+
"available": ["tea"],
83+
},
84+
}
85+
86+
87+
# ── 422 validation handler integration ─────────────────────────────
88+
#
89+
# We rebuild a fresh FastAPI app for these tests rather than reusing the
90+
# real `app.main.app`. Two reasons:
91+
# - Adding routes to the live app post-startup falls through to the SPA
92+
# middleware (returns index.html, not the test endpoint's response).
93+
# - The validation handler is the unit under test; isolating it from
94+
# the rest of the production middleware stack makes failures easy to
95+
# attribute.
96+
# We import the handler function directly from main.py and re-register it
97+
# on the test app so we're testing the same code that ships.
98+
99+
100+
def _make_validation_test_app():
101+
"""Build a fresh FastAPI app with only the validation handler from
102+
main.py wired in. Used to exercise the 422 → envelope rewrite without
103+
pulling in the SPA middleware / Clerk / database lifecycle."""
104+
from fastapi.exceptions import RequestValidationError
105+
106+
from app.main import validation_exception_handler
107+
108+
app = FastAPI()
109+
app.add_exception_handler(RequestValidationError, validation_exception_handler)
110+
111+
class Body(BaseModel):
112+
name: str
113+
port: int
114+
115+
@app.post("/validate")
116+
def _endpoint(body: Body):
117+
return {"ok": True, "name": body.name}
118+
119+
return app
120+
121+
122+
def test_validation_error_rewritten_to_apierror_envelope():
123+
"""A request that fails Pydantic validation should come back through the
124+
same envelope shape as ApiError — error code, human message, and the
125+
raw Pydantic error list preserved under detail.errors for clients that
126+
want the per-field breakdown."""
127+
app = _make_validation_test_app()
128+
129+
with TestClient(app) as client:
130+
# Missing required field "port".
131+
resp = client.post("/validate", json={"name": "x"})
132+
133+
assert resp.status_code == status.HTTP_422_UNPROCESSABLE_CONTENT
134+
body = resp.json()
135+
136+
assert "detail" in body
137+
detail = body["detail"]
138+
assert detail["error"] == "validation_failed"
139+
assert isinstance(detail["message"], str) and detail["message"]
140+
# Full per-field list preserved for callers that need it.
141+
assert isinstance(detail["errors"], list) and detail["errors"]
142+
first = detail["errors"][0]
143+
assert "loc" in first and "msg" in first
144+
145+
146+
def test_validation_error_handler_summarises_first_field_in_message():
147+
"""The summary message points at the first failing field so a toast
148+
can show ``"Field required (name)"`` instead of a generic message."""
149+
app = _make_validation_test_app()
150+
151+
with TestClient(app) as client:
152+
resp = client.post("/validate", json={})
153+
154+
body = resp.json()
155+
msg = body["detail"]["message"]
156+
# Either "name" or "port" — both are missing, ordering depends on Pydantic
157+
# internals but at least one should appear in the summary.
158+
assert any(field in msg for field in ("name", "port"))

0 commit comments

Comments
 (0)