refactor(fetch): centralize response copy state#47
Conversation
|
Warning Review limit reached
More reviews will be available in 14 minutes and 23 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 (6)
📝 WalkthroughWalkthroughBoth ChangesResponse Metadata Consolidation into _ResponseSnapshot
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 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: bbda0a6748
ℹ️ 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)
lib/src/fetch/response.js.dart (1)
163-166: 🧹 Nitpick | 🔵 Trivial | 💤 Low valueConsider using
HttpStatus.isSuccessfor consistency withresponse.io.dart.The IO version uses
HttpStatus.isSuccess(snapshot.status)while this JS version uses an inline range check. Both are functionally equivalent for the standard 200-299 success range, but using the same helper improves maintainability.♻️ Optional: align with IO version
if (snapshot != null) { - return snapshot.status >= 200 && snapshot.status <= 299; + return HttpStatus.isSuccess(snapshot.status); }🤖 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 `@lib/src/fetch/response.js.dart` around lines 163 - 166, Replace the inline range check for HTTP success status in response.js.dart with the HttpStatus.isSuccess helper method for consistency with the IO version. Specifically, in the code block where snapshot.status is compared against the 200-299 range, change the condition to use HttpStatus.isSuccess(snapshot.status) instead of the inline range check snapshot.status >= 200 && snapshot.status <= 299. This ensures both the JS and IO implementations use the same helper method for determining successful HTTP responses.
🤖 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 `@lib/src/fetch/response.js.dart`:
- Around line 163-166: Replace the inline range check for HTTP success status in
response.js.dart with the HttpStatus.isSuccess helper method for consistency
with the IO version. Specifically, in the code block where snapshot.status is
compared against the 200-299 range, change the condition to use
HttpStatus.isSuccess(snapshot.status) instead of the inline range check
snapshot.status >= 200 && snapshot.status <= 299. This ensures both the JS and
IO implementations use the same helper method for determining successful HTTP
responses.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 34b07c1f-5c9a-41ff-a4b7-fe1b6448e56d
📒 Files selected for processing (2)
lib/src/fetch/response.io.dartlib/src/fetch/response.js.dart
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 143183ce06
ℹ️ 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: 0cf98bed0c
ℹ️ 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".
| final WebResponseHost host => js_headers.headersFromHost( | ||
| host.value.headers, | ||
| ), |
There was a problem hiding this comment.
For WebResponseHost values that are direct wrappers around browser-owned responses, such as Response(await window.fetch(...)), Response.error(), or Response.redirect(...), host.value.headers can have the Fetch API's immutable guard. Returning a wrapper around that host means response.headers.set(...) or delete(...) now throws a DOM TypeError; before this getter copied the host headers into a mutable Headers, matching the package's native response behavior. The host-bound wrapper should be limited to copied/constructed web responses whose parsing methods need to observe later header mutations, rather than all raw web hosts.
Useful? React with 👍 / 👎.
1 participant
Summary
_ResponseSnapshotResponseconstruction path while passing a snapshot instead of scattered overlay parametersFollow-up to #46.
Validation
dart analyzedart 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 expandeddart test --reporter expandedSummary by CodeRabbit