Add support for HTTP listeners - #52
Conversation
|
Warning Review limit reached
Next review available in: 57 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 (16)
📝 WalkthroughWalkthroughThe change adds Node-based HTTP listener support across the Ballerina runtime, WASM bridge, and public API. It registers listeners, dispatches requests asynchronously, converts headers and bodies, supports lifecycle operations, and adds end-to-end tests. ChangesHTTP listener integration
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant NodeHTTPTransport
participant WasmBridge
participant BallerinaWASM
Client->>NodeHTTPTransport: Send HTTP request
NodeHTTPTransport->>WasmBridge: Dispatch normalized request
WasmBridge->>BallerinaWASM: Call dispatchHttpRequest
BallerinaWASM-->>WasmBridge: Return status, headers, and body
WasmBridge-->>NodeHTTPTransport: Return HTTP response
NodeHTTPTransport-->>Client: Write HTTP response
Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches 💡 1📝 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 |
4b37c84 to
99ebc27
Compare
99ebc27 to
c9d2791
Compare
c9d2791 to
cf40dd4
Compare
cf40dd4 to
af55274
Compare
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 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/ballerina`:
- Line 1: Update the tracked ballerina submodule pointer in the parent
repository to commit 1858993886be3200dc9a17169da4526320f02a9a, ensure that
commit is available in the submodule repository, and push the corrected pointer
before merging.
In `@packages/balrun/src/http-listener.ts`:
- Around line 95-104: The listener record in
packages/balrun/src/http-listener.ts lines 95-104 must remain registered until
close(listener.server) resolves; move listeners.delete after the awaited close,
while preserving immediate socket destruction. In
packages/ballerina-wasm/listeners_wasm.go lines 27-31, return the transport
close error before calling activeRunContext.unregisterHandler so handler state
remains on failure; do not add an explicit closing state unless required by the
existing dispatch behavior.
- Around line 225-235: Update readBody to enforce a finite listener request-body
limit while reading, stopping accumulation as soon as the limit is exceeded and
signaling that condition to the HTTP handler. In the surrounding request
handling flow, translate that signal into HTTP 413, while preserving normal body
parsing for requests within the limit.
In `@packages/balrun/src/wasm-bridge.ts`:
- Around line 67-74: Update WasmBridge.run to prevent concurrent executions from
sharing or overwriting the instance-level onListenerReady callback: reject a new
invocation while a run is active, preserving the existing empty-path validation
and cleanup behavior, and ensure the active-run state is reset when the promise
settles.
In `@packages/balrun/tests/fixtures/http-listener.bal`:
- Around line 17-20: Update the inspect resource function to read the name query
parameter using request.getQueryParamValue("name") instead of request.rawPath,
and include that value in the response while preserving the existing header
behavior. Update the corresponding shared expected response in the wasm bridge
test to match the query-based output.
🪄 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: 23d60d1b-7d38-4792-ab6b-154bb5e41c25
📒 Files selected for processing (16)
packages/ballerina-wasm/ballerinapackages/ballerina-wasm/diagnostic_printer.gopackages/ballerina-wasm/http_dispatch_wasm.gopackages/ballerina-wasm/listeners_wasm.gopackages/ballerina-wasm/main_wasm.gopackages/ballerina-wasm/pal_wasm.gopackages/ballerina-wasm/run_context_wasm.gopackages/balrun/src/ballerina-core.tspackages/balrun/src/ballerina.tspackages/balrun/src/http-listener.tspackages/balrun/src/index.tspackages/balrun/src/wasm-bridge.tspackages/balrun/src/wasm.d.tspackages/balrun/tests/ballerina.test.tspackages/balrun/tests/fixtures/http-listener.balpackages/balrun/tests/wasm-bridge.test.ts
6ddaa92 to
ee97870
Compare
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/balrun/tests/wasm-bridge.test.ts`:
- Around line 91-93: Update the rejection assertion for isolatedBridge.run in
the relevant test to await the promise returned by
expect(...).rejects.toThrow(), ensuring the assertion completes within the test
scope and preserves the expected error message.
🪄 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: 31fcfa70-435c-481e-bd74-b496e089237b
📒 Files selected for processing (5)
packages/ballerina-wasm/listeners_wasm.gopackages/balrun/src/http-listener.tspackages/balrun/src/wasm-bridge.tspackages/balrun/tests/fixtures/http-listener.balpackages/balrun/tests/wasm-bridge.test.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/balrun/tests/fixtures/http-listener.bal
- packages/ballerina-wasm/listeners_wasm.go
Resolves #38
Summary by CodeRabbit