fix: the API of es6-promisify is not the same as promisify-es6 (#905)

package.json

@@ -109,6 +109,7 @@
   },
   "devDependencies": {
     "@nodeutils/defaults-deep": "^1.1.0",
+    "@types/es6-promisify": "^6.0.0",
     "abortable-iterator": "^3.0.0",
     "aegir": "^29.2.0",
     "chai-bytes": "^0.1.2",

src/nat-manager.js

@@ -2,7 +2,7 @@
 
 const NatAPI = require('@motrix/nat-api')
 const debug = require('debug')
-const promisify = require('es6-promisify')
+const { promisify } = require('es6-promisify')
 const Multiaddr = require('multiaddr')
 const log = Object.assign(debug('libp2p:nat'), {
   error: debug('libp2p:nat:err')
@@ -132,14 +132,32 @@ class NatManager {
     }
 
     const client = new NatAPI(this._options)
-    const map = promisify(client.map, { context: client })
-    const destroy = promisify(client.destroy, { context: client })
-    const externalIp = promisify(client.externalIp, { context: client })
 
+    /** @type {(...any) => any} */
+    const map = promisify(client.map.bind(client))
+    /** @type {(...any) => any} */
+    const destroy = promisify(client.destroy.bind(client))
+    /** @type {(...any) => any} */
+    const externalIp = promisify(client.externalIp.bind(client))
+
+    // these are all network operations so add a retry
     this._client = {
-      // these are all network operations so add a retry
+      /**
+       * @param  {...any} args
+       * @returns {Promise<void>}
+       */
       map: (...args) => retry(() => map(...args), { onFailedAttempt: log.error, unref: true }),
+
+      /**
+       * @param  {...any} args
+       * @returns {Promise<void>}
+       */
       destroy: (...args) => retry(() => destroy(...args), { onFailedAttempt: log.error, unref: true }),
+
+      /**
+       * @param  {...any} args
+       * @returns {Promise<string>}
+       */
       externalIp: (...args) => retry(() => externalIp(...args), { onFailedAttempt: log.error, unref: true })
     }
 

test/nat-manager/nat-manager.node.js

@@ -3,6 +3,7 @@
 
 const { expect } = require('aegir/utils/chai')
 const sinon = require('sinon')
+const { networkInterfaces } = require('os')
 const AddressManager = require('../../src/address-manager')
 const TransportManager = require('../../src/transport-manager')
 const Transport = require('libp2p-tcp')
@@ -156,7 +157,7 @@ describe('Nat Manager (TCP)', () => {
       natManager,
       addressManager
     } = await createNatManager([
-      '/ip6/::/tcp/5001'
+      '/ip6/::/tcp/0'
     ])
 
     let observed = addressManager.getObservedAddrs().map(ma => ma.toString())
@@ -173,7 +174,7 @@ describe('Nat Manager (TCP)', () => {
       natManager,
       addressManager
     } = await createNatManager([
-      '/ip6/::1/tcp/5001'
+      '/ip6/::1/tcp/0'
     ])
 
     let observed = addressManager.getObservedAddrs().map(ma => ma.toString())
@@ -207,7 +208,7 @@ describe('Nat Manager (TCP)', () => {
       natManager,
       addressManager
     } = await createNatManager([
-      '/ip4/127.0.0.1/tcp/5900'
+      '/ip4/127.0.0.1/tcp/0'
     ])
 
     let observed = addressManager.getObservedAddrs().map(ma => ma.toString())
@@ -224,7 +225,7 @@ describe('Nat Manager (TCP)', () => {
       natManager,
       addressManager
     } = await createNatManager([
-      '/ip4/0.0.0.0/tcp/5900/sctp/49832'
+      '/ip4/0.0.0.0/tcp/0/sctp/0'
     ])
 
     let observed = addressManager.getObservedAddrs().map(ma => ma.toString())
@@ -241,4 +242,47 @@ describe('Nat Manager (TCP)', () => {
       new NatManager({ ttl: 5 }) // eslint-disable-line no-new
     }).to.throw().with.property('code', ERR_INVALID_PARAMETERS)
   })
+
+  it('shuts the nat api down when stopping', async function () {
+    function findRoutableAddress () {
+      const interfaces = networkInterfaces()
+
+      for (const name of Object.keys(interfaces)) {
+        for (const iface of interfaces[name]) {
+          // Skip over non-IPv4 and internal (i.e. 127.0.0.1) addresses
+          if (iface.family === 'IPv4' && !iface.internal) {
+            return iface.address
+          }
+        }
+      }
+    }
+
+    const addr = findRoutableAddress()
+
+    if (!addr) {
+      // skip test if no non-loopback address is found
+      this.skip()
+    }
+
+    const {
+      natManager
+    } = await createNatManager([
+      `/ip4/${addr}/tcp/0`
+    ])
+
+    // use the actual nat manager client not the stub
+    delete natManager._client
+
+    await natManager._start()
+
+    const client = natManager._client
+    expect(client).to.be.ok()
+
+    // ensure the client was stopped
+    const spy = sinon.spy(client, 'destroy')
+
+    await natManager.stop()
+
+    expect(spy.called).to.be.true()
+  })
 })