diff --git a/lib/internal/modules/cjs/loader.js b/lib/internal/modules/cjs/loader.js index 827655bedb65bf..b8e0f98aea9e60 100644 --- a/lib/internal/modules/cjs/loader.js +++ b/lib/internal/modules/cjs/loader.js @@ -1872,9 +1872,20 @@ function loadSource(mod, filename, formatFromNode) { } function reconstructErrorStack(err, parentPath, parentSource) { - const errLine = StringPrototypeSplit( - StringPrototypeSlice(err.stack, StringPrototypeIndexOf( - err.stack, ' at ')), '\n', 1)[0]; + // Find the stack frame that matches the parent module path. + // We cannot simply use the first frame because internal wrappers + // like TracingChannel.traceSync may appear before the user's frame. + const stackLines = StringPrototypeSplit(err.stack, '\n'); + let errLine; + for (let i = 0; i < stackLines.length; i++) { + if (StringPrototypeIndexOf(stackLines[i], parentPath) !== -1) { + errLine = stackLines[i]; + break; + } + } + if (errLine === undefined) { + return; + } const { 1: line, 2: col } = RegExpPrototypeExec(/(\d+):(\d+)\)/, errLine) || []; if (line && col) { @@ -1910,7 +1921,6 @@ function getRequireESMError(mod, pkg, content, filename) { // Continue regardless of error. } if (parentSource) { - // TODO(joyeecheung): trim off internal frames from the stack. reconstructErrorStack(err, parentPath, parentSource); } } diff --git a/test/es-module/test-cjs-esm-error-annotation.js b/test/es-module/test-cjs-esm-error-annotation.js new file mode 100644 index 00000000000000..1fb6dfaae867ea --- /dev/null +++ b/test/es-module/test-cjs-esm-error-annotation.js @@ -0,0 +1,50 @@ +'use strict'; + +// This test verifies that when a CommonJS module requires an ES module, +// the error annotation (arrow message) points to the user's require() +// call in their source file, not to an internal frame such as +// TracingChannel.traceSync in node:diagnostics_channel. +// Regression test for https://github.com/nodejs/node/issues/55350. + +const { spawnPromisified } = require('../common'); +const fixtures = require('../common/fixtures.js'); +const assert = require('node:assert'); +const path = require('node:path'); +const { execPath } = require('node:process'); +const { describe, it } = require('node:test'); + +const requiringEsm = path.resolve( + fixtures.path('/es-modules/cjs-esm-esm.js') +); + +describe('ERR_REQUIRE_ESM error annotation', { concurrency: !process.env.TEST_PARALLEL }, () => { + it('should point to the user require() call, not internal frames', async () => { + const { code, stderr } = await spawnPromisified(execPath, [ + '--no-experimental-require-module', requiringEsm, + ]); + + assert.strictEqual(code, 1); + + const stderrStr = stderr.replaceAll('\r', ''); + + // The error annotation should reference the user's file, not + // node:diagnostics_channel or any other internal module. + assert.ok( + stderrStr.includes(requiringEsm), + `Expected error output to reference ${requiringEsm}, got:\n${stderrStr}` + ); + + // The first line of the error output (before "Error [ERR_REQUIRE_ESM]") + // should contain the path to the requiring file, not "undefined". + const lines = stderrStr.split('\n'); + const fileAnnotationLine = lines[0]; + assert.ok( + fileAnnotationLine.includes(requiringEsm), + `First line should reference the requiring file, got: "${fileAnnotationLine}"` + ); + assert.ok( + !fileAnnotationLine.includes('diagnostics_channel'), + `First line should not reference diagnostics_channel, got: "${fileAnnotationLine}"` + ); + }); +});