feat(client): expose socketTimeout option (#2965)

The maximum duration (in milliseconds) that the socket can remain idle (i.e., with no data sent or received) before being automatically closed. Default reconnectionStrategy will ignore the new SocketTimeoutError, but users are allowed to have custom strategies wich handle those errors in different ways

Author: nkaradzhov

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:

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

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:

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