[jdimovska]

💡 Overview

On networks where ICE never connects (e.g. a corporate firewall blocking STUN/TURN), the SDK got stuck in an endless rejoin loop, every ~17s, instead of leaving the call with webrtc_unsupported_network.

Chrome moves ICE to disconnected and the peer connection to failed at almost the same time. The peer connection failed handler always rejoined with CONNECTION_FAILED, without checking whether ICE had ever connected. So the "ICE never connected" counter never went up, and the SDK never gave up.

📝 Implementation notes

• BasePeerConnection: a peer connection failed before ICE ever connected now rejoins with ICE_NEVER_CONNECTED, so the SDK leaves with webrtc_unsupported_network after 2 attempts. It also cancels the pre-connect watchdog, so the same failure can't trigger a second rejoin. Failures after ICE had connected still use CONNECTION_FAILED, as before.
• ClientEventReporter: this failure is now reported as ICE_CONNECTIVITY_FAILED instead of DTLS_CONNECTIVITY_FAILED. DTLS never starts if ICE doesn't connect.
• Tests: added Publisher tests for the new reason and the watchdog cancellation, and updated the reporter test for the new label.

Verified locally by blocking all media paths in the browser: the SDK now leaves after the second failure, and call.session_participant_left carries reason: "webrtc_unsupported_network".

🎫 Ticket: https://linear.app/stream/issue/REACT-1197/endless-rejoin-loop-when-ice-never-connects-webrtc-unsupported-network

Summary by CodeRabbit

• Bug Fixes
  • Improved connection failure reporting so failures are identified as ICE or DTLS connectivity issues based on the connection state.
  • Improved recovery when a connection fails before ICE connects: the client now requests a rejoin without attempting an ICE restart.
  • Prevented the pre-connection timeout from triggering a duplicate reconnection notification after a failure has already been handled.

[coderabbitai]

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: 97e91da1-82f1-40f6-b5a9-10dcd6bb7ad1

📥 Commits

Reviewing files that changed from the base of the PR and between 7c1a7da and 4855799.

📒 Files selected for processing (4)

• packages/client/src/reporting/ClientEventReporter.ts
• packages/client/src/reporting/__tests__/ClientEventReporter.test.ts
• packages/client/src/rtc/BasePeerConnection.ts
• packages/client/src/rtc/__tests__/Publisher.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 2 included reviews per hour; 1 remain after this review.

---

📝 Walkthrough

Walkthrough

Peer connection failure reporting now uses ICE state to select the failure type. Reconnection handling now distinguishes failures before ICE connects and clears the pre-connect watchdog. Tests cover failure classification and reconnection behavior.

Changes

Peer connection failure handling

Layer / File(s)	Summary
Failure reporting by ICE state packages/client/src/reporting/ClientEventReporter.ts, packages/client/src/reporting/__tests__/ClientEventReporter.test.ts	The reporter uses the DTLS failure code and reason when ICE is connected or completed; otherwise, it uses the ICE failure code and reason. The test for failure before ICE connects expects ICE_CONNECTIVITY_FAILED.
Reconnection before ICE connects packages/client/src/rtc/BasePeerConnection.ts, packages/client/src/rtc/__tests__/Publisher.test.ts	A failed peer connection uses ICE_NEVER_CONNECTED if ICE has never connected, and CONNECTION_FAILED otherwise. The pre-connect watchdog is cleared before REJOIN. Tests check the reconnection reason, that ICE restart is not called, and that the watchdog does not trigger a second notification.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Suggested reviewers: oliverlaz

Merge Risk: ⚪ Minimal · up to 48557

The change distinguishes failures before ICE connects and prevents stale watchdog notifications during reconnection. No actionable merge-blocking issue remains; merge after normal checks.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 48557

The change uses existing failure codes and reconnection controls, without an observed increase in privileges or exposure. Failure-event ordering and some downstream behavior remain unverified.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

• inferred — Network impairment can influence the changed failure classification and terminate retries for the affected Call. The inspected outcome remains within that Call's existing REJOIN and leave lifecycle; the changed branch does not select another call identity or grant additional authority.

Trust Boundaries and Controls

• observed — Telemetry classification reads local peer-state values and passes constant failure codes and reasons to the existing emitter. That emitter requires an existing call/role pair and retains centralized call/session identity fields, rather than introducing credentials or network-provided identity values.

Resilience and Maintainability Implications

• observed — The existing reconnection consumer rejects redundant pending work and inappropriate lifecycle states, checks leave generation around asynchronous steps, rate-limits full rejoins and stops at terminal states. Leave disposes peers and clears retry state, containing repeated failure notifications and interrupted recovery.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 4…
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.
Title check	✅ Passed	The title clearly and concisely describes the main change: preventing endless rejoin loops when ICE never connects.
Description check	✅ Passed	The description includes the required Overview and Implementation notes sections, identifies the ticket, explains the root cause and implementation, and documents validation results. The Docs link is …

✨ Finishing Touches

📝 Generate docstrings

• Commit to this branch
• Create a new PR

🧪 Generate unit tests (beta)

• Commit to this branch
• Create a new PR

• Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Autopilot is currently an internal CodeRabbit preview.

---

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.}

[github-actions]

Bundle size

Built package output. Sizes in KB; delta vs main@57f23a1.

Package	Unminified	Minified	Δ min vs main
@stream-io/video-client	805.7 KB	283.2 KB	+2.9 KB (+1.1%)
@stream-io/video-react-sdk	376.9 KB	229.2 KB	-10.2 KB (-4.3%)
↳ install total (+ client + react-bindings)	1212.2 KB	522.4 KB	-7.2 KB (-1.4%)
@stream-io/video-react-sdk (embedded) (cjs)	219.0 KB	127.0 KB	+564 B (+0.4%)
@stream-io/video-react-native-sdk	458.8 KB	208.5 KB	+9.2 KB (+4.6%)
↳ install total (+ client + react-bindings)	1294.1 KB	501.7 KB	+12.2 KB (+2.5%)
@stream-io/noise-cancellation-react-native	3.5 KB	1.0 KB	-234 B (-19.1%)

[github-actions]

🎉 The changes from this pull request have been released.

Shipped with:

• @stream-io/video-client@1.61.1 - release notes | npm
• @stream-io/video-react-bindings@1.20.5 - release notes | npm
• @stream-io/video-react-native-sdk@1.46.1 - release notes | npm
• @stream-io/video-react-sdk@1.43.3 - release notes | npm