fix: return 400 instead of 503 for malformed JSON and invalid Space access - #11
asasemahmed wants to merge 2 commits into
Conversation
…ace routes Malformed request bodies on /api/spaces, /api/dots, /api/conversations and /api/voice/calls, and Dot create/update with an unknown Space, fell through to the generic 503 handler, which tells the caller to check server configuration. Return 400 with a specific message instead, matching the page routes. Signed-off-by: asemabdallah <asasem547@gmail.com>
jerelvelarde
left a comment
jerelvelarde
left a comment
There was a problem hiding this comment.
I recommend fixing the error-classification regression below before merging. The intended 400 responses for malformed caller JSON are useful, but the global handler also catches JSON parsing failures from upstream services.
| }); | ||
| app.all('/copilotkit/*', (c) => platform.handle(c.req.raw)); | ||
| app.onError((error, c) => { | ||
| if (error instanceof SyntaxError) |
There was a problem hiding this comment.
[P2] Scope SyntaxError handling to request-body parsing
This handler catches upstream JSON parsing failures as well as malformed caller JSON. A valid POST to /api/voice/calls/:id/compute reaches VoiceService.compute → Platform.turn → runThreadTurn, where parsing a malformed runtime /info response throws SyntaxError. The new handler then returns 400 Invalid JSON request. even though the caller's JSON is valid; this previously returned 503. That misidentifies a service failure as a client error and can prevent clients from treating it as retryable.
This was confirmed with a focused reproduction against this PR's head: a malformed upstream response produces 400, while removing this handler restores 503. Please catch malformed JSON at the c.req.json() boundary and add a regression case showing that malformed upstream JSON still returns 503. I recommend fixing this before merging.
Catch JSON parse failures where each workspace route reads its request body instead of mapping every SyntaxError to 400 in the global handler. Malformed caller JSON still gets a 400 from the route's schema check, while a SyntaxError raised by an upstream service call stays a 503. Add a regression test for the upstream case. Signed-off-by: asemabdallah <asasem547@gmail.com>
|
Thanks, that makes sense. I moved the handling to the request-body boundary |
2 participants
Problem
On the workspace routes (
/api/spaces,/api/dots,/api/conversations,/api/voice/calls), a malformed JSON body returns503with "Check the serverconfiguration and try again." Creating or updating a Dot with a Space that does
not exist also returns
503. Both are caller errors, not server faults, so thestatus and message point the user at the wrong thing. The page routes and
/api/tasksalready answer malformed JSON with400.Change
workspaceRoutesonErrornow returns400 Invalid JSON request.for aSyntaxError.400instead of
503.Testing
typecheck, lint and the full vitest suite pass (157 tests).