feat: add err-code and cleanup logic

libp2p · Oct 1, 2018 · bfb527c · bfb527c
1 parent 244b247
commit bfb527c
Show file tree

Hide file tree

Showing 4 changed files with 17 additions and 18 deletions.
diff --git a/package.json b/package.json
@@ -38,6 +38,7 @@
   },
   "dependencies": {
     "async": "^2.6.1",
+    "err-code": "^1.1.2",
     "joi": "^13.6.0",
     "joi-browser": "^13.4.0",
     "libp2p-connection-manager": "~0.0.2",

diff --git a/src/content-routing.js b/src/content-routing.js
@@ -2,6 +2,7 @@
 
 const tryEach = require('async/tryEach')
 const parallel = require('async/parallel')
+const errCode = require('err-code')
 
 module.exports = (node) => {
   const routers = node._modules.contentRouting || []
@@ -23,14 +24,14 @@ module.exports = (node) => {
      * @returns {void}
      */
     findProviders: (key, options, callback) => {
-      if (routers.length === 0) {
-        return callback(new Error('No content routers available'))
+      if (!routers.length) {
+        return callback(errCode(new Error('No content routers available'), 'NO_ROUTERS_AVAILABLE'))
       }
 
       if (typeof options === 'function') {
         callback = options
         options = {}
-      } else if (typeof options === 'number') {
+      } else if (typeof options === 'number') { // This can be deprecated in a future release
         options = {
           maxTimeout: options
         }
@@ -44,9 +45,7 @@ module.exports = (node) => {
 
           // If we don't have any results, we need to provide an error to keep trying
           if (!results || Object.keys(results).length === 0) {
-            return cb(Object.assign(new Error('not found'), {
-              code: 'NOT_FOUND'
-            }), null)
+            return cb(errCode(new Error('not found'), 'NOT_FOUND'), null)
           }
 
           cb(null, results)
@@ -71,8 +70,8 @@ module.exports = (node) => {
      * @returns {void}
      */
     provide: (key, callback) => {
-      if (routers.length === 0) {
-        return callback(new Error('No content routers available'))
+      if (!routers.length) {
+        return callback(errCode(new Error('No content routers available'), 'NO_ROUTERS_AVAILABLE'))
       }
 
       parallel(routers.map((router) => {

diff --git a/src/peer-routing.js b/src/peer-routing.js
@@ -1,6 +1,7 @@
 'use strict'
 
 const tryEach = require('async/tryEach')
+const errCode = require('err-code')
 
 module.exports = (node) => {
   const routers = node._modules.peerRouting || []
@@ -21,8 +22,8 @@ module.exports = (node) => {
      * @returns {void}
      */
     findPeer: (id, options, callback) => {
-      if (routers.length === 0) {
-        return callback(new Error('No peer routers available'))
+      if (!routers.length) {
+        callback(errCode(new Error('No peer routers available'), 'NO_ROUTERS_AVAILABLE'))
       }
 
       if (typeof options === 'function') {
@@ -38,9 +39,7 @@ module.exports = (node) => {
 
           // If we don't have a result, we need to provide an error to keep trying
           if (!result || Object.keys(result).length === 0) {
-            return cb(Object.assign(new Error('not found'), {
-              code: 'NOT_FOUND'
-            }), null)
+            return cb(errCode(new Error('not found'), 'NOT_FOUND'), null)
           }
 
           cb(null, result)
@@ -51,7 +50,7 @@ module.exports = (node) => {
         if (err && err.code !== 'NOT_FOUND') {
           return callback(err)
         }
-        results = results || null
+        results = results || []
         callback(null, results)
       })
     }

diff --git a/test/content-routing.node.js b/test/content-routing.node.js
@@ -100,7 +100,7 @@ describe('.contentRouting', () => {
       })
 
       it('nodeE.contentRouting.findProviders for existing record', (done) => {
-        nodeE.contentRouting.findProviders(cid, 5000, (err, providers) => {
+        nodeE.contentRouting.findProviders(cid, { maxTimeout: 5000 }, (err, providers) => {
           expect(err).to.not.exist()
           expect(providers).to.have.length.above(0)
           done()
@@ -110,7 +110,7 @@ describe('.contentRouting', () => {
       it('nodeC.contentRouting.findProviders for non existing record (timeout)', (done) => {
         const cid = new CID('QmTp9VkYvnHyrqKQuFPiuZkiX9gPcqj6x5LJ1rmWuSnnnn')
 
-        nodeE.contentRouting.findProviders(cid, 5000, (err, providers) => {
+        nodeE.contentRouting.findProviders(cid, { maxTimeout: 5000 }, (err, providers) => {
           expect(err).to.not.exist()
           expect(providers).to.have.length(0)
           done()
@@ -339,7 +339,7 @@ describe('.contentRouting', () => {
           return new Error('the delegate should not have been called')
         })
 
-        nodeA.contentRouting.findProviders('a cid', 5000, (err, results) => {
+        nodeA.contentRouting.findProviders('a cid', { maxTimeout: 5000 }, (err, results) => {
           expect(err).to.not.exist()
           expect(results).to.equal(results)
           expect(dhtStub.calledOnce).to.equal(true)
@@ -355,7 +355,7 @@ describe('.contentRouting', () => {
         const dhtStub = sinon.stub(nodeA._dht, 'findProviders').callsArgWith(2, null, [])
         const delegateStub = sinon.stub(delegate, 'findProviders').callsArgWith(2, null, results)
 
-        nodeA.contentRouting.findProviders('a cid', 5000, (err, results) => {
+        nodeA.contentRouting.findProviders('a cid', { maxTimeout: 5000 }, (err, results) => {
           expect(err).to.not.exist()
           expect(results).to.deep.equal(results)
           expect(dhtStub.calledOnce).to.equal(true)