fix: properly handle instant stream errors (#1299)

redis · Mar 14, 2021 · 0327ef5 · 0327ef5
1 parent 936cb35
commit 0327ef5
Show file tree

Hide file tree

Showing 6 changed files with 60 additions and 5 deletions.
diff --git a/lib/connectors/AbstractConnector.ts b/lib/connectors/AbstractConnector.ts
@@ -5,6 +5,7 @@ export type ErrorEmitter = (type: string, err: Error) => void;
 export default abstract class AbstractConnector {
   protected connecting = false;
   protected stream: NetStream;
+  public firstError?: Error;
 
   public check(info: any): boolean {
     return true;

diff --git a/lib/connectors/SentinelConnector/index.ts b/lib/connectors/SentinelConnector/index.ts
@@ -135,6 +135,11 @@ export default class SentinelConnector extends AbstractConnector {
             } else {
               this.stream = createConnection(resolved);
             }
+
+            this.stream.once("error", (err) => {
+              this.firstError = err;
+            });
+
             this.sentinelIterator.reset(true);
             resolve(this.stream);
           } else {

diff --git a/lib/connectors/StandaloneConnector.ts b/lib/connectors/StandaloneConnector.ts
@@ -76,6 +76,10 @@ export default class StandaloneConnector extends AbstractConnector {
           return;
         }
 
+        this.stream.once("error", (err) => {
+          this.firstError = err;
+        });
+
         resolve(this.stream);
       });
     });

diff --git a/lib/redis/index.ts b/lib/redis/index.ts
@@ -287,7 +287,7 @@ Redis.prototype.setStatus = function (status, arg) {
  */
 Redis.prototype.connect = function (callback) {
   const _Promise = PromiseContainer.get();
-  const promise = new _Promise((resolve, reject) => {
+  const promise = new _Promise<void>((resolve, reject) => {
     if (
       this.status === "connecting" ||
       this.status === "connect" ||
@@ -370,11 +370,21 @@ Redis.prototype.connect = function (callback) {
               stream.setTimeout(0);
             });
           }
+        } else if (stream.destroyed) {
+          const firstError = _this.connector.firstError;
+          if (firstError) {
+            process.nextTick(() => {
+              eventHandler.errorHandler(_this)(firstError);
+            });
+          }
+          process.nextTick(eventHandler.closeHandler(_this));
         } else {
           process.nextTick(eventHandler.connectHandler(_this));
         }
-        stream.once("error", eventHandler.errorHandler(_this));
-        stream.once("close", eventHandler.closeHandler(_this));
+        if (!stream.destroyed) {
+          stream.once("error", eventHandler.errorHandler(_this));
+          stream.once("close", eventHandler.closeHandler(_this));
+        }
 
         if (options.noDelay) {
           stream.setNoDelay(true);

diff --git a/test/functional/connection.ts b/test/functional/connection.ts
@@ -5,6 +5,7 @@ import { expect } from "chai";
 import MockServer from "../helpers/mock_server";
 import * as Bluebird from "bluebird";
 import { StandaloneConnector } from "../../lib/connectors";
+import { CONNECTION_CLOSED_ERROR_MSG } from "../../lib/utils";
 
 describe("connection", function () {
   it('should emit "connect" when connected', function (done) {
@@ -240,6 +241,40 @@ describe("connection", function () {
         done();
       });
     });
+
+    it("should close if socket destroyed before being returned", function (done) {
+      const message = "instant error";
+      sinon.stub(net, "createConnection").callsFake(function () {
+        const socket = (net.createConnection as any).wrappedMethod.apply(
+          net,
+          arguments
+        ) as net.Socket;
+        socket.destroy(new Error(message));
+        return socket;
+      });
+
+      let closed = false;
+      let errored = false;
+
+      const redis = new Redis({ lazyConnect: true });
+      redis
+        .connect(() => {})
+        .catch((err) => {
+          expect(closed).to.equal(true);
+          expect(err.message).to.eql(CONNECTION_CLOSED_ERROR_MSG);
+          redis.disconnect();
+          done();
+        });
+
+      redis.on("error", (err) => {
+        expect(err.message).to.equal(message);
+        errored = true;
+      });
+      redis.on("close", () => {
+        expect(errored).to.equal(true);
+        closed = true;
+      });
+    });
   });
 
   describe("retryStrategy", function () {

diff --git a/test/unit/connectors/connector.ts b/test/unit/connectors/connector.ts
@@ -20,7 +20,7 @@ describe("StandaloneConnector", () => {
     });
 
     it("ignore path when port is set and path is null", async () => {
-      const spy = sinon.stub(net, "createConnection");
+      const spy = sinon.spy(net, "createConnection");
       const connector = new StandaloneConnector({ port: 6379, path: null });
       await connector.connect(() => {});
       expect(spy.calledOnce).to.eql(true);
@@ -29,7 +29,7 @@ describe("StandaloneConnector", () => {
     });
 
     it("supports tls", async () => {
-      const spy = sinon.stub(tls, "connect");
+      const spy = sinon.spy(tls, "connect");
       const connector = new StandaloneConnector({
         port: 6379,
         tls: { ca: "on" },