diff --git a/apps/meteor/server/services/media-call/service.ts b/apps/meteor/server/services/media-call/service.ts index e1d75b27f4f..6b1dd30b1e1 100644 --- a/apps/meteor/server/services/media-call/service.ts +++ b/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'; diff --git a/packages/media-signaling/src/definition/call/IClientMediaCall.ts b/packages/media-signaling/src/definition/call/IClientMediaCall.ts index b39feac23ee..475d8b8600a 100644 --- a/packages/media-signaling/src/definition/call/IClientMediaCall.ts +++ b/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 diff --git a/packages/media-signaling/src/definition/client.ts b/packages/media-signaling/src/definition/client.ts index 8a3168db534..c389adc4ee8 100644 --- a/packages/media-signaling/src/definition/client.ts +++ b/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 diff --git a/packages/media-signaling/src/definition/signals/client/hangup.ts b/packages/media-signaling/src/definition/signals/client/hangup.ts index af4d38089cb..f30ba8d3981 100644 --- a/packages/media-signaling/src/definition/signals/client/hangup.ts +++ b/packages/media-signaling/src/definition/signals/client/hangup.ts @@ -37,6 +37,10 @@ export const clientMediaSignalHangupSchema: JSONSchemaType ${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(); diff --git a/packages/media-signaling/src/lib/NegotiationManager.ts b/packages/media-signaling/src/lib/NegotiationManager.ts index 061228955a3..ecfbebac7b0 100644 --- a/packages/media-signaling/src/lib/NegotiationManager.ts +++ b/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; /** 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 { diff --git a/packages/media-signaling/src/lib/services/webrtc/Negotiation.ts b/packages/media-signaling/src/lib/services/webrtc/Negotiation.ts index 0c780f0cea7..94bf864871f 100644 --- a/packages/media-signaling/src/lib/services/webrtc/Negotiation.ts +++ b/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 { - 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 { 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 { 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 { + 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 { + 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 { + 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 {