Implement Early Hints to Bun.serve - #36934
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughThe PR adds ChangesHTTP Early Hints
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@coderabbitai help |
ChatThere are 3 ways to chat with CodeRabbit:
CodeRabbit commands
Other keywords and placeholders
Status, support, documentation and community
|
|
@coderabbitai full review |
✅ Action performedFull review finished. |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/runtime/http/server.mdx`:
- Around line 346-368: Add writeEarlyHints(request: Request, headers:
HeadersInit): boolean to the Server interface in the Reference section, matching
the documented method signature and boolean return contract; leave the
surrounding interface members unchanged.
In `@test/js/bun/http/serve.test.ts`:
- Around line 2230-2294: Add an HTTP/3-focused test alongside the existing
server.writeEarlyHints tests, configuring Bun.serve with HTTP/3 support and
making writeEarlyHints return false for the request. Capture the client response
and assert it contains the final response but no “103 Early Hints” status,
preserving the existing HTTP/1.1 coverage.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 32ea316f-7696-4071-85bd-a61aee2733fa
📒 Files selected for processing (7)
docs/runtime/http/server.mdxpackages/bun-types/serve.d.tssrc/runtime/server/AnyRequestContext.rssrc/runtime/server/RequestContext.rssrc/runtime/server/server.classes.tssrc/runtime/server/server_body.rstest/js/bun/http/serve.test.ts
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@test/js/bun/http/serve-http3.test.ts`:
- Line 230: Strengthen the HTTP/3 informational-response test around fetchH3 so
it inspects all parsed or raw response header blocks rather than only the final
headers returned by res.headers. Assert that no 103 response block is emitted
and no informational preload Link header appears, while preserving the existing
final-response assertion.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: abd6ba9d-b6ea-4de9-84dd-c49564d1b740
📒 Files selected for processing (2)
docs/runtime/http/server.mdxtest/js/bun/http/serve-http3.test.ts
5b7b75c to
1417bd0
Compare
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/runtime/server/RequestContext.rs`:
- Around line 4483-4501: Update RequestContext::write_informational to also
return false when self.flags.has_written_status() is true, while preserving the
existing response and HTTP/3 guards. Add a regression test that invokes
server.writeEarlyHints after a streaming response has flushed its status line,
then verifies it returns false and produces no HTTP 103 block.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: fc08f234-f2e8-44fb-9bc7-742108cf12fb
📒 Files selected for processing (9)
docs/runtime/http/server.mdxpackages/bun-types/serve.d.tssrc/runtime/server/AnyRequestContext.rssrc/runtime/server/RequestContext.rssrc/runtime/server/server.classes.tssrc/runtime/server/server_body.rstest/js/bun/http/fetch-h3.tstest/js/bun/http/serve-http3.test.tstest/js/bun/http/serve.test.ts
f9d1fff to
8bf8ed9
Compare
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
Document Server API and test HTTP/3 rejection.
8bf8ed9 to
8b6fc08
Compare
What does this PR do?
Implement to support 103 Early Hints with native Bun.serve. Currently node.js compatible
ServerResponse.writeEarlyHintsis available to use with Bun but it has performance penalty.Example of use cases:
How did you verify your code works?
We do pre-built bun locally and test with our sample code to prove a feature works, together with adding test cases to cover this cases.
should also solves #8690