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

quit vs disconnect - flipped behaviour or wrong documentation #2719

Open
7c opened this issue Mar 14, 2024 · 1 comment
Open

quit vs disconnect - flipped behaviour or wrong documentation #2719

7c opened this issue Mar 14, 2024 · 1 comment
Labels

Comments

@7c
Copy link

7c commented Mar 14, 2024

Description

based on the docs at https://redis.js.org/#node-redis-usage-disconnecting the disconnect should close the connection without flushing and quit close with flushing before closing. Current implementation is not in sync with this approach as seen here

this.#queue.flushAll(new DisconnectsClientError());

for me it looks like reversed, quit seems not try to flush commands but disconnect does.

image

this screenshot demonstrates disconnect behaviour if redis has a ping in the queue, it does try to flush and because ping does not handle .catch it calls 'Unhandled rejection'. We indeed need the .quit() or .disconnect() which should ignore the queue and just close the connection without 'Unhandled rejection' because imagine if you are using third-party-libraries you neven know how people have managed the redis commands..

is this wrong documentation or flipped implementation ?

Node.js Version

20.11.1

Redis Server Version

Node Redis Version

4.6.13

Platform

macOS

Logs

No response

@7c 7c added the Bug label Mar 14, 2024
@7c
Copy link
Author

7c commented Mar 14, 2024

Correction: .quit also seems to wait with Promise.all to quit from the redisconnection (

QUIT(): Promise<string> {
). So at the end i see no method which should break a connection gracefully.I also could not find a method to clear the redis queue to call before disconect.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

1 participant