-
-
Notifications
You must be signed in to change notification settings - Fork 490
fix HttpServerRequest.toWeb missing duplex option for streaming bodies #5899
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
Conversation
🦋 Changeset detectedLatest commit: 127167a The changes in this PR will be included in the next version bump. This PR includes changesets to release 31 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
@copilot review this you sweet robot friend. |
|
@kitlangton I've opened a new pull request, #5900, to work on those changes. Once the pull request is ready, I'll request review from you. |
|
@copilot ... You opened a new pull request instead of reviewing this one. You have failed me today, brother. |
|
@kitlangton I've opened a new pull request, #5901, to work on those changes. Once the pull request is ready, I'll request review from you. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR fixes a bug in HttpServerRequest.toWeb where Node.js throws a TypeError about a missing duplex option when converting HTTP requests with streaming bodies. The fix adds the required duplex: "half" option when constructing a Request object with a body stream.
- Added
duplex: "half"option to Request constructor when body is present - Added test coverage for POST requests with JSON bodies
- Refactored code to extract body variable for clarity
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| packages/platform/src/HttpServerRequest.ts | Added duplex option to Request constructor and refactored body handling for clarity |
| packages/platform-node/test/HttpServer.test.ts | Added test case to verify POST requests with bodies can be converted to Web Request objects |
| .changeset/tall-buses-sing.md | Added changeset documenting the bug fix |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| body, | ||
| headers: self.headers, | ||
| // @ts-expect-error - duplex is required for streaming bodies in Node 18+ | ||
| duplex: body !== undefined ? "half" : undefined |
Copilot
AI
Dec 12, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider using the spread operator to conditionally include the duplex property only when body is defined, rather than setting it to undefined. This approach is more idiomatic and avoids passing undefined values to the Request constructor. For example: ...(body !== undefined ? { duplex: "half" as const } : {})
| duplex: body !== undefined ? "half" : undefined | |
| ...(body !== undefined ? { duplex: "half" as const } : {}) |
Type
Description
Fixes
HttpServerRequest.toWebthrowingTypeError: RequestInit: duplex option is required when sending a bodyin Node.js when converting requests with streaming bodies (POST, PUT, PATCH).The Bug
When calling
HttpServerRequest.toWeb()on a Node.js server request with a body, it throws:The Fix
Add
duplex: "half"option when constructing aRequestwith a streaming body:Why it only affects Node.js
The code has an early return for when
sourceis already aRequest:In Bun/Deno/web contexts, it returns the native Request directly. The bug only manifests in Node.js where we construct a new Request from an
IncomingMessage.Test
Added test case that converts a POST request with JSON body via
toWeb()and reads it back.