Skip to content

Commit

Permalink
fix: Fixed issue with bluebird and when instrumentation where che…
Browse files Browse the repository at this point in the history
…cking active context crashed when transaction prematurely ends (newrelic#2909)
  • Loading branch information
bizob2828 authored Jan 29, 2025
1 parent 8d8608d commit 4a30d5c
Show file tree
Hide file tree
Showing 4 changed files with 70 additions and 7 deletions.
3 changes: 2 additions & 1 deletion lib/instrumentation/when/contextualizer.js
Original file line number Diff line number Diff line change
Expand Up @@ -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()
}

/**
Expand Down
11 changes: 5 additions & 6 deletions lib/shim/promise-shim.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down Expand Up @@ -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)
}
}
Expand All @@ -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)
}

Expand Down Expand Up @@ -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() {
Expand Down
17 changes: 17 additions & 0 deletions test/lib/promises/transaction-state.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
})
}
46 changes: 46 additions & 0 deletions test/unit/shim/promise-shim.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) => {
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down

0 comments on commit 4a30d5c

Please sign in to comment.