Skip to content

Commit

Permalink
feat(moderator): Make sure we resolve the sendConference promise.
Browse files Browse the repository at this point in the history
  • Loading branch information
damencho committed Sep 20, 2024
1 parent 604acf0 commit 83ba892
Show file tree
Hide file tree
Showing 4 changed files with 45 additions and 18 deletions.
7 changes: 6 additions & 1 deletion JitsiConnection.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import { getLogger } from '@jitsi/logger';

import JitsiConference from './JitsiConference';
import * as JitsiConnectionEvents from './JitsiConnectionEvents';
import FeatureFlags from './modules/flags/FeatureFlags';
Expand All @@ -8,6 +10,8 @@ import {
createConnectionFailedEvent
} from './service/statistics/AnalyticsEvents';

const logger = getLogger(__filename);

/**
* Creates a new connection object for the Jitsi Meet server side video
* conferencing service. Provides access to the JitsiConference interface.
Expand Down Expand Up @@ -67,7 +71,8 @@ JitsiConnection.prototype.connect = function(options = {}) {
this.xmpp.moderator.sendConferenceRequest(this.xmpp.getRoomJid(options.name))
.then(() => {
this.xmpp.connect(options.id, options.password);
});
})
.catch(e => logger.trace('sendConferenceRequest rejected', e));
} else {
this.xmpp.connect(options.id, options.password);
}
Expand Down
8 changes: 7 additions & 1 deletion authenticateAndUpgradeRole.js
Original file line number Diff line number Diff line change
@@ -1,10 +1,14 @@
import { getLogger } from '@jitsi/logger';

import {
CONNECTION_DISCONNECTED,
CONNECTION_ESTABLISHED,
CONNECTION_FAILED
} from './JitsiConnectionEvents';
import XMPP from './modules/xmpp/xmpp';

const logger = getLogger(__filename);

/**
* @typedef {Object} UpgradeRoleError
*
Expand Down Expand Up @@ -111,7 +115,9 @@ export default function authenticateAndUpgradeRole({
// we execute this logic in JitsiConference where we bind the current conference as `this`
// At this point we should have the new session ID
// stored in the settings. Send a new conference IQ.
this.room.xmpp.moderator.sendConferenceRequest(this.room.roomjid).finally(resolve);
this.room.xmpp.moderator.sendConferenceRequest(this.room.roomjid)
.catch(e => logger.trace('sendConferenceRequest rejected', e))
.finally(resolve);
})
.catch(({ error, message }) => {
xmpp.disconnect();
Expand Down
3 changes: 2 additions & 1 deletion modules/xmpp/ChatRoom.js
Original file line number Diff line number Diff line change
Expand Up @@ -249,7 +249,8 @@ export default class ChatRoom extends Listenable {
this.onConnStatusChanged.bind(this))
);
resolve();
});
})
.catch(e => logger.trace('PreJoin rejected', e));
});
}

Expand Down
45 changes: 30 additions & 15 deletions modules/xmpp/moderator.js
Original file line number Diff line number Diff line change
Expand Up @@ -304,14 +304,14 @@ export default class Moderator extends Listenable {
// to mark whether we have already sent a conference request
this.conferenceRequestSent = false;

return new Promise(resolve => {
return new Promise((resolve, reject) => {
if (this.mode === 'xmpp') {
logger.info(`Sending conference request over XMPP to ${this.targetJid}`);

this.connection.sendIQ(
this._createConferenceIq(roomJid),
result => this._handleIqSuccess(roomJid, result, resolve),
error => this._handleIqError(roomJid, error, resolve));
result => this._handleIqSuccess(roomJid, result, resolve, reject),
error => this._handleIqError(roomJid, error, resolve, reject));

// XXX We're pressed for time here because we're beginning a complex
// and/or lengthy conference-establishment process which supposedly
Expand All @@ -335,24 +335,24 @@ export default class Moderator extends Listenable {
&& text.indexOf('400 invalid-session') > 0;
const notAuthorized = response.status === 403;

this._handleError(roomJid, sessionError, notAuthorized, resolve);
this._handleError(roomJid, sessionError, notAuthorized, resolve, reject);
})
.catch(error => {
logger.warn(`Error: ${error}`);
this._handleError(roomJid);
this._handleError(roomJid, undefined, undefined, resolve, reject);
});

// _handleError has either scheduled a retry or fired an event indicating failure.
return;
}
response.json()
.then(resultJson => {
this._handleSuccess(roomJid, resultJson, resolve);
this._handleSuccess(roomJid, resultJson, resolve, reject);
});
})
.catch(error => {
logger.warn(`Error: ${error}`);
this._handleError(roomJid);
this._handleError(roomJid, undefined, undefined, resolve, reject);
});
}
}).then(() => {
Expand All @@ -365,9 +365,10 @@ export default class Moderator extends Listenable {
* @param roomJid
* @param conferenceRequest
* @param callback
* @param errorCallback
* @private
*/
_handleSuccess(roomJid, conferenceRequest, callback) {
_handleSuccess(roomJid, conferenceRequest, callback, errorCallback) {
// Reset the error timeout (because we haven't failed here).
this.getNextErrorTimeout(true);

Expand Down Expand Up @@ -400,6 +401,8 @@ export default class Moderator extends Listenable {

this.xmpp.eventEmitter.emit(CONNECTION_FAILED, NOT_LIVE_ERROR);

errorCallback();

return;
}

Expand All @@ -413,6 +416,8 @@ export default class Moderator extends Listenable {

this.xmpp.eventEmitter.emit(CONNECTION_REDIRECTED, conferenceRequest.vnode, conferenceRequest.focusJid);

errorCallback();

return;
}

Expand All @@ -425,7 +430,7 @@ export default class Moderator extends Listenable {
logger.info(`Not ready yet, will retry in ${waitMs} ms.`);
window.setTimeout(
() => this.sendConferenceRequest(roomJid)
.then(callback),
.then(callback).catch(errorCallback),
waitMs);
}
}
Expand All @@ -436,9 +441,10 @@ export default class Moderator extends Listenable {
* @param sessionError
* @param notAuthorized
* @param callback
* @param errorCallback
* @private
*/
_handleError(roomJid, sessionError, notAuthorized, callback) {
_handleError(roomJid, sessionError, notAuthorized, callback, errorCallback) { // eslint-disable-line max-params
// If the session is invalid, remove and try again without session ID to get
// a new one
if (sessionError) {
Expand All @@ -451,6 +457,8 @@ export default class Moderator extends Listenable {
logger.warn('Unauthorized to start the conference');
this.eventEmitter.emit(XMPPEvents.AUTHENTICATION_REQUIRED);

errorCallback();

return;
}

Expand All @@ -461,13 +469,16 @@ export default class Moderator extends Listenable {
logger.info(`Invalid session, will retry after ${waitMs} ms.`);
this.getNextTimeout(true);
window.setTimeout(() => this.sendConferenceRequest(roomJid)
.then(callback), waitMs);
.then(callback)
.catch(errorCallback), waitMs);
} else {
logger.error('Failed to get a successful response, giving up.');

// This is a "fatal" error and the user of the lib should handle it accordingly.
// TODO: change the event name to something accurate.
this.eventEmitter.emit(XMPPEvents.FOCUS_DISCONNECTED);

errorCallback();
}
}

Expand All @@ -478,8 +489,9 @@ export default class Moderator extends Listenable {
* @param error - the error result of the request that {@link sendConferenceRequest} sent
* @param {Function} callback - the function to be called back upon the
* successful allocation of the conference focus
* @param errorCallback
*/
_handleIqError(roomJid, error, callback) {
_handleIqError(roomJid, error, callback, errorCallback) {
// The reservation system only works over XMPP. Handle the error separately.
// Check for error returned by the reservation system
const reservationErr = $(error).find('>error>reservation-error');
Expand All @@ -498,6 +510,8 @@ export default class Moderator extends Listenable {
errorCode,
errorMsg);

errorCallback();

return;
}

Expand All @@ -507,7 +521,7 @@ export default class Moderator extends Listenable {
// Not authorized to create new room
const notAuthorized = $(error).find('>error>not-authorized').length > 0;

this._handleError(roomJid, invalidSession, notAuthorized, callback);
this._handleError(roomJid, invalidSession, notAuthorized, callback, errorCallback);
}

/**
Expand All @@ -518,12 +532,13 @@ export default class Moderator extends Listenable {
* @param result - the success (i.e. non-error) result of the request that {@link #sendConferenecRequest} sent
* @param {Function} callback - the function to be called back upon the
* successful allocation of the conference focus
* @param errorCallback
*/
_handleIqSuccess(roomJid, result, callback) {
_handleIqSuccess(roomJid, result, callback, errorCallback) {
// Setup config options
const conferenceRequest = this._parseConferenceIq(result);

this._handleSuccess(roomJid, conferenceRequest, callback);
this._handleSuccess(roomJid, conferenceRequest, callback, errorCallback);
}

/**
Expand Down

0 comments on commit 83ba892

Please sign in to comment.