From 01b77a3b05078114fcb66839b2e3f9039c2c1a00 Mon Sep 17 00:00:00 2001
From: luin <i@zihua.li>
Date: Mon, 15 Oct 2018 02:11:55 +0800
Subject: [PATCH 1/2] fix(cluster): quit() ignores errors caused by
 disconnected connection.

---
 lib/cluster/index.ts            | 15 ++++++---
 test/functional/cluster/quit.js | 57 +++++++++++++++++++++++++++++++++
 test/helpers/mock_server.js     |  7 +++-
 3 files changed, 73 insertions(+), 6 deletions(-)
 create mode 100644 test/functional/cluster/quit.js

diff --git a/lib/cluster/index.ts b/lib/cluster/index.ts
index 3729c38b..c8b5430b 100644
--- a/lib/cluster/index.ts
+++ b/lib/cluster/index.ts
@@ -267,11 +267,16 @@ class Cluster extends EventEmitter {
       return ret
     }
     return asCallback(
-      Promise.all(this.nodes().map(function (node) {
-        return node.quit()
-      })).then(function () {
-        return 'OK'
-      }),
+      Promise.all(this.nodes().map((node) => (
+        node.quit().catch((err) => {
+          // Ignore the error caused by disconnecting since
+          // we're disconnecting...
+          if (err.message === CONNECTION_CLOSED_ERROR_MSG) {
+            return 'OK'
+          }
+          throw err
+        })
+      ))).then(() => 'OK'),
       callback
     )
   }
diff --git a/test/functional/cluster/quit.js b/test/functional/cluster/quit.js
new file mode 100644
index 00000000..66dc4632
--- /dev/null
+++ b/test/functional/cluster/quit.js
@@ -0,0 +1,57 @@
+describe('cluster:quit', function () {
+  it('quit successfully when server is disconnecting', function (done) {
+    const slotTable = [
+      [0, 1000, ['127.0.0.1', 30001]],
+      [1001, 16383, ['127.0.0.1', 30002]]
+    ]
+    const server = new MockServer(30001, function (argv, c) {
+      if (argv[0] === 'quit') {
+        c.destroy()
+      }
+    }, slotTable)
+    new MockServer(30002, function (argv, c) {
+      if (argv[0] === 'quit') {
+        c.destroy()
+      }
+    }, slotTable)
+
+    const cluster = new Redis.Cluster([
+      { host: '127.0.0.1', port: '30001' }
+    ])
+    cluster.on('ready', () => {
+      server.disconnect()
+      cluster.quit(function () {
+        console.log(arguments)
+        done()
+      })
+    })
+  })
+
+  it('failed when quit returns error', function (done) {
+    const ERROR_MESSAGE = 'quit random error'
+    const slotTable = [
+      [0, 1000, ['127.0.0.1', 30001]],
+      [1001, 16383, ['127.0.0.1', 30002]]
+    ]
+    new MockServer(30001, function (argv, c) {
+      if (argv[0] === 'quit') {
+        return new Error(ERROR_MESSAGE)
+      }
+    }, slotTable)
+    new MockServer(30002, function (argv, c) {
+      if (argv[0] === 'quit') {
+        c.destroy()
+      }
+    }, slotTable)
+
+    const cluster = new Redis.Cluster([
+      { host: '127.0.0.1', port: '30001' }
+    ])
+    cluster.on('ready', () => {
+      cluster.quit((err) => {
+        expect(err.message).to.eql(ERROR_MESSAGE)
+        done()
+      })
+    })
+  })
+})
diff --git a/test/helpers/mock_server.js b/test/helpers/mock_server.js
index fcca9a6f..7653496a 100644
--- a/test/helpers/mock_server.js
+++ b/test/helpers/mock_server.js
@@ -29,11 +29,12 @@ afterEach(function (done) {
   }
 });
 
-function MockServer(port, handler) {
+function MockServer(port, handler, slotTable) {
   EventEmitter.call(this);
 
   this.port = port;
   this.handler = handler;
+  this.slotTable = slotTable;
 
   this.clients = [];
 
@@ -60,6 +61,10 @@ MockServer.prototype.connect = function () {
         if (reply.length === 3 && reply[0].toLowerCase() === 'client' && reply[1].toLowerCase() === 'setname') {
           c._connectionName = reply[2]
         }
+        if (_this.slotTable && reply.length === 2 && reply[0].toLowerCase() === 'cluster' && reply[1].toLowerCase() === 'slots') {
+          _this.write(c, _this.slotTable)
+          return
+        }
         _this.write(c, _this.handler && _this.handler(reply, c));
       },
       returnError: function () { }

From aa79387399c64f21d1c628d464d0333bc7613781 Mon Sep 17 00:00:00 2001
From: luin <i@zihua.li>
Date: Mon, 15 Oct 2018 02:16:20 +0800
Subject: [PATCH 2/2] fix tests

---
 test/functional/cluster/quit.js | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/test/functional/cluster/quit.js b/test/functional/cluster/quit.js
index 66dc4632..ebd2d3f1 100644
--- a/test/functional/cluster/quit.js
+++ b/test/functional/cluster/quit.js
@@ -1,15 +1,15 @@
-describe('cluster:quit', function () {
-  it('quit successfully when server is disconnecting', function (done) {
+describe('cluster:quit', () => {
+  it('quit successfully when server is disconnecting', (done) => {
     const slotTable = [
       [0, 1000, ['127.0.0.1', 30001]],
       [1001, 16383, ['127.0.0.1', 30002]]
     ]
-    const server = new MockServer(30001, function (argv, c) {
+    const server = new MockServer(30001, (argv, c) => {
       if (argv[0] === 'quit') {
         c.destroy()
       }
     }, slotTable)
-    new MockServer(30002, function (argv, c) {
+    new MockServer(30002, (argv, c) => {
       if (argv[0] === 'quit') {
         c.destroy()
       }
@@ -20,8 +20,9 @@ describe('cluster:quit', function () {
     ])
     cluster.on('ready', () => {
       server.disconnect()
-      cluster.quit(function () {
-        console.log(arguments)
+      cluster.quit((err, res) => {
+        expect(err).to.eql(null)
+        expect(res).to.eql('OK')
         done()
       })
     })