feat: Reduce pings and adds xmpp ping config (#1389)

* feat: Skips pinging when there was a recent server response.

* feat: Adds options to control xmpp ping settings.

* fix: Fixes wrong default value.

* squash: Received messages in ping interval sets failed ping to 0

doc/API.md

@@ -227,6 +227,10 @@ This objects represents the server connection. You can create new `JitsiConnecti
             - `anonymousdomain`
         4. `enableLipSync` - (optional) boolean property which enables the lipsync feature. Currently works only in Chrome and is disabled by default.
         5. `clientNode` - The name of client node advertised in XEP-0115 'c' stanza
+        6. xmppPing - (optional) JS Object - xmpp ping options
+            - `interval` - how often to send ping requests, default: 10000 (10 seconds)
+            - `timeout` - the time to wait for ping responses, default: 5000 (5 seconds)
+            - `threshold` - how many ping failures will be tolerated before the connection is killed, default: 2
 
 2. `connect(options)` - establish server connection
     - `options` - JS Object with `id` and `password` properties.

modules/xmpp/XmppConnection.js

@@ -45,8 +45,9 @@ export default class XmppConnection extends Listenable {
      * default with jitter. Pass -1 to disable. The actual interval equation is:
      * jitterDelay = (interval * 0.2) + (0.8 * interval * Math.random())
      * The keep alive is HTTP GET request to the {@link options.serviceUrl}.
+     * @param {Object} [options.xmppPing] - The xmpp ping settings.
      */
-    constructor({ enableWebsocketResume, websocketKeepAlive, serviceUrl }) {
+    constructor({ enableWebsocketResume, websocketKeepAlive, serviceUrl, xmppPing }) {
         super();
         this._options = {
             enableWebsocketResume: typeof enableWebsocketResume === 'undefined' ? true : enableWebsocketResume,
@@ -83,7 +84,9 @@ export default class XmppConnection extends Listenable {
         this.addConnectionPlugin(
             'ping',
             new PingConnectionPlugin({
-                onPingThresholdExceeded: () => this._onPingErrorThresholdExceeded()
+                getTimeSinceLastServerResponse: () => this.getTimeSinceLastSuccess(),
+                onPingThresholdExceeded: () => this._onPingErrorThresholdExceeded(),
+                pingOptions: xmppPing
             }));
     }
 

modules/xmpp/strophe.ping.js

@@ -9,28 +9,36 @@ import ConnectionPlugin from './ConnectionPlugin';
 const logger = getLogger(__filename);
 
 /**
- * Ping every 10 sec
+ * Default ping every 10 sec
  */
-const PING_INTERVAL = 10000;
+const PING_DEFAULT_INTERVAL = 10000;
 
 /**
- * Ping timeout error after 5 sec of waiting.
+ * Default ping timeout error after 5 sec of waiting.
  */
-const PING_TIMEOUT = 5000;
+const PING_DEFAULT_TIMEOUT = 5000;
 
 /**
- * How many ping failures will be tolerated before the WebSocket connection is killed.
+ * Default value for 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 = 2;
+const PING_DEFAULT_THRESHOLD = 2;
 
 /**
- * The number of timestamps of send pings to keep.
- * The current value is 2 minutes.
- * @type {number} number of timestamps.
+ * How often to send ping requests.
  */
-const PING_TIMESTAMPS_TO_KEEP = 120000 / PING_INTERVAL;
+let pingInterval;
+
+/**
+ * The time to wait for ping responses.
+ */
+let pingTimeout;
+
+/**
+ * How many ping failures will be tolerated before the connection is killed.
+ */
+let pingThreshold;
 
 /**
  * XEP-0199 ping plugin.
@@ -39,17 +47,30 @@ const PING_TIMESTAMPS_TO_KEEP = 120000 / PING_INTERVAL;
  */
 export default class PingConnectionPlugin extends ConnectionPlugin {
     /**
-     * Contructs new object
+     * Constructs new object
      * @param {Object} options
      * @param {Function} options.onPingThresholdExceeded - Callback called when ping fails too many times (controlled
      * by the {@link PING_THRESHOLD} constant).
+     * @param {Function} options._getTimeSinceLastServerResponse - A function to obtain the last seen
+     * response from the server.
+     * @param {Object} options.pingOptions - The ping options if any.
      * @constructor
      */
-    constructor({ onPingThresholdExceeded }) {
+    constructor({ getTimeSinceLastServerResponse, onPingThresholdExceeded, pingOptions = {} }) {
         super();
         this.failedPings = 0;
-        this.pingExecIntervals = new Array(PING_TIMESTAMPS_TO_KEEP);
         this._onPingThresholdExceeded = onPingThresholdExceeded;
+        this._getTimeSinceLastServerResponse = getTimeSinceLastServerResponse;
+
+        this.pingInterval = typeof pingOptions.interval === 'number' ? pingOptions.interval : PING_DEFAULT_INTERVAL;
+        this.pingTimeout = typeof pingOptions.timeout === 'number' ? pingOptions.timeout : PING_DEFAULT_TIMEOUT;
+        this.pingThreshold = typeof pingOptions.threshold === 'number'
+            ? pingOptions.threshold : PING_DEFAULT_THRESHOLD;
+
+        // The number of timestamps of send pings to keep.
+        // The current value is 2 minutes.
+        this.pingTimestampsToKeep = Math.round(120000 / this.pingInterval);
+        this.pingExecIntervals = new Array(this.pingTimestampsToKeep);
     }
 
     /**
@@ -91,27 +112,45 @@ export default class PingConnectionPlugin extends ConnectionPlugin {
      * This plugin supports only one such task and <tt>stopInterval</tt>
      * must be called before starting a new one.
      * @param remoteJid remote JID to which ping requests will be sent to.
-     * @param interval task interval in ms.
      */
-    startInterval(remoteJid, interval = PING_INTERVAL) {
+    startInterval(remoteJid) {
         clearInterval(this.intervalId);
         this.intervalId = window.setInterval(() => {
+
+            // when there were some server responses in the interval since the last time we checked (_lastServerCheck)
+            // let's skip the ping
+
+            // server response is measured on raw input and ping response time is measured after all the xmpp
+            // processing is done, and when the last server response is a ping there can be slight misalignment of the
+            // times, we give it 100ms for that processing.
+            if (this._getTimeSinceLastServerResponse() + 100 < new Date() - this._lastServerCheck) {
+                // do this just to keep in sync the intervals so we can detect suspended device
+                this._addPingExecutionTimestamp();
+
+                this._lastServerCheck = new Date();
+                this.failedPings = 0;
+
+                return;
+            }
+
             this.ping(remoteJid, () => {
+                this._lastServerCheck = new Date();
+
                 this.failedPings = 0;
             }, error => {
                 this.failedPings += 1;
                 const errmsg = `Ping ${error ? 'error' : 'timeout'}`;
 
-                if (this.failedPings >= PING_THRESHOLD) {
+                if (this.failedPings >= pingThreshold) {
                     GlobalOnErrorHandler.callErrorHandler(new Error(errmsg));
                     logger.error(errmsg, error);
                     this._onPingThresholdExceeded && this._onPingThresholdExceeded();
                 } else {
                     logger.warn(errmsg, error);
                 }
-            }, PING_TIMEOUT);
-        }, interval);
-        logger.info(`XMPP pings will be sent every ${interval} ms`);
+            }, pingTimeout);
+        }, this.pingInterval);
+        logger.info(`XMPP pings will be sent every ${this.pingInterval} ms`);
     }
 
     /**
@@ -134,7 +173,7 @@ export default class PingConnectionPlugin extends ConnectionPlugin {
         this.pingExecIntervals.push(new Date().getTime());
 
         // keep array length to PING_TIMESTAMPS_TO_KEEP
-        if (this.pingExecIntervals.length > PING_TIMESTAMPS_TO_KEEP) {
+        if (this.pingExecIntervals.length > this.pingTimestampsToKeep) {
             this.pingExecIntervals.shift();
         }
     }
@@ -172,7 +211,7 @@ export default class PingConnectionPlugin extends ConnectionPlugin {
         // remove the interval between the ping sent
         // this way in normal execution there is no suspend and the return
         // will be 0 or close to 0.
-        maxInterval -= PING_INTERVAL;
+        maxInterval -= pingInterval;
 
         // make sure we do not return less than 0
         return Math.max(maxInterval, 0);

modules/xmpp/xmpp.js

@@ -32,9 +32,10 @@ const logger = getLogger(__filename);
  * @param {string} options.serviceUrl - The service URL for XMPP connection.
  * @param {string} options.enableWebsocketResume - True to enable stream resumption.
  * @param {number} [options.websocketKeepAlive] - See {@link XmppConnection} constructor.
+ * @param {Object} [options.xmppPing] - See {@link XmppConnection} constructor.
  * @returns {XmppConnection}
  */
-function createConnection({ enableWebsocketResume, serviceUrl = '/http-bind', token, websocketKeepAlive }) {
+function createConnection({ enableWebsocketResume, serviceUrl = '/http-bind', token, websocketKeepAlive, xmppPing }) {
     // Append token as URL param
     if (token) {
         // eslint-disable-next-line no-param-reassign
@@ -44,7 +45,8 @@ function createConnection({ enableWebsocketResume, serviceUrl = '/http-bind', to
     return new XmppConnection({
         enableWebsocketResume,
         serviceUrl,
-        websocketKeepAlive
+        websocketKeepAlive,
+        xmppPing
     });
 }
 
@@ -91,6 +93,7 @@ export default class XMPP extends Listenable {
      * module try to resume the session in case the Websocket connection breaks.
      * @param {number} [options.websocketKeepAlive] - The websocket keep alive interval. See {@link XmppConnection}
      * constructor for more details.
+     * @param {Object} [options.xmppPing] - The xmpp ping settings.
      * @param {Array<Object>} options.p2pStunServers see {@link JingleConnectionPlugin} for more details.
      * @param token
      */
@@ -111,7 +114,8 @@ export default class XMPP extends Listenable {
             // FIXME remove deprecated bosh option at some point
             serviceUrl: options.serviceUrl || options.bosh,
             token,
-            websocketKeepAlive: options.websocketKeepAlive
+            websocketKeepAlive: options.websocketKeepAlive,
+            xmppPing: options.xmppPing
         });
 
         this._initStrophePlugins();