From d65f8b2e6603a4de32f5d97e69a99be78e50708b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jos=C3=A9=20Pedro=20Dias?= <jose.dias@sky.uk>
Date: Sat, 24 Apr 2021 13:44:54 +0100
Subject: [PATCH] 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>
---
 lib/command.ts                    | 21 +++++++++++++++++++++
 lib/redis/index.ts                |  8 ++------
 lib/utils/index.ts                |  2 +-
 test/functional/commandTimeout.ts | 12 ++++++++++++
 4 files changed, 36 insertions(+), 7 deletions(-)

diff --git a/lib/command.ts b/lib/command.ts
index 40159213..8b0c59ab 100644
--- a/lib/command.ts
+++ b/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) {
diff --git a/lib/redis/index.ts b/lib/redis/index.ts
index 88828e7b..7860de7a 100644
--- a/lib/redis/index.ts
+++ b/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") {
diff --git a/lib/utils/index.ts b/lib/utils/index.ts
index d9533fcd..6ba975f5 100644
--- a/lib/utils/index.ts
+++ b/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;
 }
diff --git a/test/functional/commandTimeout.ts b/test/functional/commandTimeout.ts
index 1b0a7fec..6258bbbe 100644
--- a/test/functional/commandTimeout.ts
+++ b/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();
+  });
 });