From 2a1436ded7edcebcf1e0a6ea45b31f815304c4e9 Mon Sep 17 00:00:00 2001 From: Thanh Dodeur Date: Tue, 7 Sep 2021 11:16:01 +0000 Subject: [PATCH] [FIX] mail: add more RTC connection recovery cases This commit adds RTC connection recovery attempts for some cases that were not covered, like `addIceCandidate` error or `failed` ice connection state. Part-of: odoo/odoo#76339 --- addons/mail/static/src/models/rtc/rtc.js | 47 ++++++++++++------------ 1 file changed, 23 insertions(+), 24 deletions(-) diff --git a/addons/mail/static/src/models/rtc/rtc.js b/addons/mail/static/src/models/rtc/rtc.js index 640bbd60032..e001b9df4e6 100644 --- a/addons/mail/static/src/models/rtc/rtc.js +++ b/addons/mail/static/src/models/rtc/rtc.js @@ -110,7 +110,6 @@ function factory(dependencies) { * @param {mail.rtc_session[]} currentSessions list of sessions of this call. */ async filterCallees(currentSessions) { - temporaryLog(`MEMBERS UPDATE: ${currentSessions.length} members in call`); const currentSessionsTokens = new Set(currentSessions.map(session => session.peerToken)); if (this.currentRtcSession && !currentSessionsTokens.has(this.currentRtcSession.peerToken)) { // if the current RTC session is not in the channel sessions, this call is no longer valid. @@ -157,9 +156,6 @@ function factory(dependencies) { // does handle notifications targeting a different session return; } - if (event !== 'trackChange') { - temporaryLog(`RECEIVED NOTIFICATION: ${event} from: ${rtcSession.name}`); - } if (!this.isClientRtcCompatible) { return; } @@ -168,9 +164,11 @@ function factory(dependencies) { } switch (event) { case "offer": + temporaryLog(`RECEIVED NOTIFICATION: ${event} from: ${rtcSession.name}`); await this._handleRtcTransactionOffer(sender, payload); break; case "answer": + temporaryLog(`RECEIVED NOTIFICATION: ${event} from: ${rtcSession.name}`); await this._handleRtcTransactionAnswer(sender, payload); break; case "ice-candidate": @@ -401,7 +399,6 @@ function factory(dependencies) { session.update({ connectionState: 'Disconnected: sending initial RTC offer', }); - temporaryLog('calling: ' + session.name); this._callPeer(session.peerToken); } } @@ -441,7 +438,7 @@ function factory(dependencies) { }; peerConnection.onicecandidateerror = async (error) => { temporaryLog('=== ERROR: onIceCandidate ===', { trace: error }); - this._recoverConnection(token, { delay: 15000, reason: 'ice candidate error' }); + this._recoverConnection(token, { delay: this.recoveryTimeout, reason: 'ice candidate error' }); }; peerConnection.onnegotiationneeded = async (event) => { const offer = await peerConnection.createOffer(); @@ -501,7 +498,6 @@ function factory(dependencies) { async _handleRtcTransactionAnswer(fromToken, { sdp }) { const peerConnection = this._peerConnections[fromToken]; if (!peerConnection || peerConnection.connectionState === 'closed' || peerConnection.signalingState === 'stable') { - temporaryLog('=== ERROR: Handle Answer from undefined|closed|stable === '); return; } if (peerConnection.signalingState === 'have-remote-offer') { @@ -513,7 +509,6 @@ function factory(dependencies) { await peerConnection.setRemoteDescription(rtcSessionDescription); } catch (e) { // ignored the transaction may have been resolved by another concurrent offer. - temporaryLog('=== ERROR: setRemoteDescription === ', { trace: e }); } } @@ -526,15 +521,14 @@ function factory(dependencies) { async _handleRtcTransactionICECandidate(fromToken, { candidate }) { const peerConnection = this._peerConnections[fromToken]; if (!peerConnection || peerConnection.connectionState === 'closed') { - temporaryLog('=== ERROR: Handle Ice Candidate from undefined|closed ==='); return; } const rtcIceCandidate = new window.RTCIceCandidate(candidate); try { await peerConnection.addIceCandidate(rtcIceCandidate); } catch (error) { - // ignored temporaryLog('=== ERROR: ADD ICE CANDIDATE ===', { trace: error }); + this._recoverConnection(fromToken, { delay: this.recoveryTimeout, reason: 'failed at adding ice candidate' }); } } @@ -559,7 +553,6 @@ function factory(dependencies) { await peerConnection.setRemoteDescription(rtcSessionDescription); } catch (e) { // ignored the transaction may have been resolved by another concurrent offer. - temporaryLog('=== ERROR: handle offer ===', { trace: e }); return; } await peerConnection.setRemoteDescription(rtcSessionDescription); @@ -573,7 +566,7 @@ function factory(dependencies) { event: 'answer', payload: { sdp: peerConnection.localDescription }, }); - this._recoverConnection(fromToken, { delay: 15000, reason: 'standard answer timeout' }); + this._recoverConnection(fromToken, { delay: this.recoveryTimeout, reason: 'standard answer timeout' }); } /** @@ -613,11 +606,6 @@ function factory(dependencies) { if (!targetTokens.length || !this.channel || !this.currentRtcSession) { return; } - if (event !== 'trackChange') { - const tokenSet = new Set(targetTokens); - const sessions = this.messaging.models['mail.rtc_session'].all(session => tokenSet.has(session.peerToken)); - temporaryLog(`SEND NOTIFICATION: - ${event} to: [${sessions.map(session => session.name).join(',')}] (${type})`); - } const content = JSON.stringify({ event, channelId: this.channel.id, @@ -625,6 +613,7 @@ function factory(dependencies) { }); if (type === 'server') { + temporaryLog(`SEND NOTIFICATION: - ${event} to: [${targetTokens}]`); await this.env.services.rpc({ route: '/mail/rtc/session/notify_call_members', params: { @@ -729,7 +718,7 @@ function factory(dependencies) { try { transceiver.stop(); } catch (e) { - temporaryLog('=== ERROR: stopping transceiver from remote track ===', { trace: e }); + // transceiver may already be stopped by the remote. } } } @@ -956,7 +945,6 @@ function factory(dependencies) { transceiver.direction = transceiverDirection; } catch (e) { // ignored, the track is probably already on the peerConnection. - temporaryLog('=== ERROR: replace transceiver track ===', { trace: e }); } return; } @@ -965,7 +953,6 @@ function factory(dependencies) { transceiver.direction = transceiverDirection; } catch (e) { // ignored, the transceiver is probably already removed - temporaryLog('=== ERROR: remove transceiver track ===', { trace: e }); } } @@ -990,12 +977,12 @@ function factory(dependencies) { */ async _onConnectionStateChange(state, token) { switch (state) { - case "failed": case "closed": this._removePeer(token); break; + case "failed": case "disconnected": - await this._recoverConnection(token, { delay: 500, reason: 'connection disconnected' }); + await this._recoverConnection(token, { delay: this.recoveryDelay, reason: `connection ${state}` }); break; } } @@ -1014,12 +1001,12 @@ function factory(dependencies) { connectionState, }); switch (connectionState) { - case "failed": case "closed": this._removePeer(token); break; + case "failed": case "disconnected": - await this._recoverConnection(token, { delay: 1000, reason: 'ice connection disconnected' }); + await this._recoverConnection(token, { delay: this.recoveryDelay, reason: `ice connection ${connectionState}` }); break; } } @@ -1111,6 +1098,18 @@ function factory(dependencies) { }, ], }), + /** + * How long to wait before considering a connection as needing recovery. + */ + recoveryTimeout: attr({ + default: 15000, + }), + /** + * How long to wait before recovering a connection that has failed. + */ + recoveryDelay: attr({ + default: 3000, + }), /** * True if we want to enable the video track of the current partner. */