@hono/zod-openapi: Global error handler about middleware

[cldsnchz]

Hi, I actually asked this question in the discussions: https://github.com/orgs/honojs/discussions/1803
And i was told that is the expected behavior.

I wonder if the defaultHook will be needed at all when this honojs/hono#1441 is implemented.

[yusukebe]

Is that how you would go about having a unified defaultHook while being able to split routes into dedicated files or can you think of a better approach?

I think your way is the best way!

I think it is good to throw an HTTPException with 400 or 422 when a validation error occurs, as you did.

The reason we didn't do it that way is that we didn't want the user to handle it with onError(). We wanted error handling within the Validator functionality.

However, I think either would be good!

[soulchild]

Thanks for pointing me to that issue! Having validation errors end up in the global error handler would actually be my preferred way as well. 😃 I do this in all my Express apps.

[soulchild]

Hono@v4 now throws an HTTPException when the validation fails with the built-in validator middleware. 👍

Unfortunately, this does not seem to apply to the validation provided by zod-openapi. In case of validation errors, I'm still seeing the following old response and there's no HTTPException thrown which I could catch in my global error handler:

{
  "success": false,
  "error": { 
    // ...
  }
}

@yusukebe Is this intended behavior? Maybe I'm doing something wrong… Or does zod-openapi needs to be patched as well?

[yusukebe]

Hi @soulchild

Hono@v4 now throws an HTTPException when the validation fails with the built-in validator middleware. 👍

Perhaps there is a difference in understanding between you and me. In v4, now, errors are thrown when Malformed JSON in request body errors occur. So, we still use hooks in the Validator to handle validator errors.

app.post(
  '/post',
  zValidator('json', schema, (result, c) => {
    if (!result.success) {
      return c.text(`${result.data.id} is invalid!`, 400)
    }
    const data = result.data
    return c.text(`${data.id} is valid!`)
  }),
  //...
)

So please use hooks for zod-openapi, too.

https://github.com/honojs/middleware/tree/main/packages/zod-openapi#handling-validation-errors

[yusukebe]

Or, and this is just an idea, you could throw a custom error in the defaultHook.

[soulchild]

Or, and this is just an idea, you could throw a custom error in the defaultHook.

@yusukebe That's actually what I'm currently doing. 😄 To not copy and paste my defaultHook into every route file, I have the following in a separate file:

lib/hono.ts

export const honoApp = () =>
  new OpenAPIHono<{ Variables: { user: User | null } }>({
    defaultHook: (result) => {
      if (result.success) {
        return;
      }
      const err = new HTTPException(422, {
        message: 'Validation failed',
      });
      err.cause = result.error.errors;
      throw err;
    },
  });

In my dedicated route files, I'm using the honoApp() method from above to instantiate my sub-routes:

routes/users.ts

import { honoApp } from '@/lib/hono.js';
export const userRoutes = honoApp();
userRoutes.openapi(…)

routes/other.ts

import { honoApp } from '@/lib/hono.js';
export const otherRoutes = honoApp();
otherRoutes.openapi(…)

Then I register all sub-routes in my routes entry-point file:

routes/index.ts

import { honoApp } from '@/lib/hono.js';
export const routes = honoApp();
routes.route('/users', usersRoutes);
routes.route('/other', otherRoutes);

which is then used in my app module:

app.ts

import { routes } from '@/routes/index.js';
import { honoApp } from '@/lib/hono.js';
const app = honoApp();
app.route('/api', routes);

Is that how you would go about having a unified defaultHook while being able to split routes into dedicated files or can you think of a better approach?

I think it'd be nice if we could put the defaultHook somewhere higher up on the route chain and have all sub-routes just fall back to that hook instead of having to set the defaultHook on every sub-route explicitly. What do you think?

Sorry for the lengthy post and thank you for always being very responsive and helpful! 🙏

[KamaniBhavin]

Does this setup support AWS API Gateway and Lambda integration?

Here's what I've implemented:

const send = new OpenAPIHono<{ Bindings: Bindings }>({
  defaultHook: (result, c) => {
    if (result.success) {
      return;
    }

    return c.json({ success: false, errors: result.error.errors }, { status: 400 });
  },
});

However, it's not functioning as expected; instead, it throws an error. Unfortunately, this error isn't being propagated to the API consumer. They only receive a generic "Internal Server Error" message. Upon inspecting the CloudWatch logs, I found the following error:

2024-03-06T09:28:01.778+05:30
2024-03-06T03:58:01.778Z	93458f37-3cf7-474b-99ba-e11fbfaf2b1b	ERROR	ZodError: [
  
{
    "code": "invalid_type",
    "expected": "string",
    "received": "undefined",
    "path": [
        "phone"
    ],
    "message": "Required"
}

]
    at get error [as error] (/var/task/index.js:23748:23)
    at _ZodObject.parse (/var/task/index.js:23847:18)
    at /var/task/index.js:27228:43
    at process.processTicksAndRejections (node:internal/process/task_queues:95:5)
    at async dispatch (/var/task/index.js:22027:17)
    at async /var/task/index.js:22664:25
    at async Runtime.handler (/var/task/index.js:21439:17) {
  issues: [
    {
      code: 'invalid_type',
      expected: 'string',
      received: 'undefined',
      path: [Array],
      message: 'Required'
    }
  ],
  addIssue: [Function (anonymous)],
  addIssues: [Function (anonymous)],
  errors: [
    {
      code: 'invalid_type',
      expected: 'string',
      received: 'undefined',
      path: [Array],
      message: 'Required'
    }
  ]
}

My goal is to have this error message propagate to the end user or API consumer so they can understand what's going wrong.