Split WASM PAL into focused files - #54
Conversation
|
Warning Review limit reached
Next review available in: 27 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. 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: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (6)
📝 WalkthroughWalkthroughThe WASM PAL implementation now stores its HTTP client and filesystem wrapper in separate files. The HTTP client uses Fetch and AbortController. The filesystem wrapper provides synchronized bridged file operations. ChangesWASM PAL refactor
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
🚥 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 |
8394d70 to
f6801be
Compare
f6801be to
bef7ac0
Compare
|
@coderabbitai full review |
✅ Action performedFull review finished. |
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 `@packages/ballerina-wasm/http_client_wasm.go`:
- Around line 207-219: Update extractBody to avoid Response.arrayBuffer() and
consume Response.body incrementally through a reader, tracking accumulated bytes
and stopping immediately with the configured size error when limit is exceeded.
Preserve the existing response-body error handling and return the collected
bytes only after the full stream is read within the limit.
🪄 Autofix
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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 36714d62-ddb0-4abb-bb16-4cae3aa142dd
📒 Files selected for processing (3)
packages/ballerina-wasm/http_client_wasm.gopackages/ballerina-wasm/pal_fs_wasm.gopackages/ballerina-wasm/pal_wasm.go
💤 Files with no reviewable changes (1)
- packages/ballerina-wasm/pal_wasm.go
bef7ac0 to
3e789ec
Compare
|
@coderabbitai full review |
✅ Action performedFull review finished. |
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 `@packages/ballerina-wasm/http_client_wasm.go`:
- Around line 201-202: Update the response-size limit handling in the HTTP
client flow to cancel the response body’s ReadableStream before returning the
configured size-limit error, including both affected return paths. Preserve the
size-limit error regardless of whether stream cancellation succeeds, and ensure
cancellation is performed through the existing body reader before returning.
🪄 Autofix
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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 17d34874-74e5-434b-8fec-78ce1c2e672f
📒 Files selected for processing (3)
packages/ballerina-wasm/http_client_wasm.gopackages/ballerina-wasm/pal_fs_wasm.gopackages/ballerina-wasm/pal_wasm.go
💤 Files with no reviewable changes (1)
- packages/ballerina-wasm/pal_wasm.go
0de55ac to
5f0f428
Compare
|
@coderabbitai full review |
✅ Action performedFull review finished. |
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 `@packages/ballerina-wasm/http_client_wasm.go`:
- Around line 120-132: Update buildHeaders to return (js.Value, error),
capturing errors from each Headers.set/append call—including the Content-Type
assignment—instead of allowing js.Value.Call panics to escape. Propagate the
returned error through buildFetchOptions and its caller Execute, preserving
successful header construction while returning failures for invalid header names
or values.
🪄 Autofix
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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: e515c4ce-f251-4421-b653-0fec3bf36176
📒 Files selected for processing (3)
packages/ballerina-wasm/http_client_wasm.gopackages/ballerina-wasm/pal_fs_wasm.gopackages/ballerina-wasm/pal_wasm.go
💤 Files with no reviewable changes (1)
- packages/ballerina-wasm/pal_wasm.go
5f0f428 to
5973217
Compare
|
@coderabbitai full review |
✅ Action performedFull review finished. |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@packages/ballerina-wasm/http_client_wasm.go`:
- Around line 115-120: Update fetchHTTPClient.getRedirectMode so disabling
FollowRedirects preserves raw 3xx status and headers, especially Location,
instead of using Fetch’s opaque "manual" mode; use a metadata-preserving
strategy or explicitly reject this configuration on WASM.
In `@packages/ballerina-wasm/pal_fs_wasm.go`:
- Around line 25-28: Update the existing-file branch in the directory creation
logic around fs.Stat to return a *fs.PathError using syscall.ENOTDIR, or the
equivalent bridge-defined not-directory error, instead of fs.ErrInvalid.
Preserve the current path and operation values while retaining the existing
behavior for directories and stat errors.
- Around line 16-20: Update the path contract around palFS.resolvePath and
bridgeFS so absolute run paths are either rejected before reaching the proxy or
the proxy/FS contract explicitly normalizes and supports absolute paths. Ensure
relative-name implementations such as MemFS receive relative paths, while
preserving compatible behavior for NodeFS.
🪄 Autofix
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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 818f1379-07ea-46b0-a980-8da958b53116
📒 Files selected for processing (3)
packages/ballerina-wasm/http_client_wasm.gopackages/ballerina-wasm/pal_fs_wasm.gopackages/ballerina-wasm/pal_wasm.go
💤 Files with no reviewable changes (1)
- packages/ballerina-wasm/pal_wasm.go
5973217 to
c136d7c
Compare
c136d7c to
3f0f57a
Compare
Resolved #51
Summary by CodeRabbit