fix(node): keep req.headers live and consistent across _request materialization#265
Conversation
…ialization Once the native Request materialized, `get headers()` switched to `#request.headers`, detaching any previously-taken reference: mutations through the old reference were silently lost. Instead of swapping which object the getter returns, swap the wrapper's backing store: after materialization `NodeRequestHeaders` adopts the native request's Headers and becomes a pure facade over that single mutable store. Early references stay live, `req.headers` identity is stable, and mutations through either view are visible in both `req.headers` and `_request.headers` / `clone()` / `formData()`. Resolves #245 Co-Authored-By: Claude Fable 5 <[email protected]>
|
Warning Review limit reached
Next review available in: 23 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
✨ Finishing Touches🧪 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 |
commit: |
Resolves #245
Problem
On the Node adapter, once the native (undici)
Requestmaterializes — viareq._request,clone(),formData()/blob()/bytes()/arrayBuffer(), or any lazily-inherited native property —get headers()switched from theNodeRequestHeaderswrapper to#request.headers. A reference taken earlier became detached:req.headers !== h, and mutations throughhwere silently lost. The approach tried in #243 (keep the wrapper canonical forever) fixed identity but froze_request.headers/clone()at a materialization-time snapshot, so it was reverted (1754b02).Fix
Instead of swapping which object the getter returns, swap the wrapper's backing store.
NodeRequestHeadersalready fronts a lazily-built nativeHeaders; at materialization it now_adopts the native request'sHeadersand becomes a pure facade over it. From then on there is a single mutable header store, so with no dual-write bookkeeping:req.headersidentity is stable forever,req.headersare visible in_request.headers,clone(), andformData()'s content-type sniff,_request.headersare visible throughreq.headers(which one-way write-through could never give).The untouched-headers hot path is unchanged; requests whose headers were accessed pay one extra branch + delegated call per header op after materialization.
Semantics note
After materialization the single store is the native request's
Headers, which carries the fetch spec's"request"guard: forbidden-header mutations (e.g.set("host", …)) are silently filtered where the pre-materialization wrapper allowed them. This moves toward spec behavior and only applies once a native view exists.Tests
headers reference stays live across _request materializationtest from fix(node): Node adapter correctness fixes (statusText, empty body, HEAD streaming, send errors, sync bridge) #243,_request.headersandclone(), in both directions,formData()honors acontent-typeset throughreq.headersbefore materialization.Full suite: 1041 passed, lint + typecheck clean.
🤖 Generated with Claude Code