node-redis icon indicating copy to clipboard operation
node-redis copied to clipboard

The client.isOpen is true even when the connection has failed during client.connect(), if redis server is not online.

Open 24x7soumya opened this issue 1 year ago • 3 comments

Environment:

  • Node.js Version: v16.11.1
  • Redis Server Version: Any
  • Node Redis Version: 4.3.1
  • Platform: Ubuntu 20.04.3, Windows 10

24x7soumya avatar Sep 09 '22 07:09 24x7soumya

client.isOpen = is the client open (which doesn't necessarily mean the socket is open/ready). client.isReady = is the current socket open and ready for use.

leibale avatar Sep 11 '22 13:09 leibale

Thanks @leibale. But if the connect method is called based on isReady the following response is received: Socket already opened at RedisSocket.connect (C:\service\node_modules\@redis\client\dist\lib\client\socket.js:63:19) at Commander.connect (C:\service\node_modules\@redis\client\dist\lib\client\index.js:166:70) at Backup.persist (C:\service\src\services\backup.service.js:77:23) at listOnTimeout (node:internal/timers:557:17) at processTimers (node:internal/timers:500:7)

Also, what is the utility of isOpen, then?

24x7soumya avatar Sep 11 '22 15:09 24x7soumya

isOpen is for the client to know if the socket needs to reopen

Can you please elaborate more on what you wanna do?

leibale avatar Sep 14 '22 16:09 leibale

@24x7soumya can I close this issue?

leibale avatar Nov 02 '22 22:11 leibale

Yes @leibale . Thanks.

24x7soumya avatar Nov 03 '22 03:11 24x7soumya

I am facing the same issue. isOpen is true even after the connection is broken, so isReady becomes false.

the problem is, I can't call client.connect() or client.disconnect() when isOpen = true and isReady = false

Dhruv-Garg79 avatar Dec 13 '22 04:12 Dhruv-Garg79

@leibale I have seen a similar problem here. It's my understanding that isOpen=true, isReady=false should indicate that node-redis is actively trying to connect / reconnect, but I have had some situation around socket closed unexpectedly where I am left in this state without reconnecting ever being attempted.

edkimmel avatar Mar 16 '23 15:03 edkimmel

The script doesn't work in existing form. Link: https://github.com/redis/node-redis/blob/da3face95180d5ab801716de1ff4ba97e6cbb6ab/examples/check-connection-status.js

I had to modify to this

// Check the connection status of the Redis client instance.
const redis = require('redis');

const client = redis.createClient();

console.log('Before client.connect()...');

// isOpen will return False here as the client's socket is not open yet.
// isReady will return False here, client is not yet ready to use.
console.log(`client.isOpen: ${client.isOpen}, client.isReady: ${client.isReady}`);

// Begin connection process...
// const connectPromise = client.connect();
const connectPromise = (async () => { await client.connect() })();

console.log('After client.connect()...');

// isOpen will return True here as the client's socket is open now.
// isReady will return False here as the promise hasn't resolved yet.
console.log(`client.isOpen: ${client.isOpen}, client.isReady: ${client.isReady}`);

// await connectPromise;
console.log('Afer connectPromise has resolved...');

// isOpen will return True here as the client's socket is open now.
// isReady will return True here, client is ready to use.
console.log(`client.isOpen: ${client.isOpen}, client.isReady: ${client.isReady}`);

(async () => { await client.quit() })();

However, I am getting isReady always set to false

Before client.connect()...
client.isOpen: false, client.isReady: false
After client.connect()...
client.isOpen: true, client.isReady: false
Afer connectPromise has resolved...
client.isOpen: true, client.isReady: false

Although, I am able to set a key, value pair in redis database.

Any thoughts on how to resolve this?

EngineerReversed avatar Mar 22 '23 13:03 EngineerReversed

@EngineerReversed the check-connection-status.js assume top-level-await works in your environment (node >= 14.8.0), I guess this is why it's not working for you. To make it work you can wrap the whole script (excluding imports) with an "async IIFE".

The reason your modified version prints

After connectPromise has resolved...
client.isOpen: true, client.isReady: false

is that you commented out this line:

await connectPromise;

which makes the last console.log run before the connectPromise resolves..

leibale avatar Mar 22 '23 14:03 leibale

Thanks for the advice, I did update my commented out lines with async IIFE. However, I am still facing same problem.

// Check the connection status of the Redis client instance.
const redis = require('redis');

const client = redis.createClient({
    host: "127.0.0.1",
    port: 6379
});

(async () => {  client.on('error', (err) => console.log('Redis Client Error', err)); })();

(async () => { await client.connect() })();

// isOpen will return True here as the client's socket is open now.
// isReady will return True here, client is ready to use.
console.log(`client.isOpen: ${client.isOpen}, client.isReady: ${client.isReady}`);

(async () => {  client.on('connect', () => console.log('Connected to Redis DB')); })();
(async () => {  client.on('ready', () => console.log('Ready to use Redis DB')); })();
(async () => {  client.on('end', () => console.log('Client disconnected from Redis DB')); })();

process.on('SIGINT', async () => {
    await client.disconnect();
    process.exit(0)
});

module.exports = client

client.isOpen: true, client.isReady: false

EngineerReversed avatar Mar 23 '23 09:03 EngineerReversed

@EngineerReversed this should work:

// Check the connection status of the Redis client instance.
const redis = require('redis');

const client = redis.createClient({
    host: "127.0.0.1",
    port: 6379
});

client.on('error', (err) => console.log('Redis Client Error', err));
client.on('connect', () => console.log('Connected to Redis DB'));
client.on('ready', () => console.log('Ready to use Redis DB'));
client.on('end', () => console.log('Client disconnected from Redis DB'));

(async () => {
  await client.connect();

  // isOpen will return True here as the client's socket is open now.
  // isReady will return True here, client is ready to use.
  console.log(`client.isOpen: ${client.isOpen}, client.isReady: ${client.isReady}`);
})();

process.on('SIGINT', async () => {
    await client.disconnect();
    process.exit(0)
});

module.exports = client

leibale avatar Mar 23 '23 14:03 leibale

Yes, that worked for me.

However, does that mean I can't use client outside of async based method? It would defeat the purpose since I want to use client across my module. Or Should I continue since it does not matter as I am still able to GET and SET my key and values?

EngineerReversed avatar Mar 23 '23 16:03 EngineerReversed

@EngineerReversed It depends on the use case... in case of an HTTP server that cannot work without Redis (for example - all the sessions are stored in Redis) - don't call server.listen before client.connect resolves:

// redis.js
const { createClient } = require('redis');

const client = module.exports.client = createClient();
client.on('error', err => console.error('Redis Error Client');

// server.js
const { createServer } = require('node:http');
const { client } = require)('./redis.js');

const server = module.exports.server = createServer();

(async () => {
  await client.connect();
  // expose the server once the client is ready
  server.listen(process.env.PORT);
})();

leibale avatar Mar 23 '23 18:03 leibale