Skip to content

Commit

Permalink
Handle connection after connect event was emitted
Browse files Browse the repository at this point in the history
It's possible for connect promise to resolve after the resolved
connection has already been established, which means we miss the connect
event. This can happen when Bluebird is set as the global Promise, and
we connect to an ip address host for example. This fix checks to see if
that's the case and invokes the connection handler directly. Thanks
@luin for the recommendation.

Fixes redis#977
  • Loading branch information
alavers committed Apr 5, 2020
1 parent e22cae6 commit ec76383
Show file tree
Hide file tree
Showing 2 changed files with 14 additions and 1 deletion.
6 changes: 5 additions & 1 deletion lib/redis/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -304,7 +304,11 @@ Redis.prototype.connect = function (callback) {
stream.setKeepAlive(true, options.keepAlive);
}

stream.once(CONNECT_EVENT, eventHandler.connectHandler(_this));
if (stream.connecting) {
stream.once(CONNECT_EVENT, eventHandler.connectHandler(_this));
} else {
process.nextTick(eventHandler.connectHandler(_this));
}
stream.once("error", eventHandler.errorHandler(_this));
stream.once("close", eventHandler.closeHandler(_this));

Expand Down
9 changes: 9 additions & 0 deletions test/functional/connection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import * as sinon from "sinon";
import { expect } from "chai";
import MockServer from "../helpers/mock_server";
import * as Bluebird from "bluebird";
import { StandaloneConnector } from "../../lib/connectors";

describe("connection", function () {
it('should emit "connect" when connected', function (done) {
Expand Down Expand Up @@ -423,5 +424,13 @@ describe("connection", function () {
done();
});
});

it("works when connection established before promise is resolved", (done) => {
const socket = new net.Socket();
sinon.stub(StandaloneConnector.prototype, "connect").resolves(socket);
socket.connect(6379, "127.0.0.1").on("connect", () => {
new Redis().on("connect", () => done());
});
});
});
});

0 comments on commit ec76383

Please sign in to comment.