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

hono.request method does not work in combination with hono/body-limit #2627

Closed
qoomon opened this issue May 6, 2024 · 5 comments · Fixed by #2837
Closed

hono.request method does not work in combination with hono/body-limit #2627

qoomon opened this issue May 6, 2024 · 5 comments · Fixed by #2837
Labels

Comments

@qoomon
Copy link

qoomon commented May 6, 2024

What version of Hono are you using?

4.3.2

What runtime/platform is your app running on?

Node

What steps can reproduce the bug?

import {Hono} from 'hono'
import {bodyLimit} from 'hono/body-limit'

const app = new Hono()
app.use(bodyLimit({maxSize: 100 * 1024})) // 100kb

app.post('/hello', async (context) => {
  return context.json({message: 'Hello, World!'})
})

const response = await app.request('/hello', {
  method: 'POST',
  body: JSON.stringify({foo: 'bar'}),
})
console.log('response:', response.status)

What is the expected behavior?

response: 200

What do you see instead?

TypeError: RequestInit: duplex option is required when sending a body.
    at new Request (node:internal/deps/undici/undici:5082:19)
    at bodyLimit2 (file:///.../node_modules/hono/dist/middleware/body-limit/index.js:48:17)
    at dispatch (file:///.../node_modules/hono/dist/compose.js:29:23)
    at file:///.../node_modules/hono/dist/compose.js:6:12
    at file:///.../node_modules/hono/dist/hono-base.js:188:31
    at Hono.dispatch (file:///.../node_modules/hono/dist/hono-base.js:198:7)
    at Hono.fetch (file:///.../node_modules/hono/dist/hono-base.js:201:17)
    at Hono.request (file:///.../node_modules/hono/dist/hono-base.js:213:17)
    at <anonymous> (/.../sandbox.ts:12:28)
    at ModuleJob.run (node:internal/modules/esm/module_job:235:25)
response: 500

Additional information

Same app set up is working without any error when started as a node server and fetch response, see example below

import {Hono} from 'hono'
import {bodyLimit} from 'hono/body-limit'
import {serve} from '@hono/node-server'

const app = new Hono()
app.use(bodyLimit({maxSize: 1024 * 1024})) // 1mb

app.post('/hello', async (context) => {
  return context.json({message: 'Hello, World!'})
})

// const response = await app.request('/hello', {
//   method: 'POST',
//   body: JSON.stringify({foo: 'bar'}),
// })
// console.log('response:', response.status)

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

const response = await fetch('http://localhost:3000/hello', {
  method: 'POST',
  body: JSON.stringify({foo: 'bar'}),
})
console.log('response:', response.status)
@qoomon qoomon added the bug label May 6, 2024
@qoomon
Copy link
Author

qoomon commented May 6, 2024

I could fix the error, if I do the same as in body-limit test file by prefix my test files with following workaround, see below.

I don't understand why it works. do you have any clue why it's needed for app.request?
Can this fix be done within app.request?

const GlobalRequest = globalThis.Request
globalThis.Request = class Request extends GlobalRequest {
constructor(input: Request | string, init: RequestInit) {
if (init) {
// eslint-disable-next-line @typescript-eslint/no-explicit-any
;(init as any).duplex ??= 'half'
}
super(input, init)
}
} as typeof GlobalRequest

@mgrithm
Copy link
Contributor

mgrithm commented May 7, 2024

@qoomon, if you want, I can work on it later

@qoomon
Copy link
Author

qoomon commented May 8, 2024

That would be awesome, thank you

@fzn0x
Copy link
Contributor

fzn0x commented May 28, 2024

This is issue from nodejs, I don't know which part of Hono requires this, I will also take a look on this

@fzn0x
Copy link
Contributor

fzn0x commented May 28, 2024

Looks like we need to add the duplex option in `body: Readable stream

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