fix: chunk oversized extension WebSocket responses - #19
Conversation
Large HTML captures were sent as a single response frame and tripped the default 1 MiB WebSocket limit, disconnecting the extension mid-read. Frame big responses as response-chunk messages and fall back to Playwright when an older extension still disconnects during read. Co-authored-by: Cursor <cursoragent@cursor.com>
|
Warning Review limit reached
Next review available in: 33 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. 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 (5)
📝 WalkthroughWalkthroughThe extension now chunks oversized WebSocket responses, reassembles them in ChangesExtension response reliability
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Extension
participant WebSocket
participant ExtensionHub
participant ExtensionSession
participant BrowserService
participant Playwright
Extension->>WebSocket: Send response or response-chunk frames
WebSocket->>ExtensionHub: Deliver response-chunk messages
ExtensionHub->>ExtensionSession: append_response_chunk(payload)
ExtensionSession-->>BrowserService: Resolve reassembled response
BrowserService->>Playwright: Retry read on extension failure
Playwright-->>BrowserService: Return HTML
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/browser_cli/extension/session.py (1)
129-131: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winClear partial chunk buffers when a request finishes or times out.
A stream that never delivers its final chunk leaves
_response_buffers[request.id]retained for the session lifetime after the 30-second request timeout. Remove it in thisfinallyblock.Proposed fix
finally: self._pending.pop(request.id, None) self._artifact_events.pop(request.id, None) + self._response_buffers.pop(request.id, None)🤖 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 `@src/browser_cli/extension/session.py` around lines 129 - 131, Update the finally block in the request-handling method to also remove request.id from _response_buffers, alongside _pending and _artifact_events. Ensure cleanup occurs for both completed and timed-out requests without affecting other response buffers.
🧹 Nitpick comments (1)
tests/unit/test_daemon_browser_service.py (1)
642-668: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winCover all newly added fallback patterns.
This test exercises only
Extension disconnected.; regressions inmessage too bigorexceeds limit ofwould go unnoticed. Parameterize the case over all three new messages and assert the selected message is propagated indriver_fallback["error"].Based on the added fallback patterns in this PR.
Suggested parameterization
+@pytest.mark.parametrize( + "extension_error", + ("Extension disconnected.", "message too big", "exceeds limit of 1048576"), +) def test_read_page_falls_back_to_playwright_when_extension_disconnects_mid_read( _patched_browser_service: _FakeExtensionHub, + extension_error: str, ) -> None: ... - raise RuntimeError("Extension disconnected.") + raise RuntimeError(extension_error) ... - assert "Extension disconnected" in payload["driver_fallback"]["error"] + assert extension_error in payload["driver_fallback"]["error"]🤖 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 `@tests/unit/test_daemon_browser_service.py` around lines 642 - 668, Parameterize test_read_page_falls_back_to_playwright_when_extension_disconnects_mid_read over the three supported extension failure messages: “Extension disconnected.”, “message too big”, and “exceeds limit of”. Raise the selected message from _extension_capture_html and assert that the same message is propagated in driver_fallback["error"], while preserving the existing fallback and active-driver assertions.
🤖 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 `@src/browser_cli/extension/session.py`:
- Around line 169-186: Validate that the reassembled response’s response.id
matches the chunk envelope request_id before calling resolve_response(response).
On mismatch, remove the buffered request and set its future to
OperationFailedError with error_code "EXTENSION_RESPONSE_CHUNK_INVALID",
preserving the existing invalid-response handling and preventing resolution of
the wrong request.
- Around line 68-79: The chunk handler around int(...) must reject malformed,
negative, and excessively large indexes before buffering so conversion errors
cannot terminate the session. Define and apply a bounded non-negative index
validation before updating the chunk collection, then update
ExtensionResponseChunks.assemble to verify continuity using min/max and len
without constructing range(index + 1). Add regressions covering malformed input
and an oversized index.
---
Outside diff comments:
In `@src/browser_cli/extension/session.py`:
- Around line 129-131: Update the finally block in the request-handling method
to also remove request.id from _response_buffers, alongside _pending and
_artifact_events. Ensure cleanup occurs for both completed and timed-out
requests without affecting other response buffers.
---
Nitpick comments:
In `@tests/unit/test_daemon_browser_service.py`:
- Around line 642-668: Parameterize
test_read_page_falls_back_to_playwright_when_extension_disconnects_mid_read over
the three supported extension failure messages: “Extension disconnected.”,
“message too big”, and “exceeds limit of”. Raise the selected message from
_extension_capture_html and assert that the same message is propagated in
driver_fallback["error"], while preserving the existing fallback and
active-driver assertions.
🪄 Autofix (Beta)
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: 8eb8cfae-6fb5-4344-8458-6dc99fee34b5
📒 Files selected for processing (12)
AGENTS.mdbrowser-cli-extension/manifest.jsonbrowser-cli-extension/src/background.jsbrowser-cli-extension/src/background/response_framing.jsbrowser-cli-extension/src/protocol.jsbrowser-cli-extension/tests/response_framing.test.jssrc/browser_cli/daemon/browser_service.pysrc/browser_cli/extension/__init__.pysrc/browser_cli/extension/protocol.pysrc/browser_cli/extension/session.pytests/unit/test_daemon_browser_service.pytests/unit/test_extension_transport.py
Validate chunk indexes and reassembled response ids before resolving pending requests, clean response buffers on request completion, and cover the new failure paths in transport and read-fallback tests. Co-authored-by: Cursor <cursoragent@cursor.com>
1 participant
Summary
responsepayloads asresponse-chunkmessages so large page HTML (for example WeChat articles) no longer trips the default 1 MiB WebSocket limit and disconnects mid-read.readwhen an older extension still disconnects withmessage too big/Extension disconnected.0.1.1and document the failure mode inAGENTS.md.Test plan
node --test browser-cli-extension/tests/response_framing.test.jsuv run pytest tests/unit/test_extension_transport.py tests/unit/test_daemon_browser_service.pyscripts/lint.shscripts/guard.shbrowser-cli-extension/, runbrowser-cli reload, thenbrowser-cli read https://mp.weixin.qq.com/s/TSxWeY7yCZ3HiV_zNXon-gMade with Cursor
Summary by CodeRabbit
Bug Fixes
Documentation
Chores