chore: improve webrtc negotiation timeouts (#38191)

apps/meteor/server/services/media-call/service.ts

@@ -228,6 +228,14 @@ export class MediaCallService extends ServiceClassInternal implements IMediaCall
 			return 'transferred';
 		}
 
+		if (call.hangupReason === 'not-answered') {
+			return 'not-answered';
+		}
+
+		if (call.hangupReason?.startsWith('timeout')) {
+			return 'failed';
+		}
+
 		if (call.hangupReason?.includes('error')) {
 			if (!call.activatedAt) {
 				return 'failed';

packages/media-signaling/src/definition/call/IClientMediaCall.ts

@@ -33,6 +33,10 @@ export type CallHangupReason =
 	| 'rejected' // The callee rejected the call
 	| 'unavailable' // The actor is not available
 	| 'transfer' // one of the users requested the other be transferred to someone else
+	| 'not-answered' // max ringing duration was reached with no answer from the other user
+	| 'timeout-remote-sdp' // Timeout waiting for the remote SDP
+	| 'timeout-local-sdp' // Timeout while generating the local SDP + waiting for ICE Gathering
+	| 'timeout-activation' // Timeout connecting to the negotiated session
 	| 'timeout' // The call state hasn't progressed for too long
 	| 'signaling-error' // Hanging up because of an error during the signal processing
 	| 'service-error' // Hanging up because of an error setting up the service connection

packages/media-signaling/src/definition/client.ts

@@ -2,7 +2,10 @@ export type ClientState =
 	| 'none' // The client doesn't recognize a specific call id at all
 	| 'pending' // The call is ringing
 	| 'accepting' // The client tried to accept the call and is wating for confirmation from the server
-	| 'accepted' // The call was accepted, but the client doesn't have a webrtc offer yet
+	| 'waiting-for-offer' // The call was accepted, but the client doesn't have a webrtc offer yet
+	| 'waiting-for-answer' // The call was accepted and an offer was already sent, but the client doesn't have an answer yet
+	| 'generating-local-sdp' // The client is generating its first local sdp (offer/answer)
+	| 'activating' // The WebRTC signaling has reached the stable state, but the connection is not yet active
 	| 'busy-elsewhere' // The call is happening in a different session/client
 	| 'active' // The webrtc call was established
 	| 'renegotiating' // the webrtc call was established but the client is starting a new negotiation

packages/media-signaling/src/definition/signals/client/hangup.ts

@@ -37,6 +37,10 @@ export const clientMediaSignalHangupSchema: JSONSchemaType<ClientMediaSignalHang
 				'unavailable',
 				'transfer',
 				'timeout',
+				'not-answered',
+				'timeout-remote-sdp',
+				'timeout-local-sdp',
+				'timeout-activation',
 				'signaling-error',
 				'service-error',
 				'media-error',

packages/media-signaling/src/definition/signals/client/local-state.ts

@@ -41,7 +41,19 @@ export const clientMediaSignalLocalStateSchema: JSONSchemaType<ClientMediaSignal
 		},
 		clientState: {
 			type: 'string',
-			enum: ['none', 'pending', 'accepting', 'accepted', 'busy-elsewhere', 'active', 'renegotiating', 'hangup'],
+			enum: [
+				'none',
+				'pending',
+				'accepting',
+				'waiting-for-offer',
+				'waiting-for-answer',
+				'generating-local-sdp',
+				'activating',
+				'busy-elsewhere',
+				'active',
+				'renegotiating',
+				'hangup',
+			],
 			nullable: false,
 		},
 		serviceStates: {

packages/media-signaling/src/lib/Call.ts

@@ -189,6 +189,10 @@ export class ClientMediaCall implements IClientMediaCall {
 
 	private negotiationManager: NegotiationManager;
 
+	private sentLocalSdp: boolean;
+
+	private receivedRemoteSdp: boolean;
+
 	public get audioLevel(): number {
 		return this.webrtcProcessor?.audioLevel || 0;
 	}
@@ -226,6 +230,8 @@ export class ClientMediaCall implements IClientMediaCall {
 		this.mayReportStates = true;
 		this.inputTrack = inputTrack || null;
 		this.creationTimestamp = new Date();
+		this.sentLocalSdp = false;
+		this.receivedRemoteSdp = false;
 
 		this.earlySignals = new Set();
 		this.stateTimeoutHandlers = new Set();
@@ -414,6 +420,28 @@ export class ClientMediaCall implements IClientMediaCall {
 					return 'accepting';
 				}
 				return 'pending';
+			case 'accepted':
+				if (!this.negotiationManager.currentNegotiationId) {
+					return 'waiting-for-offer';
+				}
+
+				if (this._role === 'caller') {
+					if (!this.sentLocalSdp) {
+						return 'generating-local-sdp';
+					}
+					if (!this.receivedRemoteSdp) {
+						return 'waiting-for-answer';
+					}
+				} else {
+					if (!this.receivedRemoteSdp) {
+						return 'waiting-for-offer';
+					}
+					if (!this.sentLocalSdp) {
+						return 'generating-local-sdp';
+					}
+				}
+
+				return 'activating';
 			default:
 				return this._state;
 		}
@@ -748,6 +776,12 @@ export class ClientMediaCall implements IClientMediaCall {
 		this.config.logger?.debug('ClientMediaCall.updateClientState', `${oldClientState} => ${clientState}`);
 
 		this.updateStateTimeouts();
+		// Any time the client state changes within the 'accepted' call state, set a new timeout for the new client state
+		// This ensures there will be three separate timeouts for the different negotiation stages: "generating local sdp", "waiting for remote sdp" and "connecting"
+		if (this._state === 'accepted') {
+			this.addStateTimeout(clientState, TIMEOUT_TO_PROGRESS_SIGNALING);
+		}
+
 		this.requestStateReport();
 		this.oldClientState = clientState;
 		this.emitter.emit('clientStateChange', oldClientState);
@@ -852,16 +886,20 @@ export class ClientMediaCall implements IClientMediaCall {
 
 		this.requireWebRTC();
 
-		if (signal.sdp.type === 'offer') {
-			return this.processAnswerRequest(signal);
-		}
-
-		if (signal.sdp.type !== 'answer') {
-			this.config.logger?.error('Unsupported sdp type.');
-			return;
+		switch (signal.sdp.type) {
+			case 'offer':
+				await this.processAnswerRequest(signal);
+				break;
+			case 'answer':
+				await this.negotiationManager.setRemoteDescription(signal.negotiationId, signal.sdp);
+				break;
+			default:
+				this.config.logger?.error('Unsupported sdp type.');
+				return;
 		}
 
-		await this.negotiationManager.setRemoteDescription(signal.negotiationId, signal.sdp);
+		this.receivedRemoteSdp = true;
+		this.updateClientState();
 	}
 
 	protected deliverSdp(data: { sdp: RTCSessionDescriptionInit; negotiationId: string }) {
@@ -869,6 +907,7 @@ export class ClientMediaCall implements IClientMediaCall {
 
 		if (!this.hidden) {
 			this.config.transporter.sendToServer(this.callId, 'local-sdp', data);
+			this.sentLocalSdp = true;
 		}
 
 		this.updateClientState();
@@ -954,8 +993,6 @@ export class ClientMediaCall implements IClientMediaCall {
 
 		// Both sides of the call have accepted it, we can change the state now
 		this.changeState('accepted');
-
-		this.addStateTimeout('accepted', TIMEOUT_TO_PROGRESS_SIGNALING);
 	}
 
 	private flagAsEnded(reason: CallHangupReason): void {
@@ -995,7 +1032,7 @@ export class ClientMediaCall implements IClientMediaCall {
 				if (callback) {
 					callback();
 				} else {
-					void this.hangup('timeout');
+					void this.hangup(this.getTimeoutHangupReason(state));
 				}
 			}, timeout),
 		};
@@ -1003,6 +1040,22 @@ export class ClientMediaCall implements IClientMediaCall {
 		this.stateTimeoutHandlers.add(handler);
 	}
 
+	private getTimeoutHangupReason(state: ClientState): CallHangupReason {
+		switch (state) {
+			case 'pending':
+				return 'not-answered';
+			case 'waiting-for-offer':
+			case 'waiting-for-answer':
+				return 'timeout-remote-sdp';
+			case 'generating-local-sdp':
+				return 'timeout-local-sdp';
+			case 'activating':
+				return 'timeout-activation';
+		}
+
+		return 'timeout';
+	}
+
 	private updateStateTimeouts(): void {
 		this.config.logger?.debug('ClientMediaCall.updateStateTimeouts');
 		const clientState = this.getClientState();

packages/media-signaling/src/lib/NegotiationManager.ts

@@ -10,6 +10,10 @@ export class NegotiationManager {
 		return this.currentNegotiation?.negotiationId || this.highestNegotiationId;
 	}
 
+	public get hasFinishedAnyNegotiation(): boolean {
+		return Boolean(this.highestFinishedNegotiationId);
+	}
+
 	protected negotiations: Map<string, Negotiation>;
 
 	/** negotiation actively being processed, null once completed */
@@ -29,6 +33,9 @@ export class NegotiationManager {
 	/** id of the newest negotiation, regardless of state */
 	protected highestKnownNegotiationId: string | null;
 
+	/** id of the newest negotiation that has finished processing */
+	protected highestFinishedNegotiationId: string | null;
+
 	constructor(
 		protected readonly call: INegotiationCompatibleMediaCall,
 		protected readonly config: NegotiationManagerConfig,
@@ -41,6 +48,7 @@ export class NegotiationManager {
 		this.webrtcProcessor = null;
 		this.highestNegotiationId = null;
 		this.highestKnownNegotiationId = null;
+		this.highestFinishedNegotiationId = null;
 
 		this.emitter = new Emitter();
 	}
@@ -103,13 +111,10 @@ export class NegotiationManager {
 			return;
 		}
 
-		try {
-			return this.currentNegotiation.setRemoteAnswer(remoteDescription);
-		} catch (e) {
-			this.config.logger?.error(e);
-			this.currentNegotiation = null;
-			this.emitter.emit('error', { errorCode: 'failed-to-set-remote-answer', negotiationId });
-		}
+		void this.currentNegotiation
+			.setRemoteAnswer(remoteDescription)
+			// No need to handle errors here as they are already handled by the 'error' event
+			.catch(() => null);
 	}
 
 	public setWebRTCProcessor(webrtcProcessor: IWebRTCProcessor) {
@@ -202,6 +207,9 @@ export class NegotiationManager {
 				return;
 			}
 
+			if (negotiation.finished) {
+				this.highestFinishedNegotiationId = negotiation.negotiationId;
+			}
 			this.config.logger?.debug('NegotiationManager.processNegotiation.ended');
 			this.currentNegotiation = null;
 			void this.processNegotiations();
@@ -210,6 +218,10 @@ export class NegotiationManager {
 		negotiation.emitter.on('error', ({ errorCode }) => {
 			this.config.logger?.error('Negotiation error', errorCode);
 			this.emitter.emit('error', { errorCode, negotiationId: negotiation.negotiationId });
+
+			if (this.currentNegotiation === negotiation) {
+				this.currentNegotiation.end();
+			}
 		});
 
 		negotiation.emitter.on('local-sdp', ({ sdp }) => {
@@ -217,13 +229,10 @@ export class NegotiationManager {
 			this.emitter.emit('local-sdp', { sdp, negotiationId: negotiation.negotiationId });
 		});
 
-		try {
-			return negotiation.process(this.webrtcProcessor);
-		} catch (e) {
-			this.config.logger?.error(e);
-			this.currentNegotiation = null;
-			this.emitter.emit('error', { errorCode: 'failed-to-process-negotiation', negotiationId: negotiation.negotiationId });
-		}
+		void negotiation
+			.process(this.webrtcProcessor)
+			// No need to handle errors here as they are already handled by the 'error' event
+			.catch(() => null);
 	}
 
 	protected isConfigured(): this is WebRTCNegotiationManager {

packages/media-signaling/src/lib/services/webrtc/Negotiation.ts

@@ -18,6 +18,10 @@ export class Negotiation {
 		return !this.remoteOffer;
 	}
 
+	public get finished(): boolean {
+		return this._finished;
+	}
+
 	public readonly negotiationId: string;
 
 	public readonly sequence: number;
@@ -34,6 +38,8 @@ export class Negotiation {
 
 	protected _failed: boolean;
 
+	protected _finished: boolean;
+
 	constructor(
 		negotiation: NegotiationData,
 		protected readonly logger?: IMediaSignalLogger | null,
@@ -42,6 +48,7 @@ export class Negotiation {
 		this._startedProcessing = false;
 		this._ended = false;
 		this._failed = false;
+		this._finished = false;
 		this.negotiationId = negotiation.negotiationId;
 		this.sequence = negotiation.sequence;
 		this.isPolite = negotiation.isPolite;
@@ -50,13 +57,16 @@ export class Negotiation {
 		this.emitter = new Emitter();
 	}
 
-	public end(): void {
+	public end(finished = false): void {
 		if (this._ended) {
 			return;
 		}
 
 		this.logger?.debug('Negotiation.end', this.negotiationId);
 		this._ended = true;
+		if (finished && this._startedProcessing && !this._failed) {
+			this._finished = true;
+		}
 		this.emitter.emit('ended');
 	}
 
@@ -79,7 +89,7 @@ export class Negotiation {
 	}
 
 	public async setRemoteAnswer(sdp: RTCSessionDescriptionInit): Promise<void> {
-		if (!this.webrtcProcessor) {
+		if (!this.isWebRTCNegotiation()) {
 			return;
 		}
 
@@ -90,32 +100,28 @@ export class Negotiation {
 			return;
 		}
 
-		await this.webrtcProcessor.setRemoteDescription(sdp);
+		await this.setPeerRemoteDescription(sdp);
 		// Local negotiations end when the remote description is available
-		this.end();
+		this.end(true);
 	}
 
 	protected async setLocalDescription(this: WebRTCNegotiation, sdp: RTCSessionDescriptionInit): Promise<void> {
 		this.logger?.debug('Negotiation.setLocalDescription', this.negotiationId);
 
 		this.assertNegotiationIsActive();
-		await this.webrtcProcessor.setLocalDescription(sdp);
+		await this.setPeerLocalDescription(sdp);
 
 		this.assertNegotiationIsActive();
 		await this.webrtcProcessor.waitForIceGathering();
 
 		this.assertNegotiationIsActive();
-		const localDescription = this.webrtcProcessor.getLocalDescription();
-		if (!localDescription) {
-			this.fail('implementation-error');
-			return;
-		}
+		const localDescription = this.getPeerLocalDescription();
 
 		this.emitter.emit('local-sdp', { sdp: localDescription });
 
 		// Remote negotiations end when the local description is available
 		if (!this.isLocal) {
-			this.end();
+			this.end(true);
 		}
 	}
 
@@ -123,6 +129,10 @@ export class Negotiation {
 		this.webrtcProcessor = webrtcProcessor;
 	}
 
+	protected isWebRTCNegotiation(): this is WebRTCNegotiation {
+		return !!this.webrtcProcessor;
+	}
+
 	protected assertNegotiationIsActive(): void {
 		if (this._ended) {
 			this.fail('skipped-negotiation');
@@ -142,10 +152,10 @@ export class Negotiation {
 	protected async createLocalAnswer(this: WebRTCNegotiation, remoteOffer: RTCSessionDescriptionInit): Promise<void> {
 		this.logger?.debug('Negotiation.createLocalAnswer', this.negotiationId);
 		this.assertNegotiationIsActive();
-		await this.webrtcProcessor.setRemoteDescription(remoteOffer);
+		await this.setPeerRemoteDescription(remoteOffer);
 
 		this.assertNegotiationIsActive();
-		const earlyAnswer = await this.webrtcProcessor.createAnswer();
+		const earlyAnswer = await this.createEarlyAnswer();
 
 		this.assertNegotiationIsActive();
 		await this.setLocalDescription(earlyAnswer);
@@ -160,6 +170,49 @@ export class Negotiation {
 
 		this._failed = true;
 	}
+
+	protected async setPeerRemoteDescription(this: WebRTCNegotiation, remoteDescription: RTCSessionDescriptionInit): Promise<void> {
+		try {
+			await this.webrtcProcessor.setRemoteDescription(remoteDescription);
+		} catch (err) {
+			this.logger?.error(err);
+			this.fail('failed-to-set-remote-description');
+		}
+	}
+
+	protected async createEarlyAnswer(this: WebRTCNegotiation): Promise<RTCSessionDescriptionInit> {
+		try {
+			const earlyAnswer = await this.webrtcProcessor.createAnswer();
+			return earlyAnswer;
+		} catch (err) {
+			this.logger?.error(err);
+			this.fail('failed-to-create-local-answer');
+			throw err;
+		}
+	}
+
+	protected async setPeerLocalDescription(this: WebRTCNegotiation, localDescription: RTCSessionDescriptionInit): Promise<void> {
+		try {
+			await this.webrtcProcessor.setLocalDescription(localDescription);
+		} catch (err) {
+			this.logger?.error(err);
+			this.fail('failed-to-set-local-description');
+		}
+	}
+
+	protected getPeerLocalDescription(this: WebRTCNegotiation): RTCSessionDescriptionInit {
+		try {
+			const sdp = this.webrtcProcessor.getLocalDescription();
+			if (!sdp) {
+				throw new Error('No local description');
+			}
+			return sdp;
+		} catch (err) {
+			this.logger?.error(err);
+			this.fail('failed-to-get-local-description');
+			throw err;
+		}
+	}
 }
 
 export abstract class WebRTCNegotiation extends Negotiation {