From 4b89ef976e666a7eebf6b3c617711a23619d8b32 Mon Sep 17 00:00:00 2001 From: Florian Schunk Date: Wed, 28 May 2025 11:15:10 +0100 Subject: [PATCH 1/5] add command timeout --- packages/client/lib/client/index.spec.ts | 18 ++++++++++++- packages/client/lib/client/index.ts | 33 ++++++++++++++++++++++-- packages/client/lib/errors.ts | 6 +++++ 3 files changed, 54 insertions(+), 3 deletions(-) diff --git a/packages/client/lib/client/index.spec.ts b/packages/client/lib/client/index.spec.ts index 4f752210db..0ff5d48462 100644 --- a/packages/client/lib/client/index.spec.ts +++ b/packages/client/lib/client/index.spec.ts @@ -1,7 +1,7 @@ import { strict as assert } from 'node:assert'; import testUtils, { GLOBAL, waitTillBeenCalled } from '../test-utils'; import RedisClient, { RedisClientOptions, RedisClientType } from '.'; -import { AbortError, ClientClosedError, ClientOfflineError, ConnectionTimeoutError, DisconnectsClientError, ErrorReply, MultiErrorReply, SocketClosedUnexpectedlyError, WatchError } from '../errors'; +import { AbortError, ClientClosedError, ClientOfflineError, ConnectionTimeoutError, CommandTimeoutError, DisconnectsClientError, ErrorReply, MultiErrorReply, SocketClosedUnexpectedlyError, WatchError } from '../errors'; import { defineScript } from '../lua-script'; import { spy } from 'sinon'; import { once } from 'node:events'; @@ -265,6 +265,22 @@ describe('Client', () => { }, GLOBAL.SERVERS.OPEN); }); + testUtils.testWithClient('CommandTimeoutError', async client => { + const promise = assert.rejects(client.sendCommand(['PING']), CommandTimeoutError), + start = process.hrtime.bigint(); + + while (process.hrtime.bigint() - start < 50_000_000) { + // block the event loop for 1ms, to make sure the connection will timeout + } + + await promise; + }, { + ...GLOBAL.SERVERS.OPEN, + clientOptions: { + commandTimeout: 50, + } + }); + testUtils.testWithClient('undefined and null should not break the client', async client => { await assert.rejects( client.sendCommand([null as any, undefined as any]), diff --git a/packages/client/lib/client/index.ts b/packages/client/lib/client/index.ts index a446ad8e75..97ed1d94b4 100644 --- a/packages/client/lib/client/index.ts +++ b/packages/client/lib/client/index.ts @@ -4,7 +4,7 @@ import { BasicAuth, CredentialsError, CredentialsProvider, StreamingCredentialsP import RedisCommandsQueue, { CommandOptions } from './commands-queue'; import { EventEmitter } from 'node:events'; import { attachConfig, functionArgumentsPrefix, getTransformReply, scriptArgumentsPrefix } from '../commander'; -import { ClientClosedError, ClientOfflineError, DisconnectsClientError, WatchError } from '../errors'; +import { ClientClosedError, ClientOfflineError, CommandTimeoutError, DisconnectsClientError, WatchError } from '../errors'; import { URL } from 'node:url'; import { TcpSocketConnectOpts } from 'node:net'; import { PUBSUB_TYPE, PubSubType, PubSubListener, PubSubTypeListeners, ChannelListeners } from './pub-sub'; @@ -144,6 +144,10 @@ export interface RedisClientOptions< * Tag to append to library name that is sent to the Redis server */ clientInfoTag?: string; + /** + * Provides a timeout in milliseconds. + */ + commandTimeout?: number; } type WithCommands< @@ -889,9 +893,34 @@ export default class RedisClient< return Promise.reject(new ClientOfflineError()); } + let controller: AbortController; + if (this._self.#options?.commandTimeout) { + controller = new AbortController() + options = { + ...options, + abortSignal: controller.signal + } + } const promise = this._self.#queue.addCommand(args, options); + this._self.#scheduleWrite(); - return promise; + if (!this._self.#options?.commandTimeout) { + return promise; + } + + return new Promise((resolve, reject) => { + const timeoutId = setTimeout(() => { + controller.abort(); + reject(new CommandTimeoutError()); + }, this._self.#options?.commandTimeout) + promise.then(result => { + clearInterval(timeoutId); + resolve(result) + }).catch(error => { + clearInterval(timeoutId); + reject(error) + }); + }) } async SELECT(db: number): Promise { diff --git a/packages/client/lib/errors.ts b/packages/client/lib/errors.ts index db37ec1a9b..fbeddbab93 100644 --- a/packages/client/lib/errors.ts +++ b/packages/client/lib/errors.ts @@ -22,6 +22,12 @@ export class SocketTimeoutError extends Error { } } +export class CommandTimeoutError extends Error { + constructor() { + super('Command timeout'); + } +} + export class ClientClosedError extends Error { constructor() { super('The client is closed'); From 0b98f247494b29a3ef82903cf649d00fb3e3dab4 Mon Sep 17 00:00:00 2001 From: Florian Schunk Date: Wed, 28 May 2025 11:28:49 +0100 Subject: [PATCH 2/5] update documentation --- docs/client-configuration.md | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/client-configuration.md b/docs/client-configuration.md index 57af626bf7..83a818454b 100644 --- a/docs/client-configuration.md +++ b/docs/client-configuration.md @@ -29,6 +29,7 @@ | isolationPoolOptions | | An object that configures a pool of isolated connections, If you frequently need isolated connections, consider using [createClientPool](https://github.com/redis/node-redis/blob/master/docs/pool.md#creating-a-pool) instead | | pingInterval | | Send `PING` command at interval (in ms). Useful with ["Azure Cache for Redis"](https://learn.microsoft.com/en-us/azure/azure-cache-for-redis/cache-best-practices-connection#idle-timeout) | | disableClientInfo | `false` | Disables `CLIENT SETINFO LIB-NAME node-redis` and `CLIENT SETINFO LIB-VER X.X.X` commands | +| commandTimeout | | Throw an error and abort a command if it takes longer than the specified time (in milliseconds). | ## Reconnect Strategy From e8eb95be868f825ff486947ddac1466b61edbb32 Mon Sep 17 00:00:00 2001 From: Florian Schunk Date: Wed, 18 Jun 2025 15:41:19 +0100 Subject: [PATCH 3/5] pass abort signal --- packages/client/lib/client/index.spec.ts | 17 +++++++++++++++++ packages/client/lib/client/index.ts | 9 ++++++++- 2 files changed, 25 insertions(+), 1 deletion(-) diff --git a/packages/client/lib/client/index.spec.ts b/packages/client/lib/client/index.spec.ts index 0ff5d48462..a7890b5540 100644 --- a/packages/client/lib/client/index.spec.ts +++ b/packages/client/lib/client/index.spec.ts @@ -263,6 +263,23 @@ describe('Client', () => { AbortError ); }, GLOBAL.SERVERS.OPEN); + + testUtils.testWithClient('AbortError with timeout', client => { + const controller = new AbortController(); + controller.abort(); + + return assert.rejects( + client.sendCommand(['PING'], { + abortSignal: controller.signal + }), + AbortError + ); + }, { + ...GLOBAL.SERVERS.OPEN, + clientOptions: { + commandTimeout: 50, + } + }); }); testUtils.testWithClient('CommandTimeoutError', async client => { diff --git a/packages/client/lib/client/index.ts b/packages/client/lib/client/index.ts index 97ed1d94b4..78fb46f527 100644 --- a/packages/client/lib/client/index.ts +++ b/packages/client/lib/client/index.ts @@ -896,9 +896,16 @@ export default class RedisClient< let controller: AbortController; if (this._self.#options?.commandTimeout) { controller = new AbortController() + let abortSignal = controller.signal; + if (options?.abortSignal) { + abortSignal = AbortSignal.any([ + abortSignal, + options.abortSignal + ]); + } options = { ...options, - abortSignal: controller.signal + abortSignal } } const promise = this._self.#queue.addCommand(args, options); From 19e49352365c820b1651762a3cde8e314ba25e07 Mon Sep 17 00:00:00 2001 From: Florian Date: Wed, 18 Jun 2025 16:47:06 +0100 Subject: [PATCH 4/5] add node types --- package-lock.json | 12 ++++++++---- package.json | 2 +- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/package-lock.json b/package-lock.json index d9fc9f93f9..7be21d94a1 100644 --- a/package-lock.json +++ b/package-lock.json @@ -19,7 +19,7 @@ "@istanbuljs/nyc-config-typescript": "^1.0.2", "@release-it/bumper": "^7.0.5", "@types/mocha": "^10.0.6", - "@types/node": "^20.11.16", + "@types/node": "^20.19.1", "gh-pages": "^6.1.1", "mocha": "^10.2.0", "nyc": "^15.1.0", @@ -1657,11 +1657,13 @@ "license": "MIT" }, "node_modules/@types/node": { - "version": "20.11.16", + "version": "20.19.1", + "resolved": "https://registry.npmjs.org/@types/node/-/node-20.19.1.tgz", + "integrity": "sha512-jJD50LtlD2dodAEO653i3YF04NWak6jN3ky+Ri3Em3mGR39/glWiboM/IePaRbgwSfqM1TpGXfAg8ohn/4dTgA==", "dev": true, "license": "MIT", "dependencies": { - "undici-types": "~5.26.4" + "undici-types": "~6.21.0" } }, "node_modules/@types/parse-path": { @@ -6987,7 +6989,9 @@ } }, "node_modules/undici-types": { - "version": "5.26.5", + "version": "6.21.0", + "resolved": "https://registry.npmjs.org/undici-types/-/undici-types-6.21.0.tgz", + "integrity": "sha512-iwDZqg0QAGrg9Rav5H4n0M64c3mkR59cJ6wQp+7C4nI0gsmExaedaYLNO44eT4AtBBwjbTiGPMlt2Md0T9H9JQ==", "dev": true, "license": "MIT" }, diff --git a/package.json b/package.json index e192e69d55..9479e1937e 100644 --- a/package.json +++ b/package.json @@ -23,7 +23,7 @@ "@istanbuljs/nyc-config-typescript": "^1.0.2", "@release-it/bumper": "^7.0.5", "@types/mocha": "^10.0.6", - "@types/node": "^20.11.16", + "@types/node": "^20.19.1", "gh-pages": "^6.1.1", "mocha": "^10.2.0", "nyc": "^15.1.0", From dd72feaeceba2a55f9f61d690b902c68a0160eea Mon Sep 17 00:00:00 2001 From: Florian Date: Fri, 20 Jun 2025 15:22:32 +0100 Subject: [PATCH 5/5] reuse AbortError --- packages/client/lib/client/index.spec.ts | 6 +++--- packages/client/lib/client/index.ts | 4 ++-- packages/client/lib/errors.ts | 6 ------ 3 files changed, 5 insertions(+), 11 deletions(-) diff --git a/packages/client/lib/client/index.spec.ts b/packages/client/lib/client/index.spec.ts index a7890b5540..e9ed7cdd4c 100644 --- a/packages/client/lib/client/index.spec.ts +++ b/packages/client/lib/client/index.spec.ts @@ -283,11 +283,11 @@ describe('Client', () => { }); testUtils.testWithClient('CommandTimeoutError', async client => { - const promise = assert.rejects(client.sendCommand(['PING']), CommandTimeoutError), - start = process.hrtime.bigint(); + const promise = assert.rejects(client.sendCommand(['PING']), AbortError); + const start = process.hrtime.bigint(); while (process.hrtime.bigint() - start < 50_000_000) { - // block the event loop for 1ms, to make sure the connection will timeout + // block the event loop for 50ms, to make sure the connection will timeout } await promise; diff --git a/packages/client/lib/client/index.ts b/packages/client/lib/client/index.ts index 78fb46f527..5ecdeeb76c 100644 --- a/packages/client/lib/client/index.ts +++ b/packages/client/lib/client/index.ts @@ -4,7 +4,7 @@ import { BasicAuth, CredentialsError, CredentialsProvider, StreamingCredentialsP import RedisCommandsQueue, { CommandOptions } from './commands-queue'; import { EventEmitter } from 'node:events'; import { attachConfig, functionArgumentsPrefix, getTransformReply, scriptArgumentsPrefix } from '../commander'; -import { ClientClosedError, ClientOfflineError, CommandTimeoutError, DisconnectsClientError, WatchError } from '../errors'; +import { ClientClosedError, ClientOfflineError, AbortError, DisconnectsClientError, WatchError } from '../errors'; import { URL } from 'node:url'; import { TcpSocketConnectOpts } from 'node:net'; import { PUBSUB_TYPE, PubSubType, PubSubListener, PubSubTypeListeners, ChannelListeners } from './pub-sub'; @@ -918,7 +918,7 @@ export default class RedisClient< return new Promise((resolve, reject) => { const timeoutId = setTimeout(() => { controller.abort(); - reject(new CommandTimeoutError()); + reject(new AbortError()); }, this._self.#options?.commandTimeout) promise.then(result => { clearInterval(timeoutId); diff --git a/packages/client/lib/errors.ts b/packages/client/lib/errors.ts index fbeddbab93..db37ec1a9b 100644 --- a/packages/client/lib/errors.ts +++ b/packages/client/lib/errors.ts @@ -22,12 +22,6 @@ export class SocketTimeoutError extends Error { } } -export class CommandTimeoutError extends Error { - constructor() { - super('Command timeout'); - } -} - export class ClientClosedError extends Error { constructor() { super('The client is closed');