From 6c3e796a80b92edeaed3fc454eceda40102e7954 Mon Sep 17 00:00:00 2001 From: Jacob Heun Date: Fri, 21 Sep 2018 18:42:54 +0200 Subject: [PATCH 1/2] feat: add timeout defaults License: MIT Signed-off-by: Jacob Heun --- src/index.js | 18 +++++++++++------- test/index.spec.js | 25 +++++++++++++++++++++++++ 2 files changed, 36 insertions(+), 7 deletions(-) diff --git a/src/index.js b/src/index.js index 9eb71ac..1c82679 100644 --- a/src/index.js +++ b/src/index.js @@ -11,6 +11,7 @@ const parallel = require('async/parallel') const reflect = require('async/reflect') const multiaddr = require('multiaddr') +const DEFAULT_MAX_TIMEOUT = 30e3 // 30 second default const DEFAULT_IPFS_API = { protocol: 'https', port: 443, @@ -57,20 +58,23 @@ class DelegatedContentRouting { * Search the dht for providers of the given CID. * * - call `findProviders` on the delegated node. - * - does not support the `timeout` parameter, as this is specific to the delegate node. * * @param {CID} key - * @param {number} _timeout This is ignored and is only present to comply with the dht interface + * @param {number} timeout How long the query can take. Defaults to 30 seconds * @param {function(Error, Array)} callback * @returns {void} */ - findProviders (key, _timeout, callback) { - if (typeof _timeout === 'function') { - callback = _timeout - _timeout = null + findProviders (key, timeout, callback) { + if (typeof timeout === 'function') { + callback = timeout + timeout = null } - this.dht.findprovs(key.toBaseEncodedString(), (err, results) => { + timeout = timeout || DEFAULT_MAX_TIMEOUT + + this.dht.findprovs(key.toBaseEncodedString(), { + timeout: `${timeout}ms` // The api requires specification of the time unit (s/ms) + }, (err, results) => { if (err) { return callback(err) } diff --git a/test/index.spec.js b/test/index.spec.js index 9f67f70..9d5f8b1 100644 --- a/test/index.spec.js +++ b/test/index.spec.js @@ -147,6 +147,31 @@ describe('DelegatedContentRouting', function () { } ], done) }) + + it('should be able to specify a timeout', function (done) { + async.waterfall([ + (cb) => { + const opts = delegatedNode.apiAddr.toOptions() + const routing = new DelegatedContentRouting(selfId, { + protocol: 'http', + port: opts.port, + host: opts.host + }) + const cid = new CID('QmS4ustL54uo8FzR9455qaxZwuMiUhyvMcX9Ba8nUH4uVv') + routing.findProviders(cid, 5e3, cb) + }, + (providers, cb) => { + // We should get our local node and the bootstrap node as providers. + // The delegate node is not included, because it is handling the requests + expect(providers).to.have.length(2) + expect(providers.map((p) => p.id.toB58String())).to.have.members([ + bootstrapId.id, + selfId.toB58String() + ]) + cb() + } + ], done) + }) }) describe('provide', () => { From 7f8cb7ab5189deb5a6e1ca2206acab4c1a87c56a Mon Sep 17 00:00:00 2001 From: Jacob Heun Date: Wed, 26 Sep 2018 19:29:18 +0200 Subject: [PATCH 2/2] refactor: change timeout to options License: MIT Signed-off-by: Jacob Heun --- src/index.js | 21 ++++++++++++++------- test/index.spec.js | 4 ++-- 2 files changed, 16 insertions(+), 9 deletions(-) diff --git a/src/index.js b/src/index.js index 1c82679..6ba6feb 100644 --- a/src/index.js +++ b/src/index.js @@ -60,20 +60,27 @@ class DelegatedContentRouting { * - call `findProviders` on the delegated node. * * @param {CID} key - * @param {number} timeout How long the query can take. Defaults to 30 seconds + * @param {object} options + * @param {number} options.maxTimeout How long the query can take. Defaults to 30 seconds * @param {function(Error, Array)} callback * @returns {void} */ - findProviders (key, timeout, callback) { - if (typeof timeout === 'function') { - callback = timeout - timeout = null + findProviders (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 || {} } - timeout = timeout || DEFAULT_MAX_TIMEOUT + options.maxTimeout = options.maxTimeout || DEFAULT_MAX_TIMEOUT this.dht.findprovs(key.toBaseEncodedString(), { - timeout: `${timeout}ms` // The api requires specification of the time unit (s/ms) + timeout: `${options.maxTimeout}ms` // The api requires specification of the time unit (s/ms) }, (err, results) => { if (err) { return callback(err) diff --git a/test/index.spec.js b/test/index.spec.js index 9d5f8b1..e4a70dd 100644 --- a/test/index.spec.js +++ b/test/index.spec.js @@ -148,7 +148,7 @@ describe('DelegatedContentRouting', function () { ], done) }) - it('should be able to specify a timeout', function (done) { + it('should be able to specify a maxTimeout', function (done) { async.waterfall([ (cb) => { const opts = delegatedNode.apiAddr.toOptions() @@ -158,7 +158,7 @@ describe('DelegatedContentRouting', function () { host: opts.host }) const cid = new CID('QmS4ustL54uo8FzR9455qaxZwuMiUhyvMcX9Ba8nUH4uVv') - routing.findProviders(cid, 5e3, cb) + routing.findProviders(cid, { maxTimeout: 5e3 }, cb) }, (providers, cb) => { // We should get our local node and the bootstrap node as providers.