From 241c6942b421e09014658b6ec828652ecceb1683 Mon Sep 17 00:00:00 2001 From: lukasIO Date: Thu, 30 Jul 2026 16:03:03 +0200 Subject: [PATCH 01/11] fix: recover broken publish paths and stuck reconnects 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 --- .changeset/quiet-planets-fix.md | 5 + src/room/PCTransport.ts | 10 +- src/room/PCTransportManager.test.ts | 41 ++++++ src/room/PCTransportManager.ts | 21 +++ src/room/RTCEngine.ts | 192 +++++++++++++++++++++++++++- src/room/Room.ts | 28 ++-- 6 files changed, 282 insertions(+), 15 deletions(-) create mode 100644 .changeset/quiet-planets-fix.md diff --git a/.changeset/quiet-planets-fix.md b/.changeset/quiet-planets-fix.md new file mode 100644 index 0000000000..870ffe63e3 --- /dev/null +++ b/.changeset/quiet-planets-fix.md @@ -0,0 +1,5 @@ +--- +"livekit-client": patch +--- + +fix: recover broken publish paths — act on local `ConnectionQuality.Lost`, verify ICE restarts land during resume, add outbound-RTP liveness to the connection reconcile, and reconnect (instead of disconnecting) on a detected state mismatch diff --git a/src/room/PCTransport.ts b/src/room/PCTransport.ts index c4367c7bc8..a7f8e0d069 100644 --- a/src/room/PCTransport.ts +++ b/src/room/PCTransport.ts @@ -425,9 +425,15 @@ export default class PCTransport extends (EventEmitter as new () => TypedEmitter // the only exception to this is when ICE restart is needed const currentSD = this._pc.remoteDescription; if (options?.iceRestart && currentSD) { - // TODO: handle when ICE restart is needed but we don't have a remote description - // the best thing to do is to recreate the peerconnection + // roll the remote description back in so createOffer produces a valid + // ICE-restart offer on top of the already-negotiated state await this._pc.setRemoteDescription(currentSD); + } else if (options?.iceRestart) { + // ICE restart with no remote description to restart on: `renegotiate` would stall + // (the pending offer is never answered), so throw for the caller to recreate the PC. + throw new NegotiationError( + 'ICE restart requested without a remote description, peer connection must be recreated', + ); } else { this.renegotiate = true; this.log.debug('requesting renegotiation'); diff --git a/src/room/PCTransportManager.test.ts b/src/room/PCTransportManager.test.ts index 5afc291ed4..d591cb2dfa 100644 --- a/src/room/PCTransportManager.test.ts +++ b/src/room/PCTransportManager.test.ts @@ -49,6 +49,8 @@ class FakePublisher extends EventEmitter { latestAcknowledgedOfferId = 0; + restartingIce = false; + negotiate = vi.fn(async (_onError?: (e: Error) => void) => {}); /** Simulate a publisher offer cycle: bump latestOfferId. */ @@ -253,6 +255,45 @@ describe('PCTransportManager.negotiate', () => { }); }); + describe('waitForPublisherIceRestart', () => { + it('resolves once restartingIce clears', async () => { + const { manager, pub } = makeManager(); + pub.restartingIce = true; + const p = manager.waitForPublisherIceRestart(1000); + // simulate the answer landing and clearing the flag + setTimeout(() => { + pub.restartingIce = false; + }, 20); + await expect(p).resolves.toBeUndefined(); + }); + + it('resolves immediately when restartingIce is already false', async () => { + const { manager, pub } = makeManager(); + pub.restartingIce = false; + await expect(manager.waitForPublisherIceRestart(1000)).resolves.toBeUndefined(); + }); + + it('rejects when the restart does not complete in time', async () => { + const { manager, pub } = makeManager(); + pub.restartingIce = true; + await expect(manager.waitForPublisherIceRestart(60)).rejects.toThrow(/did not complete/); + }); + + it('rejects as soon as shouldAbort returns true', async () => { + const { manager, pub } = makeManager(); + pub.restartingIce = true; + await expect(manager.waitForPublisherIceRestart(5000, () => true)).rejects.toThrow(/aborted/); + }); + + it('is a no-op when no publisher connection is required', async () => { + const { manager, pub } = makeManager(); + pub.restartingIce = true; + manager.requirePublisher(false); + // resolves despite restartingIce still being set, because there is nothing to restart + await expect(manager.waitForPublisherIceRestart(60)).resolves.toBeUndefined(); + }); + }); + // Regression test for publishing call getting stuck // With the old design, NegotiationStarted firing faster than // peerConnectionTimeout kept resetting the timer indefinitely while diff --git a/src/room/PCTransportManager.ts b/src/room/PCTransportManager.ts index 6982cdae9d..269a891b3c 100644 --- a/src/room/PCTransportManager.ts +++ b/src/room/PCTransportManager.ts @@ -173,6 +173,27 @@ export class PCTransportManager { } } + /** + * Resolves once the publisher ICE restart offer is answered (which clears `restartingIce`). + * Rejects on `timeout` or when `shouldAbort` returns true; no-op without a publisher. + */ + async waitForPublisherIceRestart(timeout: number, shouldAbort?: () => boolean) { + if (!this.needsPublisher) { + return; + } + const endTime = Date.now() + timeout; + while (Date.now() < endTime) { + if (!this.publisher.restartingIce) { + return; + } + if (shouldAbort?.()) { + throw ConnectionError.internal('ICE restart aborted before completion'); + } + await sleep(50); + } + throw ConnectionError.internal('ICE restart did not complete in time'); + } + async addIceCandidate(candidate: RTCIceCandidateInit, target: SignalTarget) { this.iceLog.debug('adding remote ICE candidate', { target, candidate }); if (target === SignalTarget.PUBLISHER) { diff --git a/src/room/RTCEngine.ts b/src/room/RTCEngine.ts index ab0a7bb4ae..333c27ca21 100644 --- a/src/room/RTCEngine.ts +++ b/src/room/RTCEngine.ts @@ -18,6 +18,7 @@ import { LeaveRequest_Action, MediaSectionsRequirement, ParticipantInfo, + ConnectionQuality as ProtoConnectionQuality, PublishDataTrackResponse, ReconnectReason, type ReconnectResponse, @@ -108,6 +109,12 @@ import { const minReconnectWait = 2 * 1000; const leaveReconnect = 'leave-reconnect'; + +/** + * How long local connection quality must stay `LOST` while connected and publishing before we + * force a full reconnect — `LOST` is the server's verdict that it isn't receiving our media. + */ +const connectionQualityLostTimeout = 5 * 1000; const reliabeReceiveStateTTL = 30_000; const initialMediaSectionsAudio = 3; @@ -246,6 +253,15 @@ export default class RTCEngine extends (EventEmitter as new () => TypedEventEmit /** used to indicate whether the browser is currently waiting to reconnect */ private isWaitingForNetworkReconnect: boolean = false; + /** set while the local participant's connection quality is `LOST`; forces a full reconnect on timeout */ + private lostQualityTimeout?: ReturnType; + + /** timestamp (ms) the primary transport entered `CONNECTING`, used to bound how long we tolerate it */ + private transportConnectingSince?: number; + + /** last observed publisher outbound `bytesSent`, used to detect a stalled publish path in {@link verifyTransport} */ + private lastPublisherBytesSent?: number; + constructor(private options: InternalRoomOptions) { super(); this.log = getLogger(options.loggerName ?? LoggerNames.Engine, () => this.logContext); @@ -271,8 +287,10 @@ export default class RTCEngine extends (EventEmitter as new () => TypedEventEmit this.client.onParticipantUpdate = (updates) => this.emit(EngineEvent.ParticipantUpdate, updates); - this.client.onConnectionQuality = (update) => + this.client.onConnectionQuality = (update) => { + this.handleLocalConnectionQuality(update); this.emit(EngineEvent.ConnectionQualityUpdate, update); + }; this.client.onRoomUpdate = (update) => this.emit(EngineEvent.RoomUpdate, update); this.client.onSubscriptionError = (resp) => this.emit(EngineEvent.SubscriptionError, resp); this.client.onSubscriptionPermissionUpdate = (update) => @@ -429,6 +447,7 @@ export default class RTCEngine extends (EventEmitter as new () => TypedEventEmit this.removeAllListeners(); this.deregisterOnLineListener(); this.clearPendingReconnect(); + this.clearLostQualityTimeout(); this.cleanupLossyDataStats(); await this.cleanupPeerConnections(); await this.cleanupClient(); @@ -622,7 +641,13 @@ export default class RTCEngine extends (EventEmitter as new () => TypedEventEmit midToTrackId, }); this.midToTrackId = midToTrackId; - await this.pcManager.setPublisherAnswer(sd, offerId); + const applied = await this.pcManager.setPublisherAnswer(sd, offerId); + if (!applied && this.attemptingReconnect && !this.fullReconnectOnNext) { + // Publisher answer rejected during a resume (e.g. stale offerId): the ICE restart + // can't land, so escalate to a full reconnect (also aborts waitForPublisherIceRestart). + this.log.warn('publisher answer rejected during resume, escalating to full reconnect'); + this.fullReconnectOnNext = true; + } }; // add candidate on trickle @@ -1157,6 +1182,73 @@ export default class RTCEngine extends (EventEmitter as new () => TypedEventEmit ); }; + /** + * A sustained local `LOST` while connected and publishing means the server isn't receiving + * our media, so force a full reconnect; any non-`LOST` value cancels a pending trigger. + */ + private handleLocalConnectionQuality(update: ConnectionQualityUpdate) { + if (!this.participantSid) { + return; + } + const localUpdate = update.updates.find((u) => u.participantSid === this.participantSid); + if (!localUpdate) { + return; + } + if (localUpdate.quality === ProtoConnectionQuality.LOST) { + this.scheduleLostQualityReconnect(); + } else { + this.clearLostQualityTimeout(); + } + } + + private scheduleLostQualityReconnect() { + if (this.lostQualityTimeout) { + // already counting down towards a reconnect + return; + } + this.lostQualityTimeout = CriticalTimers.setTimeout(() => { + this.lostQualityTimeout = undefined; + if (this._isClosed || this.pcState !== PCState.Connected || this.attemptingReconnect) { + return; + } + if (!this.hasActivePublisherSenders()) { + return; + } + this.log.warn( + 'local connection quality lost while publishing, triggering full reconnect', + this.logContext, + ); + this.fullReconnectOnNext = true; + this.handleDisconnect('connection quality lost', ReconnectReason.RR_PUBLISHER_FAILED); + }, connectionQualityLostTimeout); + } + + private clearLostQualityTimeout() { + if (this.lostQualityTimeout) { + clearTimeout(this.lostQualityTimeout); + this.lostQualityTimeout = undefined; + } + } + + /** Whether the publisher currently has any sender with a live track. */ + private hasActivePublisherSenders(): boolean { + return ( + this.pcManager?.publisher + .getSenders() + .some((sender) => !!sender.track && sender.track.readyState === 'live') ?? false + ); + } + + /** + * Forces a full reconnect while keeping the engine (and its saved credentials) alive. Used by + * Room's connection-reconcile safety net when the transport silently died but we looked connected. + * @internal + */ + reconnect(reason: ReconnectReason = ReconnectReason.RR_UNKNOWN) { + this.fullReconnectOnNext = true; + this.handleDisconnect('reconcile', reason); + } + private async attemptReconnect(reason?: ReconnectReason) { if (this._isClosed) { return; @@ -1175,15 +1267,23 @@ export default class RTCEngine extends (EventEmitter as new () => TypedEventEmit this.fullReconnectOnNext = true; } + let succeeded = false; + let performedFullReconnect = false; try { this.attemptingReconnect = true; if (this.fullReconnectOnNext) { + performedFullReconnect = true; await this.restartConnection(); } else { await this.resumeConnection(reason); } this.clearPendingReconnect(); - this.fullReconnectOnNext = false; + // Only clear the flag if we actually did a full reconnect, so a full reconnect requested + // mid-attempt (e.g. a server leave during a resume) survives a successful resume. + if (performedFullReconnect) { + this.fullReconnectOnNext = false; + } + succeeded = true; } catch (e) { this.reconnectAttempts += 1; let recoverable = true; @@ -1209,6 +1309,13 @@ export default class RTCEngine extends (EventEmitter as new () => TypedEventEmit } } finally { this.attemptingReconnect = false; + + // A full reconnect requested mid-attempt (e.g. a `RECONNECT` leave during a resume) that + // a successful attempt didn't act on; dispatch it now (the failure path already retries). + if (succeeded && this.fullReconnectOnNext && !this._isClosed) { + this.log.debug('full reconnect requested during in-progress attempt, dispatching'); + this.handleDisconnect('reconnect'); + } } } @@ -1342,6 +1449,21 @@ export default class RTCEngine extends (EventEmitter as new () => TypedEventEmit await this.pcManager.triggerIceRestart(); + // Verify the ICE restart landed (`restartingIce` clears only when a matching-offerId answer + // is applied) rather than just waiting for `connected`; escalate to a full reconnect if not. + try { + await this.pcManager.waitForPublisherIceRestart( + this.peerConnectionTimeout, + () => this.fullReconnectOnNext, + ); + } catch (e) { + this.log.warn('ICE restart did not complete during resume, escalating to full reconnect', { + error: e, + }); + this.fullReconnectOnNext = true; + throw e instanceof Error ? e : new Error(String(e)); + } + await this.waitForPCReconnected(); // re-check signal connection state before setting engine as resumed @@ -1587,15 +1709,18 @@ export default class RTCEngine extends (EventEmitter as new () => TypedEventEmit } /* @internal */ - verifyTransport(): boolean { + async verifyTransport(): Promise { if (!this.pcManager) { return false; } + const state = this.pcManager.currentState; const allowedConnectionStates: PCTransportState[] = [ PCTransportState.CONNECTING, PCTransportState.CONNECTED, ]; - if (!allowedConnectionStates.includes(this.pcManager.currentState)) { + if (!allowedConnectionStates.includes(state)) { + this.transportConnectingSince = undefined; + this.lastPublisherBytesSent = undefined; return false; } @@ -1603,9 +1728,66 @@ export default class RTCEngine extends (EventEmitter as new () => TypedEventEmit if (!this.client.ws || this.client.ws.readyState === WebSocket.CLOSED) { return false; } + + // A transport stuck in CONNECTING never reaches CONNECTED nor reports FAILED, so it would + // otherwise look healthy forever; bound how long we tolerate it. + if (state === PCTransportState.CONNECTING) { + const now = Date.now(); + if (this.transportConnectingSince === undefined) { + this.transportConnectingSince = now; + } else if (now - this.transportConnectingSince > this.peerConnectionTimeout) { + this.log.warn('transport stuck in connecting state', this.logContext); + return false; + } + // can't assert media liveness until connected + this.lastPublisherBytesSent = undefined; + return true; + } + this.transportConnectingSince = undefined; + + // Outbound-RTP liveness: with active senders `bytesSent` must keep advancing between checks; + // if it stalls while connected the publish path is broken even though the PC looks connected. + if (this.hasActivePublisherSenders()) { + const bytesSent = await this.getPublisherBytesSent(); + if (bytesSent !== undefined) { + const advanced = + this.lastPublisherBytesSent === undefined || bytesSent > this.lastPublisherBytesSent; + this.lastPublisherBytesSent = bytesSent; + if (!advanced) { + this.log.warn('publisher outbound bytes not advancing while senders active', { + ...this.logContext, + bytesSent, + }); + return false; + } + } + } else { + this.lastPublisherBytesSent = undefined; + } + return true; } + /** Sum of `bytesSent` across the publisher's outbound-rtp stats, or undefined if unavailable. */ + private async getPublisherBytesSent(): Promise { + try { + const stats = await this.pcManager?.publisher.getStats(); + if (!stats) { + return undefined; + } + let bytesSent = 0; + stats.forEach((report) => { + if (report.type === 'outbound-rtp') { + bytesSent += report.bytesSent ?? 0; + } + }); + return bytesSent; + } catch (e) { + this.log.debug('could not read publisher stats', { ...this.logContext, error: e }); + return undefined; + } + } + /** @internal */ async negotiate(): Promise { // observe signal state diff --git a/src/room/Room.ts b/src/room/Room.ts index 47b3087777..a190b5d6bb 100644 --- a/src/room/Room.ts +++ b/src/room/Room.ts @@ -2584,14 +2584,17 @@ class Room extends (EventEmitter as new () => TypedEmitter) private registerConnectionReconcile() { this.clearConnectionReconcile(); let consecutiveFailures = 0; - this.connectionReconcileInterval = CriticalTimers.setInterval(() => { + this.connectionReconcileInterval = CriticalTimers.setInterval(async () => { + // `verifyTransport` is async (it samples outbound-rtp stats), so resolve it once + // and reuse the result for both the decision and the diagnostic log. + const transportHealthy = this.engine ? await this.engine.verifyTransport() : false; if ( // ensure we didn't tear it down !this.engine || // engine detected close, but Room missed it this.engine.isClosed || // transports failed without notifying engine - !this.engine.verifyTransport() + !transportHealthy ) { consecutiveFailures++; this.log.warn('detected connection state mismatch', { @@ -2599,16 +2602,25 @@ class Room extends (EventEmitter as new () => TypedEmitter) engine: this.engine ? { closed: this.engine.isClosed, - transportsConnectedOrConnecting: this.engine.verifyTransport(), + transportsConnectedOrConnecting: transportHealthy, } : undefined, }); if (consecutiveFailures >= 3) { - this.recreateEngine(); - this.handleDisconnect( - this.options.stopLocalTrackOnUnpublish, - DisconnectReason.STATE_MISMATCH, - ); + this.clearConnectionReconcile(); + if (this.engine && !this.engine.isClosed) { + // The transport silently died while we still looked connected. Try a full reconnect + // (keeps the room alive; the engine falls back to Disconnected if it ultimately fails). + this.log.warn('detected connection state mismatch, attempting full reconnect'); + this.engine.reconnect(); + } else { + // No usable engine to reconnect with; tear down. + this.recreateEngine(); + this.handleDisconnect( + this.options.stopLocalTrackOnUnpublish, + DisconnectReason.STATE_MISMATCH, + ); + } } } else { consecutiveFailures = 0; From dcad734939d71017bf7d79257917219aeba3acd0 Mon Sep 17 00:00:00 2001 From: lukasIO Date: Fri, 31 Jul 2026 15:36:07 +0200 Subject: [PATCH 02/11] fix: drop ICE-restart-landed gate that broke signal-blip resumes Reverts the resumeConnection change that blocked the resume until the publisher's ICE restart offer was answered (`restartingIce` cleared). When only the signal blips but the media path is fine, the server may never answer the resume's ICE-restart offer, so gating on it hangs the resume and it never emits `resumed` (regressed the signalDisconnectDuringResume e2e test). A stale-but-connected publisher is instead caught at runtime by the local `ConnectionQuality.Lost` handler and the outbound-RTP liveness check in `verifyTransport()`, without breaking resume semantics. Co-Authored-By: Claude Opus 4.8 --- .changeset/quiet-planets-fix.md | 2 +- src/room/PCTransportManager.test.ts | 41 ----------------------------- src/room/PCTransportManager.ts | 21 --------------- src/room/RTCEngine.ts | 23 +--------------- 4 files changed, 2 insertions(+), 85 deletions(-) diff --git a/.changeset/quiet-planets-fix.md b/.changeset/quiet-planets-fix.md index 870ffe63e3..c843c0245d 100644 --- a/.changeset/quiet-planets-fix.md +++ b/.changeset/quiet-planets-fix.md @@ -2,4 +2,4 @@ "livekit-client": patch --- -fix: recover broken publish paths — act on local `ConnectionQuality.Lost`, verify ICE restarts land during resume, add outbound-RTP liveness to the connection reconcile, and reconnect (instead of disconnecting) on a detected state mismatch +fix: recover broken publish paths — act on local `ConnectionQuality.Lost`, add outbound-RTP liveness to the connection reconcile, recreate the peer connection when an ICE restart has no remote description, and reconnect (instead of disconnecting) on a detected connection state mismatch diff --git a/src/room/PCTransportManager.test.ts b/src/room/PCTransportManager.test.ts index d591cb2dfa..5afc291ed4 100644 --- a/src/room/PCTransportManager.test.ts +++ b/src/room/PCTransportManager.test.ts @@ -49,8 +49,6 @@ class FakePublisher extends EventEmitter { latestAcknowledgedOfferId = 0; - restartingIce = false; - negotiate = vi.fn(async (_onError?: (e: Error) => void) => {}); /** Simulate a publisher offer cycle: bump latestOfferId. */ @@ -255,45 +253,6 @@ describe('PCTransportManager.negotiate', () => { }); }); - describe('waitForPublisherIceRestart', () => { - it('resolves once restartingIce clears', async () => { - const { manager, pub } = makeManager(); - pub.restartingIce = true; - const p = manager.waitForPublisherIceRestart(1000); - // simulate the answer landing and clearing the flag - setTimeout(() => { - pub.restartingIce = false; - }, 20); - await expect(p).resolves.toBeUndefined(); - }); - - it('resolves immediately when restartingIce is already false', async () => { - const { manager, pub } = makeManager(); - pub.restartingIce = false; - await expect(manager.waitForPublisherIceRestart(1000)).resolves.toBeUndefined(); - }); - - it('rejects when the restart does not complete in time', async () => { - const { manager, pub } = makeManager(); - pub.restartingIce = true; - await expect(manager.waitForPublisherIceRestart(60)).rejects.toThrow(/did not complete/); - }); - - it('rejects as soon as shouldAbort returns true', async () => { - const { manager, pub } = makeManager(); - pub.restartingIce = true; - await expect(manager.waitForPublisherIceRestart(5000, () => true)).rejects.toThrow(/aborted/); - }); - - it('is a no-op when no publisher connection is required', async () => { - const { manager, pub } = makeManager(); - pub.restartingIce = true; - manager.requirePublisher(false); - // resolves despite restartingIce still being set, because there is nothing to restart - await expect(manager.waitForPublisherIceRestart(60)).resolves.toBeUndefined(); - }); - }); - // Regression test for publishing call getting stuck // With the old design, NegotiationStarted firing faster than // peerConnectionTimeout kept resetting the timer indefinitely while diff --git a/src/room/PCTransportManager.ts b/src/room/PCTransportManager.ts index 269a891b3c..6982cdae9d 100644 --- a/src/room/PCTransportManager.ts +++ b/src/room/PCTransportManager.ts @@ -173,27 +173,6 @@ export class PCTransportManager { } } - /** - * Resolves once the publisher ICE restart offer is answered (which clears `restartingIce`). - * Rejects on `timeout` or when `shouldAbort` returns true; no-op without a publisher. - */ - async waitForPublisherIceRestart(timeout: number, shouldAbort?: () => boolean) { - if (!this.needsPublisher) { - return; - } - const endTime = Date.now() + timeout; - while (Date.now() < endTime) { - if (!this.publisher.restartingIce) { - return; - } - if (shouldAbort?.()) { - throw ConnectionError.internal('ICE restart aborted before completion'); - } - await sleep(50); - } - throw ConnectionError.internal('ICE restart did not complete in time'); - } - async addIceCandidate(candidate: RTCIceCandidateInit, target: SignalTarget) { this.iceLog.debug('adding remote ICE candidate', { target, candidate }); if (target === SignalTarget.PUBLISHER) { diff --git a/src/room/RTCEngine.ts b/src/room/RTCEngine.ts index 333c27ca21..99b9fe3086 100644 --- a/src/room/RTCEngine.ts +++ b/src/room/RTCEngine.ts @@ -641,13 +641,7 @@ export default class RTCEngine extends (EventEmitter as new () => TypedEventEmit midToTrackId, }); this.midToTrackId = midToTrackId; - const applied = await this.pcManager.setPublisherAnswer(sd, offerId); - if (!applied && this.attemptingReconnect && !this.fullReconnectOnNext) { - // Publisher answer rejected during a resume (e.g. stale offerId): the ICE restart - // can't land, so escalate to a full reconnect (also aborts waitForPublisherIceRestart). - this.log.warn('publisher answer rejected during resume, escalating to full reconnect'); - this.fullReconnectOnNext = true; - } + await this.pcManager.setPublisherAnswer(sd, offerId); }; // add candidate on trickle @@ -1449,21 +1443,6 @@ export default class RTCEngine extends (EventEmitter as new () => TypedEventEmit await this.pcManager.triggerIceRestart(); - // Verify the ICE restart landed (`restartingIce` clears only when a matching-offerId answer - // is applied) rather than just waiting for `connected`; escalate to a full reconnect if not. - try { - await this.pcManager.waitForPublisherIceRestart( - this.peerConnectionTimeout, - () => this.fullReconnectOnNext, - ); - } catch (e) { - this.log.warn('ICE restart did not complete during resume, escalating to full reconnect', { - error: e, - }); - this.fullReconnectOnNext = true; - throw e instanceof Error ? e : new Error(String(e)); - } - await this.waitForPCReconnected(); // re-check signal connection state before setting engine as resumed From d2af71bf2429cacb5f08123e5a512b0ad29166f1 Mon Sep 17 00:00:00 2001 From: lukasIO Date: Tue, 4 Aug 2026 10:32:22 +0200 Subject: [PATCH 03/11] fix: drop publisher outbound-RTP stall check from verifyTransport MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The bytesSent-advancing heuristic false-positives on legitimate custom tracks that don't emit media continuously (static screen share, on-demand canvas, silent/push-to-talk audio), triggering unnecessary reconnects. Remove it and revert verifyTransport to a synchronous check. The "stop treating an indefinitely-connecting transport as healthy" bound is kept — it's about connection state, not media, so it doesn't share the false-positive concern. Detection of a genuinely broken publish path is left to the server-driven local ConnectionQuality.Lost handling. Co-Authored-By: Claude Opus 4.8 --- .changeset/quiet-planets-fix.md | 2 +- src/room/RTCEngine.ts | 48 +-------------------------------- src/room/Room.ts | 9 +++---- 3 files changed, 5 insertions(+), 54 deletions(-) diff --git a/.changeset/quiet-planets-fix.md b/.changeset/quiet-planets-fix.md index c843c0245d..4d025c31f9 100644 --- a/.changeset/quiet-planets-fix.md +++ b/.changeset/quiet-planets-fix.md @@ -2,4 +2,4 @@ "livekit-client": patch --- -fix: recover broken publish paths — act on local `ConnectionQuality.Lost`, add outbound-RTP liveness to the connection reconcile, recreate the peer connection when an ICE restart has no remote description, and reconnect (instead of disconnecting) on a detected connection state mismatch +fix: recover broken publish paths — act on local `ConnectionQuality.Lost`, recreate the peer connection when an ICE restart has no remote description, bound how long a transport may stay connecting, and reconnect (instead of disconnecting) on a detected connection state mismatch diff --git a/src/room/RTCEngine.ts b/src/room/RTCEngine.ts index 99b9fe3086..46a7459919 100644 --- a/src/room/RTCEngine.ts +++ b/src/room/RTCEngine.ts @@ -259,9 +259,6 @@ export default class RTCEngine extends (EventEmitter as new () => TypedEventEmit /** timestamp (ms) the primary transport entered `CONNECTING`, used to bound how long we tolerate it */ private transportConnectingSince?: number; - /** last observed publisher outbound `bytesSent`, used to detect a stalled publish path in {@link verifyTransport} */ - private lastPublisherBytesSent?: number; - constructor(private options: InternalRoomOptions) { super(); this.log = getLogger(options.loggerName ?? LoggerNames.Engine, () => this.logContext); @@ -1688,7 +1685,7 @@ export default class RTCEngine extends (EventEmitter as new () => TypedEventEmit } /* @internal */ - async verifyTransport(): Promise { + verifyTransport(): boolean { if (!this.pcManager) { return false; } @@ -1699,7 +1696,6 @@ export default class RTCEngine extends (EventEmitter as new () => TypedEventEmit ]; if (!allowedConnectionStates.includes(state)) { this.transportConnectingSince = undefined; - this.lastPublisherBytesSent = undefined; return false; } @@ -1718,55 +1714,13 @@ export default class RTCEngine extends (EventEmitter as new () => TypedEventEmit this.log.warn('transport stuck in connecting state', this.logContext); return false; } - // can't assert media liveness until connected - this.lastPublisherBytesSent = undefined; return true; } this.transportConnectingSince = undefined; - // Outbound-RTP liveness: with active senders `bytesSent` must keep advancing between checks; - // if it stalls while connected the publish path is broken even though the PC looks connected. - if (this.hasActivePublisherSenders()) { - const bytesSent = await this.getPublisherBytesSent(); - if (bytesSent !== undefined) { - const advanced = - this.lastPublisherBytesSent === undefined || bytesSent > this.lastPublisherBytesSent; - this.lastPublisherBytesSent = bytesSent; - if (!advanced) { - this.log.warn('publisher outbound bytes not advancing while senders active', { - ...this.logContext, - bytesSent, - }); - return false; - } - } - } else { - this.lastPublisherBytesSent = undefined; - } - return true; } - /** Sum of `bytesSent` across the publisher's outbound-rtp stats, or undefined if unavailable. */ - private async getPublisherBytesSent(): Promise { - try { - const stats = await this.pcManager?.publisher.getStats(); - if (!stats) { - return undefined; - } - let bytesSent = 0; - stats.forEach((report) => { - if (report.type === 'outbound-rtp') { - bytesSent += report.bytesSent ?? 0; - } - }); - return bytesSent; - } catch (e) { - this.log.debug('could not read publisher stats', { ...this.logContext, error: e }); - return undefined; - } - } - /** @internal */ async negotiate(): Promise { // observe signal state diff --git a/src/room/Room.ts b/src/room/Room.ts index a190b5d6bb..bcc0b5ae6a 100644 --- a/src/room/Room.ts +++ b/src/room/Room.ts @@ -2584,17 +2584,14 @@ class Room extends (EventEmitter as new () => TypedEmitter) private registerConnectionReconcile() { this.clearConnectionReconcile(); let consecutiveFailures = 0; - this.connectionReconcileInterval = CriticalTimers.setInterval(async () => { - // `verifyTransport` is async (it samples outbound-rtp stats), so resolve it once - // and reuse the result for both the decision and the diagnostic log. - const transportHealthy = this.engine ? await this.engine.verifyTransport() : false; + this.connectionReconcileInterval = CriticalTimers.setInterval(() => { if ( // ensure we didn't tear it down !this.engine || // engine detected close, but Room missed it this.engine.isClosed || // transports failed without notifying engine - !transportHealthy + !this.engine.verifyTransport() ) { consecutiveFailures++; this.log.warn('detected connection state mismatch', { @@ -2602,7 +2599,7 @@ class Room extends (EventEmitter as new () => TypedEmitter) engine: this.engine ? { closed: this.engine.isClosed, - transportsConnectedOrConnecting: transportHealthy, + transportsConnectedOrConnecting: this.engine.verifyTransport(), } : undefined, }); From 069367beaccf19e5098852cfe9868cf3b494356c Mon Sep 17 00:00:00 2001 From: lukasIO Date: Tue, 4 Aug 2026 11:58:04 +0200 Subject: [PATCH 04/11] test: cover local ConnectionQuality.Lost triggering a full reconnect Unit tests for the engine's local Lost-quality handling: a sustained local LOST while connected and publishing forces a full reconnect, recovery cancels the pending trigger, and it stays put when not publishing, not connected, or the LOST is for another participant. A genuine server LOST can't be produced from a browser page (a live sender keeps RTCP flowing), so this replaces the grey-box e2e attempt. Co-Authored-By: Claude Opus 4.8 --- src/room/RTCEngine.test.ts | 120 ++++++++++++++++++++++++++++++++++++- 1 file changed, 118 insertions(+), 2 deletions(-) diff --git a/src/room/RTCEngine.test.ts b/src/room/RTCEngine.test.ts index bcc00f4323..c4350012d8 100644 --- a/src/room/RTCEngine.test.ts +++ b/src/room/RTCEngine.test.ts @@ -1,5 +1,10 @@ -import { DataPacket, DataPacket_Kind, UserPacket } from '@livekit/protocol'; -import { afterEach, describe, expect, it, vi } from 'vitest'; +import { + DataPacket, + DataPacket_Kind, + ConnectionQuality as ProtoConnectionQuality, + UserPacket, +} from '@livekit/protocol'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import type { DataPacketBuffer } from '../utils/dataPacketBuffer'; import RTCEngine, { DataChannelKind } from './RTCEngine'; import { roomOptionDefaults } from './defaults'; @@ -582,4 +587,115 @@ describe('RTCEngine', () => { expect(error).not.toHaveBeenCalled(); }); }); + + describe('local connection quality Lost handling', () => { + // The engine reacts to the server's own verdict: a sustained local `LOST` while + // connected and publishing means our media isn't reaching the server, so it forces + // a full reconnect. (A genuine `LOST` can't be produced from a browser page — any + // live sender keeps RTCP flowing — so the behavior is unit tested here rather than + // in the e2e suite.) `connectionQualityLostTimeout` in RTCEngine.ts is 5s. + const LOST_TIMEOUT_MS = 5_000; + const LOCAL_SID = 'PA_local'; + + beforeEach(() => { + vi.useFakeTimers(); + }); + + afterEach(() => { + vi.useRealTimers(); + }); + + /** An engine primed to satisfy the reconnect guard: connected, publishing, not closed. */ + function primeEngine(overrides: { activeSenders?: boolean; pcState?: number } = {}) { + const engine = new RTCEngine(roomOptionDefaults); + const internals = engine as unknown as { + _isClosed: boolean; + participantSid: string; + // PCState is a private enum; Connected is 1, Reconnecting is 3. + pcState: number; + attemptingReconnect: boolean; + pcManager: unknown; + handleDisconnect: (connection: string, reason?: number) => void; + handleLocalConnectionQuality: (update: unknown) => void; + }; + internals._isClosed = false; + internals.participantSid = LOCAL_SID; + internals.pcState = overrides.pcState ?? 1; // PCState.Connected + internals.attemptingReconnect = false; + internals.pcManager = { + publisher: { + getSenders: () => + overrides.activeSenders === false ? [] : [{ track: { readyState: 'live' } }], + }, + }; + const handleDisconnect = vi.fn(); + internals.handleDisconnect = handleDisconnect; + return { engine, internals, handleDisconnect }; + } + + function qualityUpdate(sid: string, quality: ProtoConnectionQuality) { + return { updates: [{ participantSid: sid, quality }] }; + } + + it('forces a full reconnect after a sustained local Lost while publishing', () => { + const { engine, internals, handleDisconnect } = primeEngine(); + + internals.handleLocalConnectionQuality(qualityUpdate(LOCAL_SID, ProtoConnectionQuality.LOST)); + + // still pending — the reconnect only fires once the timeout elapses + expect(engine.fullReconnectOnNext).toBe(false); + expect(handleDisconnect).not.toHaveBeenCalled(); + + vi.advanceTimersByTime(LOST_TIMEOUT_MS); + + expect(engine.fullReconnectOnNext).toBe(true); + expect(handleDisconnect).toHaveBeenCalledTimes(1); + }); + + it('cancels the pending reconnect when quality recovers before the timeout', () => { + const { engine, internals, handleDisconnect } = primeEngine(); + + internals.handleLocalConnectionQuality(qualityUpdate(LOCAL_SID, ProtoConnectionQuality.LOST)); + vi.advanceTimersByTime(LOST_TIMEOUT_MS / 2); + internals.handleLocalConnectionQuality( + qualityUpdate(LOCAL_SID, ProtoConnectionQuality.EXCELLENT), + ); + vi.advanceTimersByTime(LOST_TIMEOUT_MS); + + expect(engine.fullReconnectOnNext).toBe(false); + expect(handleDisconnect).not.toHaveBeenCalled(); + }); + + it('does not reconnect on Lost when there are no active publisher senders', () => { + const { engine, internals, handleDisconnect } = primeEngine({ activeSenders: false }); + + internals.handleLocalConnectionQuality(qualityUpdate(LOCAL_SID, ProtoConnectionQuality.LOST)); + vi.advanceTimersByTime(LOST_TIMEOUT_MS); + + expect(engine.fullReconnectOnNext).toBe(false); + expect(handleDisconnect).not.toHaveBeenCalled(); + }); + + it('does not reconnect on Lost when the pc is not connected', () => { + const { engine, internals, handleDisconnect } = primeEngine({ pcState: 3 }); // Reconnecting + + internals.handleLocalConnectionQuality(qualityUpdate(LOCAL_SID, ProtoConnectionQuality.LOST)); + vi.advanceTimersByTime(LOST_TIMEOUT_MS); + + expect(engine.fullReconnectOnNext).toBe(false); + expect(handleDisconnect).not.toHaveBeenCalled(); + }); + + it('ignores Lost quality reported for other participants', () => { + const { engine, internals, handleDisconnect } = primeEngine(); + + internals.handleLocalConnectionQuality( + qualityUpdate('PA_other', ProtoConnectionQuality.LOST), + ); + vi.advanceTimersByTime(LOST_TIMEOUT_MS); + + expect(engine.fullReconnectOnNext).toBe(false); + expect(handleDisconnect).not.toHaveBeenCalled(); + }); + }); }); From 6e606ef0a4862b81da584eab6448e1a2d1db8e81 Mon Sep 17 00:00:00 2001 From: lukasIO Date: Tue, 4 Aug 2026 12:06:53 +0200 Subject: [PATCH 05/11] test: cover mid-resume full-reconnect request handling Unit tests for the attemptReconnect finally-dispatch (Fix 5): a full reconnect requested while a resume is in flight survives the successful resume and is dispatched afterwards; an ordinary successful resume does not re-dispatch; a successful full reconnect clears the flag; and the failure path is not double-dispatched. Replaces the grey-box fullReconnectDuringResume e2e test, whose triggering leave was injected client-side anyway. Co-Authored-By: Claude Opus 4.8 --- src/room/RTCEngine.test.ts | 93 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 93 insertions(+) diff --git a/src/room/RTCEngine.test.ts b/src/room/RTCEngine.test.ts index c4350012d8..b04e4912b2 100644 --- a/src/room/RTCEngine.test.ts +++ b/src/room/RTCEngine.test.ts @@ -6,6 +6,7 @@ import { } from '@livekit/protocol'; import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import type { DataPacketBuffer } from '../utils/dataPacketBuffer'; +import { PCTransportState } from './PCTransportManager'; import RTCEngine, { DataChannelKind } from './RTCEngine'; import { roomOptionDefaults } from './defaults'; import { PublishDataError, UnexpectedConnectionState } from './errors'; @@ -698,4 +699,96 @@ describe('RTCEngine', () => { expect(handleDisconnect).not.toHaveBeenCalled(); }); }); + + describe('reconnect requested mid-attempt', () => { + // A full reconnect requested while a resume is already in flight (e.g. a server + // RECONNECT leave racing the resume) sets `fullReconnectOnNext` mid-attempt. A + // successful resume must not swallow it: it survives and is dispatched afterwards. + interface ReconnectInternals { + _isClosed: boolean; + attemptingReconnect: boolean; + clientConfiguration: unknown; + pcManager: unknown; + resumeConnection: (reason?: number) => Promise; + restartConnection: (regionUrl?: string) => Promise; + clearPendingReconnect: () => void; + handleDisconnect: (connection: string, reason?: number) => void; + attemptReconnect: (reason?: number) => Promise; + } + + function primeEngine() { + const engine = new RTCEngine(roomOptionDefaults); + const internals = engine as unknown as ReconnectInternals; + internals._isClosed = false; + internals.attemptingReconnect = false; + // avoid the "resume disabled / pcManager is NEW -> force full reconnect" escalation + internals.clientConfiguration = undefined; + internals.pcManager = { currentState: PCTransportState.CONNECTED }; + internals.clearPendingReconnect = vi.fn(); + const handleDisconnect = vi.fn(); + internals.handleDisconnect = handleDisconnect; + const restartConnection = vi.fn(async () => {}); + internals.restartConnection = restartConnection; + return { engine, internals, handleDisconnect, restartConnection }; + } + + it('dispatches a full reconnect when a resume succeeds but one was requested mid-attempt', async () => { + const { engine, internals, handleDisconnect, restartConnection } = primeEngine(); + engine.fullReconnectOnNext = false; + // the resume succeeds, but a RECONNECT leave arrives while it is in flight + internals.resumeConnection = vi.fn(async () => { + engine.fullReconnectOnNext = true; + }); + + await internals.attemptReconnect(); + + expect(internals.resumeConnection).toHaveBeenCalledTimes(1); + expect(restartConnection).not.toHaveBeenCalled(); + // the mid-attempt request survived the successful resume and was dispatched + expect(engine.fullReconnectOnNext).toBe(true); + expect(handleDisconnect).toHaveBeenCalledTimes(1); + expect(handleDisconnect).toHaveBeenCalledWith('reconnect'); + }); + + it('does not dispatch a follow-up after an ordinary successful resume', async () => { + const { engine, internals, handleDisconnect, restartConnection } = primeEngine(); + engine.fullReconnectOnNext = false; + internals.resumeConnection = vi.fn(async () => {}); + + await internals.attemptReconnect(); + + expect(internals.resumeConnection).toHaveBeenCalledTimes(1); + expect(restartConnection).not.toHaveBeenCalled(); + expect(engine.fullReconnectOnNext).toBe(false); + expect(handleDisconnect).not.toHaveBeenCalled(); + }); + + it('clears the flag and does not re-dispatch after a successful full reconnect', async () => { + const { engine, internals, handleDisconnect, restartConnection } = primeEngine(); + engine.fullReconnectOnNext = true; // enters as a full reconnect + internals.resumeConnection = vi.fn(async () => {}); + + await internals.attemptReconnect(); + + expect(restartConnection).toHaveBeenCalledTimes(1); + expect(internals.resumeConnection).not.toHaveBeenCalled(); + expect(engine.fullReconnectOnNext).toBe(false); + expect(handleDisconnect).not.toHaveBeenCalled(); + }); + + it('does not add a dispatch on top of the failure path retry', async () => { + const { engine, internals, handleDisconnect } = primeEngine(); + engine.fullReconnectOnNext = false; + // resume fails after a mid-attempt request; the catch path schedules the retry + internals.resumeConnection = vi.fn(async () => { + engine.fullReconnectOnNext = true; + throw new Error('resume failed'); + }); + + await internals.attemptReconnect(); + + // exactly one dispatch (from the catch), not a second one from the finally + expect(handleDisconnect).toHaveBeenCalledTimes(1); + }); + }); }); From 477e2797f018c0a902fddaebf12bf4063339488c Mon Sep 17 00:00:00 2001 From: lukasIO Date: Tue, 4 Aug 2026 12:07:05 +0200 Subject: [PATCH 06/11] more critical timers --- src/room/RTCEngine.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/room/RTCEngine.ts b/src/room/RTCEngine.ts index 46a7459919..95f15a3658 100644 --- a/src/room/RTCEngine.ts +++ b/src/room/RTCEngine.ts @@ -484,7 +484,7 @@ export default class RTCEngine extends (EventEmitter as new () => TypedEventEmit throw new TrackInvalidError('a track with the same ID has already been published'); } return new Promise((resolve, reject) => { - const publicationTimeout = setTimeout(() => { + const publicationTimeout = CriticalTimers.setTimeout(() => { delete this.pendingTrackResolvers[req.cid]; reject( ConnectionError.timeout('publication of local track timed out, no response from server'), @@ -492,11 +492,11 @@ export default class RTCEngine extends (EventEmitter as new () => TypedEventEmit }, 10_000); this.pendingTrackResolvers[req.cid] = { resolve: (info: TrackInfo) => { - clearTimeout(publicationTimeout); + CriticalTimers.clearTimeout(publicationTimeout); resolve(info); }, reject: () => { - clearTimeout(publicationTimeout); + CriticalTimers.clearTimeout(publicationTimeout); reject(new Error('Cancelled publication by calling unpublish')); }, }; @@ -1216,7 +1216,7 @@ export default class RTCEngine extends (EventEmitter as new () => TypedEventEmit private clearLostQualityTimeout() { if (this.lostQualityTimeout) { - clearTimeout(this.lostQualityTimeout); + CriticalTimers.clearTimeout(this.lostQualityTimeout); this.lostQualityTimeout = undefined; } } From 79d6244ae026be704328728ebc22883d5b8cc3ea Mon Sep 17 00:00:00 2001 From: lukasIO Date: Tue, 4 Aug 2026 12:18:51 +0200 Subject: [PATCH 07/11] fix: derive stuck-connecting timestamp from the actual CONNECTING transition verifyTransport() owned transportConnectingSince but leaked it on several exit paths (no pcManager, ws closed, the stuck verdict) and across cleanupPeerConnections(), so a timestamp captured before a failure survived the teardown/rebuild. Since Room pauses the reconcile during Resuming/Restarting and resumes it only after recovery, the first post-recovery tick that saw CONNECTING measured against the ancient timestamp and instantly reported the transport as stuck, causing a spurious extra reconnect. Record the entry time in the pcManager state-change handler (which only fires on real transitions, overwriting on each entry into CONNECTING) and make verifyTransport() a pure read that fails open when no timestamp is recorded. Also clear it in cleanupPeerConnections() for the paths where onStateChange is detached before teardown. Co-Authored-By: Claude Opus 4.8 --- src/room/RTCEngine.test.ts | 49 ++++++++++++++++++++++++++++++++++++++ src/room/RTCEngine.ts | 34 ++++++++++++++++---------- 2 files changed, 71 insertions(+), 12 deletions(-) diff --git a/src/room/RTCEngine.test.ts b/src/room/RTCEngine.test.ts index b04e4912b2..280e1a3a71 100644 --- a/src/room/RTCEngine.test.ts +++ b/src/room/RTCEngine.test.ts @@ -791,4 +791,53 @@ describe('RTCEngine', () => { expect(handleDisconnect).toHaveBeenCalledTimes(1); }); }); + + describe('verifyTransport stuck-connecting bound', () => { + interface VerifyInternals { + pcManager: unknown; + client: unknown; + transportConnectingSince?: number; + } + + function primeEngine(currentState: PCTransportState) { + const engine = new RTCEngine(roomOptionDefaults); + const internals = engine as unknown as VerifyInternals; + internals.pcManager = { currentState }; + internals.client = { ws: { readyState: WebSocket.OPEN } }; + return { engine, internals }; + } + + it('reports the transport stuck when connecting longer than peerConnectionTimeout', () => { + const { engine, internals } = primeEngine(PCTransportState.CONNECTING); + internals.transportConnectingSince = Date.now() - (engine.peerConnectionTimeout + 1_000); + + expect(engine.verifyTransport()).toBe(false); + }); + + it('tolerates a transport still within the connecting window', () => { + const { engine, internals } = primeEngine(PCTransportState.CONNECTING); + internals.transportConnectingSince = Date.now(); + + expect(engine.verifyTransport()).toBe(true); + }); + + it('fails open (and does not record a timestamp) when connecting is untracked', () => { + // verifyTransport is a pure read now: an unrecorded CONNECTING must not be treated as + // stuck, and the method must not seed a timestamp that could later leak across teardown. + const { engine, internals } = primeEngine(PCTransportState.CONNECTING); + internals.transportConnectingSince = undefined; + + expect(engine.verifyTransport()).toBe(true); + expect(internals.transportConnectingSince).toBeUndefined(); + }); + + it('does not measure a stale connecting timestamp while connected', () => { + const { engine, internals } = primeEngine(PCTransportState.CONNECTED); + // a leftover timestamp must not affect the CONNECTED verdict, and stays for the + // state-change handler to clear rather than being mutated here + internals.transportConnectingSince = Date.now() - 10 * engine.peerConnectionTimeout; + + expect(engine.verifyTransport()).toBe(true); + }); + }); }); diff --git a/src/room/RTCEngine.ts b/src/room/RTCEngine.ts index 95f15a3658..267dcac7a5 100644 --- a/src/room/RTCEngine.ts +++ b/src/room/RTCEngine.ts @@ -458,6 +458,8 @@ export default class RTCEngine extends (EventEmitter as new () => TypedEventEmit await this.pcManager?.close(); this.pcManager = undefined; + // the connecting timestamp belongs to the transports we just tore down + this.transportConnectingSince = undefined; this.reliableReceivedState.clear(); } @@ -581,6 +583,16 @@ export default class RTCEngine extends (EventEmitter as new () => TypedEventEmit this.pcManager.onStateChange = async (connectionState, publisherState, subscriberState) => { this.log.debug(`primary PC state changed ${connectionState}`); + // Record when the primary transport actually entered CONNECTING so + // verifyTransport() can bound how long we tolerate it. Deriving it from the + // real transition (this handler only fires on state changes) rather than from + // observation time keeps it from going stale across peer-connection rebuilds. + if (connectionState === PCTransportState.CONNECTING) { + this.transportConnectingSince = Date.now(); + } else { + this.transportConnectingSince = undefined; + } + if (['closed', 'disconnected', 'failed'].includes(publisherState)) { // reset publisher connection promise this.publisherConnectionPromise = undefined; @@ -1695,7 +1707,6 @@ export default class RTCEngine extends (EventEmitter as new () => TypedEventEmit PCTransportState.CONNECTED, ]; if (!allowedConnectionStates.includes(state)) { - this.transportConnectingSince = undefined; return false; } @@ -1705,18 +1716,17 @@ export default class RTCEngine extends (EventEmitter as new () => TypedEventEmit } // A transport stuck in CONNECTING never reaches CONNECTED nor reports FAILED, so it would - // otherwise look healthy forever; bound how long we tolerate it. - if (state === PCTransportState.CONNECTING) { - const now = Date.now(); - if (this.transportConnectingSince === undefined) { - this.transportConnectingSince = now; - } else if (now - this.transportConnectingSince > this.peerConnectionTimeout) { - this.log.warn('transport stuck in connecting state', this.logContext); - return false; - } - return true; + // otherwise look healthy forever; bound how long we tolerate it. The entry time is recorded + // in the pcManager state-change handler (see configure()), so this is a pure read — an + // unrecorded CONNECTING fails open rather than measuring against a stale timestamp. + if ( + state === PCTransportState.CONNECTING && + this.transportConnectingSince !== undefined && + Date.now() - this.transportConnectingSince > this.peerConnectionTimeout + ) { + this.log.warn('transport stuck in connecting state', this.logContext); + return false; } - this.transportConnectingSince = undefined; return true; } From b5e8102d9e6761d3f5097884f2d971c701cd8a9f Mon Sep 17 00:00:00 2001 From: lukasIO Date: Tue, 4 Aug 2026 12:31:04 +0200 Subject: [PATCH 08/11] fix: cancel pending Lost-quality countdown when a reconnect begins scheduleLostQualityReconnect's countdown was only cancelled by a non-LOST quality update or close(), not when a reconnect starts. A countdown armed from a previous session's LOST verdict could fire shortly after a fast reconnect completed and force another full reconnect, before the server had evaluated the new session. Clear it at the start of attemptReconnect so the countdown always reflects the current session. attemptReconnect (rather than cleanupPeerConnections) is the single entry point that also covers the resume path, which keeps its peer connections. Co-Authored-By: Claude Opus 4.8 --- src/room/RTCEngine.test.ts | 62 ++++++++++++++++++++++++++++++++++++++ src/room/RTCEngine.ts | 6 ++++ 2 files changed, 68 insertions(+) diff --git a/src/room/RTCEngine.test.ts b/src/room/RTCEngine.test.ts index 280e1a3a71..5b48e388f5 100644 --- a/src/room/RTCEngine.test.ts +++ b/src/room/RTCEngine.test.ts @@ -840,4 +840,66 @@ describe('RTCEngine', () => { expect(engine.verifyTransport()).toBe(true); }); }); + + describe('Lost-quality countdown across reconnects', () => { + // A Lost-quality countdown armed by the previous session must not survive a reconnect + // and fire against the new session before the server has re-evaluated it. + const LOST_TIMEOUT_MS = 5_000; + + beforeEach(() => { + vi.useFakeTimers(); + }); + + afterEach(() => { + vi.useRealTimers(); + }); + + it('cancels a pending Lost countdown when a reconnect attempt begins', async () => { + const engine = new RTCEngine(roomOptionDefaults); + const internals = engine as unknown as { + _isClosed: boolean; + participantSid: string; + pcState: number; + attemptingReconnect: boolean; + clientConfiguration: unknown; + pcManager: unknown; + lostQualityTimeout?: ReturnType; + resumeConnection: (reason?: number) => Promise; + restartConnection: () => Promise; + clearPendingReconnect: () => void; + handleDisconnect: (connection: string, reason?: number) => void; + handleLocalConnectionQuality: (update: unknown) => void; + attemptReconnect: (reason?: number) => Promise; + }; + internals._isClosed = false; + internals.participantSid = 'PA_local'; + internals.pcState = 1; // PCState.Connected — the guards the countdown checks would pass + internals.attemptingReconnect = false; + internals.clientConfiguration = undefined; + internals.pcManager = { + currentState: PCTransportState.CONNECTED, + publisher: { getSenders: () => [{ track: { readyState: 'live' } }] }, + }; + internals.clearPendingReconnect = vi.fn(); + const handleDisconnect = vi.fn(); + internals.handleDisconnect = handleDisconnect; + internals.resumeConnection = vi.fn(async () => {}); + internals.restartConnection = vi.fn(async () => {}); + + // a LOST verdict from the (soon-to-be-previous) session arms the countdown + internals.handleLocalConnectionQuality({ + updates: [{ participantSid: 'PA_local', quality: ProtoConnectionQuality.LOST }], + }); + expect(internals.lostQualityTimeout).toBeDefined(); + + // a reconnect begins and completes (resume) before the countdown elapses + engine.fullReconnectOnNext = false; + await internals.attemptReconnect(); + + // the stale countdown was cancelled and cannot fire against the reconnected session + expect(internals.lostQualityTimeout).toBeUndefined(); + vi.advanceTimersByTime(LOST_TIMEOUT_MS); + expect(handleDisconnect).not.toHaveBeenCalled(); + }); + }); }); diff --git a/src/room/RTCEngine.ts b/src/room/RTCEngine.ts index 267dcac7a5..950f096d7b 100644 --- a/src/room/RTCEngine.ts +++ b/src/room/RTCEngine.ts @@ -1261,6 +1261,12 @@ export default class RTCEngine extends (EventEmitter as new () => TypedEventEmit this.log.warn('already attempting reconnect, returning early'); return; } + + // A pending Lost-quality countdown belongs to the session we're now leaving; cancel it so + // it can't fire against the reconnected session before the server has evaluated it. (A resume + // keeps the peer connections, so cleanupPeerConnections wouldn't cover this path.) + this.clearLostQualityTimeout(); + if ( this.clientConfiguration?.resumeConnection === ClientConfigSetting.DISABLED || // signaling state could change to closed due to hardware sleep From e087564ac1c1793c612960214b73cc6328a5c99d Mon Sep 17 00:00:00 2001 From: lukasIO Date: Tue, 4 Aug 2026 12:42:31 +0200 Subject: [PATCH 09/11] fix: preserve a full-reconnect request that arrives during a full reconnect MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit attemptReconnect only preserved a full-reconnect request that arrived during a resume. When the attempt itself was a full reconnect, the success path unconditionally cleared fullReconnectOnNext, discarding a request (e.g. a server RECONNECT leave) that arrived while restartConnection() was running — its zero-delay reconnect was a no-op under the attemptingReconnect guard and then wiped by clearPendingReconnect(). Consume the flag at the start of the attempt (read into a local, reset to false) so any true value seen afterwards is unambiguously a new request, handled by the existing finally dispatch for both paths. The catch still escalates a failed full reconnect back to a full reconnect. Co-Authored-By: Claude Opus 4.8 --- src/room/RTCEngine.test.ts | 17 +++++++++++++++++ src/room/RTCEngine.ts | 26 ++++++++++++++------------ 2 files changed, 31 insertions(+), 12 deletions(-) diff --git a/src/room/RTCEngine.test.ts b/src/room/RTCEngine.test.ts index 5b48e388f5..265bc541c4 100644 --- a/src/room/RTCEngine.test.ts +++ b/src/room/RTCEngine.test.ts @@ -776,6 +776,23 @@ describe('RTCEngine', () => { expect(handleDisconnect).not.toHaveBeenCalled(); }); + it('dispatches a follow-up when a full reconnect succeeds but one was requested mid-attempt', async () => { + const { engine, internals, handleDisconnect, restartConnection } = primeEngine(); + engine.fullReconnectOnNext = true; // enters as a full reconnect + // a new RECONNECT request arrives while restartConnection is running + restartConnection.mockImplementationOnce(async () => { + engine.fullReconnectOnNext = true; + }); + + await internals.attemptReconnect(); + + expect(restartConnection).toHaveBeenCalledTimes(1); + // the mid-restart request survived the successful full reconnect and was dispatched + expect(engine.fullReconnectOnNext).toBe(true); + expect(handleDisconnect).toHaveBeenCalledTimes(1); + expect(handleDisconnect).toHaveBeenCalledWith('reconnect'); + }); + it('does not add a dispatch on top of the failure path retry', async () => { const { engine, internals, handleDisconnect } = primeEngine(); engine.fullReconnectOnNext = false; diff --git a/src/room/RTCEngine.ts b/src/room/RTCEngine.ts index 950f096d7b..9785f1fe78 100644 --- a/src/room/RTCEngine.ts +++ b/src/room/RTCEngine.ts @@ -1276,22 +1276,22 @@ export default class RTCEngine extends (EventEmitter as new () => TypedEventEmit this.fullReconnectOnNext = true; } + // Consume the flag up front: capture whether this attempt is a full reconnect, then reset + // it. From here on a `true` value unambiguously represents a *new* full-reconnect request + // that arrived while this attempt was running (e.g. a server RECONNECT leave), which the + // finally block dispatches — for both the resume and full-reconnect paths. + const fullReconnect = this.fullReconnectOnNext; + this.fullReconnectOnNext = false; + let succeeded = false; - let performedFullReconnect = false; try { this.attemptingReconnect = true; - if (this.fullReconnectOnNext) { - performedFullReconnect = true; + if (fullReconnect) { await this.restartConnection(); } else { await this.resumeConnection(reason); } this.clearPendingReconnect(); - // Only clear the flag if we actually did a full reconnect, so a full reconnect requested - // mid-attempt (e.g. a server leave during a resume) survives a successful resume. - if (performedFullReconnect) { - this.fullReconnectOnNext = false; - } succeeded = true; } catch (e) { this.reconnectAttempts += 1; @@ -1300,8 +1300,9 @@ export default class RTCEngine extends (EventEmitter as new () => TypedEventEmit this.log.debug('received unrecoverable error', { error: e }); // unrecoverable recoverable = false; - } else if (!(e instanceof SignalReconnectError)) { - // cannot resume + } else if (fullReconnect || !(e instanceof SignalReconnectError)) { + // a failed full reconnect stays a full reconnect; a failed resume can only be + // resumed again for a signal-level error, otherwise it escalates this.fullReconnectOnNext = true; } @@ -1319,8 +1320,9 @@ export default class RTCEngine extends (EventEmitter as new () => TypedEventEmit } finally { this.attemptingReconnect = false; - // A full reconnect requested mid-attempt (e.g. a `RECONNECT` leave during a resume) that - // a successful attempt didn't act on; dispatch it now (the failure path already retries). + // A full reconnect requested while this attempt was running (e.g. a `RECONNECT` leave + // during a resume or a restart) that a successful attempt didn't act on; dispatch it now + // (the failure path already retries). if (succeeded && this.fullReconnectOnNext && !this._isClosed) { this.log.debug('full reconnect requested during in-progress attempt, dispatching'); this.handleDisconnect('reconnect'); From 99e1081ec998ad48811ea19da7a5d74e53884239 Mon Sep 17 00:00:00 2001 From: lukasIO Date: Tue, 4 Aug 2026 15:19:31 +0200 Subject: [PATCH 10/11] increase lost timeout --- src/room/RTCEngine.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/room/RTCEngine.ts b/src/room/RTCEngine.ts index 9785f1fe78..f571ae09a6 100644 --- a/src/room/RTCEngine.ts +++ b/src/room/RTCEngine.ts @@ -114,7 +114,7 @@ const leaveReconnect = 'leave-reconnect'; * How long local connection quality must stay `LOST` while connected and publishing before we * force a full reconnect — `LOST` is the server's verdict that it isn't receiving our media. */ -const connectionQualityLostTimeout = 5 * 1000; +const connectionQualityLostTimeout = 10 * 1000; const reliabeReceiveStateTTL = 30_000; const initialMediaSectionsAudio = 3; From 3e59e3cbddc2ce150f614f5abd74a4f6fb44f9b8 Mon Sep 17 00:00:00 2001 From: lukasIO Date: Tue, 4 Aug 2026 16:13:06 +0200 Subject: [PATCH 11/11] fix test case timeout --- src/room/RTCEngine.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/room/RTCEngine.test.ts b/src/room/RTCEngine.test.ts index 265bc541c4..1dc28a2cf6 100644 --- a/src/room/RTCEngine.test.ts +++ b/src/room/RTCEngine.test.ts @@ -595,7 +595,7 @@ describe('RTCEngine', () => { // a full reconnect. (A genuine `LOST` can't be produced from a browser page — any // live sender keeps RTCP flowing — so the behavior is unit tested here rather than // in the e2e suite.) `connectionQualityLostTimeout` in RTCEngine.ts is 5s. - const LOST_TIMEOUT_MS = 5_000; + const LOST_TIMEOUT_MS = 10_000; const LOCAL_SID = 'PA_local'; beforeEach(() => {