Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

zod-validator should show a more useful error when JSON body is missing content-type header #841

Open
Soviut opened this issue Nov 20, 2024 · 4 comments · May be fixed by honojs/hono#3707

Comments

@Soviut
Copy link

Soviut commented Nov 20, 2024

I created a very simple zod-validator to use in my test suite.

import { zValidator } from '@hono/zod-validator'
import { z } from 'zod'

export const testBodyValidator = zValidator(
  'json',
  z.object({
    key: z.string().min(1),
  })
)

My route looked like

router.post('/', testBodyValidator, async (c) => {
  return c.json(c.req.valid('json'))
})

My test looked like this

describe('POST /testing', () => {
  it('should return the request body', async () => {
    const res = await app.request('/testing', {
      method: 'POST',
      body: JSON.stringify({ key: 'value' }),
    })
    expect(await res.json()).toStrictEqual({ key: 'value' })
  })
})

However, I kept getting errors about the key field being missing.

After hours of debugging I noticed one example had the content-type header set in the app.request()

      headers: new Headers({ 'Content-Type': 'application/json' }),

That turns out to be what the problem was. While I understand why it happened, it wasn't obvious that was the issue. It would be really useful if zValidator could return an error if the validation target is json but the content-type header is missing or does not contain application/json.

@sushichan044
Copy link
Contributor

sushichan044 commented Nov 27, 2024

@Soviut
Hello.
This behavior is due to the hono/validator implementation, not zValidator.
However, it may indeed be a good idea to print a warning.

@yusukebe
Copy link
Member

If you want to show an error message, you can add a code here:

https://github.com/honojs/hono/blob/main/src/validator/validator.ts#L74-L76

We should not make it throw an error, but it's better to show a message with console.log() or console.warn().

@sushichan044
Copy link
Contributor

That sounds good 👀

@Soviut
Copy link
Author

Soviut commented Nov 28, 2024

I've added the PR here honojs/hono#3707
and a docs update here honojs/website#540

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants