fix: fix uncaught promise rejection when finding peers (#1044)

Do not abort all attempts to find peers when `findPeers` on one router throws synchronously

Co-authored-by: Robert Kiel <robert.kiel@hoprnet.io>
Co-authored-by: achingbrain <alex@achingbrain.net>

Author: 3 people

src/peer-routing.js

@@ -114,10 +114,15 @@ class PeerRouting {
 
     const output = await pipe(
       merge(
-        ...this._routers.map(router => [router.findPeer(id, options)])
+        ...this._routers.map(router => (async function * () {
+          try {
+            yield await router.findPeer(id, options)
+          } catch (err) {
+            log.error(err)
+          }
+        })())
       ),
       (source) => filter(source, Boolean),
-      // @ts-ignore findPeer resolves a Promise
       (source) => storeAddresses(source, this._peerStore),
       (source) => first(source)
     )

test/peer-routing/peer-routing.node.js

@@ -106,6 +106,95 @@ describe('peer-routing', () => {
         .to.eventually.be.rejected()
         .and.to.have.property('code', 'ERR_FIND_SELF')
     })
+
+    it('should handle error thrown synchronously during find peer', async () => {
+      const unknownPeers = await peerUtils.createPeerId({ number: 1, fixture: false })
+
+      nodes[0].peerRouting._routers = [{
+        findPeer () {
+          throw new Error('Thrown sync')
+        }
+      }]
+
+      await expect(nodes[0].peerRouting.findPeer(unknownPeers[0]))
+        .to.eventually.be.rejected()
+        .and.to.have.property('code', 'ERR_NOT_FOUND')
+    })
+
+    it('should handle error thrown asynchronously during find peer', async () => {
+      const unknownPeers = await peerUtils.createPeerId({ number: 1, fixture: false })
+
+      nodes[0].peerRouting._routers = [{
+        async findPeer () {
+          throw new Error('Thrown async')
+        }
+      }]
+
+      await expect(nodes[0].peerRouting.findPeer(unknownPeers[0]))
+        .to.eventually.be.rejected()
+        .and.to.have.property('code', 'ERR_NOT_FOUND')
+    })
+
+    it('should handle error thrown asynchronously after delay during find peer', async () => {
+      const unknownPeers = await peerUtils.createPeerId({ number: 1, fixture: false })
+
+      nodes[0].peerRouting._routers = [{
+        async findPeer () {
+          await delay(100)
+          throw new Error('Thrown async after delay')
+        }
+      }]
+
+      await expect(nodes[0].peerRouting.findPeer(unknownPeers[0]))
+        .to.eventually.be.rejected()
+        .and.to.have.property('code', 'ERR_NOT_FOUND')
+    })
+
+    it('should return value when one router errors synchronously and another returns a value', async () => {
+      const [peer] = await peerUtils.createPeerId({ number: 1, fixture: false })
+
+      nodes[0].peerRouting._routers = [{
+        findPeer () {
+          throw new Error('Thrown sync')
+        }
+      }, {
+        async findPeer () {
+          return Promise.resolve({
+            id: peer,
+            multiaddrs: []
+          })
+        }
+      }]
+
+      await expect(nodes[0].peerRouting.findPeer(peer))
+        .to.eventually.deep.equal({
+          id: peer,
+          multiaddrs: []
+        })
+    })
+
+    it('should return value when one router errors asynchronously and another returns a value', async () => {
+      const [peer] = await peerUtils.createPeerId({ number: 1, fixture: false })
+
+      nodes[0].peerRouting._routers = [{
+        async findPeer () {
+          throw new Error('Thrown sync')
+        }
+      }, {
+        async findPeer () {
+          return Promise.resolve({
+            id: peer,
+            multiaddrs: []
+          })
+        }
+      }]
+
+      await expect(nodes[0].peerRouting.findPeer(peer))
+        .to.eventually.deep.equal({
+          id: peer,
+          multiaddrs: []
+        })
+    })
   })
 
   describe('via delegate router', () => {