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

createClient not work after quit #63

Open
dontbesatisfied opened this issue Aug 12, 2021 · 2 comments
Open

createClient not work after quit #63

dontbesatisfied opened this issue Aug 12, 2021 · 2 comments

Comments

@dontbesatisfied
Copy link

dontbesatisfied commented Aug 12, 2021

Node: v12.18.2
redis: 5.0.5 (docker image)
async-redis: 2.0.0

Error occur as " AbortError: BLPOP can't be processed. The connection is already closed. "

This is ample code where an error occurs.

const redis = require('async-redis')

let client = redis.createClient("redis://localhost:6379");

let cnt = 0;

async function reconnect() {
    try {
        await client.quit()
        client = null
        client = redis.createClient("redis://localhost:6379");
        console.log('reconnect');
    } catch (error) {
        console.log(error);
    }
}


(async () => {
    while (true) {
        try {
            
            cnt++

            const v = await client.blpop('*', 1)

            if (cnt%5 === 0) {
                throw Error("Test")
            }
        } catch (error) {
            console.error(error);
            await reconnect()
            
        }
    }
})()

But below code work well.
There seems to be a problem with clients with the same option as they connected before.

is it bug?

const redis = require('async-redis')

let client = redis.createClient("redis://localhost:6379");

let cnt = 0;

async function reconnect() {
    try {
        await client.quit()
        client = null
        client = redis.createClient("redis://localhost:6379", {
            customOption: Date.now(), // Here
        });
        console.log('reconnect');
    } catch (error) {
        console.log(error);
    }
}


(async () => {
    while (true) {
        try {
            
            cnt++

            const v = await client.blpop('*', 1)

            if (cnt%5 === 0) {
                throw Error("Test")
            }
        } catch (error) {
            console.error(error);
            await reconnect()
            
        }
    }
})()
@micalevisk
Copy link

micalevisk commented Sep 6, 2021

This code also reproduces the bug
const redis = require('async-redis');
let client1 = redis.createClient("redis://localhost:6379"),
    client2 = redis.createClient("redis://localhost:6379");

console.log(
  client1.__redisClient === client2.__redisClient
) // true when it it supposed to be false

;(async () => {
  await client1.quit()
  await client2.info()
})();

Looks like this happen due to the following lines

async-redis/src/index.js

Lines 13 to 15 in fef4248

const serializedArgs = JSON.stringify(args);
if (!redisClients.has(serializedArgs)) {
redisClients.set(serializedArgs, Array.isArray(args) ? redis.createClient(...args) : redis.createClient(args));

using JSON.stringify(args) is wrong since multiple clients could have the same configuration, and thus redisClients.get(serializedArgs) will always return the first created client (due to L14).

Maybe using a Symbol or some uuid (crypto.randomUUID()) is more appropriate.

@micalevisk
Copy link

I just notice that this issue is the same as of #61

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

2 participants