fix: Fixed issue with bluebird and when instrumentation where checking active context crashed when transaction prematurely ends (#2909)

Author: bizob2828

lib/instrumentation/when/contextualizer.js

@@ -116,7 +116,8 @@ Contextualizer.prototype = Object.create(null)
 Contextualizer.prototype.isActive = function isActive() {
   const segments = this.context.segments
   const segment = segments[this.idx] || segments[this.parentIdx] || segments[0]
-  return segment && this.context.transaction.isActive()
+  const transaction = this.getTransaction()
+  return segment && transaction?.isActive()
 }
 
 /**

lib/shim/promise-shim.js

@@ -121,7 +121,7 @@ class PromiseShim extends Shim {
           const transaction = shim.tracer.getTransaction()
           // This extra property is added by `_wrapExecutorContext` in the pre step.
           const executor = args[0]
-          const context = executor && executor[symbols.executorContext]
+          const context = executor?.[symbols.executorContext]
           if (!context || !shim.isFunction(context.executor)) {
             return
           }
@@ -387,9 +387,7 @@ function _wrapExecutorContext(shim, args) {
 function _wrapResolver(context, fn) {
   return function wrappedResolveReject(val) {
     const promise = context.promise
-    if (promise && promise[symbols.context]) {
-      promise[symbols.context].getSegment().touch()
-    }
+    promise?.[symbols.context]?.getSegment()?.touch()
     fn(val)
   }
 }
@@ -416,7 +414,7 @@ function wrapHandler({ handler, index, argsLength, useAllParams, ctx, shim }) {
   }
 
   return function __NR_wrappedThenHandler() {
-    if (!ctx.handler || !ctx.handler[symbols.context]) {
+    if (!ctx?.handler?.[symbols.context]) {
       return handler.apply(this, arguments)
     }
 
@@ -591,7 +589,8 @@ class Contextualizer {
   isActive() {
     const segments = this.context.segments
     const segment = segments[this.idx] || segments[this.parentIdx] || segments[0]
-    return segment && this.context.transaction.isActive()
+    const transaction = this.getTransaction()
+    return segment && transaction?.isActive()
   }
 
   getTransaction() {

test/lib/promises/transaction-state.js

@@ -158,4 +158,21 @@ module.exports = async function runTests({ t, agent, Promise, library }) {
     })
     await plan.completed
   })
+
+  await t.test('does not propagate context when transaction ends prematurely', async function (t) {
+    const plan = tspl(t, { plan: 2 })
+
+    helper.runInTransaction(agent, function transactionWrapper(transaction) {
+      transaction.end()
+      Promise.resolve(0)
+        .then(function step1() {
+          plan.equal(agent.getTransaction(), null)
+          return 1
+        })
+        .then(function rejector(val) {
+          plan.equal(val, 1)
+        })
+    })
+    await plan.completed
+  })
 }

test/unit/shim/promise-shim.test.js

@@ -469,6 +469,22 @@ test('PromiseShim', async (t) => {
         })
       })
     })
+
+    await t.test('should not link context through to thenned callbacks when transaction ends before Promise calls', (t, end) => {
+      const { agent, shim, TestPromise } = t.nr
+      shim.setClass(TestPromise)
+      shim.wrapCast(TestPromise, 'resolve')
+      shim.wrapThen(TestPromise.prototype, 'then')
+
+      helper.runInTransaction(agent, (tx) => {
+        tx.end()
+        TestPromise.resolve().then(() => {
+          assert.equal(agent.getTransaction(), null)
+          assert.equal(shim.getActiveSegment(), null)
+          end()
+        })
+      })
+    })
   })
 
   await t.test('#wrapThen', async (t) => {
@@ -519,6 +535,21 @@ test('PromiseShim', async (t) => {
       })
     })
 
+    await t.test('should not link context through to thenned callbacks when transaction ends before Promise calls', (t, end) => {
+      const { agent, shim, TestPromise } = t.nr
+      shim.setClass(TestPromise)
+      shim.wrapThen(TestPromise.prototype, 'then')
+
+      helper.runInTransaction(agent, (tx) => {
+        tx.end()
+        TestPromise.resolve().then(() => {
+          assert.equal(agent.getTransaction(), null)
+          assert.equal(shim.getActiveSegment(), null)
+          end()
+        })
+      })
+    })
+
     await t.test('should wrap both handlers', (t) => {
       const { shim, TestPromise } = t.nr
       shim.setClass(TestPromise)
@@ -584,6 +615,21 @@ test('PromiseShim', async (t) => {
       })
     })
 
+    await t.test('should not link context through to thenned callbacks when transaction ends before promise calls', (t, end) => {
+      const { agent, shim, TestPromise } = t.nr
+      shim.setClass(TestPromise)
+      shim.wrapCatch(TestPromise.prototype, 'catch')
+
+      helper.runInTransaction(agent, (tx) => {
+        tx.end()
+        TestPromise.reject().catch(() => {
+          assert.equal(agent.getTransaction(), null)
+          assert.equal(shim.getActiveSegment(), null)
+          end()
+        })
+      })
+    })
+
     await t.test('should only wrap the rejection handler', (t) => {
       const { shim, TestPromise } = t.nr
       shim.setClass(TestPromise)