Skip to content

Commit 254bf7b

Browse files
temp fix
1 parent dff7959 commit 254bf7b

9 files changed

Lines changed: 358 additions & 108 deletions

packages/webrtc-client/src/tracks/Remote.ts

Lines changed: 49 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,9 @@ export class Remote {
4444
return remoteTrack;
4545
};
4646

47+
public getTrackByMidOrNull = (mid: string): RemoteTrack | null =>
48+
Object.values(this.remoteTracks).find((remote) => remote.mLineId === mid) ?? null;
49+
4750
public addTracks = (endpointId: EndpointId, tracks: Record<TrackId, MediaEvent_Track>) => {
4851
const endpoint: EndpointWithTrackContext | undefined = this.remoteEndpoints[endpointId];
4952

@@ -72,10 +75,18 @@ export class Remote {
7275

7376
private removeRemoteTrack = (trackId: TrackId) => {
7477
const remoteTrack = this.remoteTracks[trackId];
75-
if (!remoteTrack) throw new Error(`Track ${trackId} not found`);
78+
if (!remoteTrack) {
79+
console.warn(`Track ${trackId} not found, skipping remove`);
80+
return;
81+
}
7682

7783
const remoteEndpoint = this.remoteEndpoints[remoteTrack.trackContext.endpoint.id];
78-
if (!remoteEndpoint) throw new Error(`Endpoint ${remoteTrack.trackContext.endpoint.id} not found`);
84+
if (!remoteEndpoint) {
85+
console.warn(`Endpoint ${remoteTrack.trackContext.endpoint.id} not found, skipping remove`);
86+
delete this.remoteTracks[trackId];
87+
this.emit('trackRemoved', remoteTrack.trackContext);
88+
return;
89+
}
7990

8091
remoteEndpoint.tracks.delete(trackId);
8192
delete this.remoteTracks[trackId];
@@ -110,7 +121,10 @@ export class Remote {
110121

111122
public updateRemoteEndpoint = (endpointId: string, metadataJson?: MetadataJson) => {
112123
const endpoint: EndpointWithTrackContext | undefined = this.remoteEndpoints[endpointId];
113-
if (!endpoint) throw new Error(`Endpoint ${endpointId} not found`);
124+
if (!endpoint) {
125+
console.warn(`Endpoint ${endpointId} not found, skipping update`);
126+
return;
127+
}
114128

115129
endpoint.metadata = metadataJson ? JSON.parse(metadataJson) : undefined;
116130

@@ -119,7 +133,10 @@ export class Remote {
119133

120134
public removeRemoteEndpoint = (endpointId: EndpointId) => {
121135
const endpoint: EndpointWithTrackContext | undefined = this.remoteEndpoints[endpointId];
122-
if (!endpoint) throw new Error(`Endpoint ${endpointId} not found`);
136+
if (!endpoint) {
137+
console.warn(`Endpoint ${endpointId} not found, skipping remove`);
138+
return;
139+
}
123140

124141
const trackIds = [...endpoint.tracks.values()].map(({ trackId }) => trackId);
125142

@@ -131,10 +148,16 @@ export class Remote {
131148
};
132149

133150
public updateRemoteTrack = (endpointId: string, trackId: string, metadataJson?: MetadataJson) => {
134-
if (!this.remoteEndpoints[endpointId]) throw new Error(`Endpoint ${endpointId} not found`);
151+
if (!this.remoteEndpoints[endpointId]) {
152+
console.warn(`Endpoint ${endpointId} not found, skipping track update`);
153+
return;
154+
}
135155

136156
const remoteTrack = this.remoteTracks[trackId];
137-
if (!remoteTrack) throw new Error(`Track ${trackId} not found`);
157+
if (!remoteTrack) {
158+
console.warn(`Track ${trackId} not found, skipping update`);
159+
return;
160+
}
138161

139162
remoteTrack.trackContext.metadata = metadataJson ? JSON.parse(metadataJson) : undefined;
140163

@@ -143,7 +166,10 @@ export class Remote {
143166

144167
public disableRemoteTrackEncoding = (trackId: TrackId, encoding: Variant) => {
145168
const remoteTrack = this.remoteTracks[trackId];
146-
if (!remoteTrack) throw new Error(`Track ${trackId} not found`);
169+
if (!remoteTrack) {
170+
console.warn(`Track ${trackId} not found, skipping disableEncoding`);
171+
return;
172+
}
147173

148174
remoteTrack.disableTrackEncoding(encoding);
149175

@@ -152,7 +178,10 @@ export class Remote {
152178

153179
public enableRemoteTrackEncoding = (trackId: TrackId, encoding: Variant) => {
154180
const remoteTrack = this.remoteTracks[trackId];
155-
if (!remoteTrack) throw new Error(`Track ${trackId} not found`);
181+
if (!remoteTrack) {
182+
console.warn(`Track ${trackId} not found, skipping enableEncoding`);
183+
return;
184+
}
156185

157186
remoteTrack.enableTrackEncoding(encoding);
158187

@@ -161,7 +190,10 @@ export class Remote {
161190

162191
public setRemoteTrackEncoding = (trackId: TrackId, encoding: Variant, reason?: EncodingReason) => {
163192
const remoteTrack = this.remoteTracks[trackId];
164-
if (!remoteTrack) throw new Error(`Track ${trackId} not found`);
193+
if (!remoteTrack) {
194+
console.warn(`Track ${trackId} not found, skipping setEncoding`);
195+
return;
196+
}
165197

166198
remoteTrack.trackContext.encoding = encoding;
167199
remoteTrack.trackContext.encodingReason = reason;
@@ -171,7 +203,10 @@ export class Remote {
171203

172204
public setRemoteTrackVadStatus = (trackId: TrackId, vadStatus: MediaEvent_VadNotification_Status) => {
173205
const remoteTrack = this.remoteTracks[trackId];
174-
if (!remoteTrack) throw new Error(`Track ${trackId} not found`);
206+
if (!remoteTrack) {
207+
console.warn(`Track ${trackId} not found, skipping vad status update`);
208+
return;
209+
}
175210

176211
let nextStatus: VadStatus | null = null;
177212

@@ -208,7 +243,10 @@ export class Remote {
208243

209244
public setTargetRemoteTrackEncoding = (trackId: TrackId, variant: Variant) => {
210245
const remoteTrack = this.remoteTracks[trackId];
211-
if (!remoteTrack) throw new Error(`Track ${trackId} not found`);
246+
if (!remoteTrack) {
247+
console.warn(`Track ${trackId} not found, skipping setTargetEncoding`);
248+
return;
249+
}
212250

213251
try {
214252
remoteTrack.setTargetTrackEncoding(variant);

packages/webrtc-client/src/webRTCEndpoint.ts

Lines changed: 54 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,8 @@ export class WebRTCEndpoint extends (EventEmitter as new () => TypedEmitter<Requ
5555

5656
private clearConnectionCallbacks: (() => void) | null = null;
5757

58+
private pendingRemovals: Array<() => void> = [];
59+
5860
constructor(props: WebRTCEndpointProps = {}) {
5961
super();
6062

@@ -201,11 +203,26 @@ export class WebRTCEndpoint extends (EventEmitter as new () => TypedEmitter<Requ
201203

202204
private onTrackReady = (event: RTCTrackEvent) => {
203205
const stream = event.streams[0];
204-
if (!stream) throw new Error('Cannot find media stream');
206+
const mid = event.transceiver.mid ?? null;
205207

206-
const mid = event.transceiver.mid!;
208+
if (!stream) {
209+
this.logger.warn('ontrack without a media stream, ignoring', { mid, pcId: this.connectionManager?.pcId });
210+
return;
211+
}
207212

208-
const remoteTrack = this.remote.getTrackByMid(mid);
213+
if (mid === null) {
214+
this.logger.warn('ontrack without a mid, ignoring', { pcId: this.connectionManager?.pcId });
215+
return;
216+
}
217+
218+
const remoteTrack = this.remote.getTrackByMidOrNull(mid);
219+
if (!remoteTrack) {
220+
this.logger.warn('ontrack for unknown mid, likely a removal race', {
221+
mid,
222+
pcId: this.connectionManager?.pcId,
223+
});
224+
return;
225+
}
209226

210227
remoteTrack.setReady(stream, event.track);
211228

@@ -293,10 +310,11 @@ export class WebRTCEndpoint extends (EventEmitter as new () => TypedEmitter<Requ
293310

294311
if (this.getEndpointId() === endpointId) return;
295312

296-
this.remote.removeTracks(trackIds);
313+
this.scheduleRemoval(() => this.remote.removeTracks(trackIds));
297314
} else if (event.sdpAnswer) {
298-
this.localTrackManager.ongoingRenegotiation = false;
299315
await this.onSdpAnswer(event.sdpAnswer);
316+
this.localTrackManager.ongoingRenegotiation = false;
317+
this.flushPendingRemovals();
300318
this.commandsQueue.processNextCommand();
301319
} else if (event.candidate) {
302320
await this.onRemoteCandidate(event.candidate);
@@ -316,7 +334,7 @@ export class WebRTCEndpoint extends (EventEmitter as new () => TypedEmitter<Requ
316334

317335
if (this.getEndpointId() === endpointId) return;
318336

319-
this.remote.removeRemoteEndpoint(endpointId);
337+
this.scheduleRemoval(() => this.remote.removeRemoteEndpoint(endpointId));
320338
} else if (event.endpointUpdated) {
321339
const { endpointId, metadataJson } = event.endpointUpdated;
322340
if (this.getEndpointId() === endpointId) return;
@@ -352,6 +370,34 @@ export class WebRTCEndpoint extends (EventEmitter as new () => TypedEmitter<Requ
352370
}
353371
};
354372

373+
private scheduleRemoval = (action: () => void) => {
374+
if (this.localTrackManager.ongoingRenegotiation) {
375+
this.pendingRemovals.push(action);
376+
return;
377+
}
378+
379+
try {
380+
action();
381+
} catch (err) {
382+
this.logger.warn('removal action failed', err);
383+
}
384+
};
385+
386+
private flushPendingRemovals = () => {
387+
if (this.pendingRemovals.length === 0) return;
388+
389+
const removals = this.pendingRemovals;
390+
this.pendingRemovals = [];
391+
392+
for (const action of removals) {
393+
try {
394+
action();
395+
} catch (err) {
396+
this.logger.warn('deferred removal action failed', err);
397+
}
398+
}
399+
};
400+
355401
private onSdpAnswer = async (data: MediaEvent_SdpAnswer) => {
356402
// eslint-disable-next-line no-console
357403
console.log('[DEBUG transceivers] onSdpAnswer', {
@@ -780,6 +826,8 @@ export class WebRTCEndpoint extends (EventEmitter as new () => TypedEmitter<Requ
780826
this.dataChannelManager?.cleanup();
781827
}
782828

829+
this.pendingRemovals = [];
830+
783831
this.connectionManager = undefined;
784832
};
785833

packages/webrtc-client/tests/events/encodingSwitchedEvent.test.ts

Lines changed: 12 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { expect, it } from 'vitest';
1+
import { expect, it, vi } from 'vitest';
22

33
import { Variant, WebRTCEndpoint } from '../../src';
44
import { serializeServerMediaEvent } from '../../src/mediaEvent';
@@ -32,27 +32,26 @@ it('Change existing track encoding', () => {
3232
expect(finalTrackEncoding).toBe(Variant.VARIANT_MEDIUM);
3333
});
3434

35-
it('Changing track encoding when endpoint exist but track does not exist', () => {
35+
it('Changing track encoding when endpoint exist but track does not exist warns instead of throwing', async () => {
3636
// Given
3737
const webRTCEndpoint = new WebRTCEndpoint();
38+
const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {});
3839

3940
setupRoom(webRTCEndpoint, exampleEndpointId, exampleTrackId);
4041

4142
const initialTrackEncoding = webRTCEndpoint.getRemoteTracks()[exampleTrackId]!.encoding;
4243
expect(initialTrackEncoding).toBe(undefined);
4344

4445
// When
45-
expect(() =>
46-
webRTCEndpoint.receiveMediaEvent(
47-
serializeServerMediaEvent({
48-
trackVariantSwitched: createEncodingSwitchedEvent(
49-
exampleEndpointId,
50-
notExistingTrackId,
51-
Variant.VARIANT_MEDIUM,
52-
),
53-
}),
54-
),
55-
).rejects.toThrow(`Track ${notExistingTrackId} not found`);
46+
await webRTCEndpoint.receiveMediaEvent(
47+
serializeServerMediaEvent({
48+
trackVariantSwitched: createEncodingSwitchedEvent(exampleEndpointId, notExistingTrackId, Variant.VARIANT_MEDIUM),
49+
}),
50+
);
51+
52+
// Then
53+
expect(warnSpy).toHaveBeenCalledWith(expect.stringContaining(`Track ${notExistingTrackId} not found`));
54+
warnSpy.mockRestore();
5655
});
5756

5857
it('Changing track encoding when endpoint does not exist but track exist in other endpoint', () => {

packages/webrtc-client/tests/events/endpointRemovedEvent.test.ts

Lines changed: 32 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { expect, it } from 'vitest';
1+
import { expect, it, vi } from 'vitest';
22

33
import { WebRTCEndpoint } from '../../src';
44
import { serializeServerMediaEvent } from '../../src/mediaEvent';
@@ -12,23 +12,24 @@ import {
1212
import { mockRTCPeerConnection } from '../mocks';
1313
import { setupRoom } from '../utils';
1414

15-
it('Remove the endpoint that does not exist', () => {
15+
it('Remove the endpoint that does not exist warns instead of throwing', async () => {
1616
// Given
1717
mockRTCPeerConnection();
1818
const webRTCEndpoint = new WebRTCEndpoint();
19+
const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {});
1920

2021
webRTCEndpoint.receiveMediaEvent(
2122
serializeServerMediaEvent({ connected: createConnectedEventWithOneEndpoint(exampleEndpointId) }),
2223
);
2324

2425
// When
25-
expect(() =>
26-
webRTCEndpoint.receiveMediaEvent(
27-
serializeServerMediaEvent({ endpointRemoved: createEndpointRemoved(notExistingEndpointId) }),
28-
),
29-
)
30-
// Then
31-
.rejects.toThrow(`Endpoint ${notExistingEndpointId} not found`);
26+
await webRTCEndpoint.receiveMediaEvent(
27+
serializeServerMediaEvent({ endpointRemoved: createEndpointRemoved(notExistingEndpointId) }),
28+
);
29+
30+
// Then
31+
expect(warnSpy).toHaveBeenCalledWith(expect.stringContaining(`Endpoint ${notExistingEndpointId} not found`));
32+
warnSpy.mockRestore();
3233
});
3334

3435
it('Remove current peer', () =>
@@ -71,15 +72,16 @@ it('Remove existing endpoint should remove it from remote endpoints', () => {
7172
expect(Object.values(endpoints).length).toBe(0);
7273
});
7374

74-
it('Remove existing endpoint should remove all tracks', () => {
75+
it('Remove existing endpoint should remove all tracks', async () => {
7576
// Given
7677
mockRTCPeerConnection();
7778
const webRTCEndpoint = new WebRTCEndpoint();
7879

79-
setupRoom(webRTCEndpoint, exampleEndpointId, exampleTrackId);
80+
await setupRoom(webRTCEndpoint, exampleEndpointId, exampleTrackId);
81+
(webRTCEndpoint as any).localTrackManager.ongoingRenegotiation = false;
8082

8183
// When
82-
webRTCEndpoint.receiveMediaEvent(
84+
await webRTCEndpoint.receiveMediaEvent(
8385
serializeServerMediaEvent({ endpointRemoved: createEndpointRemoved(exampleEndpointId) }),
8486
);
8587

@@ -88,22 +90,24 @@ it('Remove existing endpoint should remove all tracks', () => {
8890
expect(Object.values(tracks).length).toBe(0);
8991
});
9092

91-
it('Remove existing endpoint should emit trackRemoved event', () =>
92-
new Promise((done) => {
93-
// Given
94-
mockRTCPeerConnection();
95-
const webRTCEndpoint = new WebRTCEndpoint();
93+
it('Remove existing endpoint should emit trackRemoved event', async () => {
94+
// Given
95+
mockRTCPeerConnection();
96+
const webRTCEndpoint = new WebRTCEndpoint();
9697

97-
setupRoom(webRTCEndpoint, exampleEndpointId, exampleTrackId);
98+
await setupRoom(webRTCEndpoint, exampleEndpointId, exampleTrackId);
99+
(webRTCEndpoint as any).localTrackManager.ongoingRenegotiation = false;
98100

99-
webRTCEndpoint.on('trackRemoved', (trackContext) => {
100-
// Then
101-
expect(trackContext.trackId).toBe(exampleTrackId);
102-
done('');
103-
});
101+
const removed: string[] = [];
102+
webRTCEndpoint.on('trackRemoved', (trackContext) => {
103+
removed.push(trackContext.trackId);
104+
});
104105

105-
// When
106-
webRTCEndpoint.receiveMediaEvent(
107-
serializeServerMediaEvent({ endpointRemoved: createEndpointRemoved(exampleEndpointId) }),
108-
);
109-
}));
106+
// When
107+
await webRTCEndpoint.receiveMediaEvent(
108+
serializeServerMediaEvent({ endpointRemoved: createEndpointRemoved(exampleEndpointId) }),
109+
);
110+
111+
// Then
112+
expect(removed).toEqual([exampleTrackId]);
113+
});

0 commit comments

Comments
 (0)