fix(sdk-js)[FDE-148]: sanitize ws close codes in browser on live session destroy

[egenthon-cmd]

Fixes an uncatchable InvalidAccessError when destroying a LiveV2Session in the browser. WebSocketSession.close() was passing reserved close codes (1001, 1006, etc.) directly to WebSocket.close(), which browsers only allow for 1000 or 3000–4999.

Sanitize close codes in browser via safeWsClose(); keep the original code/reason in onclose callbacks
Wrap the abort listener’s close() in try/catch so errors don’t bubble to window.onerror

Summary by CodeRabbit

• Bug Fixes
  • Improved WebSocket close handling so sessions now shut down more reliably in browser environments, even when certain close codes are unsupported.
  • Preserved the intended session close code and reason when a connection is closing, making disconnect behavior more consistent.
  • Prevented session-ending cleanup from failing if the underlying WebSocket rejects an abort-related close request.

[coderabbitai]

Warning

Review limit reached

@egenthon-cmd, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 50 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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 configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: ffa32f55-735b-4938-b403-d56db297ba20

📥 Commits

Reviewing files that changed from the base of the PR and between ff75863 and 05e394c.

📒 Files selected for processing (2)

• packages/sdk-js/src/network/wsClient.test.ts
• packages/sdk-js/src/network/wsClient.ts

📝 Walkthrough

Walkthrough

Adds browser-safe WebSocket close-code sanitization in wsClient.ts via a new safeWsClose helper and pendingClose state tracking, updating close()/connect()/onopen/onclose paths accordingly. Wraps LiveV2Session's abort-triggered webSocketSession.close() call in try/catch. Adds corresponding tests in both files.

Changes

WebSocket close-code sanitization fix

Layer / File(s)	Summary
Browser detection and safeWsClose helper packages/sdk-js/src/network/wsClient.ts	Adds browser-environment detection, close-code sanitization (preserving 1000 and 3000–4999), and a safeWsClose wrapper that retries with code 1000 if close throws.
WebSocketSession close/onclose wiring packages/sdk-js/src/network/wsClient.ts	Adds pendingClose tracking of requested {code, reason}, uses safeWsClose in close(), connect(), and onopen, clears pendingClose in onWsClose(), and prefers pendingClose values when forwarding onclose during CLOSING state.
wsClient close-code tests packages/sdk-js/src/network/wsClient.test.ts	Adds tests verifying sanitized close code 1000 is sent to the underlying socket while onclose reports the original code, and that InvalidAccessError from close() is swallowed with fallback to 1000.
LiveV2Session abort cleanup guard packages/sdk-js/src/v2/live/session.ts, packages/sdk-js/src/v2/live/session.test.ts	Wraps the abort handler's webSocketSession.close(1001, 'Aborted') call in try/catch to swallow thrown errors, with a test confirming endSession() does not throw when close fails with a reserved code.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Possibly related issues

• LiveV2Session raises uncatchable InvalidAccessError on session destroy in browser/Electron #42: Directly addresses the browser close-code sanitization and abort-path try/catch fixing the InvalidAccessError bug in WebSocketSession.close() and LiveV2Session.endSession() described in this issue.

Suggested reviewers: karamouche

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly matches the main change: browser WebSocket close-code sanitization during live session teardown.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch feat/error-aborted-wrong-number

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

🧹 Nitpick comments (1)

packages/sdk-js/src/network/wsClient.test.ts (1)

766-791: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Test does not exercise the catch fallback it claims to cover.

With window/document stubbed, session.close(1006, …) routes through safeWsClose, which sanitizes 1006 → 1000 before calling ws.close. Since the mock only throws for code !== 1000, ws.close(1000) never throws, so the try/catch fallback in safeWsClose is never triggered. The assertions pass purely via sanitization, making this effectively a duplicate of the previous test rather than a test of InvalidAccessError recovery.

To actually cover the catch path, make the mock throw on the sanitized code too (forcing the inner ws.close(1000) retry), or drive the fallback in a non-browser environment where sanitization is skipped.

🤖 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 `@packages/sdk-js/src/network/wsClient.test.ts` around lines 766 - 791, The
`WebSocketClient` browser-close test is only verifying sanitization, not the
`InvalidAccessError` fallback in `safeWsClose`. Update the `ws.close` mock in
this test so it also throws when called with the sanitized close code, forcing
the retry/catch path to execute. Keep the existing `WebSocketClient`,
`createSession`, and `session.close` flow, but adjust the mock and assertions so
the test truly covers recovery from `DOMException('InvalidAccessError')` rather
than duplicating the previous case.

🤖 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 `@packages/sdk-js/src/network/wsClient.test.ts`:
- Around line 766-791: The `WebSocketClient` browser-close test is only
verifying sanitization, not the `InvalidAccessError` fallback in `safeWsClose`.
Update the `ws.close` mock in this test so it also throws when called with the
sanitized close code, forcing the retry/catch path to execute. Keep the existing
`WebSocketClient`, `createSession`, and `session.close` flow, but adjust the
mock and assertions so the test truly covers recovery from
`DOMException('InvalidAccessError')` rather than duplicating the previous case.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 3aa7cfa1-6df7-42dc-bf3e-b1b823e6e2d9

📥 Commits

Reviewing files that changed from the base of the PR and between cf256e9 and ff75863.

📒 Files selected for processing (4)

• packages/sdk-js/src/network/wsClient.test.ts
• packages/sdk-js/src/network/wsClient.ts
• packages/sdk-js/src/v2/live/session.test.ts
• packages/sdk-js/src/v2/live/session.ts

[karamouche]

any reason is this fix only implemented in typescript, and not in python ?

[egenthon-cmd]

@karamouche Yes, because the pb is a browser-only WebSocket API constraint.
Python does not run in the browser, it uses the websockets library