Avoid possible exception when leaving/reconnecting in a MUC

- Unregister nickname before sending unavailable presence
- Send unavailable presence before destroying state
- Call `leave` after unregistering, otherwise the disco entry for the MUC gets removed in `leave` while it's still necessary to unregister
- Wrap `this.leave` in  try/except in `onConnectionStatusChanged` handler
- Add new MUC connection status, `CLOSING` to avoid `this.leave()` being called when `hidden` gets set to `true` while the MUC is in the process of being closed.
This commit is contained in:
JC Brand 2021-11-04 17:57:38 +01:00
parent 93e1758a0b
commit a60127e66f
2 changed files with 53 additions and 35 deletions

View File

@ -117,6 +117,7 @@ converse.ROOMSTATUS = {
ENTERED: 5, ENTERED: 5,
DESTROYED: 6, DESTROYED: 6,
BANNED: 7, BANNED: 7,
CLOSING: 8
}; };
converse.plugins.add('converse-muc', { converse.plugins.add('converse-muc', {

View File

@ -259,19 +259,20 @@ const ChatRoomMixin = {
* @method _converse.ChatRoom#onHiddenChange * @method _converse.ChatRoom#onHiddenChange
*/ */
async onHiddenChange () { async onHiddenChange () {
const roomstatus = converse.ROOMSTATUS;
const conn_status = this.session.get('connection_status'); const conn_status = this.session.get('connection_status');
if (this.get('hidden')) { if (this.get('hidden')) {
if (conn_status === converse.ROOMSTATUS.ENTERED && if (conn_status === roomstatus.ENTERED &&
api.settings.get('muc_subscribe_to_rai') && api.settings.get('muc_subscribe_to_rai') &&
this.getOwnAffiliation() !== 'none') { this.getOwnAffiliation() !== 'none') {
if (conn_status !== converse.ROOMSTATUS.DISCONNECTED) { if (conn_status !== roomstatus.DISCONNECTED && conn_status !== roomstatus.CLOSING) {
this.sendMarkerForLastMessage('received', true); this.sendMarkerForLastMessage('received', true);
await this.leave(); await this.leave();
} }
this.enableRAI(); this.enableRAI();
} }
} else { } else {
if (conn_status === converse.ROOMSTATUS.DISCONNECTED) { if (conn_status === roomstatus.DISCONNECTED) {
this.rejoin(); this.rejoin();
} }
this.clearUnreadMsgCounter(); this.clearUnreadMsgCounter();
@ -339,7 +340,11 @@ const ChatRoomMixin = {
async onConnectionStatusChanged () { async onConnectionStatusChanged () {
if (this.session.get('connection_status') === converse.ROOMSTATUS.ENTERED) { if (this.session.get('connection_status') === converse.ROOMSTATUS.ENTERED) {
if (this.get('hidden') && api.settings.get('muc_subscribe_to_rai') && this.getOwnAffiliation() !== 'none') { if (this.get('hidden') && api.settings.get('muc_subscribe_to_rai') && this.getOwnAffiliation() !== 'none') {
try {
await this.leave(); await this.leave();
} catch (e) {
log.error(e);
}
this.enableRAI(); this.enableRAI();
} else { } else {
await this.onRoomEntered(); await this.onRoomEntered();
@ -852,28 +857,35 @@ const ChatRoomMixin = {
* @param { string } [exit_msg] - Message to indicate your reason for leaving * @param { string } [exit_msg] - Message to indicate your reason for leaving
*/ */
async leave (exit_msg) { async leave (exit_msg) {
this.features.destroy(); api.connection.connected() && api.user.presence.send('unavailable', this.getRoomJIDAndNick(), exit_msg);
// Delete the features model
if (this.features) {
await new Promise(resolve =>
this.features.destroy({
'success': resolve,
'error': (m, e) => { log.error(e); resolve(); }
})
);
}
// Delete disco entity
const disco_entity = _converse.disco_entities?.get(this.get('jid')); const disco_entity = _converse.disco_entities?.get(this.get('jid'));
if (disco_entity) { if (disco_entity) {
await new Promise((success, error) => disco_entity.destroy({ success, error })); await new Promise(resolve => disco_entity.destroy({
} 'success': resolve,
if (api.connection.connected()) { 'error': (m, e) => { log.error(e); resolve(); }
api.user.presence.send('unavailable', this.getRoomJIDAndNick(), exit_msg); }));
} }
u.safeSave(this.session, { 'connection_status': converse.ROOMSTATUS.DISCONNECTED }); u.safeSave(this.session, { 'connection_status': converse.ROOMSTATUS.DISCONNECTED });
}, },
async close (ev) { async close (ev) {
u.safeSave(this.session, { 'connection_status': converse.ROOMSTATUS.CLOSING });
this.sendMarkerForLastMessage('received', true);
await this.unregisterNickname();
await this.leave(); await this.leave();
if (
api.settings.get('auto_register_muc_nickname') === 'unregister' &&
(await api.disco.supports(Strophe.NS.MUC_REGISTER, this.get('jid')))
) {
this.unregisterNickname();
}
this.occupants.clearStore();
this.occupants.clearStore();
if (ev?.name !== 'closeAllChatBoxes' && api.settings.get('muc_clear_messages_on_leave')) { if (ev?.name !== 'closeAllChatBoxes' && api.settings.get('muc_clear_messages_on_leave')) {
this.clearMessages(); this.clearMessages();
} }
@ -882,20 +894,7 @@ const ChatRoomMixin = {
await new Promise(resolve => await new Promise(resolve =>
this.session.destroy({ this.session.destroy({
'success': resolve, 'success': resolve,
'error': (m, e) => { 'error': (m, e) => { log.error(e); resolve(); }
log.error(e);
resolve();
}
})
);
// Delete the features model
await new Promise(resolve =>
this.features.destroy({
'success': resolve,
'error': (m, e) => {
log.error(e);
resolve();
}
}) })
); );
return _converse.ChatBox.prototype.close.call(this); return _converse.ChatBox.prototype.close.call(this);
@ -1647,15 +1646,31 @@ const ChatRoomMixin = {
} }
}, },
/**
* Check whether we should unregister the user from this MUC, and if so,
* call { @link _converse.ChatRoom#sendUnregistrationIQ }
* @method _converse.ChatRoom#unregisterNickname
*/
async unregisterNickname () {
if (api.settings.get('auto_register_muc_nickname') === 'unregister') {
try {
if (await api.disco.supports(Strophe.NS.MUC_REGISTER, this.get('jid'))) {
await this.sendUnregistrationIQ();
}
} catch (e) {
log.error(e);
}
}
},
/** /**
* Send an IQ stanza to the MUC to unregister this user's nickname. * Send an IQ stanza to the MUC to unregister this user's nickname.
* If the user had a 'member' affiliation, it'll be removed and their * If the user had a 'member' affiliation, it'll be removed and their
* nickname will no longer be reserved and can instead be used (and * nickname will no longer be reserved and can instead be used (and
* registered) by other users. * registered) by other users.
* @private * @method _converse.ChatRoom#sendUnregistrationIQ
* @method _converse.ChatRoom#unregisterNickname
*/ */
unregisterNickname () { sendUnregistrationIQ () {
const iq = $iq({ 'to': this.get('jid'), 'type': 'set' }) const iq = $iq({ 'to': this.get('jid'), 'type': 'set' })
.c('query', { 'xmlns': Strophe.NS.MUC_REGISTER }) .c('query', { 'xmlns': Strophe.NS.MUC_REGISTER })
.c('remove'); .c('remove');
@ -2588,7 +2603,10 @@ const ChatRoomMixin = {
async onOwnPresence (stanza) { async onOwnPresence (stanza) {
await this.occupants.fetched; await this.occupants.fetched;
const old_status = this.session.get('connection_status'); const old_status = this.session.get('connection_status');
if (stanza.getAttribute('type') !== 'unavailable' && old_status !== converse.ROOMSTATUS.ENTERED) { if (stanza.getAttribute('type') !== 'unavailable' &&
old_status !== converse.ROOMSTATUS.ENTERED &&
old_status !== converse.ROOMSTATUS.CLOSING
) {
// Set connection_status before creating the occupant, but // Set connection_status before creating the occupant, but
// only trigger afterwards, so that plugins can access the // only trigger afterwards, so that plugins can access the
// occupant in their event handlers. // occupant in their event handlers.
@ -2627,7 +2645,6 @@ const ChatRoomMixin = {
} }
} }
} }
this.session.save({ 'connection_status': converse.ROOMSTATUS.ENTERED });
}, },
/** /**