converse-ping: Refactor and update to reconnect upon ping timeout

spec/ping.js

@@ -2,49 +2,32 @@
     define(["jasmine", "mock", "test-utils"], factory);
 } (this, function (jasmine, mock, test_utils) {
     "use strict";
+    const Strophe = converse.env.Strophe;
+    const u = converse.env.utils;
 
-    describe("XMPP Ping", function () {
-        describe("Ping and pong handlers", function () {
-
-            it("are registered when _converse.js is connected",
-                mock.initConverse(
-                    null, ['rosterGroupsFetched'], {},
-                    function (done, _converse) {
 
-                spyOn(_converse, 'registerPingHandler').and.callThrough();
-                spyOn(_converse, 'registerPongHandler').and.callThrough();
-                _converse.api.trigger('connected');
-                expect(_converse.registerPingHandler).toHaveBeenCalled();
-                expect(_converse.registerPongHandler).toHaveBeenCalled();
-                done();
-            }));
+    describe("XMPP Ping", function () {
 
-            it("are registered when _converse.js reconnected",
-                mock.initConverse(
-                    null, ['rosterGroupsFetched'], {},
-                    function (done, _converse) {
+        describe("An IQ stanza", function () {
 
-                spyOn(_converse, 'registerPingHandler').and.callThrough();
-                spyOn(_converse, 'registerPongHandler').and.callThrough();
-                _converse.api.trigger('reconnected');
-                expect(_converse.registerPingHandler).toHaveBeenCalled();
-                expect(_converse.registerPongHandler).toHaveBeenCalled();
+            it("is returned when converse.js gets pinged", mock.initConverse((done, _converse) => {
+                const ping = u.toStanza(`
+                    <iq from="${_converse.domain}"
+                        to="${_converse.jid}" id="s2c1" type="get">
+                        <ping xmlns="urn:xmpp:ping"/>
+                    </iq>`);
+                _converse.connection._dataRecv(test_utils.createRequest(ping));
+                const sent_stanza = _converse.connection.IQ_stanzas.pop();
+                expect(Strophe.serialize(sent_stanza)).toBe(
+                    `<iq id="s2c1" to="${_converse.domain}" type="result" xmlns="jabber:client"/>`);
                 done();
             }));
-        });
-
-        describe("An IQ stanza", function () {
 
-            it("is sent out when _converse.js pings a server", mock.initConverse((done, _converse) => {
-                let sent_stanza, IQ_id;
-                const sendIQ = _converse.connection.sendIQ;
-                spyOn(_converse.connection, 'sendIQ').and.callFake(function (iq, callback, errback) {
-                    sent_stanza = iq;
-                    IQ_id = sendIQ.bind(this)(iq, callback, errback);
-                });
-                _converse.ping();
-                expect(sent_stanza.toLocaleString()).toBe(
-                    `<iq id="${IQ_id}" to="montague.lit" type="get" xmlns="jabber:client">`+
+            it("is sent out when converse.js pings a server", mock.initConverse((done, _converse) => {
+                _converse.api.ping();
+                const sent_stanza = _converse.connection.IQ_stanzas.pop();
+                expect(Strophe.serialize(sent_stanza)).toBe(
+                    `<iq id="${sent_stanza.getAttribute('id')}" to="montague.lit" type="get" xmlns="jabber:client">`+
                         `<ping xmlns="urn:xmpp:ping"/>`+
                     `</iq>`);
                 done();

src/headless/converse-ping.js

@@ -10,9 +10,8 @@
  * as specified in XEP-0199 XMPP Ping.
  */
 import converse from "./converse-core";
-
-// Strophe methods for building stanzas
 const { Strophe, $iq, _ } = converse.env;
+const u = converse.env.utils;
 
 Strophe.addNamespace('PING', "urn:xmpp:ping");
 
@@ -24,80 +23,99 @@ converse.plugins.add('converse-ping', {
          * loaded by converse.js's plugin machinery.
          */
         const { _converse } = this;
+        let lastStanzaDate;
 
         _converse.api.settings.update({
-            ping_interval: 180 //in seconds
+            ping_interval: 60 //in seconds
         });
 
-        _converse.ping = function (jid, success, error, timeout) {
-            // XXX: We could first check here if the server advertised that
-            // it supports PING.
-            // However, some servers don't advertise while still keeping the
-            // connection option due to pings.
-            //
-            // var feature = _converse.disco_entities[_converse.domain].features.findWhere({'var': Strophe.NS.PING});
-            _converse.lastStanzaDate = new Date();
-            jid = jid || Strophe.getDomainFromJid(_converse.bare_jid);
-            if (timeout === undefined ) { timeout = null; }
-            if (success === undefined ) { success = null; }
-            if (error === undefined ) { error = null; }
-            if (_converse.connection) {
-                const id = _converse.connection.getUniqueId('ping');
-                const iq = $iq({
-                    'type': 'get',
-                    'to': jid,
-                    'id': id
-                }).c('ping', {'xmlns': Strophe.NS.PING});
-                _converse.connection.sendIQ(iq, success, error, timeout);
-                return true;
-            }
-            return false;
-        };
-
-        _converse.pong = function (ping) {
-            _converse.lastStanzaDate = new Date();
+        function pong (ping) {
+            lastStanzaDate = new Date();
             const from = ping.getAttribute('from');
             const id = ping.getAttribute('id');
             const iq = $iq({type: 'result', to: from,id: id});
             _converse.connection.sendIQ(iq);
             return true;
-        };
+        }
 
-        _converse.registerPongHandler = function () {
+        function registerPongHandler () {
             if (_converse.connection.disco !== undefined) {
                 _converse.api.disco.own.features.add(Strophe.NS.PING);
             }
-            return _converse.connection.addHandler(_converse.pong, Strophe.NS.PING, "iq", "get");
-        };
+            return _converse.connection.addHandler(pong, Strophe.NS.PING, "iq", "get");
+        }
 
-        _converse.registerPingHandler = function () {
-            _converse.registerPongHandler();
-            if (_converse.ping_interval > 0) {
-                _converse.connection.addHandler(function () {
-                    /* Handler on each stanza, saves the received date
-                     * in order to ping only when needed.
-                     */
-                    _converse.lastStanzaDate = new Date();
-                    return true;
-                });
-                _converse.connection.addTimedHandler(1000, function () {
-                    const now = new Date();
-                    if (!_converse.lastStanzaDate) {
-                        _converse.lastStanzaDate = now;
-                    }
-                    if ((now - _converse.lastStanzaDate)/1000 > _converse.ping_interval) {
-                        return _converse.ping();
-                    }
+        function registerPingHandler () {
+            _converse.connection.addHandler(() => {
+                if (_converse.ping_interval > 0) {
+                    // Handler on each stanza, saves the received date
+                    // in order to ping only when needed.
+                    lastStanzaDate = new Date();
                     return true;
-                });
+                }
+            });
+        }
+
+        setTimeout(() => {
+            if (_converse.ping_interval > 0) {
+                const now = new Date();
+                if (!lastStanzaDate) {
+                    lastStanzaDate = now;
+                }
+                if ((now - lastStanzaDate)/1000 > _converse.ping_interval) {
+                    return _converse.api.ping();
+                }
+                return true;
             }
-        };
+        }, 1000);
+
 
         const onConnected = function () {
             // Wrapper so that we can spy on registerPingHandler in tests
-            _converse.registerPingHandler();
+            registerPongHandler();
+            registerPingHandler();
         };
         _converse.api.listen.on('connected', onConnected);
         _converse.api.listen.on('reconnected', onConnected);
+
+
+        /************************ BEGIN API ************************/
+        Object.assign(_converse.api, {
+            /**
+             * Pings the service represented by the passed in JID by sending an
+             * IQ stanza.
+             * @private
+             * @method _converse.api.ping
+             * @param { string } [jid] - The JID of the service to ping
+             */
+            async ping (jid) {
+                // XXX: We could first check here if the server advertised that it supports PING.
+                // However, some servers don't advertise while still responding to pings
+                //
+                // const feature = _converse.disco_entities[_converse.domain].features.findWhere({'var': Strophe.NS.PING});
+                lastStanzaDate = new Date();
+                jid = jid || Strophe.getDomainFromJid(_converse.bare_jid);
+                if (_converse.connection) {
+                    const iq = $iq({
+                            'type': 'get',
+                            'to': jid,
+                            'id': _converse.connection.getUniqueId('ping')
+                        }).c('ping', {'xmlns': Strophe.NS.PING});
+
+                    const result = await _converse.api.sendIQ(iq, 10000, false);
+                    if (result === null) {
+                        _converse.log(`Timeout while pinging ${jid}`, Strophe.LogLevel.WARN);
+                        if (jid === Strophe.getDomainFromJid(_converse.bare_jid)) {
+                            _converse.api.connection.reconnect();
+                        }
+                    } else if (u.isErrorStanza(result)) {
+                        _converse.log(`Error while pinging ${jid}`, Strophe.LogLevel.ERROR);
+                        _converse.log(result, Strophe.LogLevel.ERROR);
+                    }
+                    return true;
+                }
+                return false;
+            }
+        });
     }
 });

src/headless/utils/core.js

@@ -162,6 +162,12 @@ u.isErrorObject = function (o) {
     return o instanceof Error;
 }
 
+u.isErrorStanza = function (stanza) {
+    if (!_.isElement(stanza)) {
+        return false;
+    }
+    return stanza.getAttribute('type') === 'error';
+}
 
 u.isForbiddenError = function (stanza) {
     if (!_.isElement(stanza)) {