From ae273855833176683d75363c478cd06fe60f2c33 Mon Sep 17 00:00:00 2001 From: Alexander Zinin Date: Tue, 5 Nov 2024 15:42:11 +0400 Subject: [PATCH] fix: Connection Instability with socketTimeout Parameter (#9) * fix socketTimeout connection instability Signed-off-by: Aleksandr Zinin * add Datahandler constructor test Signed-off-by: Aleksandr Zinin * add socketTimeout functional tests Signed-off-by: Aleksandr Zinin --------- Signed-off-by: Aleksandr Zinin --- lib/DataHandler.ts | 6 ++- test/functional/socketTimeout.ts | 64 ++++++++++++++++++++++++++++++++ test/unit/DataHandler.ts | 30 +++++++++++++++ 3 files changed, 99 insertions(+), 1 deletion(-) create mode 100644 test/functional/socketTimeout.ts create mode 100644 test/unit/DataHandler.ts diff --git a/lib/DataHandler.ts b/lib/DataHandler.ts index 717de6de..23921590 100644 --- a/lib/DataHandler.ts +++ b/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) { diff --git a/test/functional/socketTimeout.ts b/test/functional/socketTimeout.ts new file mode 100644 index 00000000..dbbf4b52 --- /dev/null +++ b/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(); + }); + }); +}); diff --git a/test/unit/DataHandler.ts b/test/unit/DataHandler.ts new file mode 100644 index 00000000..850366a1 --- /dev/null +++ b/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); + }); + }); +});