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 <[email protected]>
libp2p · Dec 11, 2018 · 3046b54 · 3046b54
1 parent d645235
commit 3046b54
Show file tree

Hide file tree

Showing 4 changed files with 50 additions and 55 deletions.
diff --git a/src/index.js b/src/index.js
@@ -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)
@@ -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)
@@ -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<PeerInfo>)} 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,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())
@@ -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
@@ -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<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) => {
@@ -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
@@ -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,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)

diff --git 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()