fix(node-fetch/http2): handle Http2 streams correctly #2093
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
🚀 Snapshot Release (
|
| Package | Version | Info |
|---|---|---|
@whatwg-node/fetch |
0.10.5-alpha-20250222121046-869be2474b8eedcb461dd935cb99971cc0bdab21 |
npm ↗︎ unpkg ↗︎ |
@whatwg-node/node-fetch |
0.7.11-alpha-20250222121046-869be2474b8eedcb461dd935cb99971cc0bdab21 |
npm ↗︎ unpkg ↗︎ |
@whatwg-node/server |
0.9.69-alpha-20250222121046-869be2474b8eedcb461dd935cb99971cc0bdab21 |
npm ↗︎ unpkg ↗︎ |
Contributor
✅
|
Contributor
✅
|
Contributor
✅
|
Contributor
✅
|
Contributor
✅
|
Contributor
WalkthroughThis pull request applies patches to address a specific HTTP/2 error, "TypeError: bodyInit.stream is not a function," occurring during the parsing of incoming HTTP/2 requests. It refines the verification logic for Blob objects in the node-fetch package and updates the HTTP/2 test suite to use asynchronous request handling with structured JSON responses. No public interface changes were made. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Server
participant Handler
Client->>Server: Send HTTP/2 request (includes JSON payload)
Server->>Handler: Forward request asynchronously
Handler->>Handler: Parse request body as JSON
alt Successful parse
Handler->>Server: Return structured JSON response (body, headers, method, URL)
else Parsing error
Handler->>Server: Return error response
end
Server->>Client: Deliver JSON response
Possibly related PRs
Poem
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/server/test/http2.spec.ts (1)
94-111: Consider adding error case tests.While the happy path is well tested, consider adding test cases for error scenarios such as invalid JSON or stream errors to ensure robust error handling.
expect(receivedNodeRequest).toMatchObject({ data: { body: { hello: 'world', }, method: 'POST', url: expect.stringMatching(/^http:\/\/localhost:\d+\/hi$/), headers: { 'content-type': 'application/json', }, }, headers: { ':status': 418, 'content-type': 'text/plain;charset=UTF-8', 'x-is-this-http2': 'yes', }, status: 418, }); + +it('should handle invalid JSON gracefully', async () => { + const handleRequest = jest.fn(async (request: Request) => { + try { + await request.json(); + } catch (error) { + return new Response('Invalid JSON', { status: 400 }); + } + return new Response('OK'); + }); + // ... rest of the test setup +});
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.changeset/mighty-worlds-smoke.md(1 hunks)packages/node-fetch/src/Body.ts(1 hunks)packages/server/test/http2.spec.ts(4 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: unit / node 18
🔇 Additional comments (4)
packages/server/test/http2.spec.ts (2)
30-46: LGTM! Enhanced test coverage for HTTP/2 request handling.The async request handler now properly tests JSON parsing and response generation, which helps validate the fix for the
bodyInit.streamTypeError.
61-65: LGTM! Proper content-type header setup.Setting the content-type to 'application/json' and writing JSON data ensures the request is properly formatted for testing the stream handling.
packages/node-fetch/src/Body.ts (1)
511-511:✅ Verification successful
LGTM! Robust type checking for Blob objects.
The enhanced type check ensures that
streamis actually a function before treating the object as a Blob, effectively preventing theTypeError: bodyInit.stream is not a functionerror.Let's verify this fixes all potential Blob-related issues:
🏁 Script executed:
#!/bin/bash # Search for any other Blob-related type checks or stream usage rg -l "stream.*function|instanceof.*Blob|isBlob"Length of output: 189
Blob type checking update verified and approved.
The change in
packages/node-fetch/src/Body.tsnow properly ensures that thestreamproperty is not only present but also a function, which effectively prevents theTypeError: bodyInit.stream is not a functionerror. A repository-wide search shows that similar Blob-related type checks and stream usages are present in:
packages/node-fetch/src/FormData.tspackages/node-fetch/src/Blob.tspackages/node-fetch/tests/Blob.spec.tsThis consistency across the codebase confirms that the updated check in
Body.tsaligns with the overall Blob handling strategy..changeset/mighty-worlds-smoke.md (1)
1-7: LGTM! Clear and informative changeset.The changeset properly documents the issue being fixed and its impact across multiple packages.
dotansimha
approved these changes
Feb 24, 2025
This was referenced Feb 28, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes graphql-hive/graphql-yoga#3803
Fixes the bug;
TypeError: bodyInit.stream is not a functionerror thrown when@whatwg-node/serveris used withnode:http2and attempts the incoming HTTP/2 request to parse withRequest.json,Request.text,Request.formData, orRequest.blobmethods.Summary by CodeRabbit
Bug Fixes
Tests