From 559b7bf1ef8877b03dd260cee860d7f8f16f1261 Mon Sep 17 00:00:00 2001 From: JC Brand Date: Fri, 7 Feb 2025 14:30:15 +0200 Subject: [PATCH] Fixes #1677 --- src/headless/plugins/bookmarks/api.js | 2 +- src/headless/plugins/bookmarks/collection.js | 22 ++++++------ .../plugins/bookmarks/tests/bookmarks.js | 35 +++++++++++++++++++ src/headless/plugins/pubsub/api.js | 10 +++--- src/headless/types/plugins/bookmarks/api.d.ts | 4 +-- .../types/plugins/bookmarks/collection.d.ts | 2 +- src/headless/types/plugins/pubsub/api.d.ts | 6 ++-- src/shared/tests/mock.js | 16 ++++----- 8 files changed, 67 insertions(+), 30 deletions(-) diff --git a/src/headless/plugins/bookmarks/api.js b/src/headless/plugins/bookmarks/api.js index 2343e9be1a..99d9ab72dd 100644 --- a/src/headless/plugins/bookmarks/api.js +++ b/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} + * @returns {Promise} */ async get(jid) { const bookmarks = await waitUntil('bookmarksInitialized'); diff --git a/src/headless/plugins/bookmarks/collection.js b/src/headless/plugins/bookmarks/collection.js index 9a4119e60d..17863b6103 100644 --- a/src/headless/plugins/bookmarks/collection.js +++ b/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); } } diff --git a/src/headless/plugins/bookmarks/tests/bookmarks.js b/src/headless/plugins/bookmarks/tests/bookmarks.js index e16e1186b7..c44dc0791a 100644 --- a/src/headless/plugins/bookmarks/tests/bookmarks.js +++ b/src/headless/plugins/bookmarks/tests/bookmarks.js @@ -443,4 +443,39 @@ describe("A bookmark", function () { `); })); + + 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` + + + + + + + + `; + _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'); + }) + ); }); diff --git a/src/headless/plugins/pubsub/api.js b/src/headless/plugins/pubsub/api.js index 82c22dd33b..43fb5dd4e1 100644 --- a/src/headless/plugins/pubsub/api.js +++ b/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} @@ -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} */ 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; diff --git a/src/headless/types/plugins/bookmarks/api.d.ts b/src/headless/types/plugins/bookmarks/api.d.ts index 6fe8b3b009..89270ae114 100644 --- a/src/headless/types/plugins/bookmarks/api.d.ts +++ b/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} + * @returns {Promise} */ - function get(jid: string): Promise; + function get(jid: string): Promise; } //# sourceMappingURL=api.d.ts.map \ No newline at end of file diff --git a/src/headless/types/plugins/bookmarks/collection.d.ts b/src/headless/types/plugins/bookmarks/collection.d.ts index 473c1c6517..e7b49dd848 100644 --- a/src/headless/types/plugins/bookmarks/collection.d.ts +++ b/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; getUnopenedBookmarks(): Promise; } import { Collection } from '@converse/skeletor'; diff --git a/src/headless/types/plugins/pubsub/api.d.ts b/src/headless/types/plugins/pubsub/api.d.ts index 142efe48f0..a67573008f 100644 --- a/src/headless/types/plugins/pubsub/api.d.ts +++ b/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} @@ -11,7 +11,7 @@ declare namespace _default { function get(jid: string, node: string): Promise; /** * 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; } /** - * 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 diff --git a/src/shared/tests/mock.js b/src/shared/tests/mock.js index 092c26989c..aea092e11f 100644 --- a/src/shared/tests/mock.js +++ b/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 }; } }