From bd7411cdcad99a69eda9073e637e7935341881f3 Mon Sep 17 00:00:00 2001 From: om-ghante Date: Sat, 11 Apr 2026 03:28:00 +0530 Subject: [PATCH] module: fix wrong error annotation for require of ESM When a CommonJS module requires an ES module with --no-experimental-require-module, the error annotation (arrow message) was pointing to an internal frame (TracingChannel.traceSync in node:diagnostics_channel) instead of the user's actual require() call. This happened because reconstructErrorStack() was always picking the first 'at' frame from the error stack trace. When the require() call is wrapped by TracingChannel.traceSync (added in v22.4.0 via wrapModuleLoad), the first frame is the internal tracing wrapper, not the user's code. Fix reconstructErrorStack() to search the stack frames for one that matches the parent module's file path, instead of blindly using the first frame. This ensures the error annotation correctly points to the user's require() call. Also remove a TODO comment that this change addresses. Fixes: https://github.com/nodejs/node/issues/55350 --- lib/internal/modules/cjs/loader.js | 18 +++++-- .../test-cjs-esm-error-annotation.js | 50 +++++++++++++++++++ 2 files changed, 64 insertions(+), 4 deletions(-) create mode 100644 test/es-module/test-cjs-esm-error-annotation.js 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}"` + ); + }); +});