test(socket): add tests for socketTimeout option

Author: nkaradzhov

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

@@ -260,10 +260,10 @@ export default class RedisSocket extends EventEmitter {
     }
 
     if (this.#socketTimeout) {
-      socket.setTimeout(this.#socketTimeout);
       socket.once('timeout', () => {
         socket.destroy(new SocketTimeoutError(this.#socketTimeout!));
       });
+      socket.setTimeout(this.#socketTimeout);
     }
 
     socket