From ecd0e72ce46097e2bc684c48bc12bc37042c5c13 Mon Sep 17 00:00:00 2001 From: Nikolay Karadzhov Date: Tue, 26 May 2026 14:36:07 +0300 Subject: [PATCH 1/2] feat(client): align default keepAliveInitialDelay and commandTimeout Bump TCP keep-alive idle to 30s and default commandOptions.timeout to 5s across client, cluster, and sentinel constructors, per the default client library values proposal. User-supplied commandOptions are merged so partial overrides still inherit the timeout default. Co-Authored-By: Claude Opus 4.7 (1M context) --- docs/client-configuration.md | 5 +++-- docs/command-options.md | 2 ++ docs/v5-to-v6.md | 24 ++++++++++++++++++++++++ packages/client/lib/client/index.ts | 5 ++--- packages/client/lib/client/socket.ts | 9 +++------ packages/client/lib/cluster/index.ts | 8 +++----- packages/client/lib/defaults.ts | 3 +++ packages/client/lib/sentinel/index.ts | 5 ++--- 8 files changed, 42 insertions(+), 19 deletions(-) create mode 100644 packages/client/lib/defaults.ts diff --git a/docs/client-configuration.md b/docs/client-configuration.md index fd826e2e1b..44b471ec7b 100644 --- a/docs/client-configuration.md +++ b/docs/client-configuration.md @@ -13,7 +13,7 @@ | socket.socketTimeout | | The maximum duration (in milliseconds) that the socket can remain idle (i.e., with no data sent or received) before being automatically closed | | socket.noDelay | `true` | Toggle [`Nagle's algorithm`](https://nodejs.org/api/net.html#net_socket_setnodelay_nodelay) | | socket.keepAlive | `true` | Toggle [`keep-alive`](https://nodejs.org/api/net.html#socketsetkeepaliveenable-initialdelay) functionality | -| socket.keepAliveInitialDelay | `5000` | If set to a positive number, it sets the initial delay before the first keepalive probe is sent on an idle socket | +| socket.keepAliveInitialDelay | `30000` | If set to a positive number, it sets the initial delay before the first keepalive probe is sent on an idle socket | | socket.tls | | See explanation and examples [below](#TLS) | | socket.reconnectStrategy | Exponential backoff with a maximum of 2000 ms; plus 0-200 ms random jitter. | A function containing the [Reconnect Strategy](#reconnect-strategy) logic | | username | | ACL username ([see ACL guide](https://redis.io/topics/acl)) | @@ -29,7 +29,8 @@ | legacyMode | `false` | Maintain some backwards compatibility (see the [Migration Guide](./v3-to-v4.md)) | | 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 | +| disableClientInfo | `false` | Disables `CLIENT SETINFO LIB-NAME node-redis` and `CLIENT SETINFO LIB-VER X.X.X` commands | +| commandOptions.timeout | `5000` | Default per-command timeout in milliseconds. Set to `undefined` (or `0`) to disable. See [Command Options](./command-options.md). | ## Reconnect Strategy diff --git a/docs/command-options.md b/docs/command-options.md index 101cfafee0..743f7138c7 100644 --- a/docs/command-options.md +++ b/docs/command-options.md @@ -73,6 +73,8 @@ try { This option is similar to the Abort Signal one, but provides an easier way to set timeout for commands. Again, this applies to commands that haven't been written to the socket yet. +Since v6, `commandOptions.timeout` defaults to `5000` (5 seconds). Set it to `undefined` (or `0`) to disable. + ```javascript const client = createClient({ commandOptions: { diff --git a/docs/v5-to-v6.md b/docs/v5-to-v6.md index 51940c62d0..efeed8e925 100644 --- a/docs/v5-to-v6.md +++ b/docs/v5-to-v6.md @@ -114,6 +114,30 @@ const sentinel = createSentinel({ ``` +## Default socket and command options updated + +Two client defaults change in v6: + +| Option | v5 default | v6 default | +|---|---|---| +| `socket.keepAliveInitialDelay` | `5000` ms | `30000` ms | +| `commandOptions.timeout` | `undefined` (no timeout) | `5000` ms | + +```javascript +// Still gets timeout: 5000 +const client = createClient({ commandOptions: { asap: true } }); +``` + +To preserve v5 behavior, opt out explicitly: + +```javascript +const client = createClient({ + socket: { keepAliveInitialDelay: 5000 }, + commandOptions: { timeout: undefined } +}); +``` + + ## Legacy (callback) mode now uses RESP3 `createClient().legacy()` reads the parent client's RESP version. With the v6 default of RESP3, legacy callback consumers will see RESP3-shaped replies for any command whose transforms differ between protocol versions (for example, doubles arriving as `number` instead of `string`, or hash-like replies arriving as `Map`s). To keep the v5 callback reply shapes, pin `RESP: 2` on the parent client: diff --git a/packages/client/lib/client/index.ts b/packages/client/lib/client/index.ts index e05a47c9ba..85f8131f19 100644 --- a/packages/client/lib/client/index.ts +++ b/packages/client/lib/client/index.ts @@ -24,6 +24,7 @@ import EnterpriseMaintenanceManager, { MaintenanceUpdate, MovingEndpointType, SM import { ClientMetricsHandle, ClientRegistry } from '../opentelemetry'; import { ClientIdentity, ClientRole, generateClientId } from './identity'; import { trace, sanitizeArgs, publish, CHANNELS, type CommandTraceContext } from './tracing'; +import { DEFAULT_COMMAND_TIMEOUT } from '../defaults'; const noop = () => {}; @@ -726,9 +727,7 @@ export default class RedisClient< this._self.#selectedDB = options.database; } - if (options.commandOptions) { - this._commandOptions = options.commandOptions; - } + this._commandOptions = { timeout: DEFAULT_COMMAND_TIMEOUT, ...options.commandOptions }; if(options.maintNotifications !== 'disabled') { EnterpriseMaintenanceManager.setupDefaultMaintOptions(options); diff --git a/packages/client/lib/client/socket.ts b/packages/client/lib/client/socket.ts index 974feec136..9d0b062076 100644 --- a/packages/client/lib/client/socket.ts +++ b/packages/client/lib/client/socket.ts @@ -6,6 +6,7 @@ import { setTimeout } from 'node:timers/promises'; import { RedisArgument } from '../RESP/types'; import { dbgMaintenance } from './enterprise-maintenance-manager'; import { publish, CHANNELS } from './tracing'; +import { DEFAULT_KEEPALIVE_INITIAL_DELAY } from '../defaults'; type NetOptions = { tls?: false; @@ -150,14 +151,10 @@ export default class RedisSocket extends EventEmitter { // @types/node is... incorrect... // @ts-expect-error - @types/node omits socket.connect noDelay. noDelay: options?.noDelay ?? true, - // https://nodejs.org/api/tls.html#tlsconnectoptions-callback "Any socket.connect() option not already listed" - // @types/node is... incorrect... // @ts-expect-error - @types/node omits socket.connect keepAlive. keepAlive: options?.keepAlive ?? true, - // https://nodejs.org/api/tls.html#tlsconnectoptions-callback "Any socket.connect() option not already listed" - // @types/node is... incorrect... // @ts-expect-error - @types/node omits socket.connect keepAliveInitialDelay. - keepAliveInitialDelay: options?.keepAliveInitialDelay ?? 5000, + keepAliveInitialDelay: options?.keepAliveInitialDelay ?? DEFAULT_KEEPALIVE_INITIAL_DELAY, timeout: undefined, onread: undefined, readable: true, @@ -194,7 +191,7 @@ export default class RedisSocket extends EventEmitter { port: options?.port ?? 6379, noDelay: options?.noDelay ?? true, keepAlive: options?.keepAlive ?? true, - keepAliveInitialDelay: options?.keepAliveInitialDelay ?? 5000, + keepAliveInitialDelay: options?.keepAliveInitialDelay ?? DEFAULT_KEEPALIVE_INITIAL_DELAY, timeout: undefined, onread: undefined, readable: true, diff --git a/packages/client/lib/cluster/index.ts b/packages/client/lib/cluster/index.ts index a0e045711a..511499b180 100644 --- a/packages/client/lib/cluster/index.ts +++ b/packages/client/lib/cluster/index.ts @@ -15,6 +15,7 @@ import { ASKING_CMD } from '../commands/ASKING'; import SingleEntryCache from '../single-entry-cache' import { publish, CHANNELS } from '../client/tracing'; import { ClientIdentity, ClientRole, generateClusterClientId } from '../client/identity'; +import { DEFAULT_COMMAND_TIMEOUT } from '../defaults'; export type ClusterTopologyRefreshOnReconnectionAttemptStrategy = false | @@ -348,9 +349,7 @@ export default class RedisCluster< this._slots = new RedisClusterSlots(options, this.emit.bind(this), this.#identity.id); this.on(RESUBSCRIBE_LISTENERS_EVENT, this.resubscribeAllPubSubListeners.bind(this)); - if (options?.commandOptions) { - this._commandOptions = options.commandOptions; - } + this._commandOptions = { timeout: DEFAULT_COMMAND_TIMEOUT, ...options?.commandOptions }; } duplicate< @@ -458,8 +457,7 @@ export default class RedisCluster< while (true) { try { - const opts = options ?? {}; - opts.slotNumber = slotNumber; + const opts: ClusterCommandOptions = { ...options, slotNumber }; return await myFn(client, opts); } catch (_err) { const err = _err as Error; diff --git a/packages/client/lib/defaults.ts b/packages/client/lib/defaults.ts new file mode 100644 index 0000000000..b6ad90e7c4 --- /dev/null +++ b/packages/client/lib/defaults.ts @@ -0,0 +1,3 @@ +export const DEFAULT_KEEPALIVE_INITIAL_DELAY = 30000; + +export const DEFAULT_COMMAND_TIMEOUT = 5000; diff --git a/packages/client/lib/sentinel/index.ts b/packages/client/lib/sentinel/index.ts index 2aff2a0efd..ded12d5020 100644 --- a/packages/client/lib/sentinel/index.ts +++ b/packages/client/lib/sentinel/index.ts @@ -18,6 +18,7 @@ import { TcpNetConnectOpts } from 'node:net'; import { RedisTcpSocketOptions } from '../client/socket'; import { BasicPooledClientSideCache, PooledClientSideCacheProvider } from '../client/cache'; import { ClientIdentity, ClientRole, generateClientId } from '../client/identity'; +import { DEFAULT_COMMAND_TIMEOUT } from '../defaults'; interface ClientInfo { id: number; @@ -337,9 +338,7 @@ export default class RedisSentinel< }; this.#options = options; - if (options.commandOptions) { - this.#commandOptions = options.commandOptions; - } + this.#commandOptions = { timeout: DEFAULT_COMMAND_TIMEOUT, ...options.commandOptions }; this.#internal = new RedisSentinelInternal(options, this.#identity.id); this.#internal.on('error', err => this.emit('error', err)); From 79498511ae9e9d8ff79a34fc2c9017c8abb3abe6 Mon Sep 17 00:00:00 2001 From: Nikolay Karadzhov Date: Thu, 28 May 2026 15:56:24 +0300 Subject: [PATCH 2/2] fix(client): preserve default timeout through withCommandOptions and add regression tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit RedisClient.withCommandOptions(opts) set `proxy._commandOptions = opts` as an own property, shadowing the prototype's merged options and dropping the 5s default timeout introduced for v6. Merge with the current effective options so partial overrides (e.g. `{ asap: true }`) still inherit the default — matching the documented v5→v6 contract. Lock the v6 default-commandOptions contract in with unit tests across createClient, createCluster, and createSentinel (default applied, partial-override merge, explicit opt-out via `timeout: undefined`), plus a withCommandOptions regression case. Add socket-level coverage that stubs `net.createConnection` to assert `keepAliveInitialDelay: 30000` flows through and that a user override is honored verbatim. Fix two preexisting sentinel spec assertions that compared `sentinel.commandOptions` by reference against the user-supplied options object — the constructor now merges defaults in, so the reference equality no longer holds. Switched to property-level checks that prove the same propagation/no-mutation intent. Co-Authored-By: Claude Opus 4.7 (1M context) --- packages/client/lib/client/index.spec.ts | 39 ++++++++++++++ packages/client/lib/client/index.ts | 2 +- packages/client/lib/client/socket.spec.ts | 62 ++++++++++++++++++++++ packages/client/lib/cluster/index.spec.ts | 21 ++++++++ packages/client/lib/sentinel/index.spec.ts | 39 +++++++++++--- 5 files changed, 156 insertions(+), 7 deletions(-) diff --git a/packages/client/lib/client/index.spec.ts b/packages/client/lib/client/index.spec.ts index 19d0f17a4a..8513b876fc 100644 --- a/packages/client/lib/client/index.spec.ts +++ b/packages/client/lib/client/index.spec.ts @@ -45,6 +45,45 @@ describe('Client', () => { assert.deepEqual(ownKeys.typeMapping, { [RESP_TYPES.SIMPLE_STRING]: Buffer }); }); + describe('default commandOptions', () => { + type WithOptions = { _commandOptions?: { timeout?: number; asap?: boolean } }; + + it('applies the 5s default timeout when no commandOptions are passed', () => { + const client = RedisClient.create({}); + assert.equal((client as unknown as WithOptions)._commandOptions?.timeout, 5000); + }); + + it('merges the default timeout with a partial commandOptions override', () => { + const client = RedisClient.create({ commandOptions: { asap: true } }); + const opts = (client as unknown as WithOptions)._commandOptions; + assert.equal(opts?.timeout, 5000); + assert.equal(opts?.asap, true); + }); + + it('allows opting out of the default timeout with `timeout: undefined`', () => { + const client = RedisClient.create({ commandOptions: { timeout: undefined } }); + assert.equal((client as unknown as WithOptions)._commandOptions?.timeout, undefined); + }); + + it('preserves the default timeout through withCommandOptions(...)', () => { + // Regression: `proxy._commandOptions = options` set the override as an own + // property that shadowed the prototype's merged `_commandOptions`, dropping + // the default at dispatch. The proxy must merge over the current effective + // options instead. + const client = RedisClient.create({}); + const proxy = client.withCommandOptions({ asap: true }); + const opts = (proxy as unknown as WithOptions)._commandOptions; + assert.equal(opts?.timeout, 5000); + assert.equal(opts?.asap, true); + }); + + it('lets withCommandOptions(...) opt out of the default timeout', () => { + const client = RedisClient.create({}); + const proxy = client.withCommandOptions({ timeout: undefined }); + assert.equal((proxy as unknown as WithOptions)._commandOptions?.timeout, undefined); + }); + }); + it('module/function namespaces resolve to the receiver, not the original', () => { // Regression: `attachNamespace` cached the namespace as an own property // on the receiver, leaking via the prototype chain into any diff --git a/packages/client/lib/client/index.ts b/packages/client/lib/client/index.ts index 85f8131f19..ce04064068 100644 --- a/packages/client/lib/client/index.ts +++ b/packages/client/lib/client/index.ts @@ -1024,7 +1024,7 @@ export default class RedisClient< TYPE_MAPPING extends TypeMapping >(options: OPTIONS) { const proxy = Object.create(this._self); - proxy._commandOptions = options; + proxy._commandOptions = { ...this._commandOptions, ...options }; return proxy as RedisClientType< M, F, diff --git a/packages/client/lib/client/socket.spec.ts b/packages/client/lib/client/socket.spec.ts index 581da7ea76..b1da015316 100644 --- a/packages/client/lib/client/socket.spec.ts +++ b/packages/client/lib/client/socket.spec.ts @@ -177,6 +177,68 @@ describe('Socket', () => { }); }); + describe('keepAliveInitialDelay default', () => { + function captureConnectOptions() { + const original = net.createConnection; + const captured: { options?: net.TcpNetConnectOpts } = {}; + const target = net as unknown as { createConnection: unknown }; + target.createConnection = (...args: unknown[]) => { + captured.options = args[0] as net.TcpNetConnectOpts; + return (original as unknown as (...a: unknown[]) => net.Socket).apply(net, args); + }; + return { + captured, + restore() { + target.createConnection = original; + } + }; + } + + async function withCapturedConnect( + socketOptions: Partial, + fn: (options: net.TcpNetConnectOpts) => void + ) { + const server = net.createServer(); + server.on('connection', conn => conn.on('error', () => { /* ignore */ })); + await new Promise(resolve => server.listen(0, '127.0.0.1', resolve)); + const { port } = server.address() as net.AddressInfo; + + const capture = captureConnectOptions(); + try { + const socket = createSocket({ + host: '127.0.0.1', + port, + reconnectStrategy: false, + ...socketOptions + }); + await socket.connect(); + try { + assert.ok(capture.captured.options, 'captured connect options'); + fn(capture.captured.options!); + } finally { + socket.destroy(); + } + } finally { + capture.restore(); + await new Promise(resolve => server.close(() => resolve())); + } + } + + it('passes keepAliveInitialDelay: 30000 to net.createConnection by default', async () => { + await withCapturedConnect({}, options => { + // @ts-expect-error - @types/node omits keepAliveInitialDelay + assert.equal(options.keepAliveInitialDelay, 30000); + }); + }); + + it('forwards a user-supplied keepAliveInitialDelay verbatim', async () => { + await withCapturedConnect({ keepAliveInitialDelay: 1234 }, options => { + // @ts-expect-error - @types/node omits keepAliveInitialDelay + assert.equal(options.keepAliveInitialDelay, 1234); + }); + }); + }); + describe('socketTimeout', () => { const timeout = 50; testUtils.testWithClient( diff --git a/packages/client/lib/cluster/index.spec.ts b/packages/client/lib/cluster/index.spec.ts index 7ba94a0d57..a71cd0f904 100644 --- a/packages/client/lib/cluster/index.spec.ts +++ b/packages/client/lib/cluster/index.spec.ts @@ -8,6 +8,27 @@ import RedisClient from '../client'; import { RESP_TYPES } from '../RESP/decoder'; describe('Cluster', () => { + describe('default commandOptions', () => { + type WithOptions = { _commandOptions?: { timeout?: number; asap?: boolean } }; + + it('applies the 5s default timeout when no commandOptions are passed', () => { + const cluster = RedisCluster.create({ rootNodes: [] }); + assert.equal((cluster as unknown as WithOptions)._commandOptions?.timeout, 5000); + }); + + it('merges the default timeout with a partial commandOptions override', () => { + const cluster = RedisCluster.create({ rootNodes: [], commandOptions: { asap: true } }); + const opts = (cluster as unknown as WithOptions)._commandOptions; + assert.equal(opts?.timeout, 5000); + assert.equal(opts?.asap, true); + }); + + it('allows opting out of the default timeout with `timeout: undefined`', () => { + const cluster = RedisCluster.create({ rootNodes: [], commandOptions: { timeout: undefined } }); + assert.equal((cluster as unknown as WithOptions)._commandOptions?.timeout, undefined); + }); + }); + it('chained withCommandOptions(...).withTypeMapping(...) preserves earlier overrides at dispatch', () => { // Regression: cluster's `_commandOptionsProxy` used to layer via `Object.create`, // leaving earlier keys on the prototype where the dispatch-time spread dropped them. diff --git a/packages/client/lib/sentinel/index.spec.ts b/packages/client/lib/sentinel/index.spec.ts index 08ad5c626c..6dc1647a1e 100644 --- a/packages/client/lib/sentinel/index.spec.ts +++ b/packages/client/lib/sentinel/index.spec.ts @@ -14,17 +14,46 @@ import { once } from 'node:events' const execAsync = promisify(exec); describe('RedisSentinel', () => { + describe('default commandOptions', () => { + it('applies the 5s default timeout when no commandOptions are passed', () => { + const sentinel = RedisSentinel.create({ + name: 'mymaster', + sentinelRootNodes: [{ host: 'localhost', port: 26379 }] + }); + assert.equal(sentinel.commandOptions?.timeout, 5000); + }); + + it('merges the default timeout with a partial commandOptions override', () => { + const sentinel = RedisSentinel.create({ + name: 'mymaster', + sentinelRootNodes: [{ host: 'localhost', port: 26379 }], + commandOptions: { asap: true } + }); + assert.equal(sentinel.commandOptions?.timeout, 5000); + assert.equal(sentinel.commandOptions?.asap, true); + }); + + it('allows opting out of the default timeout with `timeout: undefined`', () => { + const sentinel = RedisSentinel.create({ + name: 'mymaster', + sentinelRootNodes: [{ host: 'localhost', port: 26379 }], + commandOptions: { timeout: undefined } + }); + assert.equal(sentinel.commandOptions?.timeout, undefined); + }); + }); + it('exposes top-level commandOptions via the commandOptions getter', () => { // Regression: commandOptions used to be settable on both top-level and // `nodeClientOptions`/`sentinelClientOptions`; the nested location was // silently ignored at dispatch time. Top-level is now the only place. - const commandOptions = { typeMapping: {} }; + const typeMapping = {}; const sentinel = RedisSentinel.create({ name: 'mymaster', sentinelRootNodes: [{ host: 'localhost', port: 26379 }], - commandOptions + commandOptions: { typeMapping } }); - assert.equal(sentinel.commandOptions, commandOptions); + assert.equal(sentinel.commandOptions?.typeMapping, typeMapping); }); it('withTypeMapping does not mutate the source sentinel commandOptions', () => { @@ -32,14 +61,12 @@ describe('RedisSentinel', () => { // which (because `_self` resolves to the original sentinel) corrupted shared state — // the original instance and every other proxy observed the typeMapping change. const initialTypeMapping = { [RESP_TYPES.SIMPLE_STRING]: Buffer }; - const base = { typeMapping: initialTypeMapping }; const sentinel = RedisSentinel.create({ name: 'mymaster', sentinelRootNodes: [{ host: 'localhost', port: 26379 }], - commandOptions: base + commandOptions: { typeMapping: initialTypeMapping } }); sentinel.withTypeMapping({ [RESP_TYPES.SIMPLE_STRING]: String }); - assert.equal(sentinel.commandOptions, base); assert.equal(sentinel.commandOptions?.typeMapping, initialTypeMapping); });