Repository: basho/riak-nodejs-client
Pull request #89 (2015-07-22 22:03:30)

Fix uncaught exception in defaultnodemanager when nodes are removed
See original PR #88 (CLIENTS-209) 

Fixes CLIENTS-526

Thank you @charliezhang 


Patch:
diff --git a/lib/core/defaultnodemanager.js b/lib/core/defaultnodemanager.js
index 1b458aa..8af26fc 100644
--- a/lib/core/defaultnodemanager.js
+++ b/lib/core/defaultnodemanager.js
@@ -43,11 +43,13 @@ DefaultNodeManager.prototype.executeOnNode = function(nodes, command, previous)
     var executing = false;
     var startingIndex = this._nodeIndex;
     do {
-        var node = nodes[this._nodeIndex];
-        this._nodeIndex++;
-        if (this._nodeIndex === nodes.length) {
+        // Check index before accessing {nodes} because elements can be removed from {nodes}.
+        if (this._nodeIndex >= nodes.length) {
             this._nodeIndex = 0;
         }
+        var node = nodes[this._nodeIndex];
+        this._nodeIndex++;
+
         // don't try the same node twice in a row if we have multiple nodes
         if (nodes.length > 1 && previous && previous === node) {
             continue;
@@ -66,4 +68,4 @@ DefaultNodeManager.prototype.executeOnNode = function(nodes, command, previous)
     
 };
 
-module.exports = DefaultNodeManager;
\ No newline at end of file
+module.exports = DefaultNodeManager;


Test patch:
diff --git a/test/unit/core/riakcluster.js b/test/unit/core/riakcluster.js
index 620e1f1..7b18fe4 100644
--- a/test/unit/core/riakcluster.js
+++ b/test/unit/core/riakcluster.js
@@ -44,4 +44,41 @@ describe('RiakCluster', function() {
             done();
         });
     });
+    describe('Test removing node while executing command', function() {
+        it('removing node while executing command should not throw exception', function(done) {
+            var nodeTemplate = new RiakNode.Builder().withMinConnections(10);
+            var nodeAddys = ['192.168.1.1:8087', '192.168.1.2:8087', '192.168.1.3:8087', '192.168.1.4:8087'];
+            var arrayOfNodes = RiakNode.buildNodes(nodeAddys, nodeTemplate);
+            var myCluster = new RiakCluster.Builder().withRiakNodes(arrayOfNodes).build();
+            assert.equal(myCluster.nodes.length, 4);
+            myCluster.nodes.forEach(function (n) {
+                n.state = RiakNode.State.RUNNING;
+                n.execute = function() {
+                    return true;
+                };
+            });
+            assert.equal(myCluster.nodeManager._nodeIndex, 0);
+            myCluster.execute(myCluster.nodes, {onError: function() {}});
+            assert.equal(myCluster.nodeManager._nodeIndex, 1);
+            myCluster.execute(myCluster.nodes, {onError: function() {}});
+            assert.equal(myCluster.nodeManager._nodeIndex, 2);
+
+            assert(myCluster.removeNode('192.168.1.3:8087'));
+            assert(myCluster.removeNode('192.168.1.4:8087'));
+            assert.equal(myCluster.nodes.length, 2);
+            assert.equal(myCluster.nodeManager._nodeIndex, 2);
+            assert.equal(myCluster.nodes[0].remoteAddress, '192.168.1.1');
+            assert.equal(myCluster.nodes[0].remotePort, 8087);
+            assert.equal(myCluster.nodes[1].remoteAddress, '192.168.1.2');
+            assert.equal(myCluster.nodes[1].remotePort, 8087);
+
+            myCluster.execute(myCluster.nodes, {onError: function() {}});
+            assert.equal(myCluster.nodeManager._nodeIndex, 1);
+            myCluster.execute(myCluster.nodes, {onError: function() {}});
+            assert.equal(myCluster.nodeManager._nodeIndex, 2);
+            myCluster.execute(myCluster.nodes, {onError: function() {}});
+            assert.equal(myCluster.nodeManager._nodeIndex, 1);
+            done();
+        });
+    });
 });