From 3046b54363e92964b473b48124b815b54aec0f93 Mon Sep 17 00:00:00 2001 From: Vasco Santos Date: Tue, 11 Dec 2018 11:42:36 +0000 Subject: [PATCH] chore: update options timeout property (#62) * chore: update options timeout property BREAKING CHANGE: get, getMany, findProviders and findPeer do not accept a timeout number anymore. It must be a property of an object options. Co-Authored-By: vasco-santos --- src/index.js | 55 ++++++++++++++++++++------------------------ src/private.js | 10 ++++---- src/random-walk.js | 14 +++++------ test/kad-dht.spec.js | 26 ++++++++++----------- 4 files changed, 50 insertions(+), 55 deletions(-) diff --git a/src/index.js b/src/index.js index ae46b57f..7ae0f30c 100644 --- a/src/index.js +++ b/src/index.js @@ -203,7 +203,7 @@ class KadDHT { * * @param {Buffer} key * @param {Object} options - get options - * @param {number} options.maxTimeout - optional timeout (default: 60000) + * @param {number} options.timeout - optional timeout (default: 60000) * @param {function(Error, Buffer)} callback * @returns {void} */ @@ -211,16 +211,14 @@ class KadDHT { if (typeof options === 'function') { callback = options options = {} - } else if (typeof options === 'number') { // This will be deprecated in a next release - options = { - maxTimeout: options - } } else { options = options || {} } - if (!options.maxTimeout) { - options.maxTimeout = c.minute + if (!options.maxTimeout && !options.timeout) { + options.timeout = c.minute // default + } else if (options.maxTimeout && !options.timeout) { // TODO this will be deprecated in a next release + options.timeout = options.maxTimeout } this._get(key, options, callback) @@ -232,7 +230,7 @@ class KadDHT { * @param {Buffer} key * @param {number} nvals * @param {Object} options - get options - * @param {number} options.maxTimeout - optional timeout (default: 60000) + * @param {number} options.timeout - optional timeout (default: 60000) * @param {function(Error, Array<{from: PeerId, val: Buffer}>)} callback * @returns {void} */ @@ -240,16 +238,14 @@ class KadDHT { if (typeof options === 'function') { callback = options options = {} - } else if (typeof options === 'number') { // This will be deprecated in a next release - options = { - maxTimeout: options - } } else { options = options || {} } - if (!options.maxTimeout) { - options.maxTimeout = c.minute + if (!options.maxTimeout && !options.timeout) { + options.timeout = 'c.minute' // default + } else if (options.maxTimeout && !options.timeout) { // TODO this will be deprecated in a next release + options.timeout = options.maxTimeout } this._log('getMany %b (%s)', key, nvals) @@ -322,7 +318,7 @@ class KadDHT { }) // run our query - timeout((cb) => query.run(rtp, cb), options.maxTimeout)(cb) + timeout((cb) => query.run(rtp, cb), options.timeout)(cb) } ], (err) => { // combine vals from each path @@ -485,7 +481,7 @@ class KadDHT { * * @param {CID} key * @param {Object} options - findProviders options - * @param {number} options.maxTimeout - how long the query should maximally run, in milliseconds (default: 60000) + * @param {number} options.timeout - how long the query should maximally run, in milliseconds (default: 60000) * @param {number} options.maxNumProviders - maximum number of providers to find * @param {function(Error, Array)} callback * @returns {void} @@ -494,19 +490,20 @@ class KadDHT { if (typeof options === 'function') { callback = options options = {} - } else if (typeof options === 'number') { // This will be deprecated in a next release - options = { - maxTimeout: options - } } else { options = options || {} } - options.maxTimeout = options.maxTimeout || c.minute + if (!options.maxTimeout && !options.timeout) { + options.timeout = c.minute // default + } else if (options.maxTimeout && !options.timeout) { // TODO this will be deprecated in a next release + options.timeout = options.maxTimeout + } + options.maxNumProviders = options.maxNumProviders || c.K this._log('findProviders %s', key.toBaseEncodedString()) - this._findNProviders(key, options.maxTimeout, options.maxNumProviders, callback) + this._findNProviders(key, options.timeout, options.maxNumProviders, callback) } // ----------- Peer Routing @@ -516,7 +513,7 @@ class KadDHT { * * @param {PeerId} id * @param {Object} options - findPeer options - * @param {number} options.maxTimeout - how long the query should maximally run, in milliseconds (default: 60000) + * @param {number} options.timeout - how long the query should maximally run, in milliseconds (default: 60000) * @param {function(Error, PeerInfo)} callback * @returns {void} */ @@ -524,16 +521,14 @@ class KadDHT { if (typeof options === 'function') { callback = options options = {} - } else if (typeof options === 'number') { // This will be deprecated in a next release - options = { - maxTimeout: options - } } else { options = options || {} } - if (!options.maxTimeout) { - options.maxTimeout = c.minute + if (!options.maxTimeout && !options.timeout) { + options.timeout = c.minute // default + } else if (options.maxTimeout && !options.timeout) { // TODO this will be deprecated in a next release + options.timeout = options.maxTimeout } this._log('findPeer %s', id.toB58String()) @@ -594,7 +589,7 @@ class KadDHT { timeout((cb) => { query.run(peers, cb) - }, options.maxTimeout)(cb) + }, options.timeout)(cb) }, (result, cb) => { let success = false diff --git a/src/private.js b/src/private.js index c895e7dc..5eaf607c 100644 --- a/src/private.js +++ b/src/private.js @@ -260,7 +260,7 @@ module.exports = (dht) => ({ * * @param {Buffer} key * @param {Object} options - get options - * @param {number} options.maxTimeout - optional timeout (default: 60000) + * @param {number} options.timeout - optional timeout (default: 60000) * @param {function(Error, Record)} callback * @returns {void} * @@ -269,7 +269,7 @@ module.exports = (dht) => ({ _get (key, options, callback) { dht._log('_get %b', key) waterfall([ - (cb) => dht.getMany(key, 16, options.maxTimeout, cb), + (cb) => dht.getMany(key, 16, options, cb), (vals, cb) => { const recs = vals.map((v) => v.val) let i = 0 @@ -458,14 +458,14 @@ module.exports = (dht) => ({ * Search the dht for up to `n` providers of the given CID. * * @param {CID} key - * @param {number} maxTimeout - How long the query should maximally run in milliseconds. + * @param {number} providerTimeout - How long the query should maximally run in milliseconds. * @param {number} n * @param {function(Error, Array)} callback * @returns {void} * * @private */ - _findNProviders (key, maxTimeout, n, callback) { + _findNProviders (key, providerTimeout, n, callback) { let out = new LimitedPeerList(n) dht.providers.getProviders(key, (err, provs) => { @@ -524,7 +524,7 @@ module.exports = (dht) => ({ const peers = dht.routingTable.closestPeers(key.buffer, c.ALPHA) - timeout((cb) => query.run(peers, cb), maxTimeout)((err) => { + timeout((cb) => query.run(peers, cb), providerTimeout)((err) => { // combine peers from each path paths.forEach((path) => { path.toArray().forEach((peer) => { diff --git a/src/random-walk.js b/src/random-walk.js index 17af378c..9a91f695 100644 --- a/src/random-walk.js +++ b/src/random-walk.js @@ -25,13 +25,13 @@ class RandomWalk { * * @param {number} [queries=1] - how many queries to run per period * @param {number} [period=300000] - how often to run the the random-walk process, in milliseconds (5min) - * @param {number} [maxTimeout=10000] - how long to wait for the the random-walk query to run, in milliseconds (10s) + * @param {number} [timeout=10000] - how long to wait for the the random-walk query to run, in milliseconds (10s) * @returns {void} */ - start (queries, period, maxTimeout) { + start (queries, period, timeout) { if (queries == null) { queries = 1 } if (period == null) { period = 5 * c.minute } - if (maxTimeout == null) { maxTimeout = 10 * c.second } + if (timeout == null) { timeout = 10 * c.second } // Don't run twice if (this._running) { return } @@ -66,7 +66,7 @@ class RandomWalk { // Start runner runningHandle.runPeriodically((done) => { - this._walk(queries, maxTimeout, () => done(period)) + this._walk(queries, timeout, () => done(period)) }, period) this._runningHandle = runningHandle } @@ -92,13 +92,13 @@ class RandomWalk { * Do the random walk work. * * @param {number} queries - * @param {number} maxTimeout + * @param {number} walkTimeout * @param {function(Error)} callback * @returns {void} * * @private */ - _walk (queries, maxTimeout, callback) { + _walk (queries, walkTimeout, callback) { this._kadDHT._log('random-walk:start') times(queries, (i, cb) => { @@ -106,7 +106,7 @@ class RandomWalk { (cb) => this._randomPeerId(cb), (id, cb) => timeout((cb) => { this._query(id, cb) - }, maxTimeout)(cb) + }, walkTimeout)(cb) ], (err) => { if (err) { this._kadDHT._log.error('random-walk:error', err) diff --git a/test/kad-dht.spec.js b/test/kad-dht.spec.js index ad0a076a..a5274077 100644 --- a/test/kad-dht.spec.js +++ b/test/kad-dht.spec.js @@ -77,7 +77,7 @@ function bootstrap (dhts) { }) } -function waitForWellFormedTables (dhts, minPeers, avgPeers, maxTimeout, callback) { +function waitForWellFormedTables (dhts, minPeers, avgPeers, waitTimeout, callback) { timeout((cb) => { retry({ times: 50, interval: 200 }, (cb) => { let totalPeers = 0 @@ -98,7 +98,7 @@ function waitForWellFormedTables (dhts, minPeers, avgPeers, maxTimeout, callback const done = ready.every(Boolean) cb(done ? null : new Error('not done yet')) }, cb) - }, maxTimeout)(callback) + }, waitTimeout)(callback) } function countDiffPeers (a, b) { @@ -267,7 +267,7 @@ describe('KadDHT', () => { waterfall([ (cb) => connect(dhtA, dhtB, cb), (cb) => dhtA.put(Buffer.from('/v/hello'), Buffer.from('world'), cb), - (cb) => dhtB.get(Buffer.from('/v/hello'), { maxTimeout: 1000 }, cb), + (cb) => dhtB.get(Buffer.from('/v/hello'), { timeout: 1000 }, cb), (res, cb) => { expect(res).to.eql(Buffer.from('world')) cb() @@ -291,7 +291,7 @@ describe('KadDHT', () => { waterfall([ (cb) => connect(dhtA, dhtB, cb), (cb) => dhtA.put(Buffer.from('hello'), Buffer.from('world'), cb), - (cb) => dhtB.get(Buffer.from('hello'), { maxTimeout: 1000 }, cb), + (cb) => dhtB.get(Buffer.from('hello'), { timeout: 1000 }, cb), (res, cb) => { expect(res).to.eql(Buffer.from('world')) cb() @@ -324,7 +324,7 @@ describe('KadDHT', () => { waterfall([ (cb) => connect(dhtA, dhtB, cb), (cb) => dhtA.put(Buffer.from('/ipns/hello'), Buffer.from('world'), cb), - (cb) => dhtB.get(Buffer.from('/ipns/hello'), { maxTimeout: 1000 }, cb), + (cb) => dhtB.get(Buffer.from('/ipns/hello'), { timeout: 1000 }, cb), (res, cb) => { expect(res).to.eql(Buffer.from('world')) cb() @@ -348,7 +348,7 @@ describe('KadDHT', () => { waterfall([ (cb) => connect(dhtA, dhtB, cb), (cb) => dhtA.put(Buffer.from('/v2/hello'), Buffer.from('world'), cb), - (cb) => dhtB.get(Buffer.from('/v2/hello'), { maxTimeout: 1000 }, cb) + (cb) => dhtB.get(Buffer.from('/v2/hello'), { timeout: 1000 }, cb) ], (err) => { expect(err).to.exist() expect(err.code).to.eql('ERR_UNRECOGNIZED_KEY_PREFIX') @@ -376,8 +376,8 @@ describe('KadDHT', () => { expect(err).to.not.exist() series([ - (cb) => dhtA.get(Buffer.from('/v/hello'), { maxTimeout: 1000 }, cb), - (cb) => dhtB.get(Buffer.from('/v/hello'), { maxTimeout: 1000 }, cb) + (cb) => dhtA.get(Buffer.from('/v/hello'), { timeout: 1000 }, cb), + (cb) => dhtB.get(Buffer.from('/v/hello'), { timeout: 1000 }, cb) ], (err, results) => { expect(err).to.not.exist() results.forEach((res) => { @@ -412,7 +412,7 @@ describe('KadDHT', () => { let n = 0 each(values, (v, cb) => { n = (n + 1) % 3 - dhts[n].findProviders(v.cid, { maxTimeout: 5000 }, (err, provs) => { + dhts[n].findProviders(v.cid, { timeout: 5000 }, (err, provs) => { expect(err).to.not.exist() expect(provs).to.have.length(1) expect(provs[0].id.id).to.be.eql(ids[3].id) @@ -507,7 +507,7 @@ describe('KadDHT', () => { Buffer.from('world'), cb ), - (cb) => dhts[0].get(Buffer.from('/v/hello'), { maxTimeout: 1000 }, cb), + (cb) => dhts[0].get(Buffer.from('/v/hello'), { timeout: 1000 }, cb), (res, cb) => { expect(res).to.eql(Buffer.from('world')) cb() @@ -534,7 +534,7 @@ describe('KadDHT', () => { (cb) => connect(dhts[0], dhts[1], cb), (cb) => connect(dhts[1], dhts[2], cb), (cb) => connect(dhts[2], dhts[3], cb), - (cb) => dhts[0].findPeer(ids[3], { maxTimeout: 1000 }, cb), + (cb) => dhts[0].findPeer(ids[3], { timeout: 1000 }, cb), (res, cb) => { expect(res.id.isEqual(ids[3])).to.eql(true) cb() @@ -878,7 +878,7 @@ describe('KadDHT', () => { waterfall([ (cb) => connect(dhtA, dhtB, cb), - (cb) => dhtA.get(Buffer.from('/v/hello'), { maxTimeout: 1000 }, cb) + (cb) => dhtA.get(Buffer.from('/v/hello'), { timeout: 1000 }, cb) ], (err) => { expect(err).to.exist() expect(err.code).to.be.eql(errCode) @@ -936,7 +936,7 @@ describe('KadDHT', () => { expect(err).to.not.exist() const stub = sinon.stub(dhts[0].routingTable, 'closestPeers').returns([]) - dhts[0].findPeer(ids[3], { maxTimeout: 1000 }, (err) => { + dhts[0].findPeer(ids[3], { timeout: 1000 }, (err) => { expect(err).to.exist() expect(err.code).to.eql('ERR_LOOKUP_FAILED') stub.restore()