hapijs / catbox

Multi-strategy object caching service
Other
494 stars 72 forks source link

Close the redis connection on error #49

Closed PaulMougel closed 10 years ago

PaulMougel commented 10 years ago

Before losing the reference to this.client, forcibly close the connection to the Redis server.

If the Redis server couldn’t be reached in the first place (ECONNREFUSED on connection), this also prevents node-redis from auto-reconnecting indefinitely.


I couldn't come up with a simple test case, but here is a meaningful use case:

When the Redis server isn't available, client.stop() doesn't work as expected, as something keeps the event loop busy:

var Catbox = require('./');

var client = new Catbox.Client({
    engine: 'redis',
    partition: 'examples'
});

client.start(function (err) {
    console.log(err);
    client.stop();
});

outputs:

~/tmp ❯❯❯ node test.js
[Error: Redis connection to 127.0.0.1:6379 failed - connect ECONNREFUSED]

(the program hangs there forever and needs to be stopped using ^C)

Whereas if the Redis server is available:

~/tmp ❯❯❯ node test.js
undefined
~/tmp ❯❯❯ 
LoicMahieu commented 10 years ago

+1

lock[bot] commented 4 years ago

This thread has been automatically locked due to inactivity. Please open a new issue for related bugs or questions following the new issue template instructions.