Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Connection instability when using socketTimeout parameter #1937

Merged
merged 1 commit into from
Dec 20, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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;
});
});
Loading