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: #55350

Author: omghante

lib/internal/modules/cjs/loader.js

@@ -444,13 +444,13 @@ ObjectDefineProperty(Module.prototype, 'parent', {
   get: pendingDeprecate(
     getModuleParent,
     'module.parent is deprecated due to accuracy issues. Please use ' +
-      'require.main to find program entry point instead.',
+    'require.main to find program entry point instead.',
     'DEP0144',
   ),
   set: pendingDeprecate(
     setModuleParent,
     'module.parent is deprecated due to accuracy issues. Please use ' +
-      'require.main to find program entry point instead.',
+    'require.main to find program entry point instead.',
     'DEP0144',
   ),
 });
@@ -539,7 +539,7 @@ function tryPackage(requestPath, exts, isMain, originalPath) {
     } else {
       process.emitWarning(
         `Invalid 'main' field in '${pjsonPath}' of '${pkg}'. ` +
-          'Please either fix that or report it to the module author',
+        'Please either fix that or report it to the module author',
         'DeprecationWarning',
         'DEP0128',
       );
@@ -814,7 +814,7 @@ Module._findPath = function(request, paths, isMain, conditions = getCjsCondition
 };
 
 /** `node_modules` character codes reversed */
-const nmChars = [ 115, 101, 108, 117, 100, 111, 109, 95, 101, 100, 111, 110 ];
+const nmChars = [115, 101, 108, 117, 100, 111, 109, 95, 101, 100, 111, 110];
 const nmLen = nmChars.length;
 if (isWindows) {
   /**
@@ -833,8 +833,8 @@ if (isWindows) {
     // return root node_modules when path is 'D:\\'.
     // path.resolve will make sure from.length >=3 in Windows.
     if (StringPrototypeCharCodeAt(from, from.length - 1) ===
-          CHAR_BACKWARD_SLASH &&
-        StringPrototypeCharCodeAt(from, from.length - 2) === CHAR_COLON) {
+      CHAR_BACKWARD_SLASH &&
+      StringPrototypeCharCodeAt(from, from.length - 2) === CHAR_COLON) {
       return [from + 'node_modules'];
     }
 
@@ -848,8 +848,8 @@ if (isWindows) {
       // path for drive root like 'C:\node_modules' and don't need to
       // parse drive name.
       if (code === CHAR_BACKWARD_SLASH ||
-          code === CHAR_FORWARD_SLASH ||
-          code === CHAR_COLON) {
+        code === CHAR_FORWARD_SLASH ||
+        code === CHAR_COLON) {
         if (p !== nmLen) {
           ArrayPrototypePush(
             paths,
@@ -930,7 +930,7 @@ Module._resolveLookupPaths = function(request, parent) {
 
   // Check for node modules paths.
   if (StringPrototypeCharAt(request, 0) !== '.' ||
-      (request.length > 1 &&
+    (request.length > 1 &&
       StringPrototypeCharAt(request, 1) !== '.' &&
       StringPrototypeCharAt(request, 1) !== '/' &&
       (!isWindows || StringPrototypeCharAt(request, 1) !== '\\'))) {
@@ -1019,13 +1019,13 @@ function getExportsForCircularRequire(module) {
   }
 
   if (module.exports &&
-      !isProxy(module.exports) &&
-      ObjectGetPrototypeOf(module.exports) === ObjectPrototype &&
-      // Exclude transpiled ES6 modules / TypeScript code because those may
-      // employ unusual patterns for accessing 'module.exports'. That should
-      // be okay because ES6 modules have a different approach to circular
-      // dependencies anyway.
-      !module.exports.__esModule) {
+    !isProxy(module.exports) &&
+    ObjectGetPrototypeOf(module.exports) === ObjectPrototype &&
+    // Exclude transpiled ES6 modules / TypeScript code because those may
+    // employ unusual patterns for accessing 'module.exports'. That should
+    // be okay because ES6 modules have a different approach to circular
+    // dependencies anyway.
+    !module.exports.__esModule) {
     // This is later unset once the module is done loading.
     ObjectSetPrototypeOf(
       module.exports, CircularRequirePrototypeWarningProxy);
@@ -1367,9 +1367,9 @@ Module._load = function(request, parent, isMain, internalResolveOptions = kEmpty
         }
       }
     } else if (module.exports &&
-               !isProxy(module.exports) &&
-               ObjectGetPrototypeOf(module.exports) ===
-                 CircularRequirePrototypeWarningProxy) {
+      !isProxy(module.exports) &&
+      ObjectGetPrototypeOf(module.exports) ===
+      CircularRequirePrototypeWarningProxy) {
       ObjectSetPrototypeOf(module.exports, ObjectPrototype);
     }
   }
@@ -1451,7 +1451,7 @@ Module._resolveFilename = function(request, parent, isMain, options) {
   const selfResolved = trySelf(parentPath, request, conditions);
   if (selfResolved) {
     const cacheKey = request + '\x00' +
-         (paths.length === 1 ? paths[0] : ArrayPrototypeJoin(paths, '\x00'));
+      (paths.length === 1 ? paths[0] : ArrayPrototypeJoin(paths, '\x00'));
     Module._pathCache[cacheKey] = selfResolved;
     return selfResolved;
   }
@@ -1469,7 +1469,7 @@ Module._resolveFilename = function(request, parent, isMain, options) {
   let message = `Cannot find module '${request}'`;
   if (requireStack.length > 0) {
     message = message + '\nRequire stack:\n- ' +
-              ArrayPrototypeJoin(requireStack, '\n- ');
+      ArrayPrototypeJoin(requireStack, '\n- ');
   }
   // eslint-disable-next-line no-restricted-syntax
   const err = new Error(message);
@@ -1872,9 +1872,49 @@ 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;
+    }
+  }
+  // Fallback: if no frame matched the parent path, prefer a user-land
+  // frame (skip node:internal/* and other node:-scheme frames) so the
+  // annotation points at user code, not internal wrappers.
+  if (errLine === undefined) {
+    for (let i = 0; i < stackLines.length; i++) {
+      const line = stackLines[i];
+      if (StringPrototypeIndexOf(line, '    at ') === -1) {
+        continue;
+      }
+      if (StringPrototypeIndexOf(line, 'node:internal/') !== -1) {
+        continue;
+      }
+      if (StringPrototypeIndexOf(line, '(node:') !== -1) {
+        continue;
+      }
+      errLine = line;
+      break;
+    }
+  }
+  // Last resort: if all frames are internal, use the first 'at' frame
+  // so the user still gets some error annotation rather than none.
+  if (errLine === undefined) {
+    for (let i = 0; i < stackLines.length; i++) {
+      if (StringPrototypeIndexOf(stackLines[i], '    at ') !== -1) {
+        errLine = stackLines[i];
+        break;
+      }
+    }
+  }
+  if (errLine === undefined) {
+    return;
+  }
   const { 1: line, 2: col } =
     RegExpPrototypeExec(/(\d+):(\d+)\)/, errLine) || [];
   if (line && col) {
@@ -1910,7 +1950,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);
     }
   }
@@ -2036,7 +2075,7 @@ function createRequire(filenameOrURL) {
   let filepath, fileURL;
 
   if (isURL(filenameOrURL) ||
-      (typeof filenameOrURL === 'string' && !path.isAbsolute(filenameOrURL))) {
+    (typeof filenameOrURL === 'string' && !path.isAbsolute(filenameOrURL))) {
     try {
       // It might be an URL, try to convert it.
       // If it's a relative path, it would not parse and would be considered invalid per
@@ -2064,10 +2103,10 @@ function isRelative(path) {
   if (StringPrototypeCharCodeAt(path, 0) !== CHAR_DOT) { return false; }
 
   return path.length === 1 || path === '..' ||
-  StringPrototypeStartsWith(path, './') ||
-  StringPrototypeStartsWith(path, '../') ||
-  ((isWindows && StringPrototypeStartsWith(path, '.\\')) ||
-  StringPrototypeStartsWith(path, '..\\'));
+    StringPrototypeStartsWith(path, './') ||
+    StringPrototypeStartsWith(path, '../') ||
+    ((isWindows && StringPrototypeStartsWith(path, '.\\')) ||
+      StringPrototypeStartsWith(path, '..\\'));
 }
 
 Module.createRequire = createRequire;

test/es-module/test-cjs-esm-error-annotation.js

@@ -0,0 +1,52 @@
+'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 error annotation line should contain the path to the requiring
+    // file. Do not assume it is the very first stderr line — internal
+    // throw-location lines may precede the arrow annotation.
+    const lines = stderrStr.split('\n');
+    const fileAnnotationLine = lines.find((l) => l.includes(requiringEsm));
+    assert.ok(
+      fileAnnotationLine,
+      `Expected an annotation line referencing the requiring file, got:\n` +
+      lines.slice(0, 10).join('\n')
+    );
+    assert.ok(
+      !fileAnnotationLine.includes('diagnostics_channel'),
+      `Annotation line should not reference diagnostics_channel, got: "${fileAnnotationLine}"`
+    );
+  });
+});