fix: clears commandTimeout timer as each respective command gets fulfilled (#1336)

* making commandTimeout clean successful timers and work in cluster mode

* deleting timer attribution solves an issue with transactions; cluster command doesn't need any special behaviour after all

* adding attribute annotation to keep typescript happy.

* thought setTimeout returned a number, apparently not

* this time I ran `npm run build` locally first

* addressing feedback

* Clear timeouts for commands in offline queue

When client is not ready, Redis#sendCommand() will move
commands to the offline queue, and once ready, all commands
in the offline queue will be sent with Redis#sendCommand()
again. So we should check whether the timer has been set.

* Address feedbacks

Co-authored-by: luin <i@zihua.li>

Author: JosePedroDiasSky

lib/command.ts

@@ -156,6 +156,7 @@ export default class Command implements ICommand {
   public isCustomCommand = false;
   public inTransaction = false;
   public pipelineIndex?: number;
+  private _commandTimeoutTimer?: NodeJS.Timeout;
 
   private slot?: number | null;
   private keys?: Array<string | Buffer>;
@@ -342,6 +343,12 @@ export default class Command implements ICommand {
   private _convertValue(resolve: Function): (result: any) => void {
     return (value) => {
       try {
+        const existingTimer = this._commandTimeoutTimer;
+        if (existingTimer) {
+          clearTimeout(existingTimer);
+          delete this._commandTimeoutTimer;
+        }
+
         resolve(this.transformReply(value));
         this.isResolved = true;
       } catch (err) {
@@ -370,6 +377,20 @@ export default class Command implements ICommand {
 
     return result;
   }
+
+  /**
+   * Set the wait time before terminating the attempt to execute a command
+   * and generating an error.
+   */
+  public setTimeout(ms: number) {
+    if (!this._commandTimeoutTimer) {
+      this._commandTimeoutTimer = setTimeout(() => {
+        if (!this.isResolved) {
+          this.reject(new Error("Command timed out"));
+        }
+      }, ms);
+    }
+  }
 }
 
 const msetArgumentTransformer = function (args) {

lib/redis/index.ts

@@ -693,7 +693,7 @@ addTransactionSupport(Redis.prototype);
  * ```
  * @private
  */
-Redis.prototype.sendCommand = function (command, stream) {
+Redis.prototype.sendCommand = function (command: Command, stream) {
   if (this.status === "wait") {
     this.connect().catch(noop);
   }
@@ -714,11 +714,7 @@ Redis.prototype.sendCommand = function (command, stream) {
   }
 
   if (typeof this.options.commandTimeout === "number") {
-    setTimeout(() => {
-      if (!command.isResolved) {
-        command.reject(new Error("Command timed out"));
-      }
-    }, this.options.commandTimeout);
+    command.setTimeout(this.options.commandTimeout);
   }
 
   if (command.name === "quit") {

lib/utils/index.ts

@@ -111,7 +111,7 @@ export function wrapMultiResult(arr) {
  * ```
  * @private
  */
-export function isInt(value) {
+export function isInt(value): value is string {
   const x = parseFloat(value);
   return !isNaN(value) && (x | 0) === x;
 }

test/functional/commandTimeout.ts

@@ -22,4 +22,16 @@ describe("commandTimeout", function () {
     });
     clock.tick(1000);
   });
+
+  it("does not leak timers for commands in offline queue", async function () {
+    const server = new MockServer(30001);
+
+    const redis = new Redis({ port: 30001, commandTimeout: 1000 });
+    const clock = sinon.useFakeTimers();
+    await redis.hget("foo");
+    expect(clock.countTimers()).to.eql(0);
+    clock.restore();
+    redis.disconnect();
+    await server.disconnectPromise();
+  });
 });