Show error message with option to retry when MAM query times out

CHANGES.md

@@ -5,6 +5,7 @@
 - Add a new GUI for moderator actions. You can trigger it by entering `/modtools` in a MUC.
 - Reconnect if the server doesn't respond to a `ping` within 10 seconds.
 - Don't query for MAM MUC messages before the cached messages have been restored (another cause of duplicate messages).
+- Show an error message and option to retry when fetching of the MAM archive times out
 
 ## 5.0.0 (2019-08-08)
 

sass/_core.scss

@@ -450,7 +450,7 @@ body.converse-fullscreen {
         width: 1em;
         display: block;
         text-align: center;
-        margin: 2em;
+        padding: 0.5em 0;
         font-size: 24px;
     }
     .left {

spec/http-file-upload.js

@@ -573,7 +573,7 @@
                         await u.waitUntil(() => view.el.querySelectorAll('.message').length)
                         const messages = view.el.querySelectorAll('.message.chat-error');
                         expect(messages.length).toBe(1);
-                        expect(messages[0].textContent).toBe(
+                        expect(messages[0].textContent.trim()).toBe(
                             'The size of your file, my-juliet.jpg, exceeds the maximum allowed by your server, which is 5 MB.');
                         done();
                     }));

spec/mam.js

@@ -772,7 +772,7 @@
                  *  </result>
                  *  </message>
                  */
-                const msg1 = $msg({'id':'aeb213', 'to':'juliet@capulet.lit/chamber'})
+                const msg1 = $msg({'id':'aeb212', 'to':'juliet@capulet.lit/chamber'})
                             .c('result',  {'xmlns': 'urn:xmpp:mam:2', 'queryid':queryid, 'id':'28482-98726-73623'})
                                 .c('forwarded', {'xmlns':'urn:xmpp:forward:0'})
                                     .c('delay', {'xmlns':'urn:xmpp:delay', 'stamp':'2010-07-10T23:08:25Z'}).up()
@@ -943,7 +943,7 @@
                         `</query>`+
                     `</iq>`
                 );
-                const msg1 = $msg({'id':'aeb213', 'to': contact_jid})
+                const msg1 = $msg({'id':'aeb212', 'to': contact_jid})
                             .c('result',  {'xmlns': 'urn:xmpp:mam:2', 'queryid':queryid, 'id':'28482-98726-73623'})
                                 .c('forwarded', {'xmlns':'urn:xmpp:forward:0'})
                                     .c('delay', {'xmlns':'urn:xmpp:delay', 'stamp':'2010-07-10T23:08:25Z'}).up()
@@ -974,6 +974,110 @@
                 _converse.connection._dataRecv(test_utils.createRequest(stanza));
                 done();
             }));
+
+            it("will show an error message if the MAM query times out",
+                mock.initConverse(
+                    null, ['discoInitialized'], {},
+                    async function (done, _converse) {
+
+                const sendIQ = _converse.connection.sendIQ;
+
+                let timeout_happened = false;
+                spyOn(_converse.connection, 'sendIQ').and.callFake(function (iq, callback, errback) {
+                    sendIQ.bind(this)(iq, callback, errback);
+                    if (!timeout_happened) {
+                        if (typeof(iq.tree) === "function") {
+                            iq = iq.tree();
+                        }
+                        if (sizzle('query[xmlns="urn:xmpp:mam:2"]', iq).length) {
+                            // We emulate a timeout event
+                            callback(null);
+                            timeout_happened = true;
+                        }
+                    }
+                });
+                await test_utils.waitForRoster(_converse, 'current', 1);
+                const contact_jid = mock.cur_names[0].replace(/ /g,'.').toLowerCase() + '@montague.lit';
+                await test_utils.openChatBoxFor(_converse, contact_jid);
+                await test_utils.waitUntilDiscoConfirmed(_converse, _converse.bare_jid, null, [Strophe.NS.MAM]);
+
+                const IQ_stanzas = _converse.connection.IQ_stanzas;
+                let sent_stanza = await u.waitUntil(() => IQ_stanzas.filter(iq => sizzle('query[xmlns="urn:xmpp:mam:2"]', iq).length).pop());
+                let queryid = sent_stanza.querySelector('query').getAttribute('queryid');
+
+                expect(Strophe.serialize(sent_stanza)).toBe(
+                    `<iq id="${sent_stanza.getAttribute('id')}" type="set" xmlns="jabber:client">`+
+                        `<query queryid="${queryid}" xmlns="urn:xmpp:mam:2">`+
+                            `<x type="submit" xmlns="jabber:x:data">`+
+                                `<field type="hidden" var="FORM_TYPE"><value>urn:xmpp:mam:2</value></field>`+
+                                `<field var="with"><value>mercutio@montague.lit</value></field>`+
+                            `</x>`+
+                            `<set xmlns="http://jabber.org/protocol/rsm"><max>50</max><before></before></set>`+
+                        `</query>`+
+                    `</iq>`);
+
+                const view = _converse.chatboxviews.get(contact_jid);
+                expect(view.model.messages.length).toBe(1);
+                expect(view.model.messages.at(0).get('ephemeral')).toBe(false);
+                expect(view.model.messages.at(0).get('type')).toBe('error');
+                expect(view.model.messages.at(0).get('message')).toBe('Timeout while trying to fetch archived messages.');
+
+                let err_message = view.el.querySelector('.message.chat-error');
+                err_message.querySelector('.retry').click();
+                expect(err_message.querySelector('.spinner')).not.toBe(null);
+
+                while (_converse.connection.IQ_stanzas.length) {
+                    _converse.connection.IQ_stanzas.pop();
+                }
+                sent_stanza = await u.waitUntil(() => IQ_stanzas.filter(iq => sizzle('query[xmlns="urn:xmpp:mam:2"]', iq).length).pop());
+                queryid = sent_stanza.querySelector('query').getAttribute('queryid');
+                expect(Strophe.serialize(sent_stanza)).toBe(
+                    `<iq id="${sent_stanza.getAttribute('id')}" type="set" xmlns="jabber:client">`+
+                        `<query queryid="${queryid}" xmlns="urn:xmpp:mam:2">`+
+                            `<x type="submit" xmlns="jabber:x:data">`+
+                                `<field type="hidden" var="FORM_TYPE"><value>urn:xmpp:mam:2</value></field>`+
+                                `<field var="with"><value>mercutio@montague.lit</value></field>`+
+                            `</x>`+
+                            `<set xmlns="http://jabber.org/protocol/rsm"><max>50</max><before></before></set>`+
+                        `</query>`+
+                    `</iq>`);
+
+                const msg1 = $msg({'id':'aeb212', 'to': contact_jid})
+                            .c('result',  {'xmlns': 'urn:xmpp:mam:2', 'queryid': queryid, 'id':'28482-98726-73623'})
+                                .c('forwarded', {'xmlns':'urn:xmpp:forward:0'})
+                                    .c('delay', {'xmlns':'urn:xmpp:delay', 'stamp':'2010-07-10T23:08:25Z'}).up()
+                                    .c('message', {
+                                        'xmlns':'jabber:client',
+                                        'to': contact_jid,
+                                        'from': _converse.bare_jid,
+                                        'type':'chat' })
+                                    .c('body').t("Call me but love, and I'll be new baptized;");
+                _converse.connection._dataRecv(test_utils.createRequest(msg1));
+
+                const msg2 = $msg({'id':'aeb213', 'to': contact_jid})
+                            .c('result',  {'xmlns': 'urn:xmpp:mam:2', 'queryid': queryid, 'id':'28482-98726-73624'})
+                                .c('forwarded', {'xmlns':'urn:xmpp:forward:0'})
+                                    .c('delay', {'xmlns':'urn:xmpp:delay', 'stamp':'2010-07-10T23:18:25Z'}).up()
+                                    .c('message', {
+                                        'xmlns':'jabber:client',
+                                        'to': contact_jid,
+                                        'from': _converse.bare_jid,
+                                        'type':'chat' })
+                                    .c('body').t("Henceforth I never will be Romeo.");
+                _converse.connection._dataRecv(test_utils.createRequest(msg2));
+
+                const stanza = $iq({'type': 'result', 'id': sent_stanza.getAttribute('id')})
+                    .c('fin', {'xmlns': 'urn:xmpp:mam:2', 'complete': true})
+                        .c('set',  {'xmlns': 'http://jabber.org/protocol/rsm'})
+                            .c('first', {'index': '0'}).t('28482-98726-73623').up()
+                            .c('last').t('28482-98726-73624').up()
+                            .c('count').t('2');
+                _converse.connection._dataRecv(test_utils.createRequest(stanza));
+                await u.waitUntil(() => view.model.messages.length === 2, 500);
+                err_message = view.el.querySelector('.message.chat-error');
+                expect(err_message).toBe(null);
+                done();
+            }));
         });
     });
 }));

spec/messages.js

@@ -1761,7 +1761,7 @@
                             .t('Server-to-server connection failed: Connecting failed: connection timeout');
                     _converse.connection._dataRecv(test_utils.createRequest(stanza));
                     await new Promise((resolve, reject) => view.once('messageInserted', resolve));
-                    expect(chat_content.querySelector('.chat-error').textContent).toEqual(error_txt);
+                    expect(chat_content.querySelector('.chat-error').textContent.trim()).toEqual(error_txt);
                     stanza = $msg({
                             'to': _converse.connection.jid,
                             'type': 'error',

spec/omemo.js

@@ -1430,7 +1430,7 @@
 
             await u.waitUntil(() => !view.model.get('omemo_supported'));
 
-            expect(view.el.querySelector('.chat-error').textContent).toBe(
+            expect(view.el.querySelector('.chat-error').textContent.trim()).toBe(
                 "oldguy doesn't appear to have a client that supports OMEMO. "+
                 "Encrypted chat will no longer be possible in this grouchat."
             );

src/converse-message-view.js

@@ -15,6 +15,7 @@ import tpl_file_progress from "templates/file_progress.html";
 import tpl_info from "templates/info.html";
 import tpl_message from "templates/message.html";
 import tpl_message_versions_modal from "templates/message_versions_modal.html";
+import tpl_spinner from "templates/spinner.html";
 import u from "@converse/headless/utils/emoji";
 import xss from "xss/dist/xss";
 
@@ -80,7 +81,8 @@ converse.plugins.add('converse-message-view', {
 
         _converse.MessageView = _converse.ViewWithAvatar.extend({
             events: {
-                'click .chat-msg__edit-modal': 'showMessageVersionsModal'
+                'click .chat-msg__edit-modal': 'showMessageVersionsModal',
+                'click .retry': 'onRetryClicked'
             },
 
             initialize () {
@@ -164,6 +166,16 @@ converse.plugins.add('converse-message-view', {
                 }
             },
 
+            async onRetryClicked () {
+                this.showSpinner();
+                await this.model.error.retry();
+                this.model.destroy();
+            },
+
+            showSpinner () {
+                this.el.innerHTML = tpl_spinner();
+            },
+
             onMessageEdited () {
                 if (this.model.get('is_archived')) {
                     return;

src/converse.js

@@ -2,7 +2,6 @@
  * --------------------
  * Any of the following components may be removed if they're not needed.
  */
-
 import "@converse/headless/headless";
 import "converse-autocomplete";
 import "converse-bookmark-views";  // Views for XEP-0048 Bookmarks

src/headless/converse-chatboxes.js

@@ -90,7 +90,8 @@ converse.plugins.add('converse-chatboxes', {
             defaults () {
                 return {
                     'msgid': _converse.connection.getUniqueId(),
-                    'time': (new Date()).toISOString()
+                    'time': (new Date()).toISOString(),
+                    'ephemeral': false
                 };
             },
 
@@ -134,7 +135,7 @@ converse.plugins.add('converse-chatboxes', {
             },
 
             isEphemeral () {
-                return this.isOnlyChatStateNotification() || this.get('type') === 'error';
+                return this.isOnlyChatStateNotification() || this.get('ephemeral');
             },
 
             getDisplayName () {
@@ -178,7 +179,8 @@ converse.plugins.add('converse-chatboxes', {
                     _converse.log(e, Strophe.LogLevel.ERROR);
                     return this.save({
                         'type': 'error',
-                        'message': __("Sorry, could not determine upload URL.")
+                        'message': __("Sorry, could not determine upload URL."),
+                        'ephemeral': true
                     });
                 }
                 const slot = stanza.querySelector('slot');
@@ -190,7 +192,8 @@ converse.plugins.add('converse-chatboxes', {
                 } else {
                     return this.save({
                         'type': 'error',
-                        'message': __("Sorry, could not determine file upload URL.")
+                        'message': __("Sorry, could not determine file upload URL."),
+                        'ephemeral': true
                     });
                 }
             },
@@ -228,7 +231,8 @@ converse.plugins.add('converse-chatboxes', {
                     this.save({
                         'type': 'error',
                         'upload': _converse.FAILURE,
-                        'message': message
+                        'message': message,
+                        'ephemeral': true
                     });
                 };
                 xhr.open('PUT', this.get('put'), true);
@@ -401,6 +405,13 @@ converse.plugins.add('converse-chatboxes', {
                 }
             },
 
+            createMessageFromError (error) {
+                if (error instanceof _converse.TimeoutError) {
+                    const msg = this.messages.create({'type': 'error', 'message': error.message, 'retry': true});
+                    msg.error = error;
+                }
+            },
+
             getOldestMessage () {
                 for (let i=0; i<this.messages.length; i++) {
                     const message = this.messages.at(i);
@@ -798,7 +809,8 @@ converse.plugins.add('converse-chatboxes', {
                 if (!item) {
                     this.messages.create({
                         'message': __("Sorry, looks like file upload is not supported by your server."),
-                        'type': 'error'
+                        'type': 'error',
+                        'ephemeral': true
                     });
                     return;
                 }
@@ -809,7 +821,8 @@ converse.plugins.add('converse-chatboxes', {
                 if (!slot_request_url) {
                     this.messages.create({
                         'message': __("Sorry, looks like file upload is not supported by your server."),
-                        'type': 'error'
+                        'type': 'error',
+                        'ephemeral': true
                     });
                     return;
                 }
@@ -818,7 +831,8 @@ converse.plugins.add('converse-chatboxes', {
                         return this.messages.create({
                             'message': __('The size of your file, %1$s, exceeds the maximum allowed by your server, which is %2$s.',
                                 file.name, filesize(max_file_size)),
-                            'type': 'error'
+                            'type': 'error',
+                            'ephemeral': true
                         });
                     } else {
                         const message = this.messages.create(
@@ -887,9 +901,12 @@ converse.plugins.add('converse-chatboxes', {
                     __('Sorry, an error occurred:') + ' ' + error.innerHTML;
             },
 
+            /**
+             * Given a message stanza, return the text contained in its body.
+             * @private
+             * @param { XMLElement } stanza
+             */
             getMessageBody (stanza) {
-                /* Given a message stanza, return the text contained in its body.
-                 */
                 const type = stanza.getAttribute('type');
                 if (type === 'error') {
                     return this.getErrorMessage(stanza);

src/headless/converse-core.js

@@ -117,6 +117,14 @@ _converse.Collection = Backbone.Collection.extend({
 });
 
 
+/**
+ * Custom error for indicating timeouts
+ * @namespace _converse
+ */
+class TimeoutError extends Error {}
+_converse.TimeoutError = TimeoutError;
+
+
 // Make converse pluggable
 pluggable.enable(_converse, '_converse', 'pluggable');
 

src/headless/converse-mam.js

@@ -128,6 +128,11 @@ converse.plugins.add('converse-mam', {
                 const result = await _converse.api.archive.query(query);
                 result.messages.forEach(message_handler);
 
+                if (result.error) {
+                    result.error.retry = () => this.fetchArchivedMessages(options, page);
+                    this.createMessageFromError(result.error);
+                }
+
                 if (page && result.rsm) {
                     if (page === 'forwards') {
                         options = result.rsm.next(_converse.archived_messages_page_size, options.before);
@@ -298,9 +303,9 @@ converse.plugins.add('converse-mam', {
                   * * `index`
                   * * `count`
                   * @throws {Error} An error is thrown if the XMPP server responds with an error.
-                  * @returns {Promise<Object>} A promise which resolves to an object which
-                  * will have keys `messages` and `rsm` which contains a _converse.RSM object
-                  * on which "next" or "previous" can be called before passing it in again
+                  * @returns { (Promise<Object> | _converse.TimeoutError) } A promise which resolves
+                  * to an object which will have keys `messages` and `rsm` which contains a _converse.RSM
+                  * object on which "next" or "previous" can be called before passing it in again
                   * to this method, to get the next or previous page in the result set.
                   *
                   * @example
@@ -506,17 +511,22 @@ converse.plugins.add('converse-mam', {
                         return true;
                     }, Strophe.NS.MAM);
 
-                    let iq_result, rsm;
-                    try {
-                        iq_result = await _converse.api.sendIQ(stanza, _converse.message_archiving_timeout)
-                    } catch (e) {
-                        _converse.log(
-                            "Error or timeout while trying to fetch "+
-                            "archived messages", Strophe.LogLevel.ERROR);
-                        _converse.log(e, Strophe.LogLevel.ERROR);
+                    let error;
+                    const iq_result = await _converse.api.sendIQ(stanza, _converse.message_archiving_timeout, false)
+                    if (iq_result === null) {
+                        const err_msg = "Timeout while trying to fetch archived messages.";
+                        _converse.log(err_msg, Strophe.LogLevel.ERROR);
+                        error = new _converse.TimeoutError(err_msg);
+                        return { messages, error };
+
+                    } else if (u.isErrorStanza(iq_result)) {
+                        _converse.log("Error stanza received while trying to fetch archived messages", Strophe.LogLevel.ERROR);
+                        _converse.log(iq_result, Strophe.LogLevel.ERROR);
+                        return { messages };
                     }
                     _converse.connection.deleteHandler(message_handler);
 
+                    let rsm;
                     const fin = iq_result && sizzle(`fin[xmlns="${Strophe.NS.MAM}"]`, iq_result).pop();
                     if (fin && [null, 'false'].includes(fin.getAttribute('complete'))) {
                         const set = sizzle(`set[xmlns="${Strophe.NS.RSM}"]`, fin).pop();
@@ -525,7 +535,7 @@ converse.plugins.add('converse-mam', {
                             Object.assign(rsm, Object.assign(pick(options, [...MAM_ATTRIBUTES, ..._converse.RSM_ATTRIBUTES]), rsm));
                         }
                     }
-                    return { messages, rsm }
+                    return { messages, rsm, error };
                 }
             }
         });

src/headless/converse-muc.js

@@ -1550,7 +1550,8 @@ converse.plugins.add('converse-muc', {
                     } else {
                         const attrs = {
                             'type': 'error',
-                            'message': text
+                            'message': text,
+                            'ephemeral': true
                         }
                         this.messages.create(attrs);
                     }

src/templates/info.html

@@ -1,8 +1,12 @@
-{[ if (o.render_message) { ]}
-    <!-- XXX: Should only ever be rendered if the message text has been sanitized already -->
-    <div class="message chat-info {{{o.extra_classes}}}"
-        data-isodate="{{{o.isodate}}}" {[ if (o.data_name) { ]} data-{{{o.data_name}}}="{{{o.data_value}}}"{[ } ]}>{{o.message}}</div>
+<div class="message chat-info {{{o.extra_classes}}}" data-isodate="{{{o.isodate}}}" {[ if (o.data_name) { ]} data-{{{o.data_name}}}="{{{o.data_value}}}"{[ } ]}>
+{[ if (o.render_message) {
+    // XXX: Should only ever be rendered if the message text has been sanitized already
+]}
+    {{o.message}}
 {[ } else { ]}
-    <div class="message chat-info {{{o.extra_classes}}}"
-        data-isodate="{{{o.isodate}}}" {[ if (o.data_name) { ]} data-{{{o.data_name}}}="{{{o.data_value}}}"{[ } ]}>{{{o.message}}}</div>
+    {{{o.message}}}
 {[ } ]}
+{[ if (o.retry) { ]}
+    <a class="retry">Retry</a>
+{[ } ]}
+</div>

tests/index.html

@@ -6,7 +6,6 @@
     <meta name="description" content="Converse XMPP Chat" />
     <link rel="shortcut icon" type="image/png" href="../node_modules/jasmine-core/images/jasmine_favicon.png">
     <link rel="stylesheet" type="text/css" media="screen" href="../node_modules/jasmine-core/lib/jasmine-core/jasmine.css">
-    <link type="text/css" rel="stylesheet" media="screen" href="../dist/website.css" />
     <link type="text/css" rel="stylesheet" media="screen" href="../dist/converse.css" />
     <script src="../dist/converse.js"></script>
     <script data-main="runner" src="../node_modules/requirejs/require.js"></script>