quit vs disconnect - flipped behaviour or wrong documentation

[7c]

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

node-redis/packages/client/lib/client/index.ts

Line 847 in dbf8f59

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

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

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]

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

node-redis/packages/client/lib/client/index.ts

Line 722 in dbf8f59

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.

[StiliyanKushev]

Ping, still an issue, which to this day makes our e2e tests flaky.

[nkaradzhov]

Hi, the library has changed significantly recently. Could you try to upgrade?

We have a migration guide, so, as a rule of thumb:

client.quit() → client.close() - For graceful shutdown that waits for pending commands
client.disconnect() → client.destroy() - For immediate shutdown that rejects pending commands