fix: ETIMEDOUT error with Bluebird when connecting. (#925)

There's a race condition when making connections in 4.11.x, which will happen easier with Bluebird & Unix socks. Thank you to @jeremytm & @p3x-robot. Close #918
redis · Jul 13, 2019 · 4bce38b · 4bce38b
1 parent a90f0de
commit 4bce38b
Show file tree

Hide file tree

Showing 3 changed files with 43 additions and 27 deletions.
diff --git a/lib/connectors/SentinelConnector/index.ts b/lib/connectors/SentinelConnector/index.ts
@@ -15,7 +15,6 @@ import SentinelIterator from "./SentinelIterator";
 import { ISentinelAddress } from "./types";
 import AbstractConnector, { ErrorEmitter } from "../AbstractConnector";
 import { NetStream, CallbackFunction } from "../../types";
-import * as PromiseContainer from "../../promiseContainer";
 import Redis from "../../redis";
 
 const debug = Debug("SentinelConnector");
@@ -87,10 +86,9 @@ export default class SentinelConnector extends AbstractConnector {
     this.retryAttempts = 0;
 
     let lastError;
-    const _Promise = PromiseContainer.get();
 
     const connectToNext = () =>
-      new _Promise<NetStream>((resolve, reject) => {
+      new Promise<NetStream>((resolve, reject) => {
         const endpoint = this.sentinelIterator.next();
 
         if (endpoint.done) {

diff --git a/lib/connectors/StandaloneConnector.ts b/lib/connectors/StandaloneConnector.ts
@@ -2,7 +2,6 @@ import { createConnection, TcpNetConnectOpts, IpcNetConnectOpts } from "net";
 import { connect as createTLSConnection, SecureContextOptions } from "tls";
 import { CONNECTION_CLOSED_ERROR_MSG } from "../utils";
 import AbstractConnector, { ErrorEmitter } from "./AbstractConnector";
-import * as PromiseContainer from "../promiseContainer";
 import { NetStream } from "../types";
 
 export function isIIpcConnectionOptions(
@@ -52,27 +51,31 @@ export default class StandaloneConnector extends AbstractConnector {
       Object.assign(connectionOptions, options.tls);
     }
 
-    const _Promise = PromiseContainer.get();
-    return new _Promise<NetStream>((resolve, reject) => {
-      process.nextTick(() => {
-        if (!this.connecting) {
-          reject(new Error(CONNECTION_CLOSED_ERROR_MSG));
-          return;
-        }
+    // TODO:
+    // We use native Promise here since other Promise
+    // implementation may use different schedulers that
+    // cause issue when the stream is resolved in the
+    // next tick.
+    // 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;
+      }
 
-        try {
-          if (options.tls) {
-            this.stream = createTLSConnection(connectionOptions);
-          } else {
-            this.stream = createConnection(connectionOptions);
-          }
-        } catch (err) {
-          reject(err);
-          return;
+      try {
+        if (options.tls) {
+          this.stream = createTLSConnection(connectionOptions);
+        } else {
+          this.stream = createConnection(connectionOptions);
         }
+      } catch (err) {
+        reject(err);
+        return;
+      }
 
-        resolve(this.stream);
-      });
+      resolve(this.stream);
     });
   }
 }
diff --git a/test/functional/connection.ts b/test/functional/connection.ts
@@ -1,8 +1,9 @@
-import { Socket } from "net";
+import * as net from "net";
 import Redis from "../../lib/redis";
 import * as sinon from "sinon";
 import { expect } from "chai";
 import MockServer from "../helpers/mock_server";
+import * as Bluebird from "bluebird";
 
 describe("connection", function() {
   it('should emit "connect" when connected', function(done) {
@@ -77,9 +78,9 @@ describe("connection", function() {
       var set = false;
 
       // TODO: use spy
-      // @ts-ignore
       const stub = sinon
-        .stub(Socket.prototype, "setTimeout")
+        .stub(net.Socket.prototype, "setTimeout")
+        // @ts-ignore
         .callsFake(timeout => {
           if (timeout === connectTimeout) {
             set = true;
@@ -103,9 +104,9 @@ describe("connection", function() {
       let timedoutCalled = false;
 
       // TODO: use spy
-      // @ts-ignore
       sinon
-        .stub(Socket.prototype, "setTimeout")
+        .stub(net.Socket.prototype, "setTimeout")
+        // @ts-ignore
         .callsFake((timeout, callback) => {
           if (timeout === 0) {
             if (!isReady) {
@@ -383,4 +384,18 @@ describe("connection", function() {
       });
     });
   });
+
+  describe("sync connection", () => {
+    it("works with synchronous connection", done => {
+      // @ts-ignore
+      Redis.Promise = Bluebird;
+      const redis = new Redis("/data");
+      redis.on("error", () => {
+        // @ts-ignore
+        Redis.Promise = Promise;
+        redis.disconnect();
+        done();
+      });
+    });
+  });
 });