fix: handle connecting immediately after "end" event (#929)

Introduce a connection epoch to distinguish different connections. Close #928
redis · Jul 15, 2019 · 7bcd8a8 · 7bcd8a8
1 parent 7defb6b
commit 7bcd8a8
Show file tree

Hide file tree

Showing 5 changed files with 50 additions and 15 deletions.
diff --git a/lib/connectors/StandaloneConnector.ts b/lib/connectors/StandaloneConnector.ts
@@ -59,23 +59,25 @@ export default class StandaloneConnector extends AbstractConnector {
     // Should use the provided promise in the next major
     // version and do not connect before resolved.
     return new Promise<NetStream>((resolve, reject) => {
-      if (!this.connecting) {
-        reject(new Error(CONNECTION_CLOSED_ERROR_MSG));
-        return;
-      }
+      process.nextTick(() => {
+        if (!this.connecting) {
+          reject(new Error(CONNECTION_CLOSED_ERROR_MSG));
+          return;
+        }
 
-      try {
-        if (options.tls) {
-          this.stream = createTLSConnection(connectionOptions);
-        } else {
-          this.stream = createConnection(connectionOptions);
+        try {
+          if (options.tls) {
+            this.stream = createTLSConnection(connectionOptions);
+          } else {
+            this.stream = createConnection(connectionOptions);
+          }
+        } catch (err) {
+          reject(err);
+          return;
         }
-      } catch (err) {
-        reject(err);
-        return;
-      }
 
-      resolve(this.stream);
+        resolve(this.stream);
+      });
     });
   }
 }
diff --git a/lib/redis/event_handler.ts b/lib/redis/event_handler.ts
@@ -15,8 +15,12 @@ export function connectHandler(self) {
 
     // AUTH command should be processed before any other commands
     let flushed = false;
+    const { connectionEpoch } = self;
     if (self.condition.auth) {
       self.auth(self.condition.auth, function(err) {
+        if (connectionEpoch !== self.connectionEpoch) {
+          return;
+        }
         if (err) {
           if (err.message.indexOf("no password is set") === -1) {
             flushed = true;
@@ -50,6 +54,9 @@ export function connectHandler(self) {
 
     if (self.options.enableReadyCheck) {
       self._readyCheck(function(err, info) {
+        if (connectionEpoch !== self.connectionEpoch) {
+          return;
+        }
         if (err) {
           if (!flushed) {
             self.recoverFromFatalError(

diff --git a/lib/redis/index.ts b/lib/redis/index.ts
@@ -140,6 +140,8 @@ function Redis() {
   this.resetCommandQueue();
   this.resetOfflineQueue();
 
+  this.connectionEpoch = 0;
+
   if (this.options.Connector) {
     this.connector = new this.options.Connector(this.options);
   } else if (this.options.sentinels) {
@@ -259,6 +261,7 @@ Redis.prototype.connect = function(callback) {
       reject(new Error("Redis is already connecting/connected"));
       return;
     }
+    this.connectionEpoch += 1;
     this.setStatus("connecting");
 
     const { options } = this;

diff --git a/test/functional/connection.ts b/test/functional/connection.ts
@@ -50,6 +50,29 @@ describe("connection", function() {
     });
   });
 
+  it("connects successfully immediately after end", done => {
+    const redis = new Redis();
+    redis.once("end", async () => {
+      await redis.connect();
+      done();
+    });
+
+    redis.quit();
+  });
+
+  it("connects successfully immediately after quit", done => {
+    const redis = new Redis();
+    redis.once("end", async () => {
+      await redis.connect();
+      done();
+    });
+
+    // process.nextTick ensures the connection is being made.
+    process.nextTick(() => {
+      redis.quit();
+    });
+  });
+
   describe("connectTimeout", () => {
     it("should close the connection when timeout", function(done) {
       var redis = new Redis(6379, "192.0.0.0", {

diff --git a/test/unit/connectors/connector.ts b/test/unit/connectors/connector.ts
@@ -34,7 +34,7 @@ describe("StandaloneConnector", () => {
         port: 6379,
         tls: { ca: "on" }
       });
-      connector.connect(() => {});
+      await connector.connect(() => {});
       expect(spy.calledOnce).to.eql(true);
       expect(spy.firstCall.args[0]).to.eql({ port: 6379, ca: "on" });
       connector.disconnect();