From 93b873dfaf75baf08e517476bfe54384d144b526 Mon Sep 17 00:00:00 2001
From: Zihua Li <i@zihua.li>
Date: Wed, 30 Mar 2022 22:21:09 +0800
Subject: [PATCH] fix: handle NOPERM error for monitor

Closes #1498
---
 lib/Redis.ts               | 18 +++++++++---------
 lib/redis/event_handler.ts |  6 ++++--
 package-lock.json          | 19 +++++++++++++++++++
 package.json               |  1 +
 test/functional/monitor.ts | 20 ++++++++++++++++++--
 5 files changed, 51 insertions(+), 13 deletions(-)

diff --git a/lib/Redis.ts b/lib/Redis.ts
index 5513392a..7802ddeb 100644
--- a/lib/Redis.ts
+++ b/lib/Redis.ts
@@ -364,14 +364,15 @@ class Redis extends Commander {
    * });
    * ```
    */
-  monitor(callback: Callback<Redis>): Promise<Redis> {
+  monitor(callback?: Callback<Redis>): Promise<Redis> {
     const monitorInstance = this.duplicate({
       monitor: true,
       lazyConnect: false,
     });
 
     return asCallback(
-      new Promise(function (resolve) {
+      new Promise(function (resolve, reject) {
+        monitorInstance.once("error", reject);
         monitorInstance.once("monitoring", function () {
           resolve(monitorInstance);
         });
@@ -440,16 +441,15 @@ class Redis extends Commander {
 
     if (!writable) {
       if (!this.options.enableOfflineQueue) {
-        command.reject(new Error(
-          "Stream isn't writeable and enableOfflineQueue options is false"
-        ));
+        command.reject(
+          new Error(
+            "Stream isn't writeable and enableOfflineQueue options is false"
+          )
+        );
         return command.promise;
       }
 
-      if (
-        command.name === "quit" &&
-        this.offlineQueue.length === 0
-      ) {
+      if (command.name === "quit" && this.offlineQueue.length === 0) {
         this.disconnect();
         command.resolve(Buffer.from("OK"));
         return command.promise;
diff --git a/lib/redis/event_handler.ts b/lib/redis/event_handler.ts
index 8b3b57d0..468a56bb 100644
--- a/lib/redis/event_handler.ts
+++ b/lib/redis/event_handler.ts
@@ -230,7 +230,10 @@ export function readyHandler(self) {
     self.retryAttempts = 0;
 
     if (self.options.monitor) {
-      self.call("monitor");
+      self.call("monitor").then(
+        () => self.setStatus("monitoring"),
+        (error: Error) => self.emit("error", error)
+      );
       const { sendCommand } = self;
       self.sendCommand = function (command) {
         if (Command.checkFlag("VALID_IN_MONITOR_MODE", command.name)) {
@@ -244,7 +247,6 @@ export function readyHandler(self) {
       self.once("close", function () {
         delete self.sendCommand;
       });
-      self.setStatus("monitoring");
       return;
     }
     const finalSelect = self.prevCondition
diff --git a/package-lock.json b/package-lock.json
index 40e00663..b645c491 100644
--- a/package-lock.json
+++ b/package-lock.json
@@ -25,6 +25,7 @@
         "@semantic-release/commit-analyzer": "^9.0.2",
         "@semantic-release/git": "^10.0.1",
         "@types/chai": "^4.3.0",
+        "@types/chai-as-promised": "^7.1.5",
         "@types/debug": "^4.1.5",
         "@types/lodash.defaults": "^4.2.6",
         "@types/lodash.isarguments": "^3.1.6",
@@ -1109,6 +1110,15 @@
       "integrity": "sha512-/ceqdqeRraGolFTcfoXNiqjyQhZzbINDngeoAq9GoHa8PPK1yNzTaxWjA6BFWp5Ua9JpXEMSS4s5i9tS0hOJtw==",
       "dev": true
     },
+    "node_modules/@types/chai-as-promised": {
+      "version": "7.1.5",
+      "resolved": "https://registry.npmjs.org/@types/chai-as-promised/-/chai-as-promised-7.1.5.tgz",
+      "integrity": "sha512-jStwss93SITGBwt/niYrkf2C+/1KTeZCZl1LaeezTlqppAKeoQC7jxyqYuP72sxBGKCIbw7oHgbYssIRzT5FCQ==",
+      "dev": true,
+      "dependencies": {
+        "@types/chai": "*"
+      }
+    },
     "node_modules/@types/debug": {
       "version": "4.1.7",
       "resolved": "https://registry.npmjs.org/@types/debug/-/debug-4.1.7.tgz",
@@ -10201,6 +10211,15 @@
       "integrity": "sha512-/ceqdqeRraGolFTcfoXNiqjyQhZzbINDngeoAq9GoHa8PPK1yNzTaxWjA6BFWp5Ua9JpXEMSS4s5i9tS0hOJtw==",
       "dev": true
     },
+    "@types/chai-as-promised": {
+      "version": "7.1.5",
+      "resolved": "https://registry.npmjs.org/@types/chai-as-promised/-/chai-as-promised-7.1.5.tgz",
+      "integrity": "sha512-jStwss93SITGBwt/niYrkf2C+/1KTeZCZl1LaeezTlqppAKeoQC7jxyqYuP72sxBGKCIbw7oHgbYssIRzT5FCQ==",
+      "dev": true,
+      "requires": {
+        "@types/chai": "*"
+      }
+    },
     "@types/debug": {
       "version": "4.1.7",
       "resolved": "https://registry.npmjs.org/@types/debug/-/debug-4.1.7.tgz",
diff --git a/package.json b/package.json
index 60135933..0b8deab1 100644
--- a/package.json
+++ b/package.json
@@ -57,6 +57,7 @@
     "@semantic-release/commit-analyzer": "^9.0.2",
     "@semantic-release/git": "^10.0.1",
     "@types/chai": "^4.3.0",
+    "@types/chai-as-promised": "^7.1.5",
     "@types/debug": "^4.1.5",
     "@types/lodash.defaults": "^4.2.6",
     "@types/lodash.isarguments": "^3.1.6",
diff --git a/test/functional/monitor.ts b/test/functional/monitor.ts
index 6a2cf211..0cc34e2d 100644
--- a/test/functional/monitor.ts
+++ b/test/functional/monitor.ts
@@ -1,7 +1,9 @@
 import Redis from "../../lib/Redis";
-import { expect } from "chai";
+import { expect, use } from "chai";
 import * as sinon from "sinon";
-import { waitForMonitorReady } from "../helpers/util";
+import { getRedisVersion, waitForMonitorReady } from "../helpers/util";
+
+use(require("chai-as-promised"));
 
 describe("monitor", () => {
   it("should receive commands", (done) => {
@@ -74,4 +76,18 @@ describe("monitor", () => {
       });
     });
   });
+
+  it("rejects when monitor is disabled", async () => {
+    const redis = new Redis();
+    const [major] = await getRedisVersion(redis);
+    if (major < 6) {
+      return;
+    }
+
+    await redis.acl("SETUSER", "nomonitor", "reset", "+info", ">123456", "on");
+
+    await expect(
+      new Redis({ username: "nomonitor", password: "123456" }).monitor()
+    ).to.eventually.be.rejectedWith(/NOPERM/);
+  });
 });