redis-fast-driver icon indicating copy to clipboard operation
redis-fast-driver copied to clipboard

Errors not being detected properly by `hiredis` on Linux

Open victorigualada opened this issue 3 years ago • 5 comments

This issue is related to #22 and aiming to provide more information about the real problem.

As the author states in his PR, there is a reconnect issue that appears, but just in Linux (works in Mac, not tested on Windows).

Scenario

Create test app:

$ mkdir redistest && cd redistest
$ npm i redis-fast-driver

test.js:

const RedisClient = require('redis-fast-driver');

const cli = new RedisClient({
        host: '127.0.0.1',
        port: 6379,
        maxRetries: 25,
        autoConnect: true,
        doNotRunQuitOnEnd: true,
});

cli.on('ready', () => console.log('ready'));
cli.on('reconnecting', retry => console.log('reconnecting...', retry));
cli.on('error', e => console.log(e));

run app

$ node test.js

This should trigger the reconnect behaviour and try to reconnect to redis 25 times. But it doesn't. Why is it though?

Debugging

Setting the ENABLELOG property to 1 in the binding-gyp file: https://github.com/h0x91b/redis-fast-driver/blob/2429fa3419e4fbe362d9fdfc91c82eeae9cf0e1d/binding.gyp#L15 and adding some logging showed that the error (status=1 Connection Refused) that should be captured in the ConnectCallback is not being captured https://github.com/h0x91b/redis-fast-driver/blob/2429fa3419e4fbe362d9fdfc91c82eeae9cf0e1d/src/redis-fast-driver.cc#L61-L79

How can we prove that? Testing hiredis:

// Compilation
$ git clone https://github.com/redis/hiredis.git
$ cd hiredis
$ make
$ sudo make install
$ make test

// Test
$ cc examples/example-libuv.c -o hiredis-test -lhiredis -luv
$ ./hiredis-test

Add some extra logging if needed.

This execution proved that the same error (Connection Refused on loopback interface) IS being captured by the connectCallback. So it shouldn't be a problem of hiredis either.

Another important thing is that other errors are being correctly captured and thrown, and the reconnect behavior works as expected, for example, using an invalid host such as:

const cli = new RedisClient({
        host: 'invalid-host',
        port: 6379,
        maxRetries: 25,
        autoConnect: true,
        doNotRunQuitOnEnd: true,
});

This will throw a status=2 Temporarily failure in name resolution. Although not in the connect callback but immediately after the redisAsyncConnect call: https://github.com/h0x91b/redis-fast-driver/blob/2429fa3419e4fbe362d9fdfc91c82eeae9cf0e1d/src/redis-fast-driver.cc#L134-L149

Observations

  • hiredis works as expected and captures the Connection Refused error in the connectionCallback
  • redis-fast-driver does not capture the Connection Refused error in the ConnectCallback
  • Errors that shouldn't be captured by the ConnectCallback are properly thrown and the reconnect behavior works properly
  • It works on MacOS but not in Linux ~~or Docker~~

We are out of ideas on which could be the root problem of this, and why is OS dependant. The last thing that seems to be the problem is the compilation made by node-gyp of hiredis. I'm not familiar with node-gyp so I couldn't go further into it but I saw this hiredis.gyp file with some conditions for MacOS and Solaris: https://github.com/h0x91b/redis-fast-driver/blob/2429fa3419e4fbe362d9fdfc91c82eeae9cf0e1d/deps/hiredis.gyp#L16-L24

Hope you can help with this problem! Thanks!

victorigualada avatar Jul 14 '21 09:07 victorigualada