diff --git a/docs/client-configuration.md b/docs/client-configuration.md index 0564794ac4..1c9ba51a11 100644 --- a/docs/client-configuration.md +++ b/docs/client-configuration.md @@ -9,6 +9,7 @@ | socket.family | `0` | IP Stack version (one of `4 \| 6 \| 0`) | | socket.path | | Path to the UNIX Socket | | socket.connectTimeout | `5000` | Connection timeout (in milliseconds) | +| 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 | @@ -40,7 +41,12 @@ By default the strategy uses exponential backoff, but it can be overwritten like ```javascript createClient({ socket: { - reconnectStrategy: retries => { + reconnectStrategy: (retries, cause) => { + // By default, do not reconnect on socket timeout. + if (cause instanceof SocketTimeoutError) { + return false; + } + // Generate a random jitter between 0 – 200 ms: const jitter = Math.floor(Math.random() * 200); // Delay is an exponential back off, (times^2) * 50 ms, with a maximum value of 2000 ms: diff --git a/packages/client/lib/client/socket.spec.ts b/packages/client/lib/client/socket.spec.ts index 20b238a3a3..5117cc4f49 100644 --- a/packages/client/lib/client/socket.spec.ts +++ b/packages/client/lib/client/socket.spec.ts @@ -2,13 +2,12 @@ import { strict as assert } from 'node:assert'; import { spy } from 'sinon'; import { once } from 'node:events'; import RedisSocket, { RedisSocketOptions } from './socket'; +import testUtils, { GLOBAL } from '../test-utils'; +import { setTimeout } from 'timers/promises'; describe('Socket', () => { function createSocket(options: RedisSocketOptions): RedisSocket { - const socket = new RedisSocket( - () => Promise.resolve(), - options - ); + const socket = new RedisSocket(() => Promise.resolve(), options); socket.on('error', () => { // ignore errors @@ -84,4 +83,66 @@ describe('Socket', () => { assert.equal(socket.isOpen, false); }); }); + + describe('socketTimeout', () => { + const timeout = 50; + testUtils.testWithClient( + 'should timeout with positive socketTimeout values', + async client => { + let timedOut = false; + + assert.equal(client.isReady, true, 'client.isReady'); + assert.equal(client.isOpen, true, 'client.isOpen'); + + client.on('error', err => { + assert.equal( + err.message, + `Socket timeout timeout. Expecting data, but didn't receive any in ${timeout}ms.` + ); + + assert.equal(client.isReady, false, 'client.isReady'); + + // This is actually a bug with the onSocketError implementation, + // the client should be closed before the error is emitted + process.nextTick(() => { + assert.equal(client.isOpen, false, 'client.isOpen'); + }); + + timedOut = true; + }); + await setTimeout(timeout * 2); + if (!timedOut) assert.fail('Should have timed out by now'); + }, + { + ...GLOBAL.SERVERS.OPEN, + clientOptions: { + socket: { + socketTimeout: timeout + } + } + } + ); + + testUtils.testWithClient( + 'should not timeout with undefined socketTimeout', + async client => { + + assert.equal(client.isReady, true, 'client.isReady'); + assert.equal(client.isOpen, true, 'client.isOpen'); + + client.on('error', err => { + assert.fail('Should not have timed out or errored in any way'); + }); + await setTimeout(100); + }, + { + ...GLOBAL.SERVERS.OPEN, + clientOptions: { + socket: { + socketTimeout: undefined + } + } + } + ); + }); }); diff --git a/packages/client/lib/client/socket.ts b/packages/client/lib/client/socket.ts index 603416cf9e..58ccbe0b0c 100644 --- a/packages/client/lib/client/socket.ts +++ b/packages/client/lib/client/socket.ts @@ -1,7 +1,7 @@ import { EventEmitter, once } from 'node:events'; import net from 'node:net'; import tls from 'node:tls'; -import { ConnectionTimeoutError, ClientClosedError, SocketClosedUnexpectedlyError, ReconnectStrategyError } from '../errors'; +import { ConnectionTimeoutError, ClientClosedError, SocketClosedUnexpectedlyError, ReconnectStrategyError, SocketTimeoutError } from '../errors'; import { setTimeout } from 'node:timers/promises'; import { RedisArgument } from '../RESP/types'; @@ -23,6 +23,10 @@ type RedisSocketOptionsCommon = { * 3. `(retries: number, cause: Error) => false | number | Error` -> `number` is the same as configuring a `number` directly, `Error` is the same as `false`, but with a custom error. */ reconnectStrategy?: false | number | ReconnectStrategyFunction; + /** + * The timeout (in milliseconds) after which the socket will be closed. `undefined` means no timeout. + */ + socketTimeout?: number; } type RedisTcpOptions = RedisSocketOptionsCommon & NetOptions & Omit< @@ -55,6 +59,7 @@ export default class RedisSocket extends EventEmitter { readonly #connectTimeout; readonly #reconnectStrategy; readonly #socketFactory; + readonly #socketTimeout; #socket?: net.Socket | tls.TLSSocket; @@ -85,6 +90,7 @@ export default class RedisSocket extends EventEmitter { this.#connectTimeout = options?.connectTimeout ?? 5000; this.#reconnectStrategy = this.#createReconnectStrategy(options); this.#socketFactory = this.#createSocketFactory(options); + this.#socketTimeout = options?.socketTimeout; } #createReconnectStrategy(options?: RedisSocketOptions): ReconnectStrategyFunction { @@ -103,7 +109,7 @@ export default class RedisSocket extends EventEmitter { return retryIn; } catch (err) { this.emit('error', err); - return this.defaultReconnectStrategy(retries); + return this.defaultReconnectStrategy(retries, err); } }; } @@ -253,6 +259,13 @@ export default class RedisSocket extends EventEmitter { socket.removeListener('timeout', onTimeout); } + if (this.#socketTimeout) { + socket.once('timeout', () => { + socket.destroy(new SocketTimeoutError(this.#socketTimeout!)); + }); + socket.setTimeout(this.#socketTimeout); + } + socket .once('error', err => this.#onSocketError(err)) .once('close', hadError => { @@ -341,7 +354,12 @@ export default class RedisSocket extends EventEmitter { this.#socket?.unref(); } - defaultReconnectStrategy(retries: number) { + defaultReconnectStrategy(retries: number, cause: unknown) { + // By default, do not reconnect on socket timeout. + if (cause instanceof SocketTimeoutError) { + return false; + } + // Generate a random jitter between 0 – 200 ms: const jitter = Math.floor(Math.random() * 200); // Delay is an exponential back off, (times^2) * 50 ms, with a maximum value of 2000 ms: diff --git a/packages/client/lib/errors.ts b/packages/client/lib/errors.ts index 8af4c5e5be..db37ec1a9b 100644 --- a/packages/client/lib/errors.ts +++ b/packages/client/lib/errors.ts @@ -16,6 +16,12 @@ export class ConnectionTimeoutError extends Error { } } +export class SocketTimeoutError extends Error { + constructor(timeout: number) { + super(`Socket timeout timeout. Expecting data, but didn't receive any in ${timeout}ms.`); + } +} + export class ClientClosedError extends Error { constructor() { super('The client is closed');