From 77ecdf2721a02bbe8dfea6edfb3b6a596b2df605 Mon Sep 17 00:00:00 2001 From: Leibale Date: Mon, 12 Feb 2024 18:38:00 -0500 Subject: [PATCH] fix #2679 - fix socket types, and clean some code --- docs/client-configuration.md | 53 +++--- docs/v4-to-v5.md | 38 ++-- packages/client/lib/client/index.ts | 4 +- packages/client/lib/client/socket.ts | 275 +++++++++++++++------------ 4 files changed, 206 insertions(+), 164 deletions(-) diff --git a/docs/client-configuration.md b/docs/client-configuration.md index 1854f07158..69a2db16c3 100644 --- a/docs/client-configuration.md +++ b/docs/client-configuration.md @@ -1,31 +1,32 @@ # `createClient` configuration -| Property | Default | Description | -|--------------------------|------------------------------------------|-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------| -| url | | `redis[s]://[[username][:password]@][host][:port][/db-number]` (see [`redis`](https://www.iana.org/assignments/uri-schemes/prov/redis) and [`rediss`](https://www.iana.org/assignments/uri-schemes/prov/rediss) IANA registration for more details) | -| socket | | Socket connection properties. Unlisted [`net.connect`](https://nodejs.org/api/net.html#socketconnectoptions-connectlistener) properties (and [`tls.connect`](https://nodejs.org/api/tls.html#tlsconnectoptions-callback)) are also supported | -| socket.port | `6379` | Redis server port | -| socket.host | `'localhost'` | Redis server hostname | -| 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.noDelay | `true` | Toggle [`Nagle's algorithm`](https://nodejs.org/api/net.html#net_socket_setnodelay_nodelay) | -| socket.keepAlive | `5000` | Toggle [`keep-alive`](https://nodejs.org/api/net.html#net_socket_setkeepalive_enable_initialdelay) functionality | -| socket.tls | | See explanation and examples [below](#TLS) | -| socket.reconnectStrategy | `retries => Math.min(retries * 50, 500)` | A function containing the [Reconnect Strategy](#reconnect-strategy) logic | -| username | | ACL username ([see ACL guide](https://redis.io/topics/acl)) | -| password | | ACL password or the old "--requirepass" password | -| name | | Client name ([see `CLIENT SETNAME`](https://redis.io/commands/client-setname)) | -| database | | Redis database number (see [`SELECT`](https://redis.io/commands/select) command) | -| modules | | Included [Redis Modules](../README.md#packages) | -| scripts | | Script definitions (see [Lua Scripts](../README.md#lua-scripts)) | -| functions | | Function definitions (see [Functions](../README.md#functions)) | -| commandsQueueMaxLength | | Maximum length of the client's internal command queue | -| disableOfflineQueue | `false` | Disables offline queuing, see [FAQ](./FAQ.md#what-happens-when-the-network-goes-down) | -| readonly | `false` | Connect in [`READONLY`](https://redis.io/commands/readonly) mode | -| legacyMode | `false` | Maintain some backwards compatibility (see the [Migration Guide](./v3-to-v4.md)) | -| isolationPoolOptions | | See the [Isolated Execution Guide](./isolated-execution.md) | -| 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) | +| Property | Default | Description | +|------------------------------|------------------------------------------|-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------| +| url | | `redis[s]://[[username][:password]@][host][:port][/db-number]` (see [`redis`](https://www.iana.org/assignments/uri-schemes/prov/redis) and [`rediss`](https://www.iana.org/assignments/uri-schemes/prov/rediss) IANA registration for more details) | +| socket | | Socket connection properties. Unlisted [`net.connect`](https://nodejs.org/api/net.html#socketconnectoptions-connectlistener) properties (and [`tls.connect`](https://nodejs.org/api/tls.html#tlsconnectoptions-callback)) are also supported | +| socket.port | `6379` | Redis server port | +| socket.host | `'localhost'` | Redis server hostname | +| 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.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.tls | | See explanation and examples [below](#TLS) | +| socket.reconnectStrategy | `retries => Math.min(retries * 50, 500)` | A function containing the [Reconnect Strategy](#reconnect-strategy) logic | +| username | | ACL username ([see ACL guide](https://redis.io/topics/acl)) | +| password | | ACL password or the old "--requirepass" password | +| name | | Client name ([see `CLIENT SETNAME`](https://redis.io/commands/client-setname)) | +| database | | Redis database number (see [`SELECT`](https://redis.io/commands/select) command) | +| modules | | Included [Redis Modules](../README.md#packages) | +| scripts | | Script definitions (see [Lua Scripts](../README.md#lua-scripts)) | +| functions | | Function definitions (see [Functions](../README.md#functions)) | +| commandsQueueMaxLength | | Maximum length of the client's internal command queue | +| disableOfflineQueue | `false` | Disables offline queuing, see [FAQ](./FAQ.md#what-happens-when-the-network-goes-down) | +| readonly | `false` | Connect in [`READONLY`](https://redis.io/commands/readonly) mode | +| legacyMode | `false` | Maintain some backwards compatibility (see the [Migration Guide](./v3-to-v4.md)) | +| isolationPoolOptions | | See the [Isolated Execution Guide](./isolated-execution.md) | +| 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) | ## Reconnect Strategy diff --git a/docs/v4-to-v5.md b/docs/v4-to-v5.md index 0863c7478a..fb68656d5c 100644 --- a/docs/v4-to-v5.md +++ b/docs/v4-to-v5.md @@ -1,5 +1,27 @@ # v4 to v5 migration guide +## Client Configuration + +### Keep Alive + +To better align with Node.js build-in [`net`](https://nodejs.org/api/net.html) and [`tls`](https://nodejs.org/api/tls.html) modules, the `keepAlive` option has been split into 2 options: `keepAlive` (`boolean`) and `keepAliveInitialDelay` (`number`). The defaults remain `true` and `5000`. + +### Legacy Mode + +In the previous version, you could access "legacy" mode by creating a client and passing in `{ legacyMode: true }`. Now, you can create one off of an existing client by calling the `.legacy()` function. This allows easier access to both APIs and enables better TypeScript support. + +```javascript +// use `client` for the current API +const client = createClient(); +await client.set('key', 'value'); + +// use `legacyClient` for the "legacy" API +const legacyClient = client.legacy(); +legacyClient.set('key', 'value', (err, reply) => { + // ... +}); +``` + ## Command Options In v4, command options are passed as a first optional argument: @@ -59,22 +81,6 @@ for await (const keys of client.scanIterator()) { for more information, see the [Scan Iterators guide](./scan-iterators.md). -## Legacy Mode - -In the previous version, you could access "legacy" mode by creating a client and passing in `{ legacyMode: true }`. Now, you can create one off of an existing client by calling the `.legacy()` function. This allows easier access to both APIs and enables better TypeScript support. - -```javascript -// use `client` for the current API -const client = createClient(); -await client.set('key', 'value'); - -// use `legacyClient` for the "legacy" API -const legacyClient = client.legacy(); -legacyClient.set('key', 'value', (err, reply) => { - // ... -}); -``` - ## Isolation Pool [TODO](./blocking-commands.md). diff --git a/packages/client/lib/client/index.ts b/packages/client/lib/client/index.ts index 041b0b7225..e77396d91a 100644 --- a/packages/client/lib/client/index.ts +++ b/packages/client/lib/client/index.ts @@ -1,5 +1,5 @@ import COMMANDS from '../commands'; -import RedisSocket, { RedisSocketOptions, RedisTlsSocketOptions } from './socket'; +import RedisSocket, { RedisSocketOptions } from './socket'; import RedisCommandsQueue, { CommandOptions } from './commands-queue'; import { EventEmitter } from 'node:events'; import { attachConfig, functionArgumentsPrefix, getTransformReply, scriptArgumentsPrefix } from '../commander'; @@ -244,7 +244,7 @@ export default class RedisClient< }; if (protocol === 'rediss:') { - (parsed.socket as RedisTlsSocketOptions).tls = true; + parsed!.socket!.tls = true; } else if (protocol !== 'redis:') { throw new TypeError('Invalid protocol'); } diff --git a/packages/client/lib/client/socket.ts b/packages/client/lib/client/socket.ts index ca75016d98..03dbcde154 100644 --- a/packages/client/lib/client/socket.ts +++ b/packages/client/lib/client/socket.ts @@ -1,84 +1,65 @@ -import { EventEmitter } from 'node:events'; -import * as net from 'node:net'; -import * as tls from 'node:tls'; +import { EventEmitter, once } from 'node:events'; +import net from 'node:net'; +import tls from 'node:tls'; import { ConnectionTimeoutError, ClientClosedError, SocketClosedUnexpectedlyError, ReconnectStrategyError } from '../errors'; import { setTimeout } from 'node:timers/promises'; import { RedisArgument } from '../RESP/types'; -export interface RedisSocketCommonOptions { +type NetOptions = { + tls?: false; +}; + +type TcpOptions = NetOptions & Omit< + net.TcpNetConnectOpts, + 'timeout' | 'onread' | 'readable' | 'writable' | 'port' +> & { + port?: number; +}; + +type IpcOptions = NetOptions & Omit< + net.IpcNetConnectOpts, + 'timeout' | 'onread' | 'readable' | 'writable' +>; + +type TlsOptions = { + tls: true; +} & tls.ConnectionOptions; + +type ReconnectStrategyFunction = (retries: number, cause: Error) => false | Error | number; + +export type RedisSocketOptions = { /** - * Connection Timeout (in milliseconds) + * Connection timeout (in milliseconds) */ connectTimeout?: number; - /** - * Toggle [`Nagle's algorithm`](https://nodejs.org/api/net.html#net_socket_setnodelay_nodelay) - */ - noDelay?: boolean; - /** - * Toggle [`keep-alive`](https://nodejs.org/api/net.html#net_socket_setkeepalive_enable_initialdelay) - */ - keepAlive?: number | false; /** * When the socket closes unexpectedly (without calling `.close()`/`.destroy()`), the client uses `reconnectStrategy` to decide what to do. The following values are supported: * 1. `false` -> do not reconnect, close the client and flush the command queue. * 2. `number` -> wait for `X` milliseconds before reconnecting. * 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. - * Defaults to `retries => Math.min(retries * 50, 500)` */ - reconnectStrategy?: false | number | ((retries: number, cause: Error) => false | Error | number); -} - -export interface RedisNetConnectOpts extends Omit, 'keepAlive'>, Partial, RedisSocketCommonOptions { - tls?: false; -}; - -export interface RedisTlsSocketOptions extends Partial, RedisSocketCommonOptions { - tls: true; -}; - -export type RedisSocketOptions = RedisNetConnectOpts | RedisTlsSocketOptions - -interface CreateSocketReturn { - connectEvent: string; - socket: T; -} + reconnectStrategy?: false | number | ReconnectStrategyFunction; +} & (TcpOptions | IpcOptions | TlsOptions); export type RedisSocketInitiator = () => void | Promise; export default class RedisSocket extends EventEmitter { - static #initiateOptions(options?: RedisSocketOptions): RedisSocketOptions { - options ??= {}; - if (!(options as net.IpcSocketConnectOpts).path) { - (options as net.TcpSocketConnectOpts).port ??= 6379; - (options as net.TcpSocketConnectOpts).host ??= 'localhost'; - } - - options.connectTimeout ??= 5000; - options.keepAlive ??= 5000; - options.noDelay ??= true; - - return options; - } - - static #isTlsSocket(options: RedisSocketOptions): options is RedisTlsSocketOptions { - return (options as RedisTlsSocketOptions).tls === true; - } - - readonly #initiator: RedisSocketInitiator; - - readonly #options: RedisSocketOptions; + readonly #initiator; + readonly #connectTimeout; + readonly #reconnectStrategy; + readonly #socketFactory; #socket?: net.Socket | tls.TLSSocket; #isOpen = false; - get isOpen(): boolean { + get isOpen() { return this.#isOpen; } #isReady = false; - get isReady(): boolean { + get isReady() { return this.#isReady; } @@ -88,28 +69,101 @@ export default class RedisSocket extends EventEmitter { super(); this.#initiator = initiator; - this.#options = RedisSocket.#initiateOptions(options); + this.#connectTimeout = options?.connectTimeout ?? 5000; + this.#reconnectStrategy = this.#createReconnectStrategy(options); + this.#socketFactory = this.#createSocketFactory(options); } - #reconnectStrategy(retries: number, cause: Error) { - if (this.#options.reconnectStrategy === false) { - return false; - } else if (typeof this.#options.reconnectStrategy === 'number') { - return this.#options.reconnectStrategy; - } else if (this.#options.reconnectStrategy) { - try { - const retryIn = this.#options.reconnectStrategy(retries, cause); - if (retryIn !== false && !(retryIn instanceof Error) && typeof retryIn !== 'number') { - throw new TypeError(`Reconnect strategy should return \`false | Error | number\`, got ${retryIn} instead`); - } - - return retryIn; - } catch (err) { - this.emit('error', err); - } + #createReconnectStrategy(options?: RedisSocketOptions): ReconnectStrategyFunction { + const strategy = options?.reconnectStrategy; + if (strategy === false || typeof strategy === 'number') { + return () => strategy; } - return Math.min(retries * 50, 500); + if (strategy) { + return (retries, cause) => { + try { + const retryIn = strategy(retries, cause); + if (retryIn !== false && !(retryIn instanceof Error) && typeof retryIn !== 'number') { + throw new TypeError(`Reconnect strategy should return \`false | Error | number\`, got ${retryIn} instead`); + } + return retryIn; + } catch (err) { + this.emit('error', err); + return Math.min(retries * 50, 500); + } + }; + } + + return retries => Math.min(retries * 50, 500); + } + + #createSocketFactory(options?: RedisSocketOptions) { + // TLS + if (options?.tls === true) { + const withDefaults: tls.ConnectionOptions = { + ...options, + port: options?.port ?? 6379, + // https://nodejs.org/api/tls.html#tlsconnectoptions-callback "Any socket.connect() option not already listed" + // @types/node is... incorrect... + // @ts-expect-error + 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 + 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 + keepAliveInitialDelay: options?.keepAliveInitialDelay ?? 5000, + timeout: undefined, + onread: undefined, + readable: true, + writable: true + }; + return { + create() { + return tls.connect(withDefaults); + }, + event: 'secureConnect' + }; + } + + // IPC + if (options && 'path' in options) { + const withDefaults: net.IpcNetConnectOpts = { + ...options, + timeout: undefined, + onread: undefined, + readable: true, + writable: true + }; + return { + create() { + return net.createConnection(withDefaults); + }, + event: 'connect' + }; + } + + // TCP + const withDefaults: net.TcpNetConnectOpts = { + ...options, + port: options?.port ?? 6379, + noDelay: options?.noDelay ?? true, + keepAlive: options?.keepAlive ?? true, + keepAliveInitialDelay: options?.keepAliveInitialDelay ?? 5000, + timeout: undefined, + onread: undefined, + readable: true, + writable: true + }; + return { + create() { + return net.createConnection(withDefaults); + }, + event: 'connect' + }; } #shouldReconnect(retries: number, cause: Error) { @@ -164,56 +218,37 @@ export default class RedisSocket extends EventEmitter { } } while (this.#isOpen && !this.#isReady); } + + async #createSocket(): Promise { + const socket = this.#socketFactory.create(); - #createSocket(): Promise { - return new Promise((resolve, reject) => { - const { connectEvent, socket } = RedisSocket.#isTlsSocket(this.#options) ? - this.#createTlsSocket() : - this.#createNetSocket(); + let onTimeout; + if (this.#connectTimeout !== undefined) { + onTimeout = () => socket.destroy(new ConnectionTimeoutError()); + socket.once('timeout', onTimeout); + socket.setTimeout(this.#connectTimeout); + } - if (this.#options.connectTimeout) { - socket.setTimeout(this.#options.connectTimeout, () => socket.destroy(new ConnectionTimeoutError())); - } + if (this.#isSocketUnrefed) { + socket.unref(); + } - if (this.#isSocketUnrefed) { - socket.unref(); - } + await once(socket, this.#socketFactory.event); - socket - .setNoDelay(this.#options.noDelay) - .once('error', reject) - .once(connectEvent, () => { - socket - .setTimeout(0) - // https://github.com/nodejs/node/issues/31663 - .setKeepAlive(this.#options.keepAlive !== false, this.#options.keepAlive || 0) - .off('error', reject) - .once('error', (err: Error) => this.#onSocketError(err)) - .once('close', hadError => { - if (!hadError && this.#isOpen && this.#socket === socket) { - this.#onSocketError(new SocketClosedUnexpectedlyError()); - } - }) - .on('drain', () => this.emit('drain')) - .on('data', data => this.emit('data', data)); + if (onTimeout) { + socket.removeListener('timeout', onTimeout); + } - resolve(socket); - }); - }); - } + socket + .once('error', err => this.#onSocketError(err)) + .once('close', hadError => { + if (hadError || !this.#isOpen || this.#socket !== socket) return; + this.#onSocketError(new SocketClosedUnexpectedlyError()); + }) + .on('drain', () => this.emit('drain')) + .on('data', data => this.emit('data', data)); - #createNetSocket(): CreateSocketReturn { - return { - connectEvent: 'connect', - socket: net.connect(this.#options as net.NetConnectOpts) // TODO - }; - } - - #createTlsSocket(): CreateSocketReturn { - return { - connectEvent: 'secureConnect', - socket: tls.connect(this.#options as tls.ConnectionOptions) // TODO - }; + return socket; } #onSocketError(err: Error): void { @@ -229,11 +264,11 @@ export default class RedisSocket extends EventEmitter { }); } - write(iterator: IterableIterator>): void { + write(iterable: Iterable>) { if (!this.#socket) return; this.#socket.cork(); - for (const args of iterator) { + for (const args of iterable) { for (const toWrite of args) { this.#socket.write(toWrite); } @@ -282,12 +317,12 @@ export default class RedisSocket extends EventEmitter { this.emit('end'); } - ref(): void { + ref() { this.#isSocketUnrefed = false; this.#socket?.ref(); } - unref(): void { + unref() { this.#isSocketUnrefed = true; this.#socket?.unref(); }