fix: dont call callback before it's properly set

libp2p · Nov 5, 2018 · 17b5f73 · 17b5f73
1 parent c18d2a4
commit 17b5f73
Show file tree

Hide file tree

Showing 4 changed files with 58 additions and 8 deletions.
diff --git a/src/content-routing.js b/src/content-routing.js
@@ -24,10 +24,6 @@ module.exports = (node) => {
      * @returns {void}
      */
     findProviders: (key, options, callback) => {
-      if (!routers.length) {
-        return callback(errCode(new Error('No content routers available'), 'NO_ROUTERS_AVAILABLE'))
-      }
-
       if (typeof options === 'function') {
         callback = options
         options = {}
@@ -37,6 +33,10 @@ module.exports = (node) => {
         }
       }
 
+      if (!routers.length) {
+        return callback(errCode(new Error('No content routers available'), 'NO_ROUTERS_AVAILABLE'))
+      }
+
       const tasks = routers.map((router) => {
         return (cb) => router.findProviders(key, options, (err, results) => {
           if (err) {

diff --git a/src/peer-routing.js b/src/peer-routing.js
@@ -22,15 +22,15 @@ module.exports = (node) => {
      * @returns {void}
      */
     findPeer: (id, options, callback) => {
-      if (!routers.length) {
-        callback(errCode(new Error('No peer routers available'), 'NO_ROUTERS_AVAILABLE'))
-      }
-
       if (typeof options === 'function') {
         callback = options
         options = {}
       }
 
+      if (!routers.length) {
+        callback(errCode(new Error('No peer routers available'), 'NO_ROUTERS_AVAILABLE'))
+      }
+
       const tasks = routers.map((router) => {
         return (cb) => router.findPeer(id, options, (err, result) => {
           if (err) {

diff --git a/test/content-routing.node.js b/test/content-routing.node.js
@@ -367,4 +367,29 @@ describe('.contentRouting', () => {
       })
     })
   })
+
+  describe('no routers', () => {
+    let nodeA
+    before((done) => {
+      createNode('/ip4/0.0.0.0/tcp/0', (err, node) => {
+        expect(err).to.not.exist()
+        nodeA = node
+        done()
+      })
+    })
+
+    it('.findProviders should return an error with no options', (done) => {
+      nodeA.contentRouting.findProviders('a cid', (err) => {
+        expect(err).to.exist()
+        done()
+      })
+    })
+
+    it('.findProviders should return an error with options', (done) => {
+      nodeA.contentRouting.findProviders('a cid', { maxTimeout: 5000 }, (err) => {
+        expect(err).to.exist()
+        done()
+      })
+    })
+  })
 })
diff --git a/test/peer-routing.node.js b/test/peer-routing.node.js
@@ -266,4 +266,29 @@ describe('.peerRouting', () => {
       })
     })
   })
+
+  describe('no routers', () => {
+    let nodeA
+    before((done) => {
+      createNode('/ip4/0.0.0.0/tcp/0', (err, node) => {
+        expect(err).to.not.exist()
+        nodeA = node
+        done()
+      })
+    })
+
+    it('.findPeer should return an error with no options', (done) => {
+      nodeA.peerRouting.findPeer('a cid', (err) => {
+        expect(err).to.exist()
+        done()
+      })
+    })
+
+    it('.findPeer should return an error with options', (done) => {
+      nodeA.peerRouting.findPeer('a cid', { maxTimeout: 5000 }, (err) => {
+        expect(err).to.exist()
+        done()
+      })
+    })
+  })
 })