Skip to content

Commit 984703d

Browse files
authored
fix: extended telemetry data for the signal websocket (#1881)
Adds more telemetry data collection for the signal web socket.
1 parent caaac29 commit 984703d

File tree

2 files changed

+29
-20
lines changed

2 files changed

+29
-20
lines changed

packages/client/src/StreamSfuClient.ts

Lines changed: 19 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -274,6 +274,7 @@ export class StreamSfuClient {
274274
this.signalWs = createWebSocketSignalChannel({
275275
tag: this.tag,
276276
endpoint: `${this.credentials.server.ws_endpoint}?${new URLSearchParams(params).toString()}`,
277+
tracer: this.tracer,
277278
onMessage: (message) => {
278279
this.lastMessageTimestamp = new Date();
279280
this.scheduleConnectionCheck();
@@ -285,10 +286,14 @@ export class StreamSfuClient {
285286
},
286287
});
287288

289+
let timeoutId: NodeJS.Timeout;
288290
this.signalReady = makeSafePromise(
289291
Promise.race<WebSocket>([
290292
new Promise((resolve, reject) => {
293+
let didOpen = false;
291294
const onOpen = () => {
295+
didOpen = true;
296+
clearTimeout(timeoutId);
292297
this.signalWs.removeEventListener('open', onOpen);
293298
resolve(this.signalWs);
294299
};
@@ -298,28 +303,28 @@ export class StreamSfuClient {
298303
this.signalWs.addEventListener('close', (e) => {
299304
this.handleWebSocketClose(e);
300305
// Normally, this shouldn't have any effect, because WS should never emit 'close'
301-
// before emitting 'open'. However, strager things have happened, and we don't
302-
// want to leave signalReady in pending state.
303-
reject(
304-
new Error(`SFU WS closed or connection can't be established`),
305-
);
306+
// before emitting 'open'. However, stranger things have happened, and we don't
307+
// want to leave signalReady in a pending state.
308+
const message = didOpen
309+
? `SFU WS closed: ${e.code} ${e.reason}`
310+
: `SFU WS connection can't be established: ${e.code} ${e.reason}`;
311+
this.tracer?.trace('signal.close', message);
312+
clearTimeout(timeoutId);
313+
reject(new Error(message));
306314
});
307315
}),
308316

309317
new Promise((resolve, reject) => {
310-
setTimeout(
311-
() => reject(new Error('SFU WS connection timed out')),
312-
this.joinResponseTimeout,
313-
);
318+
timeoutId = setTimeout(() => {
319+
const message = `SFU WS connection failed to open after ${this.joinResponseTimeout}ms`;
320+
this.tracer?.trace('signal.timeout', message);
321+
reject(new Error(message));
322+
}, this.joinResponseTimeout);
314323
}),
315324
]),
316325
);
317326
};
318327

319-
private cleanUpWebSocket = () => {
320-
this.signalWs.removeEventListener('close', this.handleWebSocketClose);
321-
};
322-
323328
get isHealthy() {
324329
return (
325330
this.signalWs.readyState === WebSocket.OPEN &&
@@ -343,7 +348,7 @@ export class StreamSfuClient {
343348
if (this.signalWs.readyState === WebSocket.OPEN) {
344349
this.logger('debug', `Closing SFU WS connection: ${code} - ${reason}`);
345350
this.signalWs.close(code, `js-client: ${reason}`);
346-
this.cleanUpWebSocket();
351+
this.signalWs.removeEventListener('close', this.handleWebSocketClose);
347352
}
348353
this.dispose();
349354
};

packages/client/src/rtc/signal.ts

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,28 +1,33 @@
11
import { SfuEvent } from '../gen/video/sfu/event/events';
22
import { getLogger } from '../logger';
33
import { DispatchableMessage, SfuEventKinds } from './Dispatcher';
4+
import { Tracer } from '../stats';
45

56
export const createWebSocketSignalChannel = (opts: {
67
endpoint: string;
78
onMessage: <K extends SfuEventKinds>(message: DispatchableMessage<K>) => void;
89
tag: string;
10+
tracer: Tracer | undefined;
911
}) => {
10-
const { endpoint, onMessage, tag } = opts;
12+
const { endpoint, onMessage, tag, tracer } = opts;
1113
const logger = getLogger(['SfuClientWS', tag]);
1214
logger('debug', 'Creating signaling WS channel:', endpoint);
1315
const ws = new WebSocket(endpoint);
1416
ws.binaryType = 'arraybuffer'; // do we need this?
1517

1618
ws.addEventListener('error', (e) => {
1719
logger('error', 'Signaling WS channel error', e);
20+
tracer?.trace('signal.ws.error', e);
1821
});
1922

2023
ws.addEventListener('close', (e) => {
2124
logger('info', 'Signaling WS channel is closed', e);
25+
tracer?.trace('signal.ws.close', e);
2226
});
2327

2428
ws.addEventListener('open', (e) => {
2529
logger('info', 'Signaling WS channel is open', e);
30+
tracer?.trace('signal.ws.open', e);
2631
});
2732

2833
ws.addEventListener('message', (e) => {
@@ -34,11 +39,10 @@ export const createWebSocketSignalChannel = (opts: {
3439

3540
onMessage(message as DispatchableMessage<SfuEventKinds>);
3641
} catch (err) {
37-
logger(
38-
'error',
39-
'Failed to decode a message. Check whether the Proto models match.',
40-
{ event: e, error: err },
41-
);
42+
const message =
43+
'Failed to decode a message. Check whether the Proto models match.';
44+
logger('error', message, { event: e, error: err });
45+
tracer?.trace('signal.ws.message.error', message);
4246
}
4347
});
4448
return ws;

0 commit comments

Comments
 (0)