fix(cluster): quit() ignores errors caused by disconnected connection (#720)

Author: luin

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
     )
   }

test/functional/cluster/quit.js

@@ -0,0 +1,58 @@
+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, (argv, c) => {
+      if (argv[0] === 'quit') {
+        c.destroy()
+      }
+    }, slotTable)
+    new MockServer(30002, (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((err, res) => {
+        expect(err).to.eql(null)
+        expect(res).to.eql('OK')
+        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()
+      })
+    })
+  })
+})

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 () { }