fix(client): emit termination event when reconnecting stops

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)`                                 |
 

docs/client-configuration.md

@@ -62,6 +62,15 @@ 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:
+
+```javascript
+client.on('terminated', cause => {
+  console.error('client will not reconnect:', cause);
+  queueMicrotask(() => client.destroy());
+});
+```
+
 ## TLS
 
 To enable TLS, set `socket.tls` to `true`. Below are some basic examples.

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<Error>(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();

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'));
   }

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,182 @@ 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
+      });
+
+      const events: string[] = [];
+      let terminatedCause: Error | undefined;
+      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);
+    });
+
+    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
+      });
+
+      const events: string[] = [];
+      let terminatedCause: Error | undefined;
+      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 () => {
+      // 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<void>(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 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.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();
+        await new Promise<void>(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();
+    });
+
+    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<void>(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<void>(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<void>(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<void>(resolve => server.close(() => resolve()));
+      }
+    });
+  });
+
   describe('initiator interruption (#3346)', () => {
     it('should keep retrying when the socket dies while the initiator is suspended', async () => {
       const connections: net.Socket[] = [];

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) {
@@ -215,6 +222,7 @@ const retryIn = strategy(retries, cause);
         internal: false,
         clientId: this.#clientId
       }));
+      this.emit('terminated', cause);
       this.emit('error', cause);
       return cause;
     } else if (retryIn instanceof Error) {
@@ -225,8 +233,10 @@ const retryIn = strategy(retries, cause);
         internal: false,
         clientId: this.#clientId
       }));
+      const terminatedBy = new ReconnectStrategyError(retryIn, cause);
+      this.emit('terminated', terminatedBy);
       this.emit('error', cause);
-      return new ReconnectStrategyError(retryIn, cause);
+      return terminatedBy;
     }
 
     return retryIn;
@@ -397,22 +407,48 @@ 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();
 
     if (wasReady) {
       publish(CHANNELS.CONNECTION_CLOSED, () => ({ clientId: this.#clientId, reason: 'error', wasConnected: true }));
     }
 
-    if (!wasReady || !this.#isOpen || typeof this.#shouldReconnect(0, err) !== 'number') return;
+    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',
+        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);
+    if (!this.#isOpen) return;
 
     this.emit('reconnecting');
     this.#connect().catch(() => {

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