From 8d05e685da1e7f0b32e61bd529048b3407ade31f Mon Sep 17 00:00:00 2001 From: Andres Kalle Date: Thu, 1 Apr 2021 10:15:00 +0300 Subject: [PATCH 1/5] Added timeoutPerRequest option --- lib/command.ts | 4 +++- lib/redis/RedisOptions.ts | 1 + lib/redis/index.ts | 8 ++++++++ 3 files changed, 12 insertions(+), 1 deletion(-) diff --git a/lib/command.ts b/lib/command.ts index 0218d7b9..40159213 100644 --- a/lib/command.ts +++ b/lib/command.ts @@ -160,6 +160,7 @@ export default class Command implements ICommand { private slot?: number | null; private keys?: Array; + public isResolved = false; public reject: (err: Error) => void; public resolve: (result: any) => void; public promise: Promise; @@ -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); diff --git a/lib/redis/RedisOptions.ts b/lib/redis/RedisOptions.ts index bb4535e2..d707739e 100644 --- a/lib/redis/RedisOptions.ts +++ b/lib/redis/RedisOptions.ts @@ -11,6 +11,7 @@ export interface IRedisOptions Partial { Connector?: typeof AbstractConnector; retryStrategy?: (times: number) => number | void | null; + timeoutPerRequest?: number; keepAlive?: number; noDelay?: boolean; connectionName?: string; diff --git a/lib/redis/index.ts b/lib/redis/index.ts index 54adfa75..151a74e3 100644 --- a/lib/redis/index.ts +++ b/lib/redis/index.ts @@ -710,6 +710,14 @@ Redis.prototype.sendCommand = function (command, stream) { return command.promise; } + if (typeof this.options.timeoutPerRequest === "number") { + setTimeout(() => { + if (!command.isResolved) { + command.reject(new Error("Command timed out")); + } + }, this.options.timeoutPerRequest); + } + if (command.name === "quit") { clearInterval(this._addedScriptHashesCleanInterval); this._addedScriptHashesCleanInterval = null; From 01e27e7c542964a05050e2dfa171075c4a2ffe3f Mon Sep 17 00:00:00 2001 From: Andres Kalle Date: Thu, 1 Apr 2021 16:11:15 +0300 Subject: [PATCH 2/5] Renamed timeoutPerRequest to commandTimeout --- lib/redis/RedisOptions.ts | 2 +- lib/redis/index.ts | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/lib/redis/RedisOptions.ts b/lib/redis/RedisOptions.ts index d707739e..33971d14 100644 --- a/lib/redis/RedisOptions.ts +++ b/lib/redis/RedisOptions.ts @@ -11,7 +11,7 @@ export interface IRedisOptions Partial { Connector?: typeof AbstractConnector; retryStrategy?: (times: number) => number | void | null; - timeoutPerRequest?: number; + commandTimeout?: number; keepAlive?: number; noDelay?: boolean; connectionName?: string; diff --git a/lib/redis/index.ts b/lib/redis/index.ts index 151a74e3..57459990 100644 --- a/lib/redis/index.ts +++ b/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,12 +710,12 @@ Redis.prototype.sendCommand = function (command, stream) { return command.promise; } - if (typeof this.options.timeoutPerRequest === "number") { + if (typeof this.options.commandTimeout === "number") { setTimeout(() => { if (!command.isResolved) { command.reject(new Error("Command timed out")); } - }, this.options.timeoutPerRequest); + }, this.options.commandTimeout); } if (command.name === "quit") { From 6dabb608eed9332b0c082d68c64e3ea26f39601d Mon Sep 17 00:00:00 2001 From: Andres Kalle Date: Thu, 1 Apr 2021 16:11:34 +0300 Subject: [PATCH 3/5] Documented commandTimeout --- API.md | 1 + 1 file changed, 1 insertion(+) diff --git a/API.md b/API.md index 39f84c7c..7be90deb 100644 --- a/API.md +++ b/API.md @@ -56,6 +56,7 @@ Creates a Redis instance | [options.enableOfflineQueue] | boolean | true | 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] | number | 10000 | The milliseconds before a timeout occurs during the initial connection to the Redis server. | | [options.disconnectTimeout] | number | 2000 | 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] | number | | 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] | boolean | true | After reconnected, if the previous connection was in the subscriber mode, client will auto re-subscribe these channels. | | [options.autoResendUnfulfilledCommands] | boolean | true | If true, client will resend unfulfilled commands(e.g. block commands) in the previous connection when reconnected. | | [options.lazyConnect] | boolean | false | 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 () { });` | From 2ab0f10427400d047af2c17158383e677af284b8 Mon Sep 17 00:00:00 2001 From: Andres Kalle Date: Thu, 1 Apr 2021 16:13:55 +0300 Subject: [PATCH 4/5] Added sentinelCommandTimeout option --- lib/connectors/SentinelConnector/index.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lib/connectors/SentinelConnector/index.ts b/lib/connectors/SentinelConnector/index.ts index e3b4d882..26d24bf0 100644 --- a/lib/connectors/SentinelConnector/index.ts +++ b/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, }); From c472114b897905533aaf3c46fb24842f6edf7d0d Mon Sep 17 00:00:00 2001 From: luin Date: Fri, 2 Apr 2021 22:08:37 +0800 Subject: [PATCH 5/5] Add a test case for commandTimeout --- test/functional/commandTimeout.ts | 25 +++++++++++++++++++++++++ test/helpers/mock_server.ts | 5 ++++- 2 files changed, 29 insertions(+), 1 deletion(-) create mode 100644 test/functional/commandTimeout.ts diff --git a/test/functional/commandTimeout.ts b/test/functional/commandTimeout.ts new file mode 100644 index 00000000..1b0a7fec --- /dev/null +++ b/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); + }); +}); diff --git a/test/helpers/mock_server.ts b/test/helpers/mock_server.ts index 04e6c55b..5101f064 100644 --- a/test/helpers/mock_server.ts +++ b/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(); }