Skip to content

Commit 661eb1a

Browse files
authored
fix: Updated transformer to prevent instrumenting a non-existent method on a class (#59)
1 parent 07e6472 commit 661eb1a

7 files changed

Lines changed: 90 additions & 11 deletions

File tree

lib/transformer.js

Lines changed: 34 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -95,8 +95,9 @@ class Transformer {
9595
state.operator = this.#getOperator(state)
9696

9797
esquery.traverse(ast, esquery.parse(query), (...args) => {
98-
injectionCount++
99-
this.#visit(state, ...args)
98+
if (this.#visit(state, ...args)) {
99+
injectionCount++
100+
}
100101
})
101102
}
102103

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

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

156158
state.functionIndex = ++state.functionIndex || 0
157159

158-
if (index !== null && index !== state.functionIndex) return
160+
if (index !== null && index !== state.functionIndex) return false
161+
} else {
162+
// For class nodes, validate that the method exists before instrumenting
163+
const { methodName } = state.functionQuery
164+
if (methodName) {
165+
// Handle both ClassDeclaration/ClassExpression and VariableDeclarator with ClassExpression init
166+
const classNode = node.type === 'VariableDeclarator' ? node.init : node
167+
const classBody = classNode.body
168+
const methodExists = classBody.body.some(({ key }) => key?.name === methodName)
169+
170+
if (!methodExists) {
171+
// Method doesn't exist on the class. Check if any subclass has it.
172+
const className = classNode.id?.name || node.id?.name
173+
const program = ancestry[ancestry.length - 1]
174+
175+
if (className) {
176+
const hasMethodInSubclass = esquery.query(
177+
program,
178+
`ClassDeclaration[superClass.name="${className}"] > ClassBody > MethodDefinition[key.name="${methodName}"]`
179+
).length > 0
180+
181+
if (!hasMethodInSubclass) {
182+
throw new Error(`Method '${methodName}' not found on class '${className}' or any of its subclasses`)
183+
}
184+
}
185+
}
186+
}
159187
}
160188

161189
transform(state, ...args)
190+
return true
162191
}
163192

164193
/**

tests/injection_failure/test.mjs

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,5 @@
1-
/**
2-
* Unless explicitly stated otherwise all files in this repository are licensed under the Apache-2.0 License.
3-
* This product includes software developed at Datadog (https://www.datadoghq.com/). Copyright 2025 Datadog, Inc.
4-
**/
51
import { existsSync } from 'node:fs'
62
import { assert } from '../common/preamble.js'
73

8-
const instrumented = existsSync('./instrumented.js')
9-
assert.strictEqual(instrumented, false, 'instrumented.js should not exist')
4+
const instrumented = existsSync('./instrumented.mjs')
5+
assert.strictEqual(instrumented, false, 'instrumented.mjs should not exist')
Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
class Foo {
2+
constructor () {
3+
this.value = 42
4+
}
5+
6+
doStuff () {
7+
return true
8+
}
9+
}
10+
11+
export default Foo
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
import { existsSync } from 'node:fs'
2+
import { assert } from '../common/preamble.js'
3+
4+
const instrumented = existsSync('./instrumented.mjs')
5+
assert.strictEqual(instrumented, false, 'instrumented.mjs should not exist')
Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
class Base {}
2+
3+
class Foo extends Base {
4+
doStuff () {
5+
return true
6+
}
7+
}
8+
9+
export default Foo
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
import { existsSync } from 'node:fs'
2+
import { assert } from '../common/preamble.js'
3+
4+
const instrumented = existsSync('./instrumented.mjs')
5+
assert.strictEqual(instrumented, false, 'instrumented.mjs should not exist')

tests/tests.test.mjs

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -196,6 +196,30 @@ describe('injection_failure', () => {
196196
})
197197
})
198198

199+
describe('injection_failure_method_name', () => {
200+
test('does not write instrumented file when no injection points found', () => {
201+
runTest('injection_failure_method_name', [
202+
{
203+
channelName: 'injection_failure_method_name',
204+
module: { name: TEST_MODULE_NAME, versionRange: '>=0.0.1', filePath: TEST_MODULE_PATH },
205+
functionQuery: { className: 'Foo', methodName: 'nonExistentMethod', kind: 'Async' },
206+
},
207+
], { mjs: true })
208+
})
209+
})
210+
211+
describe('injection_failure_method_name_sub_class', () => {
212+
test('does not write instrumented file when no injection points found', () => {
213+
runTest('injection_failure_method_name_subclass', [
214+
{
215+
channelName: 'injection_failures_subclass_method_name',
216+
module: { name: TEST_MODULE_NAME, versionRange: '>=0.0.1', filePath: TEST_MODULE_PATH },
217+
functionQuery: { className: 'Base', methodName: 'nonExistentMethod', kind: 'Async' },
218+
},
219+
], { mjs: true })
220+
})
221+
})
222+
199223
describe('multiple_class_method_cjs', () => {
200224
test('instruments multiple class methods', () => {
201225
runTest('multiple_class_method_cjs', [

0 commit comments

Comments
 (0)