fix: accept callbacks in agent close and destroy

Squashed commit of the following:

commit ecd6feb3db9464ab82ffd71f08026e78955fc3c6
Author: Robert Nagy <ronagy@icloud.com>
Date:   Wed Apr 7 10:31:28 2021 +0200

    fixup

commit 15b6447
Merge: 755bf6f d17b48d
Author: Robert Nagy <ronagy@icloud.com>
Date:   Wed Apr 7 10:17:02 2021 +0200

    Merge branch 'main' into fix-agent-destroy-close

commit d17b48d
Author: Yash Shah <yashah1234@gmail.com>
Date:   Wed Apr 7 13:40:36 2021 +0530

    added closed and destroyed properties on agent (#634)

    * added closed and destroyed properties on agent

    * refactored symbols frim agent/index.js to core/symbols.js

    * Removed lib/agent/index.js

    * added closed and destroyed getters to lib/agent.js

    * reset symbols.js

    * fix lint issues

commit 755bf6f
Author: Robert Nagy <ronagy@icloud.com>
Date:   Wed Apr 7 10:09:49 2021 +0200

    Update lib/agent.js

commit 16e9f23
Author: Robert Nagy <ronagy@icloud.com>
Date:   Wed Apr 7 10:09:09 2021 +0200

    Apply suggestions from code review

commit 0aef9f8
Merge: 1bf3ae0 1eeb8d1
Author: Robert Nagy <ronagy@icloud.com>
Date:   Wed Apr 7 10:06:38 2021 +0200

    Merge branch 'main' into fix-agent-destroy-close

commit 1bf3ae0
Author: Sharat Chandra <sharat9211@gmail.com>
Date:   Tue Mar 30 11:58:02 2021 +0530

    test: add tests for invalid function paramaters

commit 38c3666
Author: Sharat Chandra <sharat9211@gmail.com>
Date:   Tue Mar 30 01:11:24 2021 +0530

    test: add tests for close and destroy

commit 672f324
Author: Sharat Chandra <sharat9211@gmail.com>
Date:   Thu Mar 25 23:23:06 2021 +0530

    fix: fail early

commit 0f9794e
Author: Sharat Chandra <sharat9211@gmail.com>
Date:   Tue Mar 23 23:44:52 2021 +0530

    fix: accept callbacks in agent close and destroy

lib/agent.js

@@ -188,7 +188,15 @@ class Agent extends Dispatcher {
     return this[kClosed]
   }
 
-  close () {
+  get destroyed () {
+    return this[kDestroyed]
+  }
+
+  close (callback) {
+    if (callback != null && typeof callback !== 'function') {
+      throw new InvalidArgumentError('callback must be a function')
+    }
+
     this[kClosed] = true
 
     const closePromises = []
@@ -198,25 +206,44 @@ class Agent extends Dispatcher {
         closePromises.push(client.close())
       }
     }
-    return Promise.all(closePromises)
-  }
 
-  get destroyed () {
-    return this[kDestroyed]
+    if (!callback) {
+      return Promise.all(closePromises)
+    }
+
+    Promise.all(closePromises)
+      .then(() => process.nextTick(callback))
+      .catch((err) => process.nextTick(callback, err))
   }
 
-  destroy () {
+  destroy (err, callback) {
+    if (typeof err === 'function') {
+      callback = err
+      err = null
+    }
+
+    if (callback != null && typeof callback !== 'function') {
+      throw new InvalidArgumentError('callback must be a function')
+    }
+
     this[kClosed] = true
     this[kDestroyed] = true
 
     const destroyPromises = []
     for (const ref of this[kClients].values()) {
       const client = ref.deref()
       if (client) {
-        destroyPromises.push(client.destroy())
+        destroyPromises.push(client.destroy(err))
       }
     }
-    return Promise.all(destroyPromises)
+
+    if (!callback) {
+      return Promise.all(destroyPromises)
+    }
+
+    Promise.all(destroyPromises)
+      .then(() => process.nextTick(callback))
+      .catch((e) => process.nextTick(callback, e))
   }
 }
 

test/agent.js

@@ -33,6 +33,55 @@ test('Agent', t => {
   t.doesNotThrow(() => new Agent())
 })
 
+test('agent should call callback after closing internal pools', t => {
+  t.plan(2)
+
+  const wanted = 'payload'
+
+  const server = http.createServer((req, res) => {
+    res.setHeader('Content-Type', 'text/plain')
+    res.end(wanted)
+  })
+
+  t.teardown(server.close.bind(server))
+
+  server.listen(0, () => {
+    const dispatcher = new Agent()
+
+    const origin = `http://localhost:${server.address().port}`
+
+    request(origin, { dispatcher })
+      .then(() => {
+        t.pass('first request should resolve')
+      })
+      .catch(err => {
+        t.fail(err)
+      })
+
+    dispatcher.once('connect', () => {
+      dispatcher.close(() => {
+        request(origin, { dispatcher })
+          .then(() => {
+            t.fail('second request should not resolve')
+          })
+          .catch(err => {
+            t.ok(err instanceof errors.ClientClosedError)
+          })
+      })
+    })
+  })
+})
+
+test('agent close throws when callback is not a function', t => {
+  t.plan(1)
+  const dispatcher = new Agent()
+  try {
+    dispatcher.close({})
+  } catch (err) {
+    t.ok(err instanceof errors.InvalidArgumentError)
+  }
+})
+
 test('agent should close internal pools', t => {
   t.plan(2)
 
@@ -71,6 +120,55 @@ test('agent should close internal pools', t => {
   })
 })
 
+test('agent should destroy internal pools and call callback', t => {
+  t.plan(2)
+
+  const wanted = 'payload'
+
+  const server = http.createServer((req, res) => {
+    res.setHeader('Content-Type', 'text/plain')
+    res.end(wanted)
+  })
+
+  t.teardown(server.close.bind(server))
+
+  server.listen(0, () => {
+    const dispatcher = new Agent()
+
+    const origin = `http://localhost:${server.address().port}`
+
+    request(origin, { dispatcher })
+      .then(() => {
+        t.fail()
+      })
+      .catch(err => {
+        t.ok(err instanceof errors.ClientDestroyedError)
+      })
+
+    dispatcher.once('connect', () => {
+      dispatcher.destroy(() => {
+        request(origin, { dispatcher })
+          .then(() => {
+            t.fail()
+          })
+          .catch(err => {
+            t.ok(err instanceof errors.ClientDestroyedError)
+          })
+      })
+    })
+  })
+})
+
+test('agent destroy throws when callback is not a function', t => {
+  t.plan(1)
+  const dispatcher = new Agent()
+  try {
+    dispatcher.destroy(new Error('mock error'), {})
+  } catch (err) {
+    t.ok(err instanceof errors.InvalidArgumentError)
+  }
+})
+
 test('agent should destroy internal pools', t => {
   t.plan(2)