Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions docs/client-configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)) |
Expand All @@ -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

Expand Down
2 changes: 2 additions & 0 deletions docs/command-options.md
Original file line number Diff line number Diff line change
Expand Up @@ -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: {
Expand Down
24 changes: 24 additions & 0 deletions docs/v5-to-v6.md
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
39 changes: 39 additions & 0 deletions packages/client/lib/client/index.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
7 changes: 3 additions & 4 deletions packages/client/lib/client/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 = () => {};

Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -1025,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,
Expand Down
62 changes: 62 additions & 0 deletions packages/client/lib/client/socket.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<RedisSocketOptions>,
fn: (options: net.TcpNetConnectOpts) => void
) {
const server = net.createServer();
server.on('connection', conn => conn.on('error', () => { /* ignore */ }));
await new Promise<void>(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<void>(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(
Expand Down
9 changes: 3 additions & 6 deletions packages/client/lib/client/socket.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down
21 changes: 21 additions & 0 deletions packages/client/lib/cluster/index.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
8 changes: 3 additions & 5 deletions packages/client/lib/cluster/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 |
Expand Down Expand Up @@ -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;
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cluster withCommandOptions drops default timeout by not merging

High Severity

The cluster's withCommandOptions sets proxy._commandOptions = options without merging the existing _commandOptions, unlike the client which was correctly updated to { ...this._commandOptions, ...options }. This means calling cluster.withCommandOptions({ asap: true }) silently drops the new default timeout: 5000 because the proxy's own _commandOptions property ({ asap: true }) shadows the constructor-set value on the prototype. The cluster has no commandOptions getter to merge at access time (unlike sentinel, which is fine), so the default timeout is permanently lost for all commands dispatched through the proxy.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 7949851. Configure here.

this._commandOptions = { timeout: DEFAULT_COMMAND_TIMEOUT, ...options?.commandOptions };
Comment thread
cursor[bot] marked this conversation as resolved.
}

duplicate<
Expand Down Expand Up @@ -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;
Expand Down
3 changes: 3 additions & 0 deletions packages/client/lib/defaults.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export const DEFAULT_KEEPALIVE_INITIAL_DELAY = 30000;

export const DEFAULT_COMMAND_TIMEOUT = 5000;
39 changes: 33 additions & 6 deletions packages/client/lib/sentinel/index.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,32 +14,59 @@ 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', () => {
// Regression: `_commandOptionsProxy` used to assign to `proxy._self.#commandOptions`,
// 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);
});

Expand Down
5 changes: 2 additions & 3 deletions packages/client/lib/sentinel/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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<M, F, S, RESP, TYPE_MAPPING>(options, this.#identity.id);
this.#internal.on('error', err => this.emit('error', err));
Expand Down
Loading