feat: detect broken XMPP WebSocket using ping

If ping timeouts two times in a row (25 seconds total)
the XMPP WebSocket connection will be killed and restarted.

modules/xmpp/XmppConnection.js

@@ -6,6 +6,7 @@ import Listenable from '../util/Listenable';
 import { getJitterDelay } from '../util/Retry';
 
 import LastSuccessTracker from './StropheLastSuccess';
+import PingConnectionPlugin from './strophe.ping';
 
 const logger = getLogger(__filename);
 
@@ -80,6 +81,14 @@ export default class XmppConnection extends Listenable {
          * @private
          */
         this._deferredIQs = [];
+
+        // Ping plugin is mandatory for the Websocket mode to work correctly. It's used to detect when the connection
+        // is broken (WebSocket/TCP connection not closed gracefully).
+        this.addConnectionPlugin(
+            'ping',
+            new PingConnectionPlugin({
+                onPingThresholdExceeded: () => this._onPingErrorThresholdExceeded()
+            }));
     }
 
     /**
@@ -246,7 +255,10 @@ export default class XmppConnection extends Listenable {
             this._maybeEnableStreamResume();
             this._maybeStartWSKeepAlive();
             this._processDeferredIQs();
+            this.ping.startInterval(this.domain);
         } else if (status === Strophe.Status.DISCONNECTED) {
+            this.ping.stopInterval();
+
             // FIXME add RECONNECTING state instead of blocking the DISCONNECTED update
             blockCallback = this._tryResumingConnection();
             if (!blockCallback) {
@@ -278,7 +290,10 @@ export default class XmppConnection extends Listenable {
      * @returns {void}
      */
     closeWebsocket() {
-        this._stropheConn._proto && this._stropheConn._proto.socket && this._stropheConn._proto.socket.close();
+        if (this._stropheConn && this._stropheConn._proto) {
+            this._stropheConn._proto._closeSocket();
+            this._stropheConn._proto._onClose(null);
+        }
     }
 
     /**
@@ -459,6 +474,18 @@ export default class XmppConnection extends Listenable {
         });
     }
 
+    /**
+     * Called by the ping plugin when ping fails too many times.
+     *
+     * @returns {void}
+     */
+    _onPingErrorThresholdExceeded() {
+        if (this.isUsingWebSocket) {
+            logger.warn('Ping error threshold exceeded - killing the WebSocket');
+            this.closeWebsocket();
+        }
+    }
+
     /**
      *  Helper function to send presence stanzas. The main benefit is for sending presence stanzas for which you expect
      *  a responding presence stanza with the same id (for example when leaving a chat room).

modules/xmpp/strophe.ping.js

@@ -14,14 +14,16 @@ const logger = getLogger(__filename);
 const PING_INTERVAL = 10000;
 
 /**
- * Ping timeout error after 15 sec of waiting.
+ * Ping timeout error after 5 sec of waiting.
  */
-const PING_TIMEOUT = 15000;
+const PING_TIMEOUT = 5000;
 
 /**
- * Will close the connection after 3 consecutive ping errors.
+ * How many ping failures will be tolerated before the WebSocket connection is killed.
+ * The worst case scenario in case of ping timing out without a response is (25 seconds at the time of this writing):
+ * PING_THRESHOLD * PING_INTERVAL + PING_TIMEOUT
  */
-const PING_THRESHOLD = 3;
+const PING_THRESHOLD = 2;
 
 /**
  * The number of timestamps of send pings to keep.
@@ -38,14 +40,16 @@ const PING_TIMESTAMPS_TO_KEEP = 120000 / PING_INTERVAL;
 export default class PingConnectionPlugin extends ConnectionPlugin {
     /**
      * Contructs new object
-     * @param {XMPP} xmpp the xmpp module.
+     * @param {Object} options
+     * @param {Function} options.onPingThresholdExceeded - Callback called when ping fails too many times (controlled
+     * by the {@link PING_THRESHOLD} constant).
      * @constructor
      */
-    constructor(xmpp) {
+    constructor({ onPingThresholdExceeded }) {
         super();
         this.failedPings = 0;
-        this.xmpp = xmpp;
         this.pingExecIntervals = new Array(PING_TIMESTAMPS_TO_KEEP);
+        this._onPingThresholdExceeded = onPingThresholdExceeded;
     }
 
     /**
@@ -101,13 +105,7 @@ export default class PingConnectionPlugin extends ConnectionPlugin {
                 if (this.failedPings >= PING_THRESHOLD) {
                     GlobalOnErrorHandler.callErrorHandler(new Error(errmsg));
                     logger.error(errmsg, error);
-
-                    // FIXME it doesn't help to disconnect when 3rd PING
-                    // times out, it only stops Strophe from retrying.
-                    // Not really sure what's the right thing to do in that
-                    // situation, but just closing the connection makes no
-                    // sense.
-                    // self.connection.disconnect();
+                    this._onPingThresholdExceeded && this._onPingThresholdExceeded();
                 } else {
                     logger.warn(errmsg, error);
                 }

modules/xmpp/xmpp.js

@@ -17,7 +17,6 @@ import XmppConnection from './XmppConnection';
 import MucConnectionPlugin from './strophe.emuc';
 import JingleConnectionPlugin from './strophe.jingle';
 import initStropheLogger from './strophe.logger';
-import PingConnectionPlugin from './strophe.ping';
 import RayoConnectionPlugin from './strophe.rayo';
 import initStropheUtil from './strophe.util';
 
@@ -229,10 +228,6 @@ export default class XMPP extends Listenable {
                             `Ping NOT supported by ${pingJid} - please enable ping in your XMPP server config`);
                     }
 
-                    // It counterintuitive to start ping task when it's not supported, but since PING is now mandatory
-                    // it's done on purpose in order to print error logs and bring more attention.
-                    this.connection.ping.startInterval(pingJid);
-
                     // check for speakerstats
                     identities.forEach(identity => {
                         if (identity.type === 'speakerstats') {
@@ -646,7 +641,6 @@ export default class XMPP extends Listenable {
 
         this.connection.addConnectionPlugin('emuc', new MucConnectionPlugin(this));
         this.connection.addConnectionPlugin('jingle', new JingleConnectionPlugin(this, this.eventEmitter, iceConfig));
-        this.connection.addConnectionPlugin('ping', new PingConnectionPlugin(this));
         this.connection.addConnectionPlugin('rayo', new RayoConnectionPlugin());
     }