From 78ece6b37782885edb09d74ea624697bbe77984b Mon Sep 17 00:00:00 2001
From: borislav ivanov <borislav.ivanov@redis.com>
Date: Fri, 20 Dec 2024 11:59:41 +0200
Subject: [PATCH] fix: Connection instability when using socketTimeout
 parameter

The issue occurs when using socketTimeout, causing connections to become
unstable with repeated disconnections and reconnections. This happens due
to incorrect ordering of socket stream event handling.

Changes:
- Use prependListener() instead of on() for `DataHandler` stream data events
- Explicitly call resume() after attaching the `DataHandler` stream listener
- Add tests to verify socket timeout behavior

This ensures the parser receives and processes data before timeout checks,
preventing premature timeouts and connection instability.

Fixes #1919
---
 lib/DataHandler.ts               |  5 ++-
 test/functional/socketTimeout.ts | 54 ++++++++++++++++++++++++++++++++
 test/unit/DataHandler.ts         | 25 +++++++++++++++
 3 files changed, 83 insertions(+), 1 deletion(-)
 create mode 100644 test/functional/socketTimeout.ts
 create mode 100644 test/unit/DataHandler.ts

diff --git a/lib/DataHandler.ts b/lib/DataHandler.ts
index 717de6de..d9030517 100644
--- a/lib/DataHandler.ts
+++ b/lib/DataHandler.ts
@@ -56,9 +56,12 @@ export default class DataHandler {
       },
     });
 
-    redis.stream.on("data", (data) => {
+    // prependListener ensures the parser receives and processes data before socket timeout checks are performed
+    redis.stream.prependListener("data", (data) => {
       parser.execute(data);
     });
+    // prependListener() doesn't enable flowing mode automatically - we need to resume the stream manually
+    redis.stream.resume();
   }
 
   private returnFatalError(err: Error) {
diff --git a/test/functional/socketTimeout.ts b/test/functional/socketTimeout.ts
new file mode 100644
index 00000000..f2a62711
--- /dev/null
+++ b/test/functional/socketTimeout.ts
@@ -0,0 +1,54 @@
+import { expect } from 'chai';
+import { Done } from 'mocha';
+import Redis from '../../lib/Redis';
+
+describe('Redis Connection Socket Timeout', () => {
+  const SOCKET_TIMEOUT_MS = 500;
+
+  it('maintains stable connection with password authentication | https://github.com/redis/ioredis/issues/1919 ', (done) => {
+    const redis = createRedis({ password: 'password' });
+    assertNoTimeoutAfterConnection(redis, done);
+  });
+
+  it('maintains stable connection without initial authentication | https://github.com/redis/ioredis/issues/1919', (done) => {
+    const redis = createRedis();
+    assertNoTimeoutAfterConnection(redis, done);
+  });
+
+  it('should throw when socket timeout threshold is exceeded', (done) => {
+    const redis = createRedis()
+
+    redis.on('error', (err) => {
+      expect(err.message).to.eql(`Socket timeout. Expecting data, but didn't receive any in ${SOCKET_TIMEOUT_MS}ms.`);
+      done();
+    });
+
+    redis.connect(() => {
+      redis.stream.removeAllListeners('data');
+      redis.ping();
+    });
+  });
+
+  function createRedis(options = {}) {
+    return new Redis({
+      socketTimeout: SOCKET_TIMEOUT_MS,
+      lazyConnect: true,
+      ...options
+    });
+  }
+
+  function assertNoTimeoutAfterConnection(redisInstance: Redis, done: Done) {
+    let timeoutObj: NodeJS.Timeout;
+
+    redisInstance.on('error', (err) => {
+      clearTimeout(timeoutObj);
+      done(err.toString());
+    });
+
+    redisInstance.connect(() => {
+      timeoutObj = setTimeout(() => {
+        done();
+      }, SOCKET_TIMEOUT_MS * 2);
+    });
+  }
+});
diff --git a/test/unit/DataHandler.ts b/test/unit/DataHandler.ts
new file mode 100644
index 00000000..05e49ce1
--- /dev/null
+++ b/test/unit/DataHandler.ts
@@ -0,0 +1,25 @@
+import { expect } from 'chai';
+import * as sinon from 'sinon';
+import DataHandler from '../../lib/DataHandler';
+
+describe('DataHandler', () => {
+  it('attaches data handler to stream in correct order | https://github.com/redis/ioredis/issues/1919', () => {
+
+    const prependListener = sinon.spy((event: string, handler: Function) => {
+      expect(event).to.equal('data');
+    });
+
+    const resume = sinon.spy();
+
+    new DataHandler({
+      stream: {
+        prependListener,
+        resume
+      }
+    } as any, {} as any);
+
+    expect(prependListener.calledOnce).to.be.true;
+    expect(resume.calledOnce).to.be.true;
+    expect(resume.calledAfter(prependListener)).to.be.true;
+  });
+});
\ No newline at end of file