Browse Source

Bugfix. MUCs losing their names after reconnection

This was partly due to a race condition (timing issue) where the disco
identity was not yet resolved by the time the name was saved.

Fixed by waiting on the `waitUntilItemsFetched` promise, which only
resolves after the identity has been fetched.
JC Brand 3 months ago
parent
commit
d99062b2e5

+ 4 - 1
src/headless/plugins/disco/api.js

@@ -393,12 +393,15 @@ export default {
                 if (!entity.waitUntilFeaturesDiscovered.isPending) {
                 if (!entity.waitUntilFeaturesDiscovered.isPending) {
                     entity.waitUntilFeaturesDiscovered = getOpenPromise();
                     entity.waitUntilFeaturesDiscovered = getOpenPromise();
                 }
                 }
+                if (!entity.waitUntilItemsFetched.isPending) {
+                    entity.waitUntilItemsFetched = getOpenPromise();
+                }
                 entity.queryInfo();
                 entity.queryInfo();
             } else {
             } else {
                 // Create it if it doesn't exist
                 // Create it if it doesn't exist
                 entity = await api.disco.entities.create({ jid }, { ignore_cache: true });
                 entity = await api.disco.entities.create({ jid }, { ignore_cache: true });
             }
             }
-            return entity.waitUntilFeaturesDiscovered;
+            return entity.waitUntilItemsFetched;
         },
         },
 
 
         /**
         /**

+ 21 - 10
src/headless/plugins/disco/entity.js

@@ -62,11 +62,8 @@ class DiscoEntity extends Model {
      * @param {String} type - The identity type
      * @param {String} type - The identity type
      */
      */
     async getIdentity(category, type) {
     async getIdentity(category, type) {
-        await this.waitUntilFeaturesDiscovered;
-        return this.identities.findWhere({
-            'category': category,
-            'type': type,
-        });
+        await this.waitUntilItemsFetched;
+        return this.identities.findWhere({ category, type });
     }
     }
 
 
     /**
     /**
@@ -152,8 +149,22 @@ class DiscoEntity extends Model {
         try {
         try {
             stanza = await api.disco.info(this.get('jid'), null);
             stanza = await api.disco.info(this.get('jid'), null);
         } catch (iq) {
         } catch (iq) {
-            iq === null ? log.error(`Timeout for disco#info query for ${this.get('jid')}`) : log.error(iq);
-            this.waitUntilFeaturesDiscovered.resolve(u.isElement(iq) ? await parseErrorStanza(iq) : iq);
+            if (u.isElement(iq)) {
+                const e = await parseErrorStanza(iq);
+                if (e.message !== 'item-not-found') {
+                    log.error(`Error querying disco#info for ${this.get('jid')}: ${e.message}`);
+                }
+                this.waitUntilFeaturesDiscovered.resolve(e);
+                this.waitUntilItemsFetched.resolve(e);
+            } else {
+                if (iq === null) {
+                    log.error(`Timeout for disco#info query for ${this.get('jid')}`);
+                } else {
+                    log.error(`Error querying disco#info for ${this.get('jid')}: ${iq}`);
+                }
+                this.waitUntilFeaturesDiscovered.resolve(iq);
+                this.waitUntilItemsFetched.resolve(iq);
+            }
             return;
             return;
         }
         }
         this.onInfo(stanza);
         this.onInfo(stanza);
@@ -204,9 +215,9 @@ class DiscoEntity extends Model {
     async onInfo(stanza) {
     async onInfo(stanza) {
         Array.from(stanza.querySelectorAll('identity')).forEach((identity) => {
         Array.from(stanza.querySelectorAll('identity')).forEach((identity) => {
             this.identities.create({
             this.identities.create({
-                'category': identity.getAttribute('category'),
-                'type': identity.getAttribute('type'),
-                'name': identity.getAttribute('name'),
+                category: identity.getAttribute('category'),
+                type: identity.getAttribute('type'),
+                name: identity.getAttribute('name'),
             });
             });
         });
         });
 
 

+ 9 - 7
src/headless/plugins/muc/muc.js

@@ -1244,13 +1244,15 @@ class MUC extends ModelWithVCard(ModelWithMessages(ColorAwareModel(ChatBoxBase))
      * https://xmpp.org/extensions/xep-0045.html#disco-roominfo
      * https://xmpp.org/extensions/xep-0045.html#disco-roominfo
      * @returns {Promise}
      * @returns {Promise}
      */
      */
-    getDiscoInfo() {
-        return api.disco
-            .getIdentity("conference", "text", this.get("jid"))
-            .then((identity) => this.save({ name: identity?.get("name") }))
-            .then(() => this.getDiscoInfoFields())
-            .then(() => this.getDiscoInfoFeatures())
-            .catch((e) => log.error(e));
+    async getDiscoInfo() {
+        const identity = await api.disco.getIdentity("conference", "text", this.get("jid"));
+        if (identity?.get('name')) {
+            this.save({ name: identity.get("name") });
+        } else {
+            log.error(`No identity or name found for ${this.get("jid")}`);
+        }
+        await this.getDiscoInfoFields();
+        await this.getDiscoInfoFeatures();
     }
     }
 
 
     /**
     /**

+ 22 - 16
src/shared/tests/mock.js

@@ -215,6 +215,10 @@ async function openChatBoxFor (_converse, jid) {
     return u.waitUntil(() => _converse.chatboxviews.get(jid), 1000);
     return u.waitUntil(() => _converse.chatboxviews.get(jid), 1000);
 }
 }
 
 
+/**
+ * Returns an item-not-found disco info result, simulating that this was a
+ * new MUC being entered.
+ */
 async function waitForNewMUCDiscoInfo(_converse, muc_jid) {
 async function waitForNewMUCDiscoInfo(_converse, muc_jid) {
     const { api } = _converse;
     const { api } = _converse;
     const connection = api.connection.get();
     const connection = api.connection.get();
@@ -370,22 +374,24 @@ async function receiveOwnMUCPresence (_converse, muc_jid, nick, affiliation='own
     const sent_stanzas = _converse.api.connection.get().sent_stanzas;
     const sent_stanzas = _converse.api.connection.get().sent_stanzas;
     await u.waitUntil(() => sent_stanzas.filter(iq => sizzle('presence history', iq).length).pop());
     await u.waitUntil(() => sent_stanzas.filter(iq => sizzle('presence history', iq).length).pop());
 
 
-    const presence = $pres({
-            to: _converse.api.connection.get().jid,
-            from: `${muc_jid}/${nick}`,
-            id: u.getUniqueId()
-    }).c('x').attrs({xmlns:'http://jabber.org/protocol/muc#user'})
-        .c('item').attrs({ affiliation, role, 'jid': _converse.bare_jid }).up()
-        .c('status').attrs({code:'110'}).up().up()
-
-    if (features.includes(Strophe.NS.OCCUPANTID)) {
-        presence.c('occupant-id', {'xmlns': Strophe.NS.OCCUPANTID, 'id': u.getUniqueId() });
-    }
-
-    if (_converse.xmppstatus.get('status')) {
-       presence.c('show').t(_converse.xmppstatus.get('status'));
-    }
-    _converse.api.connection.get()._dataRecv(createRequest(presence));
+    _converse.api.connection.get()._dataRecv(createRequest(stx`
+        <presence xmlns="jabber:client"
+                to="${_converse.api.connection.get().jid}"
+                from="${muc_jid}/${nick}"
+                id="${u.getUniqueId()}">
+            <x xmlns="http://jabber.org/protocol/muc#user">
+                <item affiliation="${affiliation}" role="${role}" jid="${_converse.bare_jid}"/>
+                <status code="110"/>
+            </x>
+            ${ (features.includes(Strophe.NS.OCCUPANTID))
+                ? stx`<occupant-id xmlns="${Strophe.NS.OCCUPANTID}" id="${u.getUniqueId()}"/>`
+                : ''
+            }
+            ${ _converse.xmppstatus.get('status')
+                ? stx`<show>${_converse.xmppstatus.get('status')}</show>`
+                : ''
+            }
+        </presence>`));
 }
 }
 
 
 async function openAddMUCModal (_converse) {
 async function openAddMUCModal (_converse) {