No output when request body is invalid

[StevePotter]

Hey there, this is a great library and I'm excited to use it. I created a PUT route, and when I tested with invalid data, I didn't get any output from my curl command. It gives a 400, but I would expect to receive validation error information along with it.

Indeed, the validation error comes up in the console, but I need it to be part of the response. Validation error:

[
  {
    code: 'invalid_type',
    expected: 'string',
    received: 'undefined',
    path: [ 'name' ],
    message: 'Required'
  }
]

Simplified example:

import defineRoute from "@omer-x/next-openapi-route-handler";
import z from "zod";

export const { PUT } = defineRoute({
  operationId: "putThing",
  method: "PUT",
  summary: "Create a new thing",
  description: "Create a new thing with the given details",
  tags: ["Things"],
  requestBody: z.object({
    name: z.string(),
  }),
  action: async () => {
    return Response.json({ id: "123" });
  },
  responses: {
    200: {
      description: "Thing retrieved successfully",
      content: z.object({
        id: z.string(),
      }),
    },
  },
});

Also, when the route fails, I get a 500 but the error is not displayed in the console. If I use a vanilla nextjs route, the error shows up.

[omermecitoglu]

Hey! Thanks for feedback. I'll look into it today.

[omermecitoglu]

I made some changes in #5 but I'm not sure if it's a good starting point. I didn't need something like this before, because HTTP error codes were just enough for me. Could you give more feedback about how can we improve this?

[StevePotter]

Hey Omer, thanks for replying and for putting in that effort. You are awesome.

I took a deeper look at the code and have a better idea how it works now. A few suggestions:

1. I don't think you should wrap the user code in a try/catch. I suggest pulling this line out of that try/catch. That should solve the issue with 500 errors not showing up in the console.
2. In the end, it looks like you are using zod's safeParse, which doesn't throw. But that itself has been wrapped in a function that does throw. Then that error gets handled in definer.ts. It makes it a little hard to follow. Maybe instead, have parsePathParams, parseSearchParams, and parseBody use safeParse directly and if it fails, return a 400. Also consider 422, which I've generally used for invalid request bodies.
3. Consider adding custom functions to RouteOptions for overriding parsePathParams, parseSearchParams, parseBody. This way you can have a sensible default but people can customize it to their liking.

Happy to help with implementation if you'd like

[omermecitoglu]

Thanks for your kind words, Steve.

As it is a known issue that error handling (especially with try-catch) in JavaScript has always been a bit problematic. But the reason of why I decided to wrap the route handler with a try-catch and return some generic responses (400, 404, 500) was because I wanted to keep the code as simple as possible in action part of the definer.

I believe 500 (Internal Server Error) means the code doesn't work as expected, probably because of some untested code or database problems which is not the FE dev's concern IMO. In action function, the BE dev can wrap the code with their own try-catch to see what is wrong before it's caught by the library's error handler and can fix it. That's what I do.

But I agree that there should be some customization options with error handling. There might be some edge cases where my solution isn't sufficient. So I’ll look into how can I implement custom error handling within this week.

Thanks for sparing your valuable time.

[omermecitoglu]

Hello again @StevePotter,

As you mentioned in #5, it might be confusing for a non-JS developer to see something called zodIssues. So today, I tried to provide more detail about zodIssues in the generated OpenAPI specs. However, I noticed that reflecting the ZodIssues type in the specs would add 6 kB per defined route in the app. While I could fix this with the json-generator, I questioned whether it would be worth the effort. So, I decided to abandon that idea and focus on the custom error handler feature instead. I'm satisfied with the result and would love to hear your thoughts as well. Let me know if you think that is enough to close this issue.

[StevePotter]

Nice work. Good luck with this project. I wish nextjs didn't require so much effort to enable this type of thing. Those of us not looking to immediately set up both a frontend and backend server would really benefit from native support. Grateful for your effort and doing something about it