fix(fetch): copy wrapped response inputs#46
Conversation
|
Warning Review limit reached
More reviews will be available in 37 minutes and 29 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?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 credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan refill rate. 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, the refill rate gradually slows as usage increases. The highest same-day bursts are limited more strictly. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughFixes ChangesResponse copy/clone semantics
Sequence DiagramsequenceDiagram
participant Caller
participant Factory as Response factory
participant Helpers as Conversion helpers
participant Host as ResponseHost
Caller->>Factory: Response(upstream, init)
alt init is null
Factory->>Host: clone source host
else init provided
Factory->>Helpers: _nativeResponseFromWrappedResponse(upstream, init)
Helpers->>Helpers: extract status/statusText/headers from init or upstream
Helpers->>Helpers: conditionally delete content-type
Helpers->>Host: create fresh host with overridden values
end
Host-->>Factory: new host
Factory-->>Caller: Response with cloned/converted host
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8af940213b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 21b3f7bbad
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 49e8c10354
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
test/response_js_test.dart (1)
131-157: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick winConsider verifying upstream is still readable after consuming the copy.
The test verifies
upstream.bodyUsedremainsfalseafter consuming the wrapped response (line 156), but unlike the equivalent tests for wrapped responses (lines 99) and native responses (lines 128), it doesn't actually read the upstream to confirm it's still readable. Addingexpect(await upstream.text(), 'web body');after line 156 would provide stronger evidence that the body is truly independent.🧪 Optional test enhancement
expect(upstream.bodyUsed, isFalse); expect(await response.text(), 'web body'); expect(upstream.bodyUsed, isFalse); + expect(await upstream.text(), 'web body'); });🤖 Prompt for 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. In `@test/response_js_test.dart` around lines 131 - 157, The test 'applies init overrides when copying web responses' verifies that upstream.bodyUsed remains false after consuming the wrapped response, but it does not confirm that the upstream response is still readable. Following the pattern used in the equivalent tests for wrapped responses and native responses, add an assertion after the final upstream.bodyUsed check to verify that upstream.text() can still be called and returns the original body content 'web body', ensuring the body is truly independent.
🤖 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.
Nitpick comments:
In `@test/response_js_test.dart`:
- Around line 131-157: The test 'applies init overrides when copying web
responses' verifies that upstream.bodyUsed remains false after consuming the
wrapped response, but it does not confirm that the upstream response is still
readable. Following the pattern used in the equivalent tests for wrapped
responses and native responses, add an assertion after the final
upstream.bodyUsed check to verify that upstream.text() can still be called and
returns the original body content 'web body', ensuring the body is truly
independent.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: be13088c-2052-4724-95cc-1f5ea381df94
📒 Files selected for processing (4)
lib/src/fetch/response.io.dartlib/src/fetch/response.js.darttest/response_io_test.darttest/response_js_test.dart
|
Accepted the CodeRabbit test enhancement and added the web.Response upstream readability assertion after consuming the copied response. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f99992ab9b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 025ce1e328
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 46f28429a2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: adea8f24f1
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
1 participant
Summary
Responseinputs as copy/clone semantics instead of aliasing the source host.ResponseInitstatus, statusText, and headers when copying wrappedResponse, nativeResponse, and JSweb.Responseinputs.clone().Root Cause
The IO and JS wrapper constructors matched
Responseinputs before normal construction and reused the wrapped host directly. That meantResponse(source, init)ignored every init override, shared mutable body state withsource, and could silently construct a response whose source body had already been consumed.Validation
dart analyzedart test --reporter expandeddart test test/response_io_test.dart --reporter expandeddart test -p chrome test/response_js_test.dart --reporter expandeddart test -p chrome test/request_js_test.dart test/response_js_test.dart test/headers_js_test.dart test/url_search_params_js_test.dart test/_internal/web_fetch_utils_test.dart --reporter expandedCloses #41
Refs #36
Summary by CodeRabbit
Bug Fixes
status,statusText, andheaders, including correct application of provided overrides.bodyUsedchanges.content-typeduring copy operations.Tests