fix: recover broken publish paths and stuck reconnects - #2030
Draft
lukasIO wants to merge 1 commit into
Draft
Conversation
Adds several connection-recovery improvements aimed at the class of failures where publishing is broken even though the peer connection still reports connected: - Act on local `ConnectionQuality.Lost`: when the server reports it isn't receiving our media for a sustained period while connected and publishing, force a full reconnect. - Verify the ICE restart actually landed during a resume (wait for `restartingIce` to clear via a matching-offerId answer) instead of only waiting for `connected`, and escalate a rejected publisher answer during a resume to a full reconnect. - Recreate the peer connection (via escalation) when an ICE restart is needed but there is no remote description to restart on, rather than stalling on `renegotiate`. - Add outbound-RTP liveness to `verifyTransport()` (bytesSent must advance with active senders) and bound how long a transport may stay CONNECTING. - Preserve a full-reconnect request that arrives mid-resume so a successful resume no longer clears it. - On a detected connection state mismatch, attempt a full reconnect (keeping the room alive) instead of tearing the session down. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
🦋 Changeset detectedLatest commit: 241c694 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Contributor
size-limit report 📦
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What & why
A set of connection-recovery improvements targeting the class of failures where publishing is broken even though the peer connection still reports
connected— cases the current logic doesn't notice, so the client sits on a dead session without reconnecting.Changes
Act on local
ConnectionQuality.Lost(RTCEngine) —LOSTis the server's verdict that it isn't receiving our media. If it staysLOSTfor a sustained window (5s) while connected and actively publishing, force a full reconnect. Catches the whole class regardless of root cause. Any non-LOSTupdate cancels the pending trigger.Verify the ICE restart landed during a resume (
RTCEngine,PCTransportManager) — instead of only waiting forconnected, wait forpublisher.restartingIceto clear (which happens only when an answer with a matching offerId is applied). If it doesn't complete in time, escalate to a full reconnect. A publisher answer rejected mid-resume (setPublisherAnswer→false) now escalates too, rather than being silently swallowed.Recreate the PC when an ICE restart has no remote description (
PCTransport) — the old TODO fell through torenegotiate = true, which stalls (the pending offer is never answered). Now it throws so the caller escalates to a full reconnect (which rebuilds the peer connections). Only reachable on the resume path.Outbound-RTP liveness in
verifyTransport()(RTCEngine,Room) —verifyTransport()is now async: it checks the publisher's summedbytesSentis advancing when there are active senders, and bounds how long a transport may sit inCONNECTING(previously treated as healthy indefinitely). Room's reconcile awaits it once per tick.Don't clobber a full-reconnect request that arrives mid-resume (
RTCEngine) — a successful resume no longer clearsfullReconnectOnNextit didn't act on (e.g. a serverRECONNECTleave that arrived during the resume); it's dispatched after the attempt settles.Reconnect on a detected state mismatch instead of tearing down (
Room,RTCEngine) — when the connection-reconcile safety net trips (transport silently died while we looked connected), it now triggers a full reconnect via the existing engine (reusing saved credentials, keeping the room alive) rather than terminally disconnecting. Terminal teardown remains the fallback when there's no usable engine or the reconnect ultimately fails.Reviewer notes
DisconnectReason.STATE_MISMATCH) previously emittedRoomEvent.Disconnectedimmediately. It now goesReconnecting → (Reconnected | Disconnected), matching how every other transport failure is handled. Apps that watched for an immediateSTATE_MISMATCHdisconnect to trigger their own reconnect will see the reconnecting flow instead.verifyTransport()changed from syncbooleantoasync Promise<boolean>— it's@internal, andRoomis the only caller.PCTransportManager.waitForPublisherIceRestart(resolve/timeout/abort/no-op).Testing
pnpm type:check,pnpm lint(0 errors),pnpm throws:check, andpnpm test(668 tests, incl. 5 new) all pass.🤖 Generated with Claude Code