Skip to content

Commit

Permalink
Fix using signaling settings while being refetched
Browse files Browse the repository at this point in the history
When the external signaling server returns the error "token_expired" the
signaling settings are fetched again. However, if there are further
requests and "token_expired" is returned again the previous fetch is
canceled and a new one started instead, which caused the settings to be
temporary set to "null". The signaling settings are expected to always
be an object, so setting them to "null" could cause an error if they
were used during that time (for example, during a reconnection, which
caused the signaling object to "hang" and not do any further connection
attempt).

Preventing the settings to be nullified is only half of the story,
though; "token_expired" can be thrown when trying to connect, and errors
thrown when trying to connect cause a reconnection attempt. This could
end in a reconnection loop, as each "token_expired" error would
cause another fetch of the settings, cancelling the previous fetch and
thus preventing the settings to be updated, and as the previous settings
would be still used any connection attempt would end again in another
"token_expired" error.

To solve all that now any connection attempt done after receiving a
"token_expired" error is deferred until the signaling settings were
updated.

Note that the previous signaling settings are kept to ensure that a
signaling object is always available, even if outdated. However, any
usage of the outdated signaling settings is expected to cause a
"token_expired" error to be thrown; right now that only happens during
connections, so only that code needs to wait for the settings to be
fetched again.

Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
  • Loading branch information
danxuliu authored and backportbot-nextcloud[bot] committed Aug 17, 2023
1 parent 4fddf92 commit 1c1c408
Show file tree
Hide file tree
Showing 2 changed files with 39 additions and 1 deletion.
38 changes: 38 additions & 0 deletions src/utils/signaling.js
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,20 @@ Signaling.Base.prototype._trigger = function(ev, args) {
EventBus.$emit('signaling-' + kebabCase(ev), args)
}

Signaling.Base.prototype.setSettings = function(settings) {
if (!settings) {
// Signaling object is expected to always have a settings object
return
}

this.settings = settings

if (this._pendingUpdateSettingsPromise) {
this._pendingUpdateSettingsPromise.resolve()
delete this._pendingUpdateSettingsPromise
}
}

Signaling.Base.prototype.isNoMcuWarningEnabled = function() {
return !this.settings.hideWarning
}
Expand Down Expand Up @@ -646,6 +660,19 @@ Signaling.Standalone.prototype.connect = function() {
}, 2000)
}

if (this._pendingUpdateSettingsPromise) {
console.info('Deferring establishing signaling connection until signaling settings are updated')

this._pendingUpdateSettingsPromise.then(() => {
// "reconnect()" is called instead of "connect()", even if that
// slightly delays the connection, as "reconnect()" prevents
// duplicated connection requests.
this.reconnect()
})

return
}

console.debug('Connecting to ' + this.url + ' for ' + this.settings.token)
this.callbacks = {}
this.id = 1
Expand Down Expand Up @@ -1353,6 +1380,17 @@ Signaling.Standalone.prototype.processRoomParticipantsEvent = function(data) {
Signaling.Standalone.prototype.processErrorTokenExpired = function() {
console.info('The signaling token is expired, need to update settings')

if (!this._pendingUpdateSettingsPromise) {
let pendingUpdateSettingsPromiseResolve
this._pendingUpdateSettingsPromise = new Promise((resolve, reject) => {
// The Promise executor is run even before the Promise constructor has
// finished, so "this._pendingUpdateSettingsPromise" is not available
// yet.
pendingUpdateSettingsPromiseResolve = resolve
})
this._pendingUpdateSettingsPromise.resolve = pendingUpdateSettingsPromiseResolve
}

this._trigger('updateSettings')
}

Expand Down
2 changes: 1 addition & 1 deletion src/utils/webrtc/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ async function connectSignaling(token) {
signaling.on('updateSettings', async function() {
const settings = await getSignalingSettings(token)
console.debug('Received updated settings', settings)
signaling.settings = settings
signaling.setSettings(settings)
})

}
Expand Down

0 comments on commit 1c1c408

Please sign in to comment.