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

Connection Instability with socketTimeout Parameter #1919

Closed
pinkiesky opened this issue Oct 7, 2024 · 3 comments · Fixed by #1937
Closed

Connection Instability with socketTimeout Parameter #1919

pinkiesky opened this issue Oct 7, 2024 · 3 comments · Fixed by #1937
Labels

Comments

@pinkiesky
Copy link

pinkiesky commented Oct 7, 2024

When setting the socketTimeout parameter to a non-zero value, the Redis connection becomes unstable after startup. The connection repeatedly closes due to the socketTimeout mechanism, reconnects, and then falls into a loop of closing and reconnecting.

Code for the bug reproduction

const Redis = require("ioredis");

async function main() {
  const client = new Redis({
    host: "localhost",
    port: 6381,
    socketTimeout: 1000,
    lazyConnect: true,
    password: "123",
    reconnectOnError: () => false,
  });

  client.on("error", (err) => {
    console.count("Redis error: " + err.toString());
  });

  client.on("close", () => {
    console.count("Redis closed");
  });

  client.on("reconnecting", () => {
    console.count("Redis reconnecting");
  });

  client.on("ready", () => {
    console.count("Redis ready");
  });

  await client.connect();
}

main();

Here’s a typical log output from running the code:

~/P/ioredis-test$ node ./index.js
Redis ready: 1
Redis error: Error: Socket timeout. Expecting data, but didn't receive any in 1000ms.: 1
Redis closed: 1
Redis reconnecting: 1
Redis ready: 2
Redis error: Error: Socket timeout. Expecting data, but didn't receive any in 1000ms.: 2
Redis closed: 2
Redis reconnecting: 2
Redis ready: 3
Redis error: Error: Socket timeout. Expecting data, but didn't receive any in 1000ms.: 3
...[the same logs here]...
Redis error: Error: Socket timeout. Expecting data, but didn't receive any in 1000ms.: 9
Redis closed: 9
Redis reconnecting: 9
Redis ready: 10

In this example, the connection eventually stabilized after 10 reconnection attempts, but the number of attempts seems random and can vary between runs. My tests often have 2-3 reconnections, but this may differ for others.

My environment:

Node: v20.17.0 (npm 10.8.2)
System Version: macOS 15.0 (24A335)
Kernel Version: Darwin 24.0.0
ioredis: 5.4.1
Redis server: v7.2.5
@pinkiesky
Copy link
Author

The actual error:

Error: Socket timeout. Expecting data, but didn't receive any in 1000ms.
    at Timeout._onTimeout (/Users/rglr/Projects/ioredis-test/node_modules/ioredis/built/Redis.js:420:33)
    at listOnTimeout (node:internal/timers:581:17)
    at process.processTimers (node:internal/timers:519:7)

bobymicroby added a commit to bobymicroby/ioredis that referenced this issue Dec 20, 2024
…#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
bobymicroby added a commit to bobymicroby/ioredis that referenced this issue Dec 20, 2024
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
github-actions bot pushed a commit that referenced this issue Dec 20, 2024
## [5.4.2](v5.4.1...v5.4.2) (2024-12-20)

### Bug Fixes

* Connection instability when using socketTimeout parameter ([#1937](#1937)) ([ca5e940](ca5e940)), closes [#1919](#1919)
Copy link

🎉 This issue has been resolved in version 5.4.2 🎉

The release is available on:

Your semantic-release bot 📦🚀

@bobymicroby
Copy link
Member

@pinkiesky Thanks for opening this issue and submitting the fix! Your contribution is much appreciated!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants