Fixes #1677

src/headless/plugins/bookmarks/api.js

@@ -26,7 +26,7 @@ const bookmarks = {
     /**
      * @method api.bookmarks.get
      * @param {string} jid - The JID of the bookmark to return.
-     * @returns {Promise<import('./model').default>}
+     * @returns {Promise<import('./model').default|undefined>}
      */
     async get(jid) {
         const bookmarks = await waitUntil('bookmarksInitialized');

src/headless/plugins/bookmarks/collection.js

@@ -8,6 +8,7 @@ import Bookmark from './model.js';
 import _converse from '../../shared/_converse.js';
 import api from '../../shared/api/index.js';
 import converse from '../../shared/api/public.js';
+import { parseErrorStanza } from '../../shared/parsers.js';
 import log from '../../log.js';
 import { initStorage } from '../../utils/storage.js';
 import { parseStanzaForBookmarks } from './parsers.js';
@@ -262,9 +263,9 @@ class Bookmarks extends Collection {
      * @param {Object} deferred
      * @param {Element} iq
      */
-    onBookmarksReceivedError(deferred, iq) {
-        const { __ } = _converse;
+    async onBookmarksReceivedError(deferred, iq) {
         if (iq === null) {
+            const { __ } = _converse;
             log.error('Error: timeout while fetching bookmarks');
             api.alert('error', __('Timeout Error'), [
                 __(
@@ -272,19 +273,20 @@ class Bookmarks extends Collection {
                         'You can reload the page to request them again.'
                 ),
             ]);
-        } else if (deferred) {
-            if (iq.querySelector('error[type="cancel"] item-not-found')) {
+            deferred?.reject(new Error('Could not fetch bookmarks'));
+
+        } else {
+            const { errors } = converse.env;
+            const e = await parseErrorStanza(iq);
+            if (e instanceof errors.ItemNotFoundError) {
                 // Not an exception, the user simply doesn't have any bookmarks.
                 window.sessionStorage.setItem(this.fetched_flag, 'true');
-                return deferred.resolve();
+                deferred?.resolve();
             } else {
                 log.error('Error while fetching bookmarks');
-                log.error(iq);
-                return deferred.reject(new Error('Could not fetch bookmarks'));
+                if (iq) log.error(iq);
+                deferred?.reject(new Error('Could not fetch bookmarks'));
             }
-        } else {
-            log.error('Error while fetching bookmarks');
-            log.error(iq);
         }
     }
 

src/headless/plugins/bookmarks/tests/bookmarks.js

@@ -443,4 +443,39 @@ describe("A bookmark", function () {
                 </pubsub>
             </iq>`);
     }));
+
+    it("handles missing bookmarks gracefully when server responds with item-not-found", mock.initConverse(
+        ['chatBoxesFetched'], {}, async (_converse) => {
+
+            await mock.waitForRoster(_converse, 'current', 0);
+            await mock.waitUntilDiscoConfirmed(
+                _converse, _converse.bare_jid,
+                [{'category': 'pubsub', 'type': 'pep'}],
+                ['http://jabber.org/protocol/pubsub#publish-options', 'urn:xmpp:bookmarks:1#compat']
+            );
+
+            const IQ_stanzas = _converse.api.connection.get().IQ_stanzas;
+            const sent_stanza = await u.waitUntil(
+                () => IQ_stanzas.filter(s => sizzle(`items[node="urn:xmpp:bookmarks:1"]`, s).length).pop());
+
+            // Simulate server response with item-not-found error
+            const error_stanza = stx`
+                <iq xmlns="jabber:client" type="error"
+                        id="${sent_stanza.getAttribute('id')}"
+                        from="${sent_stanza.getAttribute('to')}"
+                        to="${sent_stanza.getAttribute('from')}">
+                    <pubsub xmlns="http://jabber.org/protocol/pubsub">
+                        <items node="urn:xmpp:bookmarks:1"/>
+                    </pubsub>
+                    <error code="404" type="cancel">
+                        <item-not-found xmlns="urn:ietf:params:xml:ns:xmpp-stanzas"/>
+                    </error>
+                </iq>`;
+            _converse.api.connection.get()._dataRecv(mock.createRequest(error_stanza));
+
+            const cache_key = `converse.room-bookmarksromeo@montague.litfetched`;
+            const result = await u.waitUntil(() => window.sessionStorage.getItem(cache_key));
+            expect(result).toBe('true');
+        })
+    );
 });

src/headless/plugins/pubsub/api.js

@@ -25,7 +25,7 @@ export default {
         config: {
             /**
              * Fetches the configuration for a PubSub node
-             * @method _converse.api.pubsub.configure
+             * @method _converse.api.pubsub.config.get
              * @param {string} jid - The JID of the pubsub service where the node resides
              * @param {string} node - The node to configure
              * @returns {Promise<import('./types').PubSubConfigOptions>}
@@ -56,7 +56,7 @@ export default {
 
             /**
              * Configures a PubSub node
-             * @method _converse.api.pubsub.configure
+             * @method _converse.api.pubsub.config.set
              * @param {string} jid The JID of the pubsub service where the node resides
              * @param {string} node The node to configure
              * @param {PubSubConfigOptions} config The configuration options
@@ -99,7 +99,7 @@ export default {
         },
 
         /**
-         * Publshes an item to a PubSub node
+         * Publishes an item to a PubSub node
          * @method _converse.api.pubsub.publish
          * @param {string} jid The JID of the pubsub service where the node resides.
          * @param {string} node The node being published to
@@ -113,8 +113,8 @@ export default {
          * @returns {Promise<void|Element>}
          */
         async publish(jid, node, item, options, strict_options = true) {
-            if (!node) throw new Error('api.pubsub.config.publish: node value required');
-            if (!item) throw new Error('api.pubsub.config.publish: item value required');
+            if (!node) throw new Error('api.pubsub.publish: node value required');
+            if (!item) throw new Error('api.pubsub.publish: item value required');
 
             const bare_jid = _converse.session.get('bare_jid');
             const entity_jid = jid || bare_jid;

src/headless/types/plugins/bookmarks/api.d.ts

@@ -16,8 +16,8 @@ declare namespace bookmarks {
     /**
      * @method api.bookmarks.get
      * @param {string} jid - The JID of the bookmark to return.
-     * @returns {Promise<import('./model').default>}
+     * @returns {Promise<import('./model').default|undefined>}
      */
-    function get(jid: string): Promise<import("./model").default>;
+    function get(jid: string): Promise<import("./model").default | undefined>;
 }
 //# sourceMappingURL=api.d.ts.map

src/headless/types/plugins/bookmarks/collection.d.ts

@@ -59,7 +59,7 @@ declare class Bookmarks extends Collection {
      * @param {Object} deferred
      * @param {Element} iq
      */
-    onBookmarksReceivedError(deferred: any, iq: Element): any;
+    onBookmarksReceivedError(deferred: any, iq: Element): Promise<void>;
     getUnopenedBookmarks(): Promise<any>;
 }
 import { Collection } from '@converse/skeletor';

src/headless/types/plugins/pubsub/api.d.ts

@@ -3,7 +3,7 @@ declare namespace _default {
         namespace config {
             /**
              * Fetches the configuration for a PubSub node
-             * @method _converse.api.pubsub.configure
+             * @method _converse.api.pubsub.config.get
              * @param {string} jid - The JID of the pubsub service where the node resides
              * @param {string} node - The node to configure
              * @returns {Promise<import('./types').PubSubConfigOptions>}
@@ -11,7 +11,7 @@ declare namespace _default {
             function get(jid: string, node: string): Promise<import("./types").PubSubConfigOptions>;
             /**
              * Configures a PubSub node
-             * @method _converse.api.pubsub.configure
+             * @method _converse.api.pubsub.config.set
              * @param {string} jid The JID of the pubsub service where the node resides
              * @param {string} node The node to configure
              * @param {PubSubConfigOptions} config The configuration options
@@ -20,7 +20,7 @@ declare namespace _default {
             function set(jid: string, node: string, config: import("./types").PubSubConfigOptions): Promise<import("./types").PubSubConfigOptions>;
         }
         /**
-         * Publshes an item to a PubSub node
+         * Publishes an item to a PubSub node
          * @method _converse.api.pubsub.publish
          * @param {string} jid The JID of the pubsub service where the node resides.
          * @param {string} node The node being published to

src/shared/tests/mock.js

@@ -737,14 +737,14 @@ function getMockVcardFetcher (settings) {
         const vcard_el = vcard.tree();
 
         return {
-            'stanza': vcard_el,
-            'fullname': vcard_el.querySelector('FN')?.textContent,
-            'nickname': vcard_el.querySelector('NICKNAME')?.textContent,
-            'image': vcard_el.querySelector('PHOTO BINVAL')?.textContent,
-            'image_type': vcard_el.querySelector('PHOTO TYPE')?.textContent,
-            'url': vcard_el.querySelector('URL')?.textContent,
-            'vcard_updated': dayjs().format(),
-            'vcard_error': undefined
+            stanza: vcard_el,
+            fullname: vcard_el.querySelector('FN')?.textContent,
+            nickname: vcard_el.querySelector('NICKNAME')?.textContent,
+            image: vcard_el.querySelector('PHOTO BINVAL')?.textContent,
+            image_type: vcard_el.querySelector('PHOTO TYPE')?.textContent,
+            url: vcard_el.querySelector('URL')?.textContent,
+            vcard_updated: dayjs().format(),
+            vcard_error: undefined
         };
     }
 }