deprecate QUIT and disconnect, implement close and destroy

Author: leibale

docs/v4-to-v5.md

@@ -41,9 +41,7 @@ To override just a specific option, use the following functions:
 
 The `QUIT` command has been deprecated in Redis 7.2 and should now also be considered deprecated in Node-Redis. Instead of sending a `QUIT` command to the server, the client can simply close the network connection.
 
-Rather than using `client.quit()`, your code should use `client.close()` or `client.disconnect()`.
-
-TODO difference between `close` and `disconnect`...
+`client.QUIT/quit()` is replaced by `client.close()`. and, to avoid confusion, `client.disconnect()` has been renamed to `client.destroy()`.
 
 ## Scan Iterators
 
@@ -62,7 +60,7 @@ const client = createClient(),
 // use `client` for the new API
 await client.set('key', 'value');
 
-// use `legacyClient` for the "legacy" callback API
+// use `legacyClient` for the "legacy" API
 legacyClient.set('key', 'value', (err, reply) => {
   // ...
 });

packages/client/lib/client/commands-queue.ts

@@ -316,4 +316,11 @@ export default class RedisCommandsQueue {
       );
     }
   }
+
+  isEmpty() {
+    return (
+      this._waitingToBeSent.length === 0 &&
+      this._waitingForReply.length === 0
+    );
+  }
 }

packages/client/lib/client/index.ts

@@ -657,29 +657,19 @@ export default class RedisClient<
     );
   }
 
-  QUIT(): Promise<string> {
-    return this._socket.quit(async () => {
-      const quitPromise = this._queue.addCommand<string>(['QUIT']);
-      this._tick();
-      return quitPromise;
-    });
-  }
-
-  quit = this.QUIT;
-
   private _tick(force = false): void {
     if (this._socket.writableNeedDrain || (!force && !this._socket.isReady)) {
       return;
     }
 
     this._socket.cork();
 
-    while (!this._socket.writableNeedDrain) {
+    do {
       const args = this._queue.getCommandToSend();
       if (args === undefined) break;
 
       this._socket.writeCommand(args);
-    }
+    } while (!this._socket.writableNeedDrain);
   }
 
   private _addMultiCommands(
@@ -782,9 +772,57 @@ export default class RedisClient<
     } while (cursor !== 0);
   }
 
+  /**
+   * @deprecated use .close instead
+   */
+  QUIT(): Promise<string> {
+    return this._socket.quit(async () => {
+      const quitPromise = this._queue.addCommand<string>(['QUIT']);
+      this._tick();
+      return quitPromise;
+    });
+  }
+
+  quit = this.QUIT;
+
+  /**
+   * @deprecated use .destroy instead
+   */
   disconnect() {
+    return Promise.resolve(this.destroy());
+  }
+
+  private _resolveClose?: () => unknown;
+
+  /**
+   * Close the client. Wait for pending replies.
+   */
+  close() {
+    return new Promise<void>(resolve => {
+      this._socket.close();
+
+      if (this._queue.isEmpty()) {
+        this._socket.destroySocket();
+        return resolve();
+      }
+
+      const maybeClose = () => {
+        if (!this._queue.isEmpty()) return;
+        
+        this._socket.off('data', maybeClose);
+        this._socket.destroySocket();
+        resolve();
+      };
+      this._socket.on('data', maybeClose);
+    });
+  }
+
+  /**
+   * Destroy the client. Rejects all commands immediately.
+   */
+  destroy() {
     this._queue.flushAll(new DisconnectsClientError());
-    this._socket.disconnect();
+    this._socket.destroy();
   }
 
   ref() {

packages/client/lib/client/socket.ts

@@ -19,7 +19,7 @@ export interface RedisSocketCommonOptions {
    */
   keepAlive?: number | false;
   /**
-   * When the socket closes unexpectedly (without calling `.quit()`/`.disconnect()`), the client uses `reconnectStrategy` to decide what to do. The following values are supported:
+   * When the socket closes unexpectedly (without calling `.close()`/`.destroy()`), the client uses `reconnectStrategy` to decide what to do. The following values are supported:
    * 1. `false` -> do not reconnect, close the client and flush the command queue.
    * 2. `number` -> wait for `X` milliseconds before reconnecting.
    * 3. `(retries: number, cause: Error) => false | number | Error` -> `number` is the same as configuring a `number` directly, `Error` is the same as `false`, but with a custom error.
@@ -250,35 +250,43 @@ export default class RedisSocket extends EventEmitter {
     }
   }
 
-  disconnect(): void {
+  async quit<T>(fn: () => Promise<T>): Promise<T> {
     if (!this.#isOpen) {
       throw new ClientClosedError();
     }
 
     this.#isOpen = false;
-    this.#disconnect();
+    const reply = await fn();
+    this.destroySocket();
+    return reply;
   }
 
-  #disconnect(): void {
-    this.#isReady = false;
-
-    if (this.#socket) {
-      this.#socket.destroy();
-      this.#socket = undefined;
+  close() {
+    if (!this.#isOpen) {
+      throw new ClientClosedError();
     }
 
-    this.emit('end');
+    this.#isOpen = false;
   }
 
-  async quit<T>(fn: () => Promise<T>): Promise<T> {
+  destroy() {
     if (!this.#isOpen) {
       throw new ClientClosedError();
     }
 
     this.#isOpen = false;
-    const reply = await fn();
-    this.#disconnect();
-    return reply;
+    this.destroySocket();
+  }
+
+  destroySocket() {
+    this.#isReady = false;
+
+    if (this.#socket) {
+      this.#socket.destroy();
+      this.#socket = undefined;
+    }
+
+    this.emit('end');
   }
 
   #isCorked = false;