-
Notifications
You must be signed in to change notification settings - Fork 3
Description
One thing we could do, it just occurred to me, is make them look more like real API errors so that the UI would handle them naturally. As in, where right now we just return the Zod error directly as JSON, we could stick it somewhere in an object that looks like
{ "error_code": "InvalidRequest", "message": "..." }. The Zod error would probably look like garbage stringified, but maybe we put a short version of it in message (with "Zod" in there to be clear it's a mock API error) and then log the full thing to the console.
https://github.com/oxidecomputer/console/pull/1854/files#r1425571596
Right now, when there's a validation failure in the mock API, we return the list of Zod "issues" more or less directly as the response body.
oxide.ts/client/msw-handlers.ts
Lines 1070 to 1073 in 9406dfb
| const { params, paramsErr } = paramSchema | |
| ? validateParams(paramSchema, req, pathParams) | |
| : { params: {}, paramsErr: undefined }; | |
| if (paramsErr) return json(paramsErr, { status: 400 }); |
oxide.ts/client/msw-handlers.ts
Lines 1050 to 1052 in 9406dfb
| const { issues } = result.error; | |
| const status = issues.some((e) => e.path[0] === "path") ? 404 : 400; | |
| return { paramsErr: json(issues, { status }) }; |
This is a bit silly because it means that our code in the console to display messages from API 400s chokes and displays "Unknown server error" instead of a nicer message that we can assert about in the E2Es. As I said in the quote above, I propose we jam these errors into an object that more closely resembles an API error response, which looks like this:
{
error_code: "InvalidRequest",
message: "Invalid IP address",
request_id: "xxx",
}We could also stick the full list of Zod issues in the response under another key — I don't think anything would break as a result. The point is to have a nicer message to display in the console for dev and testing.