Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 34 additions & 5 deletions lib/transformer.js
Original file line number Diff line number Diff line change
Expand Up @@ -95,8 +95,9 @@ class Transformer {
state.operator = this.#getOperator(state)

esquery.traverse(ast, esquery.parse(query), (...args) => {
injectionCount++
this.#visit(state, ...args)
if (this.#visit(state, ...args)) {
injectionCount++
}
})
}

Expand Down Expand Up @@ -137,11 +138,12 @@ class Transformer {
*
* @param {object} state - Merged config + runtime state for this traversal.
* @param {...unknown} args - `(node, parent, ancestry)` from esquery traverse.
* @returns {boolean} True if injection should be counted, false otherwise.
*/
#visit (state, ...args) {
const transform = this.#customTransforms[state.operator] ?? transforms[state.operator]
const { index = 0 } = state.functionQuery
const [node] = args
const [node, , ancestry] = args
const type = node.init?.type || node.type

// Class nodes are visited for traceInstanceMethod (missing method patching),
Expand All @@ -151,14 +153,41 @@ class Transformer {
// nested class like `let Server = (() => { class Server {} })()`) is only
// matched because `[id.name="Server"]` is broad. It is not a function node
// and should not be instrumented or counted toward the function index.
if (node.type === 'VariableDeclarator') return
if (node.type === 'VariableDeclarator') return false

state.functionIndex = ++state.functionIndex || 0

if (index !== null && index !== state.functionIndex) return
if (index !== null && index !== state.functionIndex) return false
} else {
// For class nodes, validate that the method exists before instrumenting
const { methodName } = state.functionQuery
if (methodName) {
// Handle both ClassDeclaration/ClassExpression and VariableDeclarator with ClassExpression init
const classNode = node.type === 'VariableDeclarator' ? node.init : node
const classBody = classNode.body
const methodExists = classBody.body.some(({ key }) => key?.name === methodName)

if (!methodExists) {
// Method doesn't exist on the class. Check if any subclass has it.
const className = classNode.id?.name || node.id?.name
const program = ancestry[ancestry.length - 1]

if (className) {
const hasMethodInSubclass = esquery.query(
program,
`ClassDeclaration[superClass.name="${className}"] > ClassBody > MethodDefinition[key.name="${methodName}"]`
).length > 0

if (!hasMethodInSubclass) {
throw new Error(`Method '${methodName}' not found on class '${className}' or any of its subclasses`)
}
}
}
}
}

transform(state, ...args)
return true
}

/**
Expand Down
8 changes: 2 additions & 6 deletions tests/injection_failure/test.mjs
Original file line number Diff line number Diff line change
@@ -1,9 +1,5 @@
/**
* Unless explicitly stated otherwise all files in this repository are licensed under the Apache-2.0 License.
* This product includes software developed at Datadog (https://www.datadoghq.com/). Copyright 2025 Datadog, Inc.
**/
import { existsSync } from 'node:fs'
import { assert } from '../common/preamble.js'

const instrumented = existsSync('./instrumented.js')
assert.strictEqual(instrumented, false, 'instrumented.js should not exist')
const instrumented = existsSync('./instrumented.mjs')
assert.strictEqual(instrumented, false, 'instrumented.mjs should not exist')
11 changes: 11 additions & 0 deletions tests/injection_failure_method_name/mod.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
class Foo {
constructor () {
this.value = 42
}

doStuff () {
return true
}
}

export default Foo
5 changes: 5 additions & 0 deletions tests/injection_failure_method_name/test.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
import { existsSync } from 'node:fs'
import { assert } from '../common/preamble.js'

const instrumented = existsSync('./instrumented.mjs')
assert.strictEqual(instrumented, false, 'instrumented.mjs should not exist')
9 changes: 9 additions & 0 deletions tests/injection_failure_method_name_subclass/mod.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
class Base {}

class Foo extends Base {
doStuff () {
return true
}
}

export default Foo
5 changes: 5 additions & 0 deletions tests/injection_failure_method_name_subclass/test.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
import { existsSync } from 'node:fs'
import { assert } from '../common/preamble.js'

const instrumented = existsSync('./instrumented.mjs')
assert.strictEqual(instrumented, false, 'instrumented.mjs should not exist')
24 changes: 24 additions & 0 deletions tests/tests.test.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -196,6 +196,30 @@ describe('injection_failure', () => {
})
})

describe('injection_failure_method_name', () => {
test('does not write instrumented file when no injection points found', () => {
runTest('injection_failure_method_name', [
{
channelName: 'injection_failure_method_name',
module: { name: TEST_MODULE_NAME, versionRange: '>=0.0.1', filePath: TEST_MODULE_PATH },
functionQuery: { className: 'Foo', methodName: 'nonExistentMethod', kind: 'Async' },
},
], { mjs: true })
})
})

describe('injection_failure_method_name_sub_class', () => {
test('does not write instrumented file when no injection points found', () => {
runTest('injection_failure_method_name_subclass', [
{
channelName: 'injection_failures_subclass_method_name',
module: { name: TEST_MODULE_NAME, versionRange: '>=0.0.1', filePath: TEST_MODULE_PATH },
functionQuery: { className: 'Base', methodName: 'nonExistentMethod', kind: 'Async' },
},
], { mjs: true })
})
})

describe('multiple_class_method_cjs', () => {
test('instruments multiple class methods', () => {
runTest('multiple_class_method_cjs', [
Expand Down
Loading