From 6ac912545bb3badef5cecc1dec1acb7c6cc671bb Mon Sep 17 00:00:00 2001 From: Braydon Fuller Date: Thu, 9 Jun 2016 11:12:52 -0400 Subject: [PATCH] bitcoind: _tryAll -> _tryAllClients Fixes a timing bug with not all clients being tried --- lib/services/bitcoind.js | 41 +++++++++++-------- test/services/bitcoind.unit.js | 73 ++++++++++++++++++++-------------- 2 files changed, 68 insertions(+), 46 deletions(-) diff --git a/lib/services/bitcoind.js b/lib/services/bitcoind.js index fd0ee0b4..04dd812c 100644 --- a/lib/services/bitcoind.js +++ b/lib/services/bitcoind.js @@ -428,8 +428,15 @@ Bitcoin.prototype._resetCaches = function() { this.blockOverviewCache.reset(); }; -Bitcoin.prototype._tryAll = function(func, callback) { - async.retry({times: this.nodes.length, interval: this.tryAllInterval || 1000}, func, callback); +Bitcoin.prototype._tryAllClients = function(func, callback) { + var self = this; + var nodesIndex = 0; + var retry = function(done) { + var client = self.nodes[nodesIndex].client; + nodesIndex++; + func(client, done); + }; + async.retry({times: this.nodes.length, interval: this.tryAllInterval || 1000}, retry, callback); }; Bitcoin.prototype._wrapRPCError = function(errObj) { @@ -1537,8 +1544,8 @@ Bitcoin.prototype.getAddressSummary = function(addressArg, options, callback) { Bitcoin.prototype._maybeGetBlockHash = function(blockArg, callback) { var self = this; if (_.isNumber(blockArg) || (blockArg.length < 40 && /^[0-9]+$/.test(blockArg))) { - self._tryAll(function(done) { - self.client.getBlockHash(blockArg, function(err, response) { + self._tryAllClients(function(client, done) { + client.getBlockHash(blockArg, function(err, response) { if (err) { return done(self._wrapRPCError(err)); } @@ -1563,7 +1570,7 @@ Bitcoin.prototype.getRawBlock = function(blockArg, callback) { if (err) { return callback(err); } - self._tryAll(function(done) { + self._tryAllClients(function(client, done) { self.client.getBlock(blockhash, false, function(err, response) { if (err) { return done(self._wrapRPCError(err)); @@ -1603,8 +1610,8 @@ Bitcoin.prototype.getBlockOverview = function(blockArg, callback) { callback(null, cachedBlock); }); } else { - self._tryAll(function(done) { - self.client.getBlock(blockhash, true, function(err, response) { + self._tryAllClients(function(client, done) { + client.getBlock(blockhash, true, function(err, response) { if (err) { return done(self._wrapRPCError(err)); } @@ -1654,8 +1661,8 @@ Bitcoin.prototype.getBlock = function(blockArg, callback) { callback(null, cachedBlock); }); } else { - self._tryAll(function(done) { - self.client.getBlock(blockhash, false, function(err, response) { + self._tryAllClients(function(client, done) { + client.getBlock(blockhash, false, function(err, response) { if (err) { return done(self._wrapRPCError(err)); } @@ -1713,8 +1720,8 @@ Bitcoin.prototype.getBlockHeader = function(blockArg, callback) { if (err) { return callback(err); } - self._tryAll(function(done) { - self.client.getBlockHeader(blockhash, function(err, response) { + self._tryAllClients(function(client, done) { + client.getBlockHeader(blockhash, function(err, response) { if (err) { return done(self._wrapRPCError(err)); } @@ -1795,8 +1802,8 @@ Bitcoin.prototype.getRawTransaction = function(txid, callback) { callback(null, tx); }); } else { - self._tryAll(function(done) { - self.client.getRawTransaction(txid, function(err, response) { + self._tryAllClients(function(client, done) { + client.getRawTransaction(txid, function(err, response) { if (err) { return done(self._wrapRPCError(err)); } @@ -1822,8 +1829,8 @@ Bitcoin.prototype.getTransaction = function(txid, callback) { callback(null, tx); }); } else { - self._tryAll(function(done) { - self.client.getRawTransaction(txid, function(err, response) { + self._tryAllClients(function(client, done) { + client.getRawTransaction(txid, function(err, response) { if (err) { return done(self._wrapRPCError(err)); } @@ -1937,8 +1944,8 @@ Bitcoin.prototype.getDetailedTransaction = function(txid, callback) { callback(null, tx); }); } else { - self._tryAll(function(done) { - self.client.getRawTransaction(txid, 1, function(err, response) { + self._tryAllClients(function(client, done) { + client.getRawTransaction(txid, 1, function(err, response) { if (err) { return done(self._wrapRPCError(err)); } diff --git a/test/services/bitcoind.unit.js b/test/services/bitcoind.unit.js index 4f602649..4eca90ae 100644 --- a/test/services/bitcoind.unit.js +++ b/test/services/bitcoind.unit.js @@ -525,46 +525,61 @@ describe('Bitcoin Service', function() { }); }); - describe('#_tryAll', function() { - it('will retry the number of bitcoind nodes', function(done) { + describe('#_tryAllClients', function() { + it('will retry for each node client', function(done) { var bitcoind = new BitcoinService(baseConfig); bitcoind.tryAllInterval = 1; - bitcoind.nodes.push({}); - bitcoind.nodes.push({}); - bitcoind.nodes.push({}); - var count = 0; - var func = function(callback) { - count++; - if (count <= 2) { - callback(new Error('test')); - } else { - callback(); + bitcoind.nodes.push({ + client: { + getInfo: sinon.stub().callsArgWith(0, new Error('test')) } - }; - bitcoind._tryAll(function(next) { - func(next); - }, function() { - count.should.equal(3); + }); + bitcoind.nodes.push({ + client: { + getInfo: sinon.stub().callsArgWith(0, new Error('test')) + } + }); + bitcoind.nodes.push({ + client: { + getInfo: sinon.stub().callsArg(0) + } + }); + bitcoind._tryAllClients(function(client, next) { + client.getInfo(next); + }, function(err) { + if (err) { + return done(err); + } + bitcoind.nodes[0].client.getInfo.callCount.should.equal(1); + bitcoind.nodes[1].client.getInfo.callCount.should.equal(1); + bitcoind.nodes[2].client.getInfo.callCount.should.equal(1); done(); }); }); - it('will get error if all fail', function(done) { + it('will get error if all clients fail', function(done) { var bitcoind = new BitcoinService(baseConfig); bitcoind.tryAllInterval = 1; - bitcoind.nodes.push({}); - bitcoind.nodes.push({}); - bitcoind.nodes.push({}); - var count = 0; - var func = function(callback) { - count++; - callback(new Error('test')); - }; - bitcoind._tryAll(function(next) { - func(next); + bitcoind.nodes.push({ + client: { + getInfo: sinon.stub().callsArgWith(0, new Error('test')) + } + }); + bitcoind.nodes.push({ + client: { + getInfo: sinon.stub().callsArgWith(0, new Error('test')) + } + }); + bitcoind.nodes.push({ + client: { + getInfo: sinon.stub().callsArgWith(0, new Error('test')) + } + }); + bitcoind._tryAllClients(function(client, next) { + client.getInfo(next); }, function(err) { should.exist(err); + err.should.be.instanceOf(Error); err.message.should.equal('test'); - count.should.equal(3); done(); }); });