From b2705d2bb91f84af208238145d63f082fb7dc6b6 Mon Sep 17 00:00:00 2001 From: GiHoon1123 Date: Mon, 31 Aug 2026 11:18:54 +0900 Subject: [PATCH 1/7] fix(client): emit termination event when reconnecting stops --- docs/client-configuration.md | 8 ++ packages/client/lib/client/index.spec.ts | 20 +++++ packages/client/lib/client/index.ts | 1 + packages/client/lib/client/socket.spec.ts | 103 ++++++++++++++++++++++ packages/client/lib/client/socket.ts | 12 ++- 5 files changed, 143 insertions(+), 1 deletion(-) diff --git a/docs/client-configuration.md b/docs/client-configuration.md index 46d351bc2eb..78f7ef61e48 100644 --- a/docs/client-configuration.md +++ b/docs/client-configuration.md @@ -62,6 +62,14 @@ createClient({ }); ``` +An `'error'` event fires on every disconnect, including ones the client is about to retry, so it can't tell you whether reconnection is still in progress. Once `reconnectStrategy` gives up (returns `false` or an `Error`), the client also emits a `'terminated'` event with the reason — that's the signal that reconnection has permanently stopped and the client needs to be recreated: + +```javascript +client.on('terminated', cause => { + console.error('client will not reconnect:', cause); +}); +``` + ## TLS To enable TLS, set `socket.tls` to `true`. Below are some basic examples. diff --git a/packages/client/lib/client/index.spec.ts b/packages/client/lib/client/index.spec.ts index 2621c7de7ca..a15dafe2377 100644 --- a/packages/client/lib/client/index.spec.ts +++ b/packages/client/lib/client/index.spec.ts @@ -1551,6 +1551,26 @@ describe('Client', () => { describe("socket errors during handshake", () => { + it("should re-emit terminated from the socket", async () => { + const client = createClient({ + socket: { + host: "error", + connectTimeout: 1, + reconnectStrategy: false + } + }); + client.on("error", () => {}); + + const terminated = new Promise(resolve => { + client.once("terminated", resolve); + }); + await assert.rejects(client.connect()); + + const cause = await terminated; + assert.ok(cause instanceof Error); + client.destroy(); + }); + it("should successfully connect when server accepts connection immediately", async () => { const { log, client, teardown } = await setup({}, 0); await client.connect(); diff --git a/packages/client/lib/client/index.ts b/packages/client/lib/client/index.ts index d9ab2eab422..dd3dfa12157 100644 --- a/packages/client/lib/client/index.ts +++ b/packages/client/lib/client/index.ts @@ -1053,6 +1053,7 @@ export default class RedisClient< this.#maybeScheduleWrite(); }) .on('reconnecting', () => this.emit('reconnecting')) + .on('terminated', cause => this.emit('terminated', cause)) .on('drain', () => this.#maybeScheduleWrite()) .on('end', () => this.emit('end')); } diff --git a/packages/client/lib/client/socket.spec.ts b/packages/client/lib/client/socket.spec.ts index 260bc9af97c..47e073fa06d 100644 --- a/packages/client/lib/client/socket.spec.ts +++ b/packages/client/lib/client/socket.spec.ts @@ -5,6 +5,7 @@ import net from 'node:net'; import RedisSocket, { RedisSocketOptions } from './socket'; import testUtils, { GLOBAL } from '../test-utils'; import { setTimeout } from 'timers/promises'; +import { ReconnectStrategyError } from '../errors'; describe('Socket', () => { const CLIENT_ID = 'test-client-id'; @@ -87,6 +88,108 @@ describe('Socket', () => { }); }); + describe('terminated event (#2948)', () => { + it('should emit `terminated` when reconnectStrategy gives up on the initial connection', async () => { + const socket = createSocket({ + host: 'error', + connectTimeout: 1, + reconnectStrategy: false + }); + + // `node:events`' `once()` special-cases `'error'` — it resolves/rejects + // as soon as *either* the awaited event or an `'error'` fires, so it + // can't be used to await `'terminated'` here without racing the + // `'error'` this same give-up also emits. A plain listener sidesteps that. + let terminatedCause: Error | undefined; + socket.on('terminated', cause => { terminatedCause = cause; }); + + await assert.rejects(socket.connect()); + + assert.ok(terminatedCause instanceof Error); + assert.equal(socket.isOpen, false); + }); + + it('should emit `terminated` with the wrapped error when a custom reconnectStrategy gives up', async () => { + const reconnectStrategy = spy((retries: number) => { + if (retries === 1) return new Error('done'); + return 0; + }); + + const socket = createSocket({ + host: 'error', + connectTimeout: 1, + reconnectStrategy + }); + + let terminatedCause: Error | undefined; + socket.on('terminated', cause => { terminatedCause = cause; }); + + await assert.rejects(socket.connect()); + + assert.ok(terminatedCause instanceof ReconnectStrategyError, 'terminated cause should be a ReconnectStrategyError'); + }); + + it('should emit `terminated` — not just `error` — when the connection is lost after being ready', async () => { + // This is the scenario from #2948: `error` fires on *every* disconnect, + // including ones the client is about to retry, so a listener can't tell + // "still retrying" apart from "reconnectStrategy gave up, this client + // is dead". Before this fix, losing an already-ready connection only + // ever emitted `error`, indistinguishable from a transient one. + const connections: net.Socket[] = []; + const server = net.createServer(conn => { + conn.on('error', () => { /* ignore */ }); + connections.push(conn); + }); + await new Promise(resolve => server.listen(0, '127.0.0.1', resolve)); + const { port } = server.address() as net.AddressInfo; + const firstConnection = once(server, 'connection') as Promise<[net.Socket]>; + + try { + const socket = createSocket({ + host: '127.0.0.1', + port, + // Give up as soon as the live connection dies, instead of retrying. + reconnectStrategy: false + }); + + await socket.connect(); + assert.equal(socket.isReady, true, 'socket.isReady'); + + const terminatedCauses: Error[] = []; + socket.on('terminated', cause => terminatedCauses.push(cause)); + + const [conn] = await firstConnection; + conn.destroy(); + const [errCause] = await once(socket, 'error') as [Error]; + + assert.equal(terminatedCauses.length, 1, 'terminated should have fired exactly once'); + assert.equal(terminatedCauses[0], errCause, 'terminated should carry the same cause as error'); + assert.equal(socket.isOpen, false, 'socket.isOpen'); + } finally { + for (const conn of connections) conn.destroy(); + await new Promise(resolve => server.close(() => resolve())); + } + }); + + it('should not emit `terminated` when the reconnectStrategy schedules a retry', async () => { + const socket = createSocket({ + host: 'error', + connectTimeout: 1, + reconnectStrategy: 0 + }); + + let terminatedCount = 0; + socket.on('terminated', () => terminatedCount++); + + socket.connect(); + await once(socket, 'error'); + assert.equal(socket.isOpen, true); + assert.equal(terminatedCount, 0, 'terminated must not fire while still retrying'); + + socket.destroy(); + }); + }); + describe('initiator interruption (#3346)', () => { it('should keep retrying when the socket dies while the initiator is suspended', async () => { const connections: net.Socket[] = []; diff --git a/packages/client/lib/client/socket.ts b/packages/client/lib/client/socket.ts index 3c30a2a638e..38c63e6dfe5 100644 --- a/packages/client/lib/client/socket.ts +++ b/packages/client/lib/client/socket.ts @@ -205,6 +205,13 @@ const retryIn = strategy(retries, cause); }; } + /** + * The single choke point where `reconnectStrategy` giving up (`false` or an + * `Error`) is handled: closes the socket for good and emits `'terminated'` + * so a caller reacting only to `'error'` — which also fires on every + * *retried* disconnect — can tell "still retrying" apart from "reconnection + * has permanently stopped, the client is unusable from here on". + */ #shouldReconnect(retries: number, cause: Error) { const retryIn = this.#reconnectStrategy(retries, cause); if (retryIn === false) { @@ -216,6 +223,7 @@ const retryIn = strategy(retries, cause); clientId: this.#clientId })); this.emit('error', cause); + this.emit('terminated', cause); return cause; } else if (retryIn instanceof Error) { this.#isOpen = false; @@ -226,7 +234,9 @@ const retryIn = strategy(retries, cause); clientId: this.#clientId })); this.emit('error', cause); - return new ReconnectStrategyError(retryIn, cause); + const terminatedBy = new ReconnectStrategyError(retryIn, cause); + this.emit('terminated', terminatedBy); + return terminatedBy; } return retryIn; From 6b9516352c67de17423b36485e564c5d6cf86298 Mon Sep 17 00:00:00 2001 From: GiHoon1123 Date: Mon, 31 Aug 2026 11:30:16 +0900 Subject: [PATCH 2/7] docs(client): document terminated event --- README.md | 1 + docs/client-configuration.md | 3 ++- packages/redis/README.md | 1 + 3 files changed, 4 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index 5a96c2061f6..c5c738ee7d2 100644 --- a/README.md +++ b/README.md @@ -388,6 +388,7 @@ The Node Redis client class is an Nodejs EventEmitter and it emits an event each | `end` | Connection has been closed (via `.close()` or `.destroy()`) | _No arguments_ | | `error` | An error has occurred—usually a network issue such as "Socket closed unexpectedly" | `(error: Error)` | | `reconnecting` | Client is trying to reconnect to the server | _No arguments_ | +| `terminated` | Reconnection has stopped because `reconnectStrategy` returned `false` or an `Error` | `(cause: Error)` | | `sharded-channel-moved` | See [here](https://github.com/redis/node-redis/blob/master/docs/pub-sub.md#sharded-channel-moved-event) | See [here](https://github.com/redis/node-redis/blob/master/docs/pub-sub.md#sharded-channel-moved-event) | | `invalidate` | Client Tracking is on with `emitInvalidate` and a key is invalidated | `(key: RedisItem \| null)` | diff --git a/docs/client-configuration.md b/docs/client-configuration.md index 78f7ef61e48..2793ba96968 100644 --- a/docs/client-configuration.md +++ b/docs/client-configuration.md @@ -62,11 +62,12 @@ createClient({ }); ``` -An `'error'` event fires on every disconnect, including ones the client is about to retry, so it can't tell you whether reconnection is still in progress. Once `reconnectStrategy` gives up (returns `false` or an `Error`), the client also emits a `'terminated'` event with the reason — that's the signal that reconnection has permanently stopped and the client needs to be recreated: +An `'error'` event fires on every disconnect, including ones the client is about to retry, so it can't tell you whether reconnection is still in progress. Once `reconnectStrategy` gives up (returns `false` or an `Error`), the client also emits a `'terminated'` event with the reason — that's the signal that reconnection has permanently stopped. Call `destroy()` before replacing the client so its resources are released: ```javascript client.on('terminated', cause => { console.error('client will not reconnect:', cause); + client.destroy(); }); ``` diff --git a/packages/redis/README.md b/packages/redis/README.md index 2242229b50b..9c124060b79 100644 --- a/packages/redis/README.md +++ b/packages/redis/README.md @@ -302,6 +302,7 @@ The Node Redis client class is an Nodejs EventEmitter and it emits an event each | `end` | Connection has been closed (via `.disconnect()`) | _No arguments_ | | `error` | An error has occurred—usually a network issue such as "Socket closed unexpectedly" | `(error: Error)` | | `reconnecting` | Client is trying to reconnect to the server | _No arguments_ | +| `terminated` | Reconnection has stopped because `reconnectStrategy` returned `false` or an `Error` | `(cause: Error)` | | `sharded-channel-moved` | See [here](https://github.com/redis/node-redis/blob/master/docs/pub-sub.md#sharded-channel-moved-event) | See [here](https://github.com/redis/node-redis/blob/master/docs/pub-sub.md#sharded-channel-moved-event) | > :warning: You **MUST** listen to `error` events. If a client doesn't have at least one `error` listener registered and From 87a1fc7299a33cf15cba70cacf7d2e166d0956c6 Mon Sep 17 00:00:00 2001 From: GiHoon1123 Date: Mon, 31 Aug 2026 12:26:29 +0900 Subject: [PATCH 3/7] fix(client): emit termination before error --- docs/client-configuration.md | 2 +- packages/client/lib/client/socket.spec.ts | 20 ++++++++++++++------ packages/client/lib/client/socket.ts | 4 ++-- 3 files changed, 17 insertions(+), 9 deletions(-) diff --git a/docs/client-configuration.md b/docs/client-configuration.md index 2793ba96968..e0b450e2e66 100644 --- a/docs/client-configuration.md +++ b/docs/client-configuration.md @@ -62,7 +62,7 @@ createClient({ }); ``` -An `'error'` event fires on every disconnect, including ones the client is about to retry, so it can't tell you whether reconnection is still in progress. Once `reconnectStrategy` gives up (returns `false` or an `Error`), the client also emits a `'terminated'` event with the reason — that's the signal that reconnection has permanently stopped. Call `destroy()` before replacing the client so its resources are released: +An `'error'` event fires on every disconnect, including ones the client is about to retry, so it can't tell you whether reconnection is still in progress. Once `reconnectStrategy` gives up (returns `false` or an `Error`), the client emits a `'terminated'` event before the companion `'error'` event. This is the signal that reconnection has permanently stopped. Call `destroy()` before replacing the client so its resources are released: ```javascript client.on('terminated', cause => { diff --git a/packages/client/lib/client/socket.spec.ts b/packages/client/lib/client/socket.spec.ts index 47e073fa06d..beeb95667bc 100644 --- a/packages/client/lib/client/socket.spec.ts +++ b/packages/client/lib/client/socket.spec.ts @@ -96,16 +96,18 @@ describe('Socket', () => { reconnectStrategy: false }); - // `node:events`' `once()` special-cases `'error'` — it resolves/rejects - // as soon as *either* the awaited event or an `'error'` fires, so it - // can't be used to await `'terminated'` here without racing the - // `'error'` this same give-up also emits. A plain listener sidesteps that. + const events: string[] = []; let terminatedCause: Error | undefined; - socket.on('terminated', cause => { terminatedCause = cause; }); + socket.on('terminated', cause => { + events.push('terminated'); + terminatedCause = cause; + }); + socket.on('error', () => events.push('error')); await assert.rejects(socket.connect()); assert.ok(terminatedCause instanceof Error); + assert.deepEqual(events.slice(-2), ['terminated', 'error']); assert.equal(socket.isOpen, false); }); @@ -121,12 +123,18 @@ describe('Socket', () => { reconnectStrategy }); + const events: string[] = []; let terminatedCause: Error | undefined; - socket.on('terminated', cause => { terminatedCause = cause; }); + socket.on('terminated', cause => { + events.push('terminated'); + terminatedCause = cause; + }); + socket.on('error', () => events.push('error')); await assert.rejects(socket.connect()); assert.ok(terminatedCause instanceof ReconnectStrategyError, 'terminated cause should be a ReconnectStrategyError'); + assert.deepEqual(events.slice(-2), ['terminated', 'error']); }); it('should emit `terminated` — not just `error` — when the connection is lost after being ready', async () => { diff --git a/packages/client/lib/client/socket.ts b/packages/client/lib/client/socket.ts index 38c63e6dfe5..7191a62d59e 100644 --- a/packages/client/lib/client/socket.ts +++ b/packages/client/lib/client/socket.ts @@ -222,8 +222,8 @@ const retryIn = strategy(retries, cause); internal: false, clientId: this.#clientId })); - this.emit('error', cause); this.emit('terminated', cause); + this.emit('error', cause); return cause; } else if (retryIn instanceof Error) { this.#isOpen = false; @@ -233,9 +233,9 @@ const retryIn = strategy(retries, cause); internal: false, clientId: this.#clientId })); - this.emit('error', cause); const terminatedBy = new ReconnectStrategyError(retryIn, cause); this.emit('terminated', terminatedBy); + this.emit('error', cause); return terminatedBy; } From 36a1037980c4089fcd6d938ac9d777971ebb2fbd Mon Sep 17 00:00:00 2001 From: GiHoon1123 Date: Mon, 31 Aug 2026 12:59:08 +0900 Subject: [PATCH 4/7] fix(client): order terminal socket events --- packages/client/lib/client/socket.spec.ts | 13 +++++++--- packages/client/lib/client/socket.ts | 30 ++++++++++++++++------- 2 files changed, 30 insertions(+), 13 deletions(-) diff --git a/packages/client/lib/client/socket.spec.ts b/packages/client/lib/client/socket.spec.ts index beeb95667bc..7a71c1d13d8 100644 --- a/packages/client/lib/client/socket.spec.ts +++ b/packages/client/lib/client/socket.spec.ts @@ -163,15 +163,20 @@ describe('Socket', () => { await socket.connect(); assert.equal(socket.isReady, true, 'socket.isReady'); - const terminatedCauses: Error[] = []; - socket.on('terminated', cause => terminatedCauses.push(cause)); + const events: string[] = []; + let terminatedCause: Error | undefined; + socket.on('terminated', cause => { + events.push('terminated'); + terminatedCause = cause; + }); + socket.on('error', () => events.push('error')); const [conn] = await firstConnection; conn.destroy(); const [errCause] = await once(socket, 'error') as [Error]; - assert.equal(terminatedCauses.length, 1, 'terminated should have fired exactly once'); - assert.equal(terminatedCauses[0], errCause, 'terminated should carry the same cause as error'); + assert.deepEqual(events, ['terminated', 'error']); + assert.equal(terminatedCause, errCause, 'terminated should carry the same cause as error'); assert.equal(socket.isOpen, false, 'socket.isOpen'); } finally { for (const conn of connections) conn.destroy(); diff --git a/packages/client/lib/client/socket.ts b/packages/client/lib/client/socket.ts index 7191a62d59e..2d47f680e30 100644 --- a/packages/client/lib/client/socket.ts +++ b/packages/client/lib/client/socket.ts @@ -407,14 +407,6 @@ const retryIn = strategy(retries, cause); this.#isReady = false; const socket = this.#socket; this.#socket = undefined; - publish(CHANNELS.ERROR, () => ({ - error: err, - origin: 'client', - internal: false, - clientId: this.#clientId - })); - this.emit('error', err); - socket?.removeAllListeners('data'); socket?.destroy(); @@ -422,7 +414,27 @@ const retryIn = strategy(retries, cause); publish(CHANNELS.CONNECTION_CLOSED, () => ({ clientId: this.#clientId, reason: 'error', wasConnected: true })); } - if (!wasReady || !this.#isOpen || typeof this.#shouldReconnect(0, err) !== 'number') return; + if (!wasReady || !this.#isOpen) { + publish(CHANNELS.ERROR, () => ({ + error: err, + origin: 'client', + internal: false, + clientId: this.#clientId + })); + this.emit('error', err); + return; + } + + const retryIn = this.#shouldReconnect(0, err); + if (typeof retryIn !== 'number') return; + + publish(CHANNELS.ERROR, () => ({ + error: err, + origin: 'client', + internal: false, + clientId: this.#clientId + })); + this.emit('error', err); this.emit('reconnecting'); this.#connect().catch(() => { From a04d4f81d452ecb0561d9dc96a094346730ebc82 Mon Sep 17 00:00:00 2001 From: GiHoon1123 Date: Mon, 31 Aug 2026 13:21:00 +0900 Subject: [PATCH 5/7] fix(client): handle terminal event edge cases --- docs/client-configuration.md | 2 +- packages/client/lib/client/socket.spec.ts | 61 +++++++++++++++++++++++ packages/client/lib/client/socket.ts | 16 +++++- 3 files changed, 77 insertions(+), 2 deletions(-) diff --git a/docs/client-configuration.md b/docs/client-configuration.md index e0b450e2e66..1c5de1799c2 100644 --- a/docs/client-configuration.md +++ b/docs/client-configuration.md @@ -67,7 +67,7 @@ An `'error'` event fires on every disconnect, including ones the client is about ```javascript client.on('terminated', cause => { console.error('client will not reconnect:', cause); - client.destroy(); + queueMicrotask(() => client.destroy()); }); ``` diff --git a/packages/client/lib/client/socket.spec.ts b/packages/client/lib/client/socket.spec.ts index 7a71c1d13d8..b8df7b2757e 100644 --- a/packages/client/lib/client/socket.spec.ts +++ b/packages/client/lib/client/socket.spec.ts @@ -201,6 +201,67 @@ describe('Socket', () => { socket.destroy(); }); + + it('should not reconnect when an error listener destroys the socket', async () => { + const connections: net.Socket[] = []; + const server = net.createServer(conn => { + conn.on('error', () => { /* ignore */ }); + connections.push(conn); + }); + await new Promise(resolve => server.listen(0, '127.0.0.1', resolve)); + const { port } = server.address() as net.AddressInfo; + const firstConnection = once(server, 'connection') as Promise<[net.Socket]>; + + try { + const socket = createSocket({ + host: '127.0.0.1', + port, + reconnectStrategy: 0 + }); + socket.on('error', () => socket.destroy()); + + await socket.connect(); + const [conn] = await firstConnection; + conn.destroy(); + await once(socket, 'end'); + await setTimeout(10); + + assert.equal(connections.length, 1, 'destroyed socket must not reconnect'); + assert.equal(socket.isOpen, false, 'socket.isOpen'); + } finally { + for (const conn of connections) conn.destroy(); + await new Promise(resolve => server.close(() => resolve())); + } + }); + + it('should emit `terminated` before `error` when the handshake fails', async () => { + const connections: net.Socket[] = []; + const server = net.createServer(conn => { + conn.on('error', () => { /* ignore */ }); + connections.push(conn); + setImmediate(() => conn.destroy()); + }); + await new Promise(resolve => server.listen(0, '127.0.0.1', resolve)); + const { port } = server.address() as net.AddressInfo; + + try { + const socket = new RedisSocket(() => new Promise(() => { /* wait for socket failure */ }), CLIENT_ID, { + host: '127.0.0.1', + port, + reconnectStrategy: false + }); + const events: string[] = []; + socket.on('terminated', () => events.push('terminated')); + socket.on('error', () => events.push('error')); + + await assert.rejects(socket.connect()); + + assert.deepEqual(events, ['terminated', 'error']); + } finally { + for (const conn of connections) conn.destroy(); + await new Promise(resolve => server.close(() => resolve())); + } + }); }); describe('initiator interruption (#3346)', () => { diff --git a/packages/client/lib/client/socket.ts b/packages/client/lib/client/socket.ts index 2d47f680e30..8220015bcfe 100644 --- a/packages/client/lib/client/socket.ts +++ b/packages/client/lib/client/socket.ts @@ -414,7 +414,20 @@ const retryIn = strategy(retries, cause); publish(CHANNELS.CONNECTION_CLOSED, () => ({ clientId: this.#clientId, reason: 'error', wasConnected: true })); } - if (!wasReady || !this.#isOpen) { + if (!wasReady) { + if (!this.#isOpen) { + publish(CHANNELS.ERROR, () => ({ + error: err, + origin: 'client', + internal: false, + clientId: this.#clientId + })); + this.emit('error', err); + } + return; + } + + if (!this.#isOpen) { publish(CHANNELS.ERROR, () => ({ error: err, origin: 'client', @@ -435,6 +448,7 @@ const retryIn = strategy(retries, cause); clientId: this.#clientId })); this.emit('error', err); + if (!this.#isOpen) return; this.emit('reconnecting'); this.#connect().catch(() => { From 104bc2e919b8ebd9f53e39802f7bb1af73ff915f Mon Sep 17 00:00:00 2001 From: GiHoon1123 Date: Mon, 31 Aug 2026 13:47:54 +0900 Subject: [PATCH 6/7] fix(client): preserve terminal error handling --- packages/client/lib/client/socket.ts | 107 +++++++++++---------------- 1 file changed, 45 insertions(+), 62 deletions(-) diff --git a/packages/client/lib/client/socket.ts b/packages/client/lib/client/socket.ts index 8220015bcfe..0bf791c4d01 100644 --- a/packages/client/lib/client/socket.ts +++ b/packages/client/lib/client/socket.ts @@ -13,6 +13,10 @@ type NetOptions = { }; type ReconnectStrategyFunction = (retries: number, cause: Error) => false | Error | number; +type ReconnectStrategyResult = { + retryIn: false | Error | number; + error?: Error; +}; type RedisSocketOptionsCommon = { /** @@ -111,34 +115,30 @@ export default class RedisSocket extends EventEmitter { this.#clientId = clientId; } - #createReconnectStrategy(options?: RedisSocketOptions): ReconnectStrategyFunction { + #createReconnectStrategy(options?: RedisSocketOptions): (retries: number, cause: Error) => ReconnectStrategyResult { const strategy = options?.reconnectStrategy; if (strategy === false || typeof strategy === 'number') { - return () => strategy; + return () => ({ retryIn: strategy }); } if (strategy) { return (retries, cause) => { try { -const retryIn = strategy(retries, cause); + 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; + return { retryIn }; } catch (err) { - publish(CHANNELS.ERROR, () => ({ - error: err as Error, - origin: 'client', - internal: false, - clientId: this.#clientId - })); - this.emit('error', err); - return this.defaultReconnectStrategy(retries, err); + return { + retryIn: this.defaultReconnectStrategy(retries, err), + error: err as Error + }; } }; } - return this.defaultReconnectStrategy; + return (retries, cause) => ({ retryIn: this.defaultReconnectStrategy(retries, cause) }); } #createSocketFactory(options?: RedisSocketOptions) { @@ -213,33 +213,31 @@ const retryIn = strategy(retries, cause); * has permanently stopped, the client is unusable from here on". */ #shouldReconnect(retries: number, cause: Error) { - const retryIn = this.#reconnectStrategy(retries, cause); + const { retryIn, error: strategyError } = this.#reconnectStrategy(retries, cause); if (retryIn === false) { this.#isOpen = false; - publish(CHANNELS.ERROR, () => ({ - error: cause, - origin: 'client', - internal: false, - clientId: this.#clientId - })); this.emit('terminated', cause); - this.emit('error', cause); - return cause; + this.#emitError(cause); + return { retryIn: cause, strategyError }; } else if (retryIn instanceof Error) { this.#isOpen = false; - publish(CHANNELS.ERROR, () => ({ - error: cause, - origin: 'client', - internal: false, - clientId: this.#clientId - })); const terminatedBy = new ReconnectStrategyError(retryIn, cause); this.emit('terminated', terminatedBy); - this.emit('error', cause); - return terminatedBy; + this.#emitError(cause); + return { retryIn: terminatedBy, strategyError }; } - return retryIn; + return { retryIn, strategyError }; + } + + #emitError(err: Error) { + publish(CHANNELS.ERROR, () => ({ + error: err, + origin: 'client', + internal: false, + clientId: this.#clientId + })); + this.emit('error', err); } async connect(): Promise { @@ -264,9 +262,14 @@ const retryIn = strategy(retries, cause); // Check if socket was closed/destroyed during initiator execution if (!this.#socket || this.#socket.destroyed || !this.#socket.readable || !this.#socket.writable) { - const retryIn = this.#shouldReconnect(retries++, new SocketClosedUnexpectedlyError()); + const error = new SocketClosedUnexpectedlyError(); + const { retryIn, strategyError } = this.#shouldReconnect(retries++, error); if (typeof retryIn !== 'number') { throw retryIn; } + this.#emitError(error); + if (strategyError) this.#emitError(strategyError); + if (!this.#isOpen) throw new ClientClosedError(); await setTimeout(retryIn); + if (!this.#isOpen) throw new ClientClosedError(); this.emit('reconnecting'); continue; } @@ -292,19 +295,16 @@ const retryIn = strategy(retries, cause); // reconnecting or scheduling a retry — the shutdown is intentional. if (!this.#isOpen) throw err; - const retryIn = this.#shouldReconnect(retries++, err as Error); + const { retryIn, strategyError } = this.#shouldReconnect(retries++, err as Error); if (typeof retryIn !== 'number') { throw retryIn; } - publish(CHANNELS.ERROR, () => ({ - error: err as Error, - origin: 'client', - internal: false, - clientId: this.#clientId - })); - this.emit('error', err); + this.#emitError(err as Error); + if (strategyError) this.#emitError(strategyError); + if (!this.#isOpen) throw err; await setTimeout(retryIn); + if (!this.#isOpen) throw err; this.emit('reconnecting'); } } while (this.#isOpen && !this.#isReady); @@ -416,38 +416,21 @@ const retryIn = strategy(retries, cause); if (!wasReady) { if (!this.#isOpen) { - publish(CHANNELS.ERROR, () => ({ - error: err, - origin: 'client', - internal: false, - clientId: this.#clientId - })); - this.emit('error', err); + this.#emitError(err); } return; } if (!this.#isOpen) { - publish(CHANNELS.ERROR, () => ({ - error: err, - origin: 'client', - internal: false, - clientId: this.#clientId - })); - this.emit('error', err); + this.#emitError(err); return; } - const retryIn = this.#shouldReconnect(0, err); + const { retryIn, strategyError } = this.#shouldReconnect(0, err); if (typeof retryIn !== 'number') return; - publish(CHANNELS.ERROR, () => ({ - error: err, - origin: 'client', - internal: false, - clientId: this.#clientId - })); - this.emit('error', err); + this.#emitError(err); + if (strategyError) this.#emitError(strategyError); if (!this.#isOpen) return; this.emit('reconnecting'); From f74c465dae7e07fce97cc626128e79eaceb7eb38 Mon Sep 17 00:00:00 2001 From: Gihoon1123 Date: Mon, 31 Aug 2026 21:19:58 +0900 Subject: [PATCH 7/7] docs(client): clarify terminated event ordering --- README.md | 2 +- docs/client-configuration.md | 2 +- packages/redis/README.md | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index c5c738ee7d2..d7c4658842e 100644 --- a/README.md +++ b/README.md @@ -388,7 +388,7 @@ The Node Redis client class is an Nodejs EventEmitter and it emits an event each | `end` | Connection has been closed (via `.close()` or `.destroy()`) | _No arguments_ | | `error` | An error has occurred—usually a network issue such as "Socket closed unexpectedly" | `(error: Error)` | | `reconnecting` | Client is trying to reconnect to the server | _No arguments_ | -| `terminated` | Reconnection has stopped because `reconnectStrategy` returned `false` or an `Error` | `(cause: Error)` | +| `terminated` | Reconnection has stopped because `reconnectStrategy` returned `false` or an `Error`; emitted before the companion `error` | `(cause: Error)` | | `sharded-channel-moved` | See [here](https://github.com/redis/node-redis/blob/master/docs/pub-sub.md#sharded-channel-moved-event) | See [here](https://github.com/redis/node-redis/blob/master/docs/pub-sub.md#sharded-channel-moved-event) | | `invalidate` | Client Tracking is on with `emitInvalidate` and a key is invalidated | `(key: RedisItem \| null)` | diff --git a/docs/client-configuration.md b/docs/client-configuration.md index 1c5de1799c2..d2169853b92 100644 --- a/docs/client-configuration.md +++ b/docs/client-configuration.md @@ -62,7 +62,7 @@ createClient({ }); ``` -An `'error'` event fires on every disconnect, including ones the client is about to retry, so it can't tell you whether reconnection is still in progress. Once `reconnectStrategy` gives up (returns `false` or an `Error`), the client emits a `'terminated'` event before the companion `'error'` event. This is the signal that reconnection has permanently stopped. Call `destroy()` before replacing the client so its resources are released: +An `'error'` event fires on every disconnect, including ones the client is about to retry, so it can't tell you whether reconnection is still in progress. Once `reconnectStrategy` gives up (returns `false` or an `Error`), the client emits a `'terminated'` event before the companion `'error'` event. This ordering lets `await events.once(client, 'terminated')` resolve with the cause before the companion error is emitted, and distinguishes a permanent termination from a transient retry. Call `destroy()` before replacing the client so its resources are released: ```javascript client.on('terminated', cause => { diff --git a/packages/redis/README.md b/packages/redis/README.md index 9c124060b79..8e7bfe5eb9a 100644 --- a/packages/redis/README.md +++ b/packages/redis/README.md @@ -302,7 +302,7 @@ The Node Redis client class is an Nodejs EventEmitter and it emits an event each | `end` | Connection has been closed (via `.disconnect()`) | _No arguments_ | | `error` | An error has occurred—usually a network issue such as "Socket closed unexpectedly" | `(error: Error)` | | `reconnecting` | Client is trying to reconnect to the server | _No arguments_ | -| `terminated` | Reconnection has stopped because `reconnectStrategy` returned `false` or an `Error` | `(cause: Error)` | +| `terminated` | Reconnection has stopped because `reconnectStrategy` returned `false` or an `Error`; emitted before the companion `error` | `(cause: Error)` | | `sharded-channel-moved` | See [here](https://github.com/redis/node-redis/blob/master/docs/pub-sub.md#sharded-channel-moved-event) | See [here](https://github.com/redis/node-redis/blob/master/docs/pub-sub.md#sharded-channel-moved-event) | > :warning: You **MUST** listen to `error` events. If a client doesn't have at least one `error` listener registered and