Skip to content

Commit

Permalink
Add correct ConnectionErrorReason when cancelling ongoing connection …
Browse files Browse the repository at this point in the history
…attempts (#1315)

* Add ConnectionErrorReason when cancelling ongoing connection attempt

* Create fifty-ties-jog.md

* ensure connection error reason is set everywhere
  • Loading branch information
lukasIO authored Nov 18, 2024
1 parent 4dbab2c commit 924f518
Show file tree
Hide file tree
Showing 7 changed files with 56 additions and 13 deletions.
5 changes: 5 additions & 0 deletions .changeset/fifty-ties-jog.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"livekit-client": patch
---

Add ConnectionErrorReason when cancelling ongoing connection attempt
22 changes: 19 additions & 3 deletions src/api/SignalClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -272,12 +272,22 @@ export class SignalClient {
const abortHandler = async () => {
this.close();
clearTimeout(wsTimeout);
reject(new ConnectionError('room connection has been cancelled (signal)'));
reject(
new ConnectionError(
'room connection has been cancelled (signal)',
ConnectionErrorReason.Cancelled,
),
);
};

const wsTimeout = setTimeout(() => {
this.close();
reject(new ConnectionError('room connection has timed out (signal)'));
reject(
new ConnectionError(
'room connection has timed out (signal)',
ConnectionErrorReason.ServerUnreachable,
),
);
}, opts.websocketTimeout);

if (abortSignal?.aborted) {
Expand Down Expand Up @@ -391,6 +401,7 @@ export class SignalClient {
reject(
new ConnectionError(
`did not receive join response, got ${resp.message?.case} instead`,
ConnectionErrorReason.InternalError,
),
);
}
Expand All @@ -407,7 +418,12 @@ export class SignalClient {

this.ws.onclose = (ev: CloseEvent) => {
if (this.isEstablishingConnection) {
reject(new ConnectionError('Websocket got closed during a (re)connection attempt'));
reject(
new ConnectionError(
'Websocket got closed during a (re)connection attempt',
ConnectionErrorReason.InternalError,
),
);
}

this.log.warn(`websocket closed`, {
Expand Down
7 changes: 6 additions & 1 deletion src/room/PCTransportManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -342,7 +342,12 @@ export class PCTransportManager {

const connectTimeout = CriticalTimers.setTimeout(() => {
abortController?.signal.removeEventListener('abort', abortHandler);
reject(new ConnectionError('could not establish pc connection'));
reject(
new ConnectionError(
'could not establish pc connection',
ConnectionErrorReason.InternalError,
),
);
}, timeout);

while (this.state !== PCTransportState.CONNECTED) {
Expand Down
16 changes: 13 additions & 3 deletions src/room/RTCEngine.ts
Original file line number Diff line number Diff line change
Expand Up @@ -313,7 +313,10 @@ export default class RTCEngine extends (EventEmitter as new () => TypedEventEmit
const publicationTimeout = setTimeout(() => {
delete this.pendingTrackResolvers[req.cid];
reject(
new ConnectionError('publication of local track timed out, no response from server'),
new ConnectionError(
'publication of local track timed out, no response from server',
ConnectionErrorReason.InternalError,
),
);
}, 10_000);
this.pendingTrackResolvers[req.cid] = {
Expand Down Expand Up @@ -1061,7 +1064,10 @@ export default class RTCEngine extends (EventEmitter as new () => TypedEventEmit
} catch (e: any) {
// TODO do we need a `failed` state here for the PC?
this.pcState = PCState.Disconnected;
throw new ConnectionError(`could not establish PC connection, ${e.message}`);
throw new ConnectionError(
`could not establish PC connection, ${e.message}`,
ConnectionErrorReason.InternalError,
);
}
}

Expand Down Expand Up @@ -1126,7 +1132,10 @@ export default class RTCEngine extends (EventEmitter as new () => TypedEventEmit
const transport = subscriber ? this.pcManager.subscriber : this.pcManager.publisher;
const transportName = subscriber ? 'Subscriber' : 'Publisher';
if (!transport) {
throw new ConnectionError(`${transportName} connection not set`);
throw new ConnectionError(
`${transportName} connection not set`,
ConnectionErrorReason.InternalError,
);
}

let needNegotiation = false;
Expand Down Expand Up @@ -1167,6 +1176,7 @@ export default class RTCEngine extends (EventEmitter as new () => TypedEventEmit

throw new ConnectionError(
`could not establish ${transportName} connection, state: ${transport.getICEConnectionState()}`,
ConnectionErrorReason.InternalError,
);
}

Expand Down
4 changes: 3 additions & 1 deletion src/room/RegionUrlProvider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,9 @@ export class RegionUrlProvider {
} else {
throw new ConnectionError(
`Could not fetch region settings: ${regionSettingsResponse.statusText}`,
regionSettingsResponse.status === 401 ? ConnectionErrorReason.NotAllowed : undefined,
regionSettingsResponse.status === 401
? ConnectionErrorReason.NotAllowed
: ConnectionErrorReason.InternalError,
regionSettingsResponse.status,
);
}
Expand Down
11 changes: 8 additions & 3 deletions src/room/Room.ts
Original file line number Diff line number Diff line change
Expand Up @@ -714,7 +714,10 @@ class Room extends (EventEmitter as new () => TypedEmitter<RoomEventCallbacks>)
} catch (err) {
await this.engine.close();
this.recreateEngine();
const resultingError = new ConnectionError(`could not establish signal connection`);
const resultingError = new ConnectionError(
`could not establish signal connection`,
ConnectionErrorReason.ServerUnreachable,
);
if (err instanceof Error) {
resultingError.message = `${resultingError.message}: ${err.message}`;
}
Expand All @@ -732,7 +735,7 @@ class Room extends (EventEmitter as new () => TypedEmitter<RoomEventCallbacks>)
if (abortController.signal.aborted) {
await this.engine.close();
this.recreateEngine();
throw new ConnectionError(`Connection attempt aborted`);
throw new ConnectionError(`Connection attempt aborted`, ConnectionErrorReason.Cancelled);
}

try {
Expand Down Expand Up @@ -783,7 +786,9 @@ class Room extends (EventEmitter as new () => TypedEmitter<RoomEventCallbacks>)
this.log.warn('abort connection attempt', this.logContext);
this.abortController?.abort();
// in case the abort controller didn't manage to cancel the connection attempt, reject the connect promise explicitly
this.connectFuture?.reject?.(new ConnectionError('Client initiated disconnect'));
this.connectFuture?.reject?.(
new ConnectionError('Client initiated disconnect', ConnectionErrorReason.Cancelled),
);
this.connectFuture = undefined;
}
// send leave
Expand Down
4 changes: 2 additions & 2 deletions src/room/errors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,9 @@ export const enum ConnectionErrorReason {
export class ConnectionError extends LivekitError {
status?: number;

reason?: ConnectionErrorReason;
reason: ConnectionErrorReason;

constructor(message?: string, reason?: ConnectionErrorReason, status?: number) {
constructor(message: string, reason: ConnectionErrorReason, status?: number) {
super(1, message);
this.status = status;
this.reason = reason;
Expand Down

0 comments on commit 924f518

Please sign in to comment.