Clarify cluster connection behaviour and error handling

[nicokaiser]

It seems to me that starting from version 4 error handling in Redis.Cluster is either incomplete or unclear:

1. Errors on auto-connect are ignored:

const startupNodes = [
    { host: 'existingnode.domain', port: 6379 },
    { host: 'nonexistingnode.domain', port: 6379 }
];

const redis = new Redis.Cluster(startupNodes, { lazyConnect: false });

redis.on('error', err => {
    // this is never called
});

redis.publish(channel, data)
    .then(() => {
        // this is never called, as there are no timeouts
    })
    .catch(err => {
        // this is also never called
    });

This can be mitigated by explicitly calling connect():

const redis = new Redis.Cluster(startupNodes, { lazyConnect: true });

redis.connect().catch(err => {
    // this is called
});

redis.on('error', err => {
    // this is still never called
});

2. DNS Errors on reconnect are ignored:

I guess (!) these kind of errors can only occur during initial connect, not during reconnect. Otherwise this issue would be even much more problematic, as errors during reconnect are also ignored: https://github.com/luin/ioredis/blob/v4.2.3/lib/cluster/index.ts#L214

It this behaviour intentional? If so, this should be noted somewhere in the docs, as it is different from how ioredis 3.x behaved. In 3.x, any connection errors are handled by the reconnection procedure and thus connection is re-tried until forever (or at least, until the clusterRetryStrategy returns no number anymore).

I'm not sure if this is related to #709, but it feels like something else, as DNS errors are the only errors that can have Cluster#resolveStartupNodeHostNames() (and thus, Cluster#connect()) reject.

[nicokaiser]

Update: this can also happen on reconnect, e.g. if the DNS for one cluster node is not available anymore. I consider this as critical, as error handling in these cases is not possible and ioredis stops reconnecting without the application noticing.

ioredis should either emit an "error" event in these cases, or – like in version 3 – retry connecting following the clusterRetryStrategy.

[ioredis-robot]

🎉 This issue has been resolved in version 4.3.1 🎉

The release is available on:

• npm package (@next dist-tag)
• GitHub release

Your semantic-release bot 📦🚀

[knoxcard]

This is still a problem, why is this soooo difficult?

[abhimanyuPathania]

Following is still an issue in 4.22.0

redis.on('error', err => {
    // this is never called
});