fix(api/v1): flatten error response shape to match OpenAPI Error schema - #58
Conversation
The OpenAPI Error schema declares `error` as a string:
Error:
type: object
properties:
error: { type: string }
message: { type: string }
But twelve v1 route handlers were returning the nested `{ error: { message: '...' } }` shape. Callers following the OpenAPI contract null-deref on `.error.message`; callers reading the live API see two different shapes depending on which endpoint they hit.
Audit doc finding #9 cited the four most-visible offenders; the grep
turned up eight more in the same area, all the same shape. Fixing
them all in one mechanical pass — splitting per-route would leave the
contract half-broken between merges.
Routes updated (all v1):
- /forms (POST validation + GET/POST 500)
- /forms/[formId] (GET/DELETE 404, PATCH 500)
- /forms/[formId]/duplicate (500)
- /forms/[formId]/publish (500)
- /forms/[formId]/submissions (GET 500, POST 422 validation + 500)
- /forms/[formId]/submissions/[submissionId] (GET 404, PATCH 400 + 500)
- /forms/[formId]/submissions/export (400 + 500 + 404 + 500)
- /forms/[formId]/analytics (403 entitlement + 500)
- /analytics/trends (400 + 500)
- /reports/custom (GET 500, POST 400 + 500)
- /reports/custom/_entitlement helper (403)
- /ai/usage (500)
The 422 validation response on /submissions keeps its `details: err.validationErrors` sibling — flat-shape plus a domain-specific extra is consistent with the spec; the nested wrapper added no value.
Verified:
- grep across app/api/v1/ for `error: {` returns zero hits.
- npx tsc --noEmit -p tsconfig.typecheck.json: clean.
- npx jest __tests__/api/v1: 36/36 pass.
- No test asserted the nested shape (the two `error: { message }`
references in v1 tests are Supabase-DB-error mocks, not API
response assertions).
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f71b022de5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| details: err.validationErrors, | ||
| }, | ||
| error: 'Validation failed', | ||
| details: err.validationErrors, |
There was a problem hiding this comment.
Wrap validation details to match Error schema
When a form submission fails validation, this newly promoted top-level details value is err.validationErrors, which is an array of field errors. The OpenAPI Error component this change is trying to satisfy declares details as an object (openapi.json:2096-2098), so generated clients or response validators still see a schema mismatch for 422 responses; wrap the array in an object such as details: { validationErrors: ... } or update the schema accordingly.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Pull request overview
This PR standardizes /api/v1/* error responses to align with the OpenAPI Error schema by flattening { error: { message: string } } into { error: string }, eliminating inconsistent response shapes across v1 endpoints.
Changes:
- Updated v1 route handlers to return
erroras a string instead of a nested{ message }object. - Preserved existing domain-specific siblings (e.g., validation
details) while flattening the top-level error shape. - Updated entitlement/permission error responses to the flattened shape.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 22 comments.
Show a summary per file
| File | Description |
|---|---|
| app/api/v1/reports/custom/route.ts | Flattens error responses for custom reports list/create endpoints. |
| app/api/v1/reports/custom/_entitlement.ts | Flattens entitlement-denied error response. |
| app/api/v1/forms/route.ts | Flattens error responses for forms list/create endpoints. |
| app/api/v1/forms/[formId]/submissions/route.ts | Flattens error responses; keeps 422 validation details as a sibling. |
| app/api/v1/forms/[formId]/submissions/export/route.ts | Flattens error responses for submission export flow. |
| app/api/v1/forms/[formId]/submissions/[submissionId]/route.ts | Flattens error responses for submission fetch/review endpoints. |
| app/api/v1/forms/[formId]/route.ts | Flattens error responses for form fetch/update/archive endpoints. |
| app/api/v1/forms/[formId]/publish/route.ts | Flattens error response for publish endpoint. |
| app/api/v1/forms/[formId]/duplicate/route.ts | Flattens error response for duplicate endpoint. |
| app/api/v1/forms/[formId]/analytics/route.ts | Flattens error responses for analytics endpoint. |
| app/api/v1/analytics/trends/route.ts | Flattens error responses for trends endpoint. |
| app/api/v1/ai/usage/route.ts | Flattens error response for AI usage endpoint. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| } catch (err) { | ||
| return Response.json( | ||
| { | ||
| error: { | ||
| message: | ||
| err instanceof Error ? err.message : 'Failed to list reports', | ||
| }, | ||
| error: | ||
| err instanceof Error ? err.message : 'Failed to list reports', | ||
| }, | ||
| { status: 500 }, | ||
| ); |
| if (!name || typeof name !== 'string') { | ||
| return Response.json( | ||
| { error: { message: 'name is required' } }, | ||
| { error: 'name is required' }, | ||
| { status: 400 }, | ||
| ); | ||
| } |
| } catch (err) { | ||
| return Response.json( | ||
| { | ||
| error: { | ||
| message: | ||
| err instanceof Error ? err.message : 'Failed to create report', | ||
| }, | ||
| error: | ||
| err instanceof Error ? err.message : 'Failed to create report', | ||
| }, | ||
| { status: 500 }, | ||
| ); |
| @@ -33,11 +33,7 @@ export async function GET(request: Request) { | |||
| ); | |||
| } catch (err) { | |||
| return Response.json( | |||
| if (!title || typeof title !== 'string') { | ||
| return Response.json( | ||
| { error: { message: 'title is required' } }, | ||
| { error: 'title is required' }, | ||
| { status: 400 }, | ||
| ); | ||
| } |
| } catch { | ||
| return Response.json( | ||
| { error: { message: 'Form not found' } }, | ||
| { error: 'Form not found' }, | ||
| { status: 404 }, | ||
| ); |
| } catch (err) { | ||
| return Response.json( | ||
| { | ||
| error: { | ||
| message: | ||
| err instanceof Error ? err.message : 'Failed to get analytics', | ||
| }, | ||
| error: | ||
| err instanceof Error ? err.message : 'Failed to get analytics', | ||
| }, | ||
| { status: 500 }, | ||
| ); |
| if (!from || !to) { | ||
| return Response.json( | ||
| { error: { message: 'from and to query parameters are required' } }, | ||
| { error: 'from and to query parameters are required' }, | ||
| { status: 400 }, | ||
| ); |
| } catch (err) { | ||
| return Response.json( | ||
| { | ||
| error: { | ||
| message: err instanceof Error ? err.message : 'Failed to get trends', | ||
| }, | ||
| }, | ||
| { error: err instanceof Error ? err.message : 'Failed to get trends' }, | ||
| { status: 500 }, | ||
| ); |
| } catch (err) { | ||
| return Response.json( | ||
| { | ||
| error: { | ||
| message: | ||
| err instanceof Error ? err.message : 'Failed to get AI usage', | ||
| }, | ||
| error: | ||
| err instanceof Error ? err.message : 'Failed to get AI usage', | ||
| }, | ||
| { status: 500 }, | ||
| ); |
♿ Accessibility Test Results✅ PASSED - No critical accessibility issues found Tests Performed:
Artifacts: Download the accessibility reports from the "Artifacts" section for detailed results. |
Phase A (audit dimensions) is now walked end-to-end. This PR adds: §9b — Dead-entitlement removals (records the #57 deletion of soc2_certification + executive_rollup; closes audit row #14). §9c — A→B→C data flow findings (4 verified rows: HIGH role-change surface gap, HIGH trial-state stale Zustand, MED org-name client cache, MED revokeInvitation org-id input-trust gap). Plus 12 rejected-with-reason rows from the data-flow agent pass that did not survive direct inspection (force-dynamic at /app/* layout killed most of the "stale SSR cache" claims; the Stripe webhook already calls revalidatePath; the email-mismatch invite-accept scenario is explicitly guarded; "if a user is added to multiple orgs in the future" is speculative not actual). Also reclassifies: #19 (onboarding framework provisioning race) HIGH → MED. The proposed audit fix doesn't actually close the race; the real fix is a "provisioning in progress" state on /app/compliance/ frameworks/[slug], which is a different surface from onboarding. #9 (v1 error-shape consistency) — added "scope larger than first cited" note since the grep found 12 files, all flattened in #58. PR refs added to rows #9 and #14. The audit now records 36 verified findings; the open HIGH set is #19 (reclassified MED), #33 (no role-change action), #34 (trial state stale in Zustand). Co-authored-by: ejaz <ejaz@local> Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
Summary
Errorschema declareserroras a string. Twelve v1 route handlers were returning the nested{ error: { message: '...' } }shape — callers following the spec null-deref on.error.message; callers reading the live API see two different shapes depending on which endpoint they hit.What changed
Twelve files in
app/api/v1/:/forms,/forms/[formId],/forms/[formId]/duplicate,/forms/[formId]/publish/forms/[formId]/submissions,/forms/[formId]/submissions/[submissionId],/forms/[formId]/submissions/export,/forms/[formId]/analytics/analytics/trends,/reports/custom,/reports/custom/_entitlement,/ai/usageThe 422 validation response on
/submissionskeeps itsdetails: err.validationErrorssibling — flat shape plus a domain-specific extra is consistent with the spec; the nested wrapper added no value.Test plan
grep -rE "error: ?\{" app/api/v1/→ zero hits after edit.npx tsc --noEmit -p tsconfig.typecheck.json→ clean.npx jest __tests__/api/v1→ 36/36 pass.error: { message }references in v1 tests are Supabase-DB-error mocks, not API response assertions).🤖 Generated with Claude Code
Co-Authored-By: Claude Opus 4.7 noreply@anthropic.com