fix(rtc_engine): escalate a PeerConnection that stays disconnected

[xianshijing-lk]

Before you submit your PR

• I have read the contributing guidelines and validated that this PR will be accepted.
• I have read and followed the principles regarding breaking changes, testing, and code quality.

Stacked on #1331. Targets sxian/CLT-3249/resume-requires-pc-recovery-evidence; review that one first. It touches the same RtcEvent::ConnectionChange handler, so it does not apply cleanly to main on its own.

PR description

Only PeerConnectionState::Failed drove recovery. libwebrtc does not reach Failed until ICE consent expires — tens of seconds after a transport actually stops working — while Disconnected, which it reports within a couple of seconds, was ignored entirely. Nothing acted in between, so a session whose media plane had died stayed "connected" and silently deaf for the whole consent window before anything began recovering.

The clearest case is steady state, with no signalling failure at all: a NAT rebind, a Wi-Fi→cellular handover, a route change. There is no resume in flight to notice, Disconnected is dropped on the floor, and recovery does not begin until Failed finally arrives.

It also lengthened node-failure recovery, though #1331 already removes the worst of that: with the resume no longer accepting a stale Connected, it rejects the dead transport and escalates at its own bound. This PR shortens that further and, more importantly, covers the case where nothing is waiting.

Approach

Disconnected is deliberately not treated as an immediate failure: brief disconnects during ordinary network disturbance are normal and self-healing, and tearing down a session for one would be worse than the disturbance itself. So a transport entering Disconnected starts a grace period (PC_DISCONNECTED_GRACE, 5s — comfortably past a transient blip, far short of consent expiry) and is judged on its state when the countdown elapses, not on the transition that started it.

Judging on the final state rather than tracking the transition has a nice property: a connection that recovered on its own needs no cancellation bookkeeping at all. It simply reads as connected when the countdown lapses, and nothing happens. Repeated Disconnected transitions collapse onto the countdown already running rather than each spawning their own, so a flapping transport cannot accumulate timers that all fire and re-report the same failure.

Escalation is delegated to the engine rather than decided locally, because the engine already interprets a transport failure in context: outside a reconnect it starts one; during a reconnect its existing sticky-escalation logic converts the cycle to a full reconnect instead of looping on resume. Failed is left to the existing branch, so it is still reported the instant it occurs and is not double-reported by the countdown.

Comparison with client-sdk-js

Worth recording, because it is not a straight port: client-sdk-js does not act on disconnected either. PCTransportManager.updateState has no branch for it — none of its conditions match, so this.state silently retains its previous value — which means verifyTransport keeps accepting the transport and the connection-reconcile loop cannot catch it.

JS's actual net for this case is server-driven: ConnectionQuality::LOST on the local participant → scheduleLostQualityReconnect (RTCEngine.ts:1207), which is the same arm/re-check/guard shape used here. That signal catches a publisher the server has stopped receiving, but not a subscriber that has stopped receiving the server. Acting locally on disconnected covers both directions.

Rust currently forwards ConnectionQuality to the application and does nothing with it (rtc_engine/mod.rs), so adopting the JS-style LOST trigger as well would be a reasonable complementary follow-up — the two are not redundant.

Breaking changes

None. No public API change; PC_DISCONNECTED_GRACE is new and additive.

Behaviourally, a transport that stays disconnected for 5s now triggers recovery where previously it was ignored until Failed. That is the point of the change, but it does mean sessions on genuinely marginal links may now reconnect where they previously sat in a degraded-but-quiet state. The grace period is the tuning point if that proves too eager in the field.

MSRV

Unchanged.

Testing

cargo test -p livekit --lib — 88 passed.

The escalation decision is split into should_escalate_after_grace(state) so each branch is asserted directly, without waiting out a real countdown:

• transport_still_down_after_grace_is_escalated — the defect itself: Disconnected (and Connecting, i.e. never got back) at the end of the window must escalate rather than wait for Failed.
• transport_that_recovered_during_grace_is_left_alone — the reason this is a countdown and not an immediate reaction; guards against regressing to "escalate on Disconnected".
• failed_is_not_double_reported_after_grace — Failed is already reported by the existing branch.
• closed_transport_is_not_escalated_after_grace — a deliberate teardown (session close, or a full reconnect replacing this session) must not be fought.

Plus, against a real PeerConnection:

• disconnect_grace_admits_one_countdown_at_a_time — asserts repeated transitions collapse onto one countdown, and that a fresh one can be armed afterwards, so a transport that recovers and dies again is still escalated. The second half is the one that matters: a dedupe flag that is never released would silently disable all subsequent detection.

Not covered by unit tests: the wall-clock behaviour of the timer itself, and the engine-side escalation it triggers (already covered by the existing reconnect tests and the __lk-e2e-test fault injection). End-to-end confirmation belongs with the private e2e suite; I have not run it — no staging credentials.

Async

Uses livekit_runtime::spawn and livekit_runtime::sleep, no direct runtime dependency. on_rtc_event takes self: &Arc<Self> so the countdown can keep the session alive for its duration; it checks the session's closed flag on wake and bails, so a closed session does not escalate.

The delay here is not waiting for state to "catch up" — it is the substance of the fix. Reacting to Disconnected without it would reconnect on every transient blip. The two new unit tests are synchronous; the existing #[tokio::test] convention in these modules is unchanged.

[devin-ai-integration]

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no potential bugs to report.

View in Devin Review to see 1 additional finding.