fix: Connection Instability with socketTimeout Parameter (#9)

* fix socketTimeout connection instability

Signed-off-by: Aleksandr Zinin <[email protected]>

* add Datahandler constructor test

Signed-off-by: Aleksandr Zinin <[email protected]>

* add socketTimeout functional tests

Signed-off-by: Aleksandr Zinin <[email protected]>

---------

Signed-off-by: Aleksandr Zinin <[email protected]>

lib/DataHandler.ts

@@ -56,9 +56,13 @@ export default class DataHandler {
       },
     });
 
-    redis.stream.on("data", (data) => {
+    redis.stream.prependListener("data", (data) => {
       parser.execute(data);
     });
+
+    // `prependListener` not switching a stream to flowing mode as `on` does,
+    // so we need to do it manually in case if our listener is the first one
+    redis.stream.resume();
   }
 
   private returnFatalError(err: Error) {

test/functional/socketTimeout.ts

@@ -0,0 +1,64 @@
+import { expect } from "chai";
+import Redis from "../../lib/Redis";
+
+describe("socketTimeout", () => {
+  const timeoutMs = 500;
+
+  it("should ensure correct startup with password (https://github.com/redis/ioredis/issues/1919)", (done) => {
+    let timeoutObj: NodeJS.Timeout;
+
+    const redis = new Redis({
+      socketTimeout: timeoutMs,
+      lazyConnect: true,
+      password: "foobared",
+    });
+
+    redis.on("error", (err) => {
+      clearTimeout(timeoutObj);
+      done(err.toString());
+    });
+
+    redis.connect(() => {
+      timeoutObj = setTimeout(() => {
+        done();
+      }, timeoutMs * 2);
+    });
+  });
+
+  it("should not throw error when socketTimeout is set and no command is sent", (done) => {
+    let timeoutObj: NodeJS.Timeout;
+
+    const redis = new Redis({
+      socketTimeout: timeoutMs,
+      lazyConnect: true,
+    });
+
+    redis.on("error", (err) => {
+      clearTimeout(timeoutObj);
+      done(err.toString());
+    });
+
+    redis.connect(() => {
+      timeoutObj = setTimeout(() => {
+        done();
+      }, timeoutMs * 2);
+    });
+  });
+
+  it("should throw if socket timeout is reached", (done) => {
+    const redis = new Redis({
+      socketTimeout: timeoutMs,
+      lazyConnect: true,
+    });
+
+    redis.on("error", (err) => {
+      expect(err.message).to.include("Socket timeout");
+      done();
+    });
+
+    redis.connect(() => {
+      redis.stream.removeAllListeners("data");
+      redis.ping();
+    });
+  });
+});

test/unit/DataHandler.ts

@@ -0,0 +1,30 @@
+import * as sinon from "sinon";
+import { expect } from "chai";
+import DataHandler from "../../lib/DataHandler";
+
+describe("DataHandler", () => {
+  afterEach(() => {
+    sinon.restore();
+  });
+
+  describe("constructor()", () => {
+    it("should add a data handler to the redis stream properly", () => {
+      const dataHandledable = {
+        stream: {
+          prependListener: sinon.spy(),
+          resume: sinon.spy(),
+        },
+      };
+      new DataHandler(dataHandledable, {});
+
+      expect(dataHandledable.stream.prependListener.calledOnce).to.eql(true);
+      expect(dataHandledable.stream.resume.calledOnce).to.eql(true);
+
+      expect(
+        dataHandledable.stream.resume.calledAfter(
+          dataHandledable.stream.prependListener
+        )
+      ).to.eql(true);
+    });
+  });
+});