From a0d7470fc49432cb4b83ae84b0dd73f2ecd33cca Mon Sep 17 00:00:00 2001 From: Bob Evans Date: Thu, 25 Sep 2025 18:17:39 -0400 Subject: [PATCH 1/2] fix: Updated module patch to only wrap _compile as it calls `_resolveFileName` --- index.js | 45 +++++++++++++++++---------------------------- test/index.test.js | 19 ------------------- 2 files changed, 17 insertions(+), 47 deletions(-) diff --git a/index.js b/index.js index f351f16..2e994d3 100644 --- a/index.js +++ b/index.js @@ -9,45 +9,36 @@ class ModulePatch { constructor({ instrumentations = [] } = {}) { this.packages = new Set(instrumentations.map(i => i.module.name)) this.instrumentator = create(instrumentations) - this.transformers = new Map() - this.resolve = Module._resolveFilename this.compile = Module.prototype._compile } /** - * Patches the Node.js module class methods that are responsible for resolving filePaths and compiling code. + * Patches the Node.js module class method that is responsible for compiling code. * If a module is found that has an instrumentator, it will transform the code before compiling it * with tracing channel methods. */ patch() { const self = this - Module._resolveFilename = function wrappedResolveFileName() { - const resolvedName = self.resolve.apply(this, arguments) - const resolvedModule = parse(resolvedName) + Module.prototype._compile = function wrappedCompile(...args) { + const [content, filename] = args + const resolvedModule = parse(filename) if (resolvedModule && self.packages.has(resolvedModule.name)) { + debug('found resolved module, checking if there is a transformer %s', filename) const version = getPackageVersion(resolvedModule.basedir, resolvedModule.name) const transformer = self.instrumentator.getTransformer(resolvedModule.name, version, resolvedModule.path) if (transformer) { - self.transformers.set(resolvedName, transformer) - } - } - return resolvedName - } - - Module.prototype._compile = function wrappedCompile(...args) { - const [content, filename] = args - if (self.transformers.has(filename)) { - const transformer = self.transformers.get(filename) - try { - const transformedCode = transformer.transform(content, 'unknown') - args[0] = transformedCode?.code - if (process.env.TRACING_DUMP) { - dump(args[0], filename) + debug('transforming file %s', filename) + try { + const transformedCode = transformer.transform(content, 'unknown') + args[0] = transformedCode?.code + if (process.env.TRACING_DUMP) { + dump(args[0], filename) + } + } catch (error) { + debug('Error transforming module %s: %o', filename, error) + } finally { + transformer.free() } - } catch (error) { - debug('Error transforming module %s: %o', filename, error) - } finally { - transformer.free() } } @@ -56,12 +47,10 @@ class ModulePatch { } /** - * Clears all the transformers and restores the original Module methods that were wrapped. + * Restores the original Module.prototype._compile method * **Note**: This is intended to be used in testing only. */ unpatch() { - this.transformers.clear() - Module._resolveFilename = this.resolve Module.prototype._compile = this.compile } } diff --git a/test/index.test.js b/test/index.test.js index aeab88f..fffe146 100644 --- a/test/index.test.js +++ b/test/index.test.js @@ -41,26 +41,7 @@ test('should init ModulePatch', (t) => { const { modulePatch } = t.ctx assert.ok(modulePatch instanceof ModulePatch) assert.ok(modulePatch.instrumentator) - assert.equal(modulePatch.resolve, Module._resolveFilename) assert.ok(modulePatch.compile, Module.prototype._compile) - assert.ok(modulePatch.transformers instanceof Map) -}) - -test('should set a transformer for a matched patch', (t) => { - const { modulePath, modulePatch } = t.ctx - modulePatch.patch() - Module._resolveFilename(modulePath, null, false) - assert.ok(modulePatch.transformers.has(modulePath)) - modulePatch.unpatch() - assert.equal(modulePatch.transformers.size, 0) -}) - -test('should not set a transformer for an unmatched patch', (t) => { - const { modulePatch } = t.ctx - modulePatch.patch() - const modulePath = path.join(__dirname, './example-deps/lib/node_modules/pkg-2/index.js') - Module._resolveFilename(modulePath, null, false) - assert.equal(modulePatch.transformers.size, 0) }) test('should rewrite code for a match transformer', async (t) => { From ea255820eb5931f5748ffa9382c40b4be83ea3fd Mon Sep 17 00:00:00 2001 From: Bob Evans Date: Thu, 25 Sep 2025 18:47:02 -0400 Subject: [PATCH 2/2] chore: updated snapshots --- .snapshots/7f8b6c257a075df56e54361c8433cb48/0.json | 2 +- .snapshots/814238ad8cda0e2502409f2591339137/0.json | 2 +- .snapshots/e9d3709e54638f0a8fb975da4ec13452/0.json | 2 +- .snapshots/f9e55bce8f9e3f3009403bfd870b4e79/0.json | 2 +- package.json | 5 +++-- 5 files changed, 7 insertions(+), 6 deletions(-) diff --git a/.snapshots/7f8b6c257a075df56e54361c8433cb48/0.json b/.snapshots/7f8b6c257a075df56e54361c8433cb48/0.json index 1240cbb..c6cdef6 100644 --- a/.snapshots/7f8b6c257a075df56e54361c8433cb48/0.json +++ b/.snapshots/7f8b6c257a075df56e54361c8433cb48/0.json @@ -1 +1 @@ -"const { tracingChannel: tr_ch_apm_tracingChannel } = require(\"diagnostics_channel\");\nconst tr_ch_apm$unitTestCjs = tr_ch_apm_tracingChannel(\"orchestrion:pkg-1:unitTestCjs\");\nclass Foo {\n constructor(){\n this.name = 'foo';\n }\n doStuff() {\n const __apm$original_args = arguments;\n const __apm$traced = async ()=>{\n const __apm$wrapped = async ()=>{\n return 'doing stuff';\n };\n return __apm$wrapped.apply(null, __apm$original_args);\n };\n if (!tr_ch_apm$unitTestCjs.hasSubscribers) return __apm$traced();\n return tr_ch_apm$unitTestCjs.tracePromise(__apm$traced, {\n arguments,\n self: this,\n moduleVersion: \"1.0.0\"\n });\n }\n}\nmodule.exports = Foo;\n" \ No newline at end of file +"const { tracingChannel: tr_ch_apm_tracingChannel } = require(\"diagnostics_channel\");\nconst tr_ch_apm$unitTestCjs = tr_ch_apm_tracingChannel(\"orchestrion:pkg-1:unitTestCjs\");\nclass Foo {\n constructor(){\n this.name = 'foo';\n }\n doStuff() {\n const __apm$original_args = arguments;\n const __apm$traced = ()=>{\n const __apm$wrapped = ()=>{\n return 'doing stuff';\n };\n return __apm$wrapped.apply(null, __apm$original_args);\n };\n if (!tr_ch_apm$unitTestCjs.hasSubscribers) return __apm$traced();\n return tr_ch_apm$unitTestCjs.tracePromise(__apm$traced, {\n arguments,\n self: this,\n moduleVersion: \"1.0.0\"\n });\n }\n}\nmodule.exports = Foo;\n" \ No newline at end of file diff --git a/.snapshots/814238ad8cda0e2502409f2591339137/0.json b/.snapshots/814238ad8cda0e2502409f2591339137/0.json index 1240cbb..c6cdef6 100644 --- a/.snapshots/814238ad8cda0e2502409f2591339137/0.json +++ b/.snapshots/814238ad8cda0e2502409f2591339137/0.json @@ -1 +1 @@ -"const { tracingChannel: tr_ch_apm_tracingChannel } = require(\"diagnostics_channel\");\nconst tr_ch_apm$unitTestCjs = tr_ch_apm_tracingChannel(\"orchestrion:pkg-1:unitTestCjs\");\nclass Foo {\n constructor(){\n this.name = 'foo';\n }\n doStuff() {\n const __apm$original_args = arguments;\n const __apm$traced = async ()=>{\n const __apm$wrapped = async ()=>{\n return 'doing stuff';\n };\n return __apm$wrapped.apply(null, __apm$original_args);\n };\n if (!tr_ch_apm$unitTestCjs.hasSubscribers) return __apm$traced();\n return tr_ch_apm$unitTestCjs.tracePromise(__apm$traced, {\n arguments,\n self: this,\n moduleVersion: \"1.0.0\"\n });\n }\n}\nmodule.exports = Foo;\n" \ No newline at end of file +"const { tracingChannel: tr_ch_apm_tracingChannel } = require(\"diagnostics_channel\");\nconst tr_ch_apm$unitTestCjs = tr_ch_apm_tracingChannel(\"orchestrion:pkg-1:unitTestCjs\");\nclass Foo {\n constructor(){\n this.name = 'foo';\n }\n doStuff() {\n const __apm$original_args = arguments;\n const __apm$traced = ()=>{\n const __apm$wrapped = ()=>{\n return 'doing stuff';\n };\n return __apm$wrapped.apply(null, __apm$original_args);\n };\n if (!tr_ch_apm$unitTestCjs.hasSubscribers) return __apm$traced();\n return tr_ch_apm$unitTestCjs.tracePromise(__apm$traced, {\n arguments,\n self: this,\n moduleVersion: \"1.0.0\"\n });\n }\n}\nmodule.exports = Foo;\n" \ No newline at end of file diff --git a/.snapshots/e9d3709e54638f0a8fb975da4ec13452/0.json b/.snapshots/e9d3709e54638f0a8fb975da4ec13452/0.json index ba2845e..903e5cb 100644 --- a/.snapshots/e9d3709e54638f0a8fb975da4ec13452/0.json +++ b/.snapshots/e9d3709e54638f0a8fb975da4ec13452/0.json @@ -1 +1 @@ -"import { tracingChannel as tr_ch_apm_tracingChannel } from \"diagnostics_channel\";\nconst tr_ch_apm$unitTestEsm = tr_ch_apm_tracingChannel(\"orchestrion:esm-pkg:unitTestEsm\");\nclass Foo {\n constructor(){\n this.name = 'foo';\n }\n doStuff() {\n const __apm$original_args = arguments;\n const __apm$traced = async ()=>{\n const __apm$wrapped = async ()=>{\n return 'doing stuff';\n };\n return __apm$wrapped.apply(null, __apm$original_args);\n };\n if (!tr_ch_apm$unitTestEsm.hasSubscribers) return __apm$traced();\n return tr_ch_apm$unitTestEsm.tracePromise(__apm$traced, {\n arguments,\n self: this,\n moduleVersion: \"1.0.0\"\n });\n }\n}\nexport default Foo;\n" \ No newline at end of file +"import { tracingChannel as tr_ch_apm_tracingChannel } from \"diagnostics_channel\";\nconst tr_ch_apm$unitTestEsm = tr_ch_apm_tracingChannel(\"orchestrion:esm-pkg:unitTestEsm\");\nclass Foo {\n constructor(){\n this.name = 'foo';\n }\n doStuff() {\n const __apm$original_args = arguments;\n const __apm$traced = ()=>{\n const __apm$wrapped = ()=>{\n return 'doing stuff';\n };\n return __apm$wrapped.apply(null, __apm$original_args);\n };\n if (!tr_ch_apm$unitTestEsm.hasSubscribers) return __apm$traced();\n return tr_ch_apm$unitTestEsm.tracePromise(__apm$traced, {\n arguments,\n self: this,\n moduleVersion: \"1.0.0\"\n });\n }\n}\nexport default Foo;\n" \ No newline at end of file diff --git a/.snapshots/f9e55bce8f9e3f3009403bfd870b4e79/0.json b/.snapshots/f9e55bce8f9e3f3009403bfd870b4e79/0.json index 2d653d1..546a0f5 100644 --- a/.snapshots/f9e55bce8f9e3f3009403bfd870b4e79/0.json +++ b/.snapshots/f9e55bce8f9e3f3009403bfd870b4e79/0.json @@ -1 +1 @@ -"class Foo {\n constructor(){\n this.name = 'foo';\n }\n doStuff() {\n const __apm$original_args = arguments;\n const __apm$traced = async ()=>{\n const __apm$wrapped = async ()=>{\n return 'doing stuff';\n };\n return __apm$wrapped.apply(null, __apm$original_args);\n };\n if (!tr_ch_apm$unitTest.hasSubscribers) return __apm$traced();\n return tr_ch_apm$unitTest.tracePromise(__apm$traced, {\n arguments,\n self: this,\n moduleVersion: \"1.0.0\"\n });\n }\n}" \ No newline at end of file +"class Foo {\n constructor(){\n this.name = 'foo';\n }\n doStuff() {\n const __apm$original_args = arguments;\n const __apm$traced = ()=>{\n const __apm$wrapped = ()=>{\n return 'doing stuff';\n };\n return __apm$wrapped.apply(null, __apm$original_args);\n };\n if (!tr_ch_apm$unitTest.hasSubscribers) return __apm$traced();\n return tr_ch_apm$unitTest.tracePromise(__apm$traced, {\n arguments,\n self: this,\n moduleVersion: \"1.0.0\"\n });\n }\n}" \ No newline at end of file diff --git a/package.json b/package.json index 2ad9e3f..2331826 100644 --- a/package.json +++ b/package.json @@ -10,7 +10,8 @@ }, "main": "index.js", "scripts": { - "test": "c8 borp 'test/**/*.test.{js,mjs}'" + "test": "c8 borp 'test/**/*.test.{js,mjs}'", + "test:update-snapshots": "SNAP_UPDATE=1 npm test" }, "files": [ "index.js", @@ -27,4 +28,4 @@ "borp": "^0.20.1", "c8": "^10.1.3" } -} \ No newline at end of file +}