Skip to content

Commit c2eb132

Browse files
fix: stop putting the subscriber into restartingIce on a reconnect (#2054)
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 4a900ed commit c2eb132

3 files changed

Lines changed: 60 additions & 4 deletions

File tree

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
---
2+
'livekit-client': patch
3+
---
4+
5+
Fix the subscriber silently buffering remote ICE candidates after a reconnect
6+
7+
`triggerIceRestart` put the subscriber into `restartingIce` on every reconnect, but only
8+
`setRemoteDescription` clears that — and the server re-offers the subscriber only when the
9+
reconnect moved the participant to a different node. After an ordinary signal-only resume no
10+
offer arrives, so the flag stayed set for the lifetime of the transport and every subsequent
11+
remote candidate was queued instead of applied, leaving the subscriber unable to adopt any new
12+
network path the server proposed. The subscriber no longer enters that state: the server does
13+
not send candidates ahead of the offer that introduces them, so queueing them gains nothing.

src/room/PCTransportManager.test.ts

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -279,3 +279,38 @@ describe('PCTransportManager.negotiate', () => {
279279
await expect(p).resolves.toBeUndefined();
280280
});
281281
});
282+
283+
describe('PCTransportManager.triggerIceRestart', () => {
284+
let originalRTCPeerConnection: unknown;
285+
286+
beforeEach(() => {
287+
originalRTCPeerConnection = (globalThis as unknown as { RTCPeerConnection?: unknown })
288+
.RTCPeerConnection;
289+
(globalThis as unknown as { RTCPeerConnection: unknown }).RTCPeerConnection = StubPC;
290+
});
291+
292+
afterEach(() => {
293+
(globalThis as unknown as { RTCPeerConnection: unknown }).RTCPeerConnection =
294+
originalRTCPeerConnection;
295+
});
296+
297+
/**
298+
* The subscriber must keep applying remote candidates across a reconnect.
299+
*
300+
* Putting it into `restartingIce` would queue them until a new remote description arrives —
301+
* but the server only re-offers the subscriber when the reconnect moved us to another node,
302+
* so on an ordinary signal-only resume nothing would ever flush that queue, and the
303+
* transport would stop adopting new network paths for the rest of the session. The server
304+
* also never sends candidates ahead of the offer that introduces them, so queueing buys
305+
* nothing in exchange.
306+
*/
307+
it('does not stop the subscriber applying remote candidates', async () => {
308+
const manager = new PCTransportManager('subscriber-primary', {});
309+
const publisher = new FakePublisher();
310+
(manager as unknown as { publisher: FakePublisher }).publisher = publisher;
311+
312+
await manager.triggerIceRestart();
313+
314+
expect(manager.subscriber?.restartingIce).toBe(false);
315+
});
316+
});

src/room/PCTransportManager.ts

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -162,12 +162,20 @@ export class PCTransportManager {
162162
this.updateState();
163163
}
164164

165+
/**
166+
* Restarts ICE on the transports that need it. Only the publisher: the server restarts the
167+
* subscriber's ICE itself and follows with a fresh offer.
168+
*
169+
* The subscriber deliberately does NOT enter `restartingIce` here. Queueing its remote
170+
* candidates would guard against candidates for a new generation arriving before the offer
171+
* that introduces it, but the server does not send them in that order -- on a same-node
172+
* resume it buffers them until the offer has gone out, and on a reconnect that lands on
173+
* another node it withholds subscriber candidates until immediately before creating the
174+
* offer. Setting the flag only risks withholding candidates during the window that decides
175+
* whether the reconnect succeeded.
176+
*/
165177
async triggerIceRestart() {
166178
this.iceLog.warn('triggering ICE restart');
167-
if (this.subscriber) {
168-
this.subscriber.restartingIce = true;
169-
}
170-
// only restart publisher if it's needed
171179
if (this.needsPublisher) {
172180
await this.createAndSendPublisherOffer({ iceRestart: true });
173181
}

0 commit comments

Comments
 (0)