fix reconnect strategy + docs

Author: leibale

docs/client-configuration.md

@@ -29,28 +29,17 @@
 
 ## Reconnect Strategy
 
-TODO: `false | number | (retries: number, cause: unknown) => number | Error`
+When the socket closes unexpectedly (without calling `.quit()`/`.disconnect()`) the client uses `reconnectStrategy` to decide what to do:
+1. `false` -> do not reconnect, close the client and flush all commands in the queue.
+2. `number` -> wait for `X` milliseconds before reconnecting.
+3. `(retries: number, cause: Error) => number | Error` -> `number` is the same as configuration a `number` directly, `Error` is the same as `false`, but with a custom error.
 
-You can implement a custom reconnect strategy as a function:
-When a network error occurs the client will automatically try to reconnect, following a default linear strategy (the more attempts, the more waiting before trying to reconnect).
-
-This strategy can be overridden by providing a `socket.reconnectStrategy` option during the client's creation.
-
-The `socket.reconnectStrategy` is a function that:
-
-- Receives the number of retries attempted so far and the causing error.
-- Returns `number | Error`:
-    - `number`: wait time in milliseconds prior to attempting a reconnect.
-    - `Error`: closes the client and flushes internal command queues.
-
-The example below shows the default `reconnectStrategy` and how to override it.
+By default the strategy is `Math.min(retries * 50, 500)`, but it can be overriten:
 
 ```typescript
-import { createClient } from 'redis';
-
-const client = createClient({
+createClient({
   socket: {
-    reconnectStrategy: retries => Math.min(retries * 50, 500)
+    reconnectStrategy: retries => Math.min(retries * 50, 1000)
   }
 });
 ```

packages/client/lib/client/socket.ts

@@ -9,7 +9,7 @@ export interface RedisSocketCommonOptions {
     connectTimeout?: number;
     noDelay?: boolean;
     keepAlive?: number | false;
-    reconnectStrategy?: false | number | ((retries: number, cause: unknown) => number | Error);
+    reconnectStrategy?: false | number | ((retries: number, cause: Error) => false | Error | number);
 }
 
 type RedisNetSocketOptions = Partial<net.SocketConnectOpts> & {
@@ -83,7 +83,7 @@ export default class RedisSocket extends EventEmitter {
         this.#options = RedisSocket.#initiateOptions(options);
     }
 
-    reconnectStrategy(retries: number, cause: unknown): false | number | Error {
+    #reconnectStrategy(retries: number, cause: Error) {
         if (this.#options.reconnectStrategy === false) {
             return false;
         } else if (typeof this.#options.reconnectStrategy === 'number') {
@@ -92,18 +92,33 @@ export default class RedisSocket extends EventEmitter {
             try {
                 const retryIn = this.#options.reconnectStrategy(retries, cause);
                 if (typeof retryIn !== 'number' && !(retryIn instanceof Error)) {
-                    throw new TypeError(`Reconnect strategy should return \`number | Error\`, got ${retryIn} instead`);
+                    throw new TypeError(`Reconnect strategy should return \`number | Error\`, got ${retryIn} instead`)
                 }
 
                 return retryIn;
             } catch (err) {
-                this.emit('error', err);
+                this.emit('error', err);    
             }
         }
 
         return Math.min(retries * 50, 500);
     }
 
+    #shouldReconnect(retries: number, cause: Error) {
+        const retryIn = this.#reconnectStrategy(retries, cause);
+        if (retryIn === false) {
+            this.#isOpen = false;
+            this.emit('error', cause);
+            return cause;
+        } else if (retryIn instanceof Error) {
+            this.#isOpen = false;
+            this.emit('error', cause);
+            return new ReconnectStrategyError(retryIn, cause);
+        }
+
+        return retryIn;
+    }
+
     async connect(): Promise<void> {
         if (this.#isOpen) {
             throw new Error('Socket already opened');
@@ -113,13 +128,9 @@ export default class RedisSocket extends EventEmitter {
         return this.#connect();
     }
 
-    async #connect(hadError?: boolean): Promise<void> {
+    async #connect(): Promise<void> {
         let retries = 0;
         do {
-            if (retries > 0 || hadError) {
-                this.emit('reconnecting');
-            }
-
             try {
                 this.#socket = await this.#createSocket();
                 this.#writableNeedDrain = false;
@@ -135,21 +146,17 @@ export default class RedisSocket extends EventEmitter {
                 this.#isReady = true;
                 this.emit('ready');
             } catch (err) {
-                const retryIn = this.reconnectStrategy(retries, err);
-                if (retryIn === false) {
-                    this.#isOpen = false;
-                    this.emit('error', err);
-                    throw err;
-                } else if (retryIn instanceof Error) {
-                    this.#isOpen = false;
-                    this.emit('error', err);
-                    throw new ReconnectStrategyError(retryIn, err);
+                const retryIn = this.#shouldReconnect(retries, err as Error);
+                if (typeof retryIn !== 'number') {
+                    throw retryIn;
                 }
 
                 this.emit('error', err);
                 await promiseTimeout(retryIn);
             }
+
             retries++;
+            this.emit('reconnecting');
         } while (this.#isOpen && !this.#isReady);
     }
 
@@ -211,9 +218,10 @@ export default class RedisSocket extends EventEmitter {
         this.#isReady = false;
         this.emit('error', err);
 
-        if (!this.#isOpen) return;
-
-        this.#connect(true).catch(() => {
+        if (!this.#isOpen || typeof this.#shouldReconnect(0, err) !== 'number') return;
+        
+        this.emit('reconnecting');
+        this.#connect().catch(() => {
             // the error was already emitted, silently ignore it
         });
     }