@hono/zod-openapi: Global error handler

[soulchild]

I just started playing around with Hono for the purpose of building a RESTful API with its zod-openapi middleware and so far I'm really digging it. 👍

One thing I was unable to accomplish, was having my routes split into dedicated modules/routers AND override the default validation handler in one place (my top-level application module) as suggested here:

hello-world.ts

export const helloWorldRouter = new OpenAPIHono();

// ... schema definitions go here …

helloWorldRouter.openapi(route, (c) => {
  const { what } = c.req.valid('param');
  return c.json({
    hello: what,
  });
});

app.ts

import { helloWorldRouter } from './routes/hello-world';

const app = new OpenAPIHono({
  defaultHook: (result, c) => {
    if (result.success) {
      return;
    }
    return c.json(
      {
        message: 'Validation failed',
        cause: formatZodErrors(result),
      },
      422
    );
  }
});

app.route('/hello', helloWorldRouter);

The defaultHook only triggers for routes directly applied to the app instance and not for the routes defined in helloWorldRouter.

I could put the defaultHook in a dedicated function and use it in all my route modules when instantiating the OpenAPIHono() class, but IMHO that's too easy to forget and could lead to inconsistent error handling. A better alternative would be creating my own custom class which wraps OpenAPIHono and always injects the defaultHook, but before I do that I wanted to ask whether the current behavior is intentional or maybe a bug?

Thank you for creating Hono and any hints which might help me solve the issue.

[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.

[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! 🙏

[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!

[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.