feat: added commandTimeout option (#1320)

* Added timeoutPerRequest option

* Renamed timeoutPerRequest to commandTimeout

* Documented commandTimeout

* Added sentinelCommandTimeout option

* Add a test case for commandTimeout

Co-authored-by: luin <[email protected]>

Author: mjomble

API.md

@@ -56,6 +56,7 @@ Creates a Redis instance
 | [options.enableOfflineQueue]            | <code>boolean</code>                                              | <code>true</code>                     | By default, if there is no active connection to the Redis server, commands are added to a queue and are executed once the connection is "ready" (when `enableReadyCheck` is `true`, "ready" means the Redis server has loaded the database from disk, otherwise means the connection to the Redis server has been established). If this option is false, when execute the command when the connection isn't ready, an error will be returned. |
 | [options.connectTimeout]                | <code>number</code>                                               | <code>10000</code>                    | The milliseconds before a timeout occurs during the initial connection to the Redis server.                                                                                                                                                                                                                                                                                                                                                   |
 | [options.disconnectTimeout]             | <code>number</code>                                               | <code>2000</code>                     | The milliseconds before [socket.destroy()](https://nodejs.org/dist/latest-v14.x/docs/api/net.html#net_socket_destroy_error) is called after [socket.end()](https://nodejs.org/dist/latest-v14.x/docs/api/net.html#net_socket_end_data_encoding_callback) if the connection remains half-open during disconnection.                                                                                                                            |
+| [options.commandTimeout]                | <code>number</code>                                               |                                       | The milliseconds before a timeout occurs when executing a single command. By default, there is no timeout and the client will wait indefinitely. The timeout is enforced only on the client side, not server side. The server may still complete the operation after a timeout error occurs on the client side.                                                                                                                               |
 | [options.autoResubscribe]               | <code>boolean</code>                                              | <code>true</code>                     | After reconnected, if the previous connection was in the subscriber mode, client will auto re-subscribe these channels.                                                                                                                                                                                                                                                                                                                       |
 | [options.autoResendUnfulfilledCommands] | <code>boolean</code>                                              | <code>true</code>                     | If true, client will resend unfulfilled commands(e.g. block commands) in the previous connection when reconnected.                                                                                                                                                                                                                                                                                                                            |
 | [options.lazyConnect]                   | <code>boolean</code>                                              | <code>false</code>                    | By default, When a new `Redis` instance is created, it will connect to Redis server automatically. If you want to keep the instance disconnected until a command is called, you can pass the `lazyConnect` option to the constructor: `javascript var redis = new Redis({ lazyConnect: true }); // No attempting to connect to the Redis server here. // Now let's connect to the Redis server redis.get('foo', function () { });`            |

lib/command.ts

@@ -160,6 +160,7 @@ export default class Command implements ICommand {
   private slot?: number | null;
   private keys?: Array<string | Buffer>;
 
+  public isResolved = false;
   public reject: (err: Error) => void;
   public resolve: (result: any) => void;
   public promise: Promise<any>;
@@ -342,6 +343,7 @@ export default class Command implements ICommand {
     return (value) => {
       try {
         resolve(this.transformReply(value));
+        this.isResolved = true;
       } catch (err) {
         this.reject(err);
       }
@@ -392,7 +394,7 @@ const hsetArgumentTransformer = function (args) {
     }
   }
   return args;
-}
+};
 
 Command.setArgumentTransformer("mset", msetArgumentTransformer);
 Command.setArgumentTransformer("msetnx", msetArgumentTransformer);

lib/connectors/SentinelConnector/index.ts

@@ -42,6 +42,7 @@ export interface ISentinelConnectionOptions extends ITcpConnectionOptions {
   preferredSlaves?: PreferredSlaves;
   connectTimeout?: number;
   disconnectTimeout?: number;
+  sentinelCommandTimeout?: number;
   enableTLSForSentinelMode?: boolean;
   sentinelTLS?: ConnectionOptions;
   natMap?: INatMap;
@@ -265,6 +266,7 @@ export default class SentinelConnector extends AbstractConnector {
       retryStrategy: null,
       enableReadyCheck: false,
       connectTimeout: this.options.connectTimeout,
+      commandTimeout: this.options.sentinelCommandTimeout,
       dropBufferSupport: true,
     });
 

lib/redis/RedisOptions.ts

@@ -11,6 +11,7 @@ export interface IRedisOptions
     Partial<IClusterOptions> {
   Connector?: typeof AbstractConnector;
   retryStrategy?: (times: number) => number | void | null;
+  commandTimeout?: number;
   keepAlive?: number;
   noDelay?: boolean;
   connectionName?: string;

lib/redis/index.ts

@@ -98,8 +98,8 @@ const debug = Debug("redis");
  * @param {NatMap} [options.natMap=null] NAT map for sentinel connector.
  * @param {boolean} [options.updateSentinels=true] - Update the given `sentinels` list with new IP
  * addresses when communicating with existing sentinels.
-* @param {boolean} [options.enableAutoPipelining=false] - When enabled, all commands issued during an event loop 
- * iteration are automatically wrapped in a pipeline and sent to the server at the same time. 
+* @param {boolean} [options.enableAutoPipelining=false] - When enabled, all commands issued during an event loop
+ * iteration are automatically wrapped in a pipeline and sent to the server at the same time.
  * This can dramatically improve performance.
  * @param {string[]} [options.autoPipeliningIgnoredCommands=[]] - The list of commands which must not be automatically wrapped in pipelines.
  * @param {number} [options.maxScriptsCachingTime=60000] Default script definition caching time.
@@ -710,6 +710,14 @@ Redis.prototype.sendCommand = function (command, stream) {
     return command.promise;
   }
 
+  if (typeof this.options.commandTimeout === "number") {
+    setTimeout(() => {
+      if (!command.isResolved) {
+        command.reject(new Error("Command timed out"));
+      }
+    }, this.options.commandTimeout);
+  }
+
   if (command.name === "quit") {
     clearInterval(this._addedScriptHashesCleanInterval);
     this._addedScriptHashesCleanInterval = null;

test/functional/commandTimeout.ts

@@ -0,0 +1,25 @@
+import { expect } from "chai";
+import * as sinon from "sinon";
+import Redis from "../../lib/redis";
+import MockServer from "../helpers/mock_server";
+
+describe("commandTimeout", function () {
+  it("rejects if command timed out", function (done) {
+    const server = new MockServer(30001, function (argv, socket, flags) {
+      if (argv[0] === "hget") {
+        flags.hang = true;
+        return;
+      }
+    });
+
+    const redis = new Redis({ port: 30001, commandTimeout: 1000 });
+    const clock = sinon.useFakeTimers();
+    redis.hget("foo", (err) => {
+      expect(err.message).to.eql("Command timed out");
+      clock.restore();
+      redis.disconnect();
+      server.disconnect(() => done());
+    });
+    clock.tick(1000);
+  });
+});

test/helpers/mock_server.ts

@@ -34,6 +34,7 @@ export function getConnectionName(socket: Socket): string | undefined {
 
 interface IFlags {
   disconnect?: boolean;
+  hang?: boolean;
 }
 export type MockServerHandler = (
   reply: any,
@@ -93,7 +94,9 @@ export default class MockServer extends EventEmitter {
           }
           const flags: IFlags = {};
           const handlerResult = this.handler && this.handler(reply, c, flags);
-          this.write(c, handlerResult);
+          if (!flags.hang) {
+            this.write(c, handlerResult);
+          }
           if (flags.disconnect) {
             this.disconnect();
           }