Skip to content

Commit

Permalink
fix: Connection instability when using socketTimeout parameter (redis…
Browse files Browse the repository at this point in the history
…#1919)

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 redis#1919
  • Loading branch information
bobymicroby committed Dec 20, 2024
1 parent af83275 commit 1e6e080
Show file tree
Hide file tree
Showing 3 changed files with 83 additions and 1 deletion.
5 changes: 4 additions & 1 deletion lib/DataHandler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,9 +56,12 @@ export default class DataHandler {
},
});

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

private returnFatalError(err: Error) {
Expand Down
54 changes: 54 additions & 0 deletions test/functional/socketTimeout.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
import { expect } from 'chai';
import { Done } from 'mocha';
import Redis from '../../lib/Redis';

describe('Redis Connection Socket Timeout', () => {
const SOCKET_TIMEOUT_MS = 500;

it('maintains stable connection with password authentication | https://github.com/redis/ioredis/issues/1919 ', (done) => {
const redis = createRedis({ password: 'password' });
assertNoTimeoutAfterConnection(redis, done);
});

it('maintains stable connection without initial authentication | https://github.com/redis/ioredis/issues/1919', (done) => {
const redis = createRedis();
assertNoTimeoutAfterConnection(redis, done);
});

it('should throw when socket timeout threshold is exceeded', (done) => {
const redis = createRedis()

redis.on('error', (err) => {
expect(err.message).to.eql(`Socket timeout. Expecting data, but didn't receive any in ${SOCKET_TIMEOUT_MS}ms.`);
done();
});

redis.connect(() => {
redis.stream.removeAllListeners('data');
redis.ping();
});
});

function createRedis(options = {}) {
return new Redis({
socketTimeout: SOCKET_TIMEOUT_MS,
lazyConnect: true,
...options
});
}

function assertNoTimeoutAfterConnection(redisInstance: Redis, done: Done) {
let timeoutObj: NodeJS.Timeout;

redisInstance.on('error', (err) => {
clearTimeout(timeoutObj);
done(err.toString());
});

redisInstance.connect(() => {
timeoutObj = setTimeout(() => {
done();
}, SOCKET_TIMEOUT_MS * 2);
});
}
});
25 changes: 25 additions & 0 deletions test/unit/DataHandler.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
import { expect } from 'chai';
import * as sinon from 'sinon';
import DataHandler from '../../lib/DataHandler';

describe('DataHandler', () => {
it('attaches data handler to stream in correct order | https://github.com/redis/ioredis/issues/1919', () => {

const prependListener = sinon.spy((event: string, handler: Function) => {
expect(event).to.equal('data');
});

const resume = sinon.spy();

new DataHandler({
stream: {
prependListener,
resume
}
} as any, {} as any);

expect(prependListener.calledOnce).to.be.true;
expect(resume.calledOnce).to.be.true;
expect(resume.calledAfter(prependListener)).to.be.true;
});
});

0 comments on commit 1e6e080

Please sign in to comment.