fix: deferring pings should only delay by at most one interval

Author: mikuso

lib/client.js

@@ -1,6 +1,7 @@
 const { randomUUID} = require('crypto');
 const { EventEmitter, once } = require('events');
 const { setTimeout } = require('timers/promises');
+const { setTimeout: setTimeoutCb } = require('timers');
 const WebSocket = require('ws');
 const { ExponentialStrategy } = require('backoff');
 const { CONNECTING, OPEN, CLOSING, CLOSED } = WebSocket;
@@ -30,7 +31,6 @@ class RPCClient extends EventEmitter {
         this._keepAliveAbortController = undefined;
         this._pendingPingResponse = false;
         this._lastPingTime = 0;
-        this._skipNextPing = false;
         this._closePromise = undefined;
         this._protocolOptions = [];
         this._protocol = undefined;
@@ -398,12 +398,12 @@ class RPCClient extends EventEmitter {
         ws.on('error', err => this.emit('socketError', err));
         ws.on('ping', () => {
             if (this._options.deferPingsOnActivity) {
-                this._skipNextPing = true;
+                this._deferNextPing();
             }
         });
         ws.on('pong', () => {
             if (this._options.deferPingsOnActivity) {
-                this._skipNextPing = true;
+                this._deferNextPing();
             }
             this._pendingPingResponse = false;
             const rtt = Date.now() - this._lastPingTime;
@@ -557,10 +557,24 @@ class RPCClient extends EventEmitter {
         return this._connectPromise;
     }
 
+    _deferNextPing() {
+        if (!this._nextPingTimeout) {
+            return;
+        }
+
+        this._nextPingTimeout.refresh();
+    }
+
     async _keepAlive() {
         // abort any previously running keepAlive
         this._keepAliveAbortController?.abort();
 
+        const timerEmitter = new EventEmitter();
+        const nextPingTimeout = setTimeoutCb(()=>{
+            timerEmitter.emit('next')
+        }, this._options.pingIntervalMs);
+        this._nextPingTimeout = nextPingTimeout;
+
         try {
             if (this._state !== OPEN) {
                 // don't start pinging if connection not open
@@ -574,20 +588,16 @@ class RPCClient extends EventEmitter {
 
             // setup new abort controller
             this._keepAliveAbortController = new AbortController();
-
+            
             while (true) {
-                await setTimeout(this._options.pingIntervalMs, undefined, {signal: this._keepAliveAbortController.signal});
-                
+                await once(timerEmitter, 'next', {signal: this._keepAliveAbortController.signal}),
+                this._keepAliveAbortController.signal.throwIfAborted();
+
                 if (this._state !== OPEN) {
                     // keepalive no longer required
                     break;
                 }
 
-                if (this._skipNextPing) {
-                    this._skipNextPing = false;
-                    continue;
-                }
-
                 if (this._pendingPingResponse) {
                     // we didn't get a response to our last ping
                     throw Error("Ping timeout");
@@ -596,13 +606,17 @@ class RPCClient extends EventEmitter {
                 this._lastPingTime = Date.now();
                 this._pendingPingResponse = true;                
                 this._ws.ping();
+                nextPingTimeout.refresh();
             }
 
         } catch (err) {
+            // console.log('keepalive failed', err);
             if (err.name !== 'AbortError') {
                 // throws on ws.ping() error
                 this._ws.terminate();
             }
+        } finally {
+            clearTimeout(nextPingTimeout);
         }
     }
 
@@ -648,7 +662,7 @@ class RPCClient extends EventEmitter {
 
     _onMessage(buffer) {
         if (this._options.deferPingsOnActivity) {
-            this._skipNextPing = true;
+            this._deferNextPing();
         }
 
         const message = buffer.toString('utf8');