Only send out ping if we're actually authenticated

Fixes #2562

src/headless/plugins/ping/api.js

@@ -7,18 +7,19 @@ const { Strophe, $iq, u } = converse.env;
 export default {
     /**
      * Pings the entity represented by the passed in JID by sending an IQ stanza to it.
-     * If we already know we're not connected, no ping is sent out and `false` is returned.
-     * If the ping is sent out to the user's bare JID and no response is received it will attempt to reconnect.
      * @method api.ping
      * @param { String } [jid] - The JID of the service to ping
+     *  If the ping is sent out to the user's bare JID and no response is received it will attempt to reconnect.
      * @param { Integer } [timeout] - The amount of time in
      *  milliseconds to wait for a response. The default is 10000;
-     * @returns { Boolean } Whether the pinged entity responded with a non-error IQ stanza.
+     * @returns { Boolean | null }
+     *  Whether the pinged entity responded with a non-error IQ stanza.
+     *  If we already know we're not connected, no ping is sent out and `null` is returned.
      */
     async ping (jid, timeout) {
-        if (!api.connection.connected()) {
-            log.warn("Not pinging when we know we're not connected");
-            return false;
+        if (!api.connection.authenticated()) {
+            log.warn("Not pinging when we know we're not authenticated");
+            return null;
         }
 
         // XXX: We could first check here if the server advertised that it supports PING.

src/headless/plugins/ping/index.js

@@ -7,7 +7,7 @@
  */
 import ping_api from './api.js';
 import { api, converse } from "@converse/headless/core.js";
-import { onEverySecond, onWindowStateChanged, onConnected } from './utils.js';
+import { onWindowStateChanged, registerHandlers, unregisterIntervalHandler } from './utils.js';
 
 const { Strophe } = converse.env;
 
@@ -24,10 +24,9 @@ converse.plugins.add('converse-ping', {
 
         Object.assign(api, ping_api);
 
-        setInterval(onEverySecond, 1000);
-
-        api.listen.on('connected', onConnected);
-        api.listen.on('reconnected', onConnected);
+        api.listen.on('connected', registerHandlers);
+        api.listen.on('reconnected', registerHandlers);
+        api.listen.on('disconnected', unregisterIntervalHandler);
         api.listen.on('windowStateChanged', onWindowStateChanged);
     }
 });

src/headless/plugins/ping/tests/ping.js

@@ -32,7 +32,7 @@ describe("XMPP Ping", function () {
 
         it("is not sent out if we're not connected", mock.initConverse(async (_converse) => {
             spyOn(_converse.connection, 'send');
-            expect(await _converse.api.ping()).toBe(false);
+            expect(await _converse.api.ping()).toBe(null);
             expect(_converse.connection.send.calls.count()).toBe(0);
         }));
     });

src/headless/plugins/ping/utils.js

@@ -5,9 +5,7 @@ const { Strophe, $iq } = converse.env;
 let lastStanzaDate;
 
 export function onWindowStateChanged (data) {
-    if (data.state === 'visible' && api.connection.connected()) {
-        api.ping(null, 5000);
-    }
+    data.state === 'visible' && api.ping(null, 5000);
 }
 
 export function setLastStanzaDate (date) {
@@ -42,22 +40,28 @@ export function registerPingHandler () {
     });
 }
 
-export function onConnected () {
+let intervalId;
+
+export function registerHandlers () {
     // Wrapper so that we can spy on registerPingHandler in tests
     registerPongHandler();
     registerPingHandler();
+    clearInterval(intervalId);
+    intervalId = setInterval(onEverySecond, 1000);
+}
+
+export function unregisterIntervalHandler () {
+    clearInterval(intervalId);
 }
 
 export function onEverySecond () {
-    if (_converse.isTestEnv() || !api.connection.connected()) {
+    if (_converse.isTestEnv() || !api.connection.authenticated()) {
         return;
     }
     const ping_interval = api.settings.get('ping_interval');
     if (ping_interval > 0) {
         const now = new Date();
-        if (!lastStanzaDate) {
-            lastStanzaDate = now;
-        }
+        lastStanzaDate = lastStanzaDate ?? now;
         if ((now - lastStanzaDate)/1000 > ping_interval) {
             api.ping();
         }

src/headless/shared/connection/api.js

@@ -8,6 +8,16 @@ import { Strophe } from 'strophe.js/src/strophe.js';
  * @memberOf _converse.api
  */
 export default {
+
+    /**
+     * @method _converse.api.connection.authenticated
+     * @memberOf _converse.api.connection
+     * @returns {boolean} Whether we're authenticated to the XMPP server or not
+     */
+    authenticated () {
+        return _converse?.connection?.authenticated && true;
+    },
+
     /**
      * @method _converse.api.connection.connected
      * @memberOf _converse.api.connection