feat(fetch): implement Response.ok/statusText and Body.text/bytes/arrayBuffer - #271
feat(fetch): implement Response.ok/statusText and Body.text/bytes/arrayBuffer#271Sonic-Y3k wants to merge 2 commits into
Conversation
|
Warning Review limit reached
More reviews will be available in 53 minutes and 31 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 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 see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (5)
WalkthroughThis PR extends the fetch API implementation by adding Response convenience methods and Body content readers. The core work introduces Response.Ok() (status range check) and Response.StatusText() (reason phrase extraction) to internal/fetch/fetch.go, along with unit tests. The code generator configuration is updated to treat these as custom implementations. The scripting layer then exposes these methods as JavaScript-callable bindings (Response_ok, Response_statusText, Body_text, Body_bytes, Body_arrayBuffer), and integration tests validate the complete behavior across success and error scenarios. Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
scripting/internal/fetch/body.go (1)
11-70: 🧹 Nitpick | 🔵 Trivial | 💤 Low valueConsider extracting common Body consumer pattern.
All four Body consumer methods (
Body_json,Body_text,Body_bytes,Body_arrayBuffer) repeat the same pattern:instance, err := js.As[fetch.Body](cbCtx.Instance()) if err != nil { return nil, err } return codec.EncodePromise(cbCtx, promise.ReadAll(instance), encoder)A helper function could eliminate this duplication and improve maintainability:
func encodeBodyPromise[T any]( cbCtx js.CallbackContext[T], encoder codec.Encoder[T, []byte], ) (js.Value[T], error) { instance, err := js.As[fetch.Body](cbCtx.Instance()) if err != nil { return nil, err } return codec.EncodePromise(cbCtx, promise.ReadAll(instance), encoder) }Then each Body method becomes a one-liner:
return encodeBodyPromise(cbCtx, encoderFunc).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: dd1f5527-2b0a-43be-8c21-56830306707c
📒 Files selected for processing (8)
internal/code-gen/scripting/configuration/fetch_configuration.gointernal/fetch/fetch.gointernal/fetch/response_test.goscripting/internal/fetch/body.goscripting/internal/fetch/body_generated.goscripting/internal/fetch/response.goscripting/internal/fetch/response_generated.goscripting/internal/scripttests/fetch_suite.go
💤 Files with no reviewable changes (2)
- scripting/internal/fetch/response_generated.go
- scripting/internal/fetch/body_generated.go
…ayBuffer Response.ok (status in 200-299), Response.statusText, and the Body.text(), Body.bytes() and Body.arrayBuffer() consumers were previously unimplemented and threw "Not implemented". Implemented via the code generator (custom implementations) with unit tests and a scripttests "Response surface" case.
4f09dcb to
acb4c6c
Compare
|
Follow-up in d14cd8a:
No generated files changed (codegen is still idempotent), and the suite passes on both the |
…helper Add doc comments to the Response/Body script bindings, their tests, and the code-gen Fetch configuration so the docstring-coverage check passes, and extract a shared encodeBodyPromise helper to remove the duplicated js.As/EncodePromise pattern across the json/text/bytes/arrayBuffer consumers.
d14cd8a to
5a92f33
Compare
Response.ok,Response.statusText, and theBody.text(),Body.bytes()andBody.arrayBuffer()consumers were unimplemented and threw"Not implemented". (Response.ok's error message invited an issue/PR.)Response.Ok()(status in 200–299) andResponse.StatusText()added to the Gofetch.Responsetype.Body.text/bytes/arrayBufferimplemented as custom implementations.fetch_configuration.go); the regenerated files are committed and the codegen is idempotent.Independent of #270 — both touch
internal/fetchbut in separate regions/files, so they merge in any order.Testing:
TestResponseOk/TestResponseStatusTextplus a scripttests "Response surface" case (ok/status/statusText/text/json/bytes/arrayBuffer + 404).AI disclosure: This change was developed with the help of an AI coding assistant. I've reviewed and tested it myself; it follows the existing conventions and the full test suite (main module,
v8engine,sobekengine) passes locally.