fix: apm.removePatch(name, somethingNotRegistered) would remove last …

…handler (#2315)

There was a bug in the Instrumentation class's data structure for added
patches where removePatch(name, somethingBogus) would "find" that
handler at index -1 and remove the last handler.

CHANGELOG.asciidoc

@@ -44,6 +44,10 @@ Notes:
 [float]
 ===== Bug fixes
 
+* Fix a bug in `apm.removePatch(module, aHandler)` that would remove the
+  last registered handler if `aHandler` did not match any currently
+  registered handlers. ({pull}2315[#2315])
+
 * Fix a crash in instrumentation of the old Elasticsearch client
   (`elasticsearch`) for some rarer cases of client options -- for example
   passing multiple hosts. ({pull}2312[#2312])

lib/instrumentation/named-array.js

@@ -34,9 +34,11 @@ class NamedArray {
     const array = this.get(key)
     if (array) {
       const index = array.indexOf(value)
-      array.splice(index, 1)
-      if (!array.length) {
-        this.clear(key)
+      if (index !== -1) {
+        array.splice(index, 1)
+        if (!array.length) {
+          this.clear(key)
+        }
       }
     }
   }

test/agent.test.js

@@ -1680,6 +1680,38 @@ test('patches', function (t) {
     agent.destroy()
     t.end()
   })
+
+  t.test('#removePatch(name, oops) does not remove patches', function (t) {
+    const agent = new Agent().start(agentOptsNoopTransport)
+
+    const moduleName = 'removePatch-test-module'
+    t.notOk(agent._instrumentation._patches.has(moduleName))
+
+    const handler1 = function (exports) { return exports }
+    const handler2 = function (exports) { return exports }
+    agent.addPatch(moduleName, handler1)
+    agent.addPatch(moduleName, handler2)
+    const modulePatches = agent._instrumentation._patches.get(moduleName)
+    t.ok(modulePatches.length === 2 &&
+      modulePatches[0] === handler1 &&
+      modulePatches[1] === handler2, 'module patches are as expected')
+
+    agent.removePatch(moduleName)
+    t.equal(agent._instrumentation._patches.get(moduleName).length, 2,
+      'still have 2 patches after removePatch(name)')
+    agent.removePatch(moduleName, 'this is not one of the registered handlers')
+    t.equal(agent._instrumentation._patches.get(moduleName).length, 2,
+      'still have 2 patches after removePatch(name, oops)')
+    agent.removePatch(moduleName, function oops () {})
+    t.equal(agent._instrumentation._patches.get(moduleName).length, 2,
+      'still have 2 patches after removePatch(name, function oops () {})')
+
+    agent.removePatch(moduleName, handler2)
+    agent.removePatch(moduleName, handler1)
+    agent.destroy()
+
+    t.end()
+  })
 })
 
 test('#registerMetric(name, labels, callback)', function (t) {