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

Unhandled error event: Error: connect ECONNREFUSED on reconnect #1192

Closed
stieg opened this issue Sep 17, 2020 · 2 comments
Closed

Unhandled error event: Error: connect ECONNREFUSED on reconnect #1192

stieg opened this issue Sep 17, 2020 · 2 comments

Comments

@stieg
Copy link

stieg commented Sep 17, 2020

Note this issue is similar to #1077, #1157, & #1102 . I'm opening a new ticket since I want to focus on the primary issue here which is the Unhandled error event" portion. Take the following code:

const Redis = require("ioredis");

async function sleep(ms) {
  return new Promise((resolve) => setTimeout(resolve, ms));
}

async function main() {
  let redis;
  try {
    redis = new Redis('redis://localhost:6379/');
    while(true) {
      const info = await redis.info('Server');
      console.log(`Server Info at ${new Date()}:\n${info}`);
      await sleep(3000);
    }
  } catch (err) {
    console.error('Caught Error: ', err);
  } finally {
    await redis.disconnect();
  }
}

main();

And follow the steps below:

  1. Ensure Redis is running locally
  2. Start the demo app
  3. Wait for demo app to connect and start displaying server details
  4. Stop the redis server,
  5. Observe the errors
[ioredis] Unhandled error event: Error: connect ECONNREFUSED 127.0.0.1:6379
    at TCPConnectWrap.afterConnect [as oncomplete] (net.js:1141:16)

Now an error being thrown here is reasonable as the Redis server is not available. The problem is that these are unhandled errors. This is very not good as it implies that these errors are being thrown before an "error" event handler can be registered on the socket. In our code base we listen for unhandled errors and begin a shutdown if they are encountered. Something like this:

process.on('uncaughtException', exitHandler(1, 'Unexpected Error'))
process.on('unhandledRejection', exitHandler(1, 'Unhandled Promise'))

So it appears that an error event is being thrown before ioredis attaches its error handler to catch it. From reading the code I believe this error is happening before eventHandler.errorHandler is attached to the socket during the reconnect attempt in built/redis/index.js in the connect method.

Let me know if clarification is needed:

node: 12.18.3
ioredis: 4.17.3

@stieg
Copy link
Author

stieg commented Sep 17, 2020

So my initial theory was quite wrong. The error handler is being attached to the socket as expected. Where things fall apart is in the silentEmit method prototype. In that method several checks happen, mainly to see if there are listeners. If there are then the event goes to the listener. But if there isn't then the error event is logged and then false is returned. The false being returned indicates to the previous emitter that there were no emitters that handled it. And in the case of an error event (which is a special event type of course) that means the error event will become an uncaughtException as per node event rules. This explains why I am seeing this behavior.

This behavior contradicts your description of how the error event needs to be handled in your README file. Namely this statement is incorrect:

However, ioredis emits all error events silently (only emits when there's at least one listener) so that your application won't crash if you're not listening to the error event.

So... what is the behavior supposed to be? Is the code wrong or are the documents wrong?

@stieg
Copy link
Author

stieg commented Sep 17, 2020

I'm gonna close this for now since I may just be all twisted around. Will reopen when I am more confident in the issue. Sorry for the noise.

@stieg stieg closed this as completed Sep 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant