Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ice restart trigger #1750

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
57 changes: 38 additions & 19 deletions modules/connectivity/IceFailedHandling.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,11 @@ import * as JitsiConferenceEvents from '../../JitsiConferenceEvents';

const logger = getLogger(__filename);

/**
* Maximum number of consecutive failed restart attempts before trying a reload instead.
*/
const MAXIMUM_ICE_RESTART_ATTEMPTS = 3;

/**
* This class deals with shenanigans around JVB media session's ICE failed status handling.
*
Expand Down Expand Up @@ -55,31 +60,45 @@ export default class IceFailedHandling {
}

const jvbConnection = this._conference.jvbJingleSession;
const jvbConnIceState = jvbConnection && jvbConnection.getIceConnectionState();

if (!jvbConnection) {
logger.warn('Not sending ICE failed - no JVB connection');
} else if (jvbConnIceState === 'connected') {

return;
}

const jvbConnIceState = jvbConnection.getIceConnectionState();

if (jvbConnIceState === 'connected') {
logger.info('ICE connection restored - not sending ICE failed');
} else {
logger.info('Sending ICE failed - the connection did not recover, '
+ `ICE state: ${jvbConnIceState}, `
+ `use 'session-terminate': ${useTerminateForRestart}`);
if (useTerminateForRestart) {
this._conference.jvbJingleSession.terminate(
() => {
logger.info('session-terminate for ice restart - done');
},
error => {
logger.error(`session-terminate for ice restart - error: ${error.message}`);
}, {
reason: 'connectivity-error',
reasonDescription: 'ICE FAILED',
requestRestart: true,
sendSessionTerminate: true
});
const numAttempts = jvbConnection.iceRestartAttempts;

if (numAttempts >= MAXIMUM_ICE_RESTART_ATTEMPTS) {
logger.info('ICE failed, but maximum number of ICE restart attempts has been reached.');
this._conference.eventEmitter.emit(
JitsiConferenceEvents.CONFERENCE_FAILED,
JitsiConferenceErrors.ICE_FAILED);
} else {
this._conference.jvbJingleSession.sendIceFailedNotification();
logger.info('Sending ICE failed - the connection did not recover, '
+ `ICE state: ${jvbConnIceState}, `
+ `use 'session-terminate': ${useTerminateForRestart}`);
if (useTerminateForRestart) {
this._conference.jvbJingleSession.terminate(
() => {
logger.info('session-terminate for ice restart - done');
},
error => {
logger.error(`session-terminate for ice restart - error: ${error.message}`);
}, {
reason: 'connectivity-error',
reasonDescription: 'ICE FAILED',
requestRestart: true,
sendSessionTerminate: true
});
} else {
this._conference.jvbJingleSession.sendIceFailedNotification();
}
}
}
}
Expand Down
27 changes: 26 additions & 1 deletion modules/xmpp/JingleSessionPC.js
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,12 @@ const DEFAULT_MAX_STATS = 300;
*/
const ICE_CAND_GATHERING_TIMEOUT = 150;

/**
* The amount of time to wait in the ice disconnected state before giving up and triggering a restart.
* @type {number} timeout in ms.
*/
const ICE_DISCONNECT_TIMEOUT = 5000;

/**
* Reads the endpoint ID given a string which represents either the endpoint's full JID, or the endpoint ID itself.
* @param {String} jidOrEndpointId A string which is either the full JID of a participant, or the ID of an
Expand Down Expand Up @@ -246,6 +252,8 @@ export default class JingleSessionPC extends JingleSession {

this.lasticecandidate = false;
this.closed = false;
this.disconnectTimerId = 0;
this.iceRestartAttempts = 0;

/**
* Indicates whether or not this <tt>JingleSessionPC</tt> is used in
Expand Down Expand Up @@ -488,9 +496,16 @@ export default class JingleSessionPC extends JingleSession {
XMPPEvents.ICE_CONNECTION_STATE_CHANGED,
this,
this.peerconnection.iceConnectionState);

if (this.disconnectTimerId > 0) {
clearTimeout(this.disconnectTimerId);
this.disconnectTimerId = 0;
}

switch (this.peerconnection.iceConnectionState) {
case 'checking':
this._iceCheckingStartedTimestamp = now;
++this.iceRestartAttempts;
break;
case 'connected':
// Informs interested parties that the connection has been restored. This includes the case when
Expand Down Expand Up @@ -551,6 +566,7 @@ export default class JingleSessionPC extends JingleSession {
XMPPEvents.CONNECTION_ESTABLISHED, this);
}
this.isReconnect = false;
this.iceRestartAttempts = 0;
break;
case 'disconnected':
this.isReconnect = true;
Expand All @@ -561,8 +577,18 @@ export default class JingleSessionPC extends JingleSession {
this.room.eventEmitter.emit(
XMPPEvents.CONNECTION_INTERRUPTED, this);
}

this.lasticecandidate = false;

this.disconnectTimerId = setTimeout(() => {
this.room.eventEmitter.emit(
XMPPEvents.CONNECTION_ICE_FAILED, this);

}, ICE_DISCONNECT_TIMEOUT);
break;
case 'failed':
this.lasticecandidate = false;

this.room.eventEmitter.emit(
XMPPEvents.CONNECTION_ICE_FAILED, this);
break;
Expand Down Expand Up @@ -683,7 +709,6 @@ export default class JingleSessionPC extends JingleSession {
} else {
logger.log(`${this} sendIceCandidate: last candidate`);

// FIXME: remember to re-think in ICE-restart
this.lasticecandidate = true;
}
}
Expand Down