Skip to content
This repository has been archived by the owner on Jul 21, 2023. It is now read-only.

Commit

Permalink
chore: update options timeout property (#62)
Browse files Browse the repository at this point in the history 
* 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 <vasco.santos@ua.pt>
  • Loading branch information
@vasco-santos
vasco-santos committed Dec 11, 2018
1 parent d645235 commit 3046b54
Show file tree
Hide file tree
Showing 4 changed files with 50 additions and 55 deletions.
55 changes: 25 additions & 30 deletions src/index.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -203,24 +203,22 @@ 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}
*/
get (key, options, callback) {
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)
Expand All @@ -232,24 +230,22 @@ 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}
*/
getMany (key, nvals, options, callback) {
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)
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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<PeerInfo>)} callback
* @returns {void}
Expand All @@ -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
Expand All @@ -516,24 +513,22 @@ 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}
*/
findPeer (id, options, callback) {
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())
Expand Down Expand Up @@ -594,7 +589,7 @@ class KadDHT {

timeout((cb) => {
query.run(peers, cb)
}, options.maxTimeout)(cb)
}, options.timeout)(cb)
},
(result, cb) => {
let success = false
Expand Down
10 changes: 5 additions & 5 deletions src/private.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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}
*
Expand All @@ -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
Expand Down Expand Up @@ -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<PeerInfo>)} 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) => {
Expand Down Expand Up @@ -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) => {
Expand Down
14 changes: 7 additions & 7 deletions src/random-walk.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 }

Expand Down Expand Up @@ -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
}
Expand All @@ -92,21 +92,21 @@ 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) => {
waterfall([
(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)
Expand Down
26 changes: 13 additions & 13 deletions test/kad-dht.spec.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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) {
Expand Down Expand Up @@ -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()
Expand All @@ -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()
Expand Down Expand Up @@ -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()
Expand All @@ -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')
Expand Down Expand Up @@ -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) => {
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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()
Expand All @@ -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()
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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()
Expand Down

0 comments on commit 3046b54

Please sign in to comment.