Skip to content

Commit ca5e940

Browse files
authored
fix: Connection instability when using socketTimeout parameter (#1937)
The issue occurs when using socketTimeout, causing connections to become unstable with repeated disconnections and reconnections. This happens due to incorrect ordering of socket stream event handling. Changes: - Use prependListener() instead of on() for `DataHandler` stream data events - Explicitly call resume() after attaching the `DataHandler` stream listener - Add tests to verify socket timeout behavior This ensures the parser receives and processes data before timeout checks, preventing premature timeouts and connection instability. Fixes #1919
1 parent af83275 commit ca5e940

File tree

3 files changed

+83
-1
lines changed

3 files changed

+83
-1
lines changed

lib/DataHandler.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,9 +56,12 @@ export default class DataHandler {
5656
},
5757
});
5858

59-
redis.stream.on("data", (data) => {
59+
// prependListener ensures the parser receives and processes data before socket timeout checks are performed
60+
redis.stream.prependListener("data", (data) => {
6061
parser.execute(data);
6162
});
63+
// prependListener() doesn't enable flowing mode automatically - we need to resume the stream manually
64+
redis.stream.resume();
6265
}
6366

6467
private returnFatalError(err: Error) {

test/functional/socketTimeout.ts

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
import { expect } from 'chai';
2+
import { Done } from 'mocha';
3+
import Redis from '../../lib/Redis';
4+
5+
describe('Redis Connection Socket Timeout', () => {
6+
const SOCKET_TIMEOUT_MS = 500;
7+
8+
it('maintains stable connection with password authentication | https://github.com/redis/ioredis/issues/1919 ', (done) => {
9+
const redis = createRedis({ password: 'password' });
10+
assertNoTimeoutAfterConnection(redis, done);
11+
});
12+
13+
it('maintains stable connection without initial authentication | https://github.com/redis/ioredis/issues/1919', (done) => {
14+
const redis = createRedis();
15+
assertNoTimeoutAfterConnection(redis, done);
16+
});
17+
18+
it('should throw when socket timeout threshold is exceeded', (done) => {
19+
const redis = createRedis()
20+
21+
redis.on('error', (err) => {
22+
expect(err.message).to.eql(`Socket timeout. Expecting data, but didn't receive any in ${SOCKET_TIMEOUT_MS}ms.`);
23+
done();
24+
});
25+
26+
redis.connect(() => {
27+
redis.stream.removeAllListeners('data');
28+
redis.ping();
29+
});
30+
});
31+
32+
function createRedis(options = {}) {
33+
return new Redis({
34+
socketTimeout: SOCKET_TIMEOUT_MS,
35+
lazyConnect: true,
36+
...options
37+
});
38+
}
39+
40+
function assertNoTimeoutAfterConnection(redisInstance: Redis, done: Done) {
41+
let timeoutObj: NodeJS.Timeout;
42+
43+
redisInstance.on('error', (err) => {
44+
clearTimeout(timeoutObj);
45+
done(err.toString());
46+
});
47+
48+
redisInstance.connect(() => {
49+
timeoutObj = setTimeout(() => {
50+
done();
51+
}, SOCKET_TIMEOUT_MS * 2);
52+
});
53+
}
54+
});

test/unit/DataHandler.ts

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
import { expect } from 'chai';
2+
import * as sinon from 'sinon';
3+
import DataHandler from '../../lib/DataHandler';
4+
5+
describe('DataHandler', () => {
6+
it('attaches data handler to stream in correct order | https://github.com/redis/ioredis/issues/1919', () => {
7+
8+
const prependListener = sinon.spy((event: string, handler: Function) => {
9+
expect(event).to.equal('data');
10+
});
11+
12+
const resume = sinon.spy();
13+
14+
new DataHandler({
15+
stream: {
16+
prependListener,
17+
resume
18+
}
19+
} as any, {} as any);
20+
21+
expect(prependListener.calledOnce).to.be.true;
22+
expect(resume.calledOnce).to.be.true;
23+
expect(resume.calledAfter(prependListener)).to.be.true;
24+
});
25+
});

0 commit comments

Comments
 (0)