Skip to content

fix(request): forward duplex to the native request - #409

Open
Ahmad-Mosha wants to merge 1 commit into
honojs:mainfrom
Ahmad-Mosha:fix/408-duplex
Open

Ahmad-Mosha wants to merge 1 commit into
honojs:mainfrom
Ahmad-Mosha:fix/408-duplex

Conversation

@Ahmad-Mosha

Copy link
Copy Markdown

Fixes #408

On Node.js 26.11, passing the lightweight request as RequestInit throws TypeError [ERR_INVALID_THIS]: Illegal invocation:

new Request(new URL('/rewritten', c.req.url), c.req.raw)

Hono's own app.mount() does the same thing (new Request(url, request)), so mounted apps return 500 on 26.11.

Node.js 26.11.0 updated undici to 8.11.2, which includes nodejs/undici#5792 (released in 8.11.0). That change replaced the prototype-based Request brand checks with private-field checks. The lightweight request inherits from Request.prototype but has no private state. Any getter it doesn't define falls through to the native one, and that getter now throws. duplex is the only RequestInit member that falls through. It is read in two places: by our constructor (options.duplex ??= 'half') and by undici's RequestInit conversion.

This adds duplex to the getters forwarded to the cached native request, the same way keepalive was added in #148. The native getter always returns 'half'. When the request is used as RequestInit, body is read before duplex, so the native request already exists and no extra work is done. A standalone duplex read now builds the native request, like the other forwarded getters. Hono itself never reads it.

Tests

  • Added a duplex assertion to the standard Request compatibility test. Added two tests that pass a lightweight request as the init option: a GET, and a POST with a body. The GET test covers undici's RequestInit conversion. The POST test covers duplex ??= 'half' in our constructor.
  • These tests fail on main with Node.js 26.11.0 and 26.11.1 and pass with this change. On Node.js 26.10 and earlier they pass either way, because the old brand check accepted the lightweight request. This means the Node versions in the CI matrix can't catch a regression here.
  • Full suite: 505/505 on Node 20.20.2, 22.23.2 and 24.21.0. On Node 26.10.0, 26.11.0 and 26.11.1, 4 tests fail both with and without this change (stream chunking and disconnect recovery in request.test.ts and server.test.ts). They are unrelated to this issue. format, lint, typecheck and build are clean.
  • Ran the reproduction from On Node 26.11, reading duplex on the lightweight request throws ERR_INVALID_THIS, so new Request(url, c.req.raw) fails #408 and an app.mount() request against a packed build: 200 on Node 20, 22, 24, 26.10, 26.11.0 and 26.11.1.

Since undici 8.11 (Node.js 26.11), Request getters use private-field brand
checks, so reading `duplex` on the lightweight request falls through to the
native getter and throws ERR_INVALID_THIS. This breaks passing the request
as RequestInit, e.g. `new Request(url, c.req.raw)` and `app.mount()`.

Fixes honojs#408
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 this pull request may close these issues.

On Node 26.11, reading duplex on the lightweight request throws ERR_INVALID_THIS, so new Request(url, c.req.raw) fails

1 participant