fix(response): allow status and headers staged during the first stream chunk#1512
Conversation
…m chunk Two related regressions surfaced by #1510: - `HTTPResponse.status` / `.statusText` defaulted to `200` / `"OK"`, so `res.status || preparedRes?.status` in `prepareResponse` always picked `200` and silently discarded a status staged on `event.res` by the handler or a middleware. Any util returning an `HTTPResponse` (`html()`, `iterable()`, ...) was affected. They now return `undefined` when unset, meaning "inherit". - `iterable()` built its `ReadableStream` synchronously, so the response was created before the producer ran. Pull the first chunk before constructing the `HTTPResponse` — the same window h3 v1 had — so status and headers set while producing it are still applied. `iterable()` now returns a promise. Co-Authored-By: Claude Opus 5 <[email protected]>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthrough
ChangesIterable response timing
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
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/status.test.ts`:
- Around line 84-97: Update the test around the “own status wins” handler to
stage a conflicting event.res.statusText value as well as event.res.status, then
assert the returned HTTPResponse’s statusText remains “teapot.” Ensure the
assertions cover both response-owned status and statusText so reversed fallback
precedence would fail.
🪄 Autofix (Beta)
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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 380312fe-cf51-4226-97d0-51d15c6d4cf0
📒 Files selected for processing (6)
docs/2.utils/2.response.mdsrc/_deprecated.tssrc/response.tssrc/utils/response.tstest/iterable.test.tstest/status.test.ts
Closes #1510 (partially — see caveat).
What I found
Reproduced the report on
main.iterable()(suggested in the issue thread) did not help either — two independent problems:1.
HTTPResponsesilently discardsevent.res.statusPre-existing and unrelated to streaming:
prepareResponsedoesres.status || preparedRes?.status, butresis theHTTPResponseand its getter wasthis.#init?.status || 200— neverundefined, so the staged status was dead code for every util returning anHTTPResponse(html(),iterable(), user-built ones). Same forstatusText.HTTPResponse.status/.statusTextnow returnundefinedwhen unset, meaning "inherit":event.res.statusis used, falling back to200.prepareResponseitself is unchanged — the|| preparedRes?.statusfallback finally does what it was written to do.2.
iterable()built its response before the producer ranThe
ReadableStreamwas constructed synchronously, sotoResponsehad already built theResponseby the timepullfirst ran.iterable()now awaits the first chunk before constructing theHTTPResponse— the same window v1 had, where headers went out on first write:Side effect: a throw before the first chunk now renders a proper error response instead of tearing a stream mid-flight.
Caveat
Returning a raw
ReadableStream(the exact snippet in #1510) still cannot support this — theResponsemust be constructed before anything reads the stream, and eagerly priming every returned stream would stall headers for SSE/long-poll bodies.iterable()is the supported way to keep a v1-style window; documented on the util.Breaking
HTTPResponse.statusisnumber | undefined(wasnumber, always ≥200);statusTextlikewise.iterable()returnsPromise<HTTPResponse>(as does the deprecatedsendIterable). Returning it from a handler is unaffected —toResponsealready awaits.Size
Bundle budgets unchanged and still green — this comes out ~11 bytes smaller than
main(dropping the|| 200/|| "OK"defaults pays for the fix).Tests added in
test/status.test.ts(staged status vs. own status) andtest/iterable.test.ts(status/headers before, during and after the first chunk; throw before the first chunk). Full suite + lint green.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Documentation