Handle malformed saved gigs JSON#183
Conversation
Greptile SummaryThis PR guards
Confidence Score: 5/5Safe to merge — the change is narrowly scoped to intercepting JSON parse failures before they reach Zod or Supabase, with no modifications to the auth, validation, or database write paths. The introduced helper is small and its logic is straightforward: it wraps request.json() and returns a discriminated union. Both handlers correctly guard on the error branch before proceeding. The formatting-only changes to the rest of the file carry no functional risk. No files require special attention. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Incoming Request] --> B[getAuthContext]
B -- no auth --> C[401 Unauthorized]
B -- authenticated --> D[parseJsonBody]
D -- parse error --> E[400 Invalid JSON body]
D -- valid JSON --> F[Zod safeParse]
F -- invalid schema --> G[400 Validation error]
F -- valid schema --> H[Supabase query]
H -- db error --> I[400 DB error]
H -- success --> J[200 / 201 Response]
Reviews (3): Last reviewed commit: "Handle malformed saved gig bodies" | Re-trigger Greptile |
| describe("saved gigs invalid JSON handling", () => { | ||
| beforeEach(() => { | ||
| vi.clearAllMocks(); | ||
| mockGetAuthContext.mockResolvedValue({ | ||
| user: { id: "user-1" }, | ||
| supabase: mockSupabase, | ||
| }); | ||
| }); | ||
|
|
||
| it("returns 400 for malformed POST bodies without querying Supabase", async () => { | ||
| const res = await POST(makeRequest("POST", "{")); | ||
| const body = await res.json(); | ||
|
|
||
| expect(res.status).toBe(400); | ||
| expect(body.error).toBe("Invalid JSON body"); | ||
| expect(mockFrom).not.toHaveBeenCalled(); | ||
| }); | ||
|
|
||
| it("returns 400 for malformed DELETE bodies without querying Supabase", async () => { | ||
| const res = await DELETE(makeRequest("DELETE", "{")); | ||
| const body = await res.json(); | ||
|
|
||
| expect(res.status).toBe(400); | ||
| expect(body.error).toBe("Invalid JSON body"); | ||
| expect(mockFrom).not.toHaveBeenCalled(); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
Test suite covers only one error case
The two tests verify that malformed JSON returns 400 and skips Supabase, which is the targeted regression. However, there is no test for valid JSON that fails Zod validation (e.g., a missing or non-UUID gig_id) nor for the normal success path (valid body → Supabase insert/delete called). The route.test.ts is a new file, so this is the only test coverage that exists for these handlers; a subsequent regression in Zod validation or the Supabase write path would go undetected.
| async function parseJsonBody(request: NextRequest) { | ||
| try { | ||
| return { body: await request.json() }; | ||
| } catch { | ||
| return { | ||
| response: NextResponse.json({ error: "Invalid JSON body" }, { status: 400 }), | ||
| }; | ||
| } | ||
| } |
There was a problem hiding this comment.
Broad catch swallows non-parse errors
request.json() can throw for reasons beyond a SyntaxError (e.g., a body-read interruption). The bare catch {} maps all of those to a 400 "Invalid JSON body", which misrepresents infrastructure-level failures as client errors. Narrowing the catch to SyntaxError (or re-throwing anything else) would keep the 400 semantically accurate and let genuine 500-class errors surface to the outer handler.
| async function parseJsonBody(request: NextRequest) { | |
| try { | |
| return { body: await request.json() }; | |
| } catch { | |
| return { | |
| response: NextResponse.json({ error: "Invalid JSON body" }, { status: 400 }), | |
| }; | |
| } | |
| } | |
| async function parseJsonBody(request: NextRequest) { | |
| try { | |
| return { body: await request.json() }; | |
| } catch (err) { | |
| if (err instanceof SyntaxError) { | |
| return { | |
| response: NextResponse.json({ error: "Invalid JSON body" }, { status: 400 }), | |
| }; | |
| } | |
| throw err; | |
| } | |
| } |
|
Want your agent to iterate on Greptile's feedback? Try greploops. |
Summary
POST /api/saved-gigsandDELETE /api/saved-gigsrequest bodies.Fixes #182
Verification
./node_modules/.bin/vitest run src/app/api/saved-gigs/route.test.ts./node_modules/.bin/prettier --check src/app/api/saved-gigs/route.ts src/app/api/saved-gigs/route.test.ts./node_modules/.bin/eslint src/app/api/saved-gigs/route.ts src/app/api/saved-gigs/route.test.tsnpm run type-checkgit diff --cached --check