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

NotFound Handler + Compress does not return body #2061

Closed
sebastianwessel opened this issue Jan 22, 2024 · 6 comments · Fixed by #2080
Closed

NotFound Handler + Compress does not return body #2061

sebastianwessel opened this issue Jan 22, 2024 · 6 comments · Fixed by #2080
Labels

Comments

@sebastianwessel
Copy link

What version of Hono are you using?

3.12.6

What runtime/platform is your app running on?

Node

What steps can reproduce the bug?

import { serve } from '@hono/node-server'
import { Hono } from 'hono'
import { compress } from 'hono/compress'

const app = new Hono()
app.use('*', compress())
app.notFound((c) => c.json({ message: 'error message' }, 400))

serve({
  fetch: app.fetch,
  port: 3000,
})

What is the expected behavior?

Returns error body:

{ "message": "error message" }

What do you see instead?

No body content - browser default error page

Additional information

Might be an issue in hono + compress or maybe in @hono/node-server package, as there were similar issues in regular handlers in older versions

@ariskemper
Copy link
Contributor

ariskemper commented Jan 23, 2024

@sebastianwessel i tested with 3.12.6 and wrote this test, it works as expected, will check with @hono/node-server

describe('Not Found JSON', () => {
  const app = new Hono()
  app.use('*', compress())
  app.notFound((c) => {
    return c.json({ message: 'error message' }, 400)
  })

  it('Custom 404 Not Found', async () => {
    const res = await app.request('http://localhost/foo')
    expect(res.status).toBe(400)
    expect(await res.json()).toEqual({ message: 'error message' })
  })
})

@ariskemper
Copy link
Contributor

ariskemper commented Jan 23, 2024

@sebastianwessel issue was that compress + hono + hono/node-server were not working correctly, when middleware compress is used. I fixed this so when content-encoding is gzip or deflate it is compressing the body with specific encoding. Hono currently implements gzip and deflates. Tomorrow i will issue PR for this fix in hono/node-server.

@ariskemper
Copy link
Contributor

ariskemper commented Jan 25, 2024

@usualoma thanks for the solution, but there is another bug, when running with this test:

  const app = new Hono()

  app.use('*', compress())

  app.notFound((c) => {
    return c.json({ message: 'error message' }, 400)
  })

  it('custom error message 400 Not Found', async () => {
    const res = await app.request('http://localhost/err', {
      method: 'GET',
      headers: new Headers({ 'Accept-Encoding': 'gzip' }),
    })
    expect(res.status).toBe(400)
    expect(res.headers.get('Content-Encoding')).toEqual('gzip')
    expect(res.headers.get('Content-Length')).toBeNull()
    expect(await res.json()).toEqual({ message: 'error message' })
  })

test fails with SyntaxError: Unexpected token in JSON at position 0

@usualoma
Copy link
Member

Hi @ariskemper
Thank you for your comment.

I haven't actually tried it, but I think in this case you need to do the gunzip process yourself. I think you are getting an error because you are not doing that here.

expect(JSON.parse(zlib.gunzipSync(await res.text())))

@ariskemper
Copy link
Contributor

ariskemper commented Jan 25, 2024

@usualoma thanks for the hint, ok looks good to me, will bump hono to v3.12.7 in @hono/node-server and will add some extra tests in @hono/node-server with content decompression, what do you think?

@usualoma
Copy link
Member

@ariskemper Thank you. I am happy to have the test added.

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

Successfully merging a pull request may close this issue.

3 participants