module: fix wrong error annotation for require of ESM

[om-ghante]

When a CommonJS module requires an ES module with
--no-experimental-require-module, the error annotation (arrow message)
points to an internal frame instead of the user's actual require() call.

Before (broken):
The error annotation incorrectly points to node:diagnostics_channel:315
with content undefined, because TracingChannel.traceSync is the first
frame in the stack trace.

After (fixed):
The error annotation correctly points to the user's file and line where
require() was called, showing the actual source line.

Root Cause

The reconstructErrorStack function always picked the first at frame
from the error stack trace. Since v22.4.0, require() is wrapped by
TracingChannel.traceSync via wrapModuleLoad, so the first frame is
the internal tracing wrapper instead of the user's code.

Fix

Search the stack frames for one matching the parent module's file path,
instead of blindly using the first frame. If no frame matches (e.g.,
on a different Node.js version or with an unexpected internal wrapper),
fall back to the old behavior of using the first at frame so the
user still gets some error annotation rather than none. This ensures
robust behavior across Node.js versions while correctly skipping
internal wrappers when possible.

Fixes: #55350

[nodejs-github-bot]

Review requested:

• @nodejs/loaders

[afurm]

Nice improvement to the error annotation. The loop to find the matching stack frame is more robust than grabbing the first frame.

One concern: err may be mutated. Between getRequireESMError throwing and reconstructErrorStack running, other code could potentially mutate err.stack (especially in userland or with async context). The loop assumes err.stack is stable by the time it runs. Is there any scenario where the stack could be reformatted or truncated before this executes? The early return when errLine === undefined handles the missing frame case safely.

Also: The TODO comment // TODO(joyeecheung): trim off internal frames from the stack is removed but the issue is not fully solved by this change — internal frames from TracingChannel.traceSync are now skipped, but other internal wrappers could still appear before the user's frame. Is there a more general approach being considered, or is this fix scoped to the reported issue? Not blocking, just curious about the longer-term direction.

[om-ghante]

Nice improvement to the error annotation. The loop to find the matching stack frame is more robust than grabbing the first frame.

One concern: err may be mutated. Between getRequireESMError throwing and reconstructErrorStack running, other code could potentially mutate err.stack (especially in userland or with async context). The loop assumes err.stack is stable by the time it runs. Is there any scenario where the stack could be reformatted or truncated before this executes? The early return when errLine === undefined handles the missing frame case safely.

Also: The TODO comment // TODO(joyeecheung): trim off internal frames from the stack is removed but the issue is not fully solved by this change — internal frames from TracingChannel.traceSync are now skipped, but other internal wrappers could still appear before the user's frame. Is there a more general approach being considered, or is this fix scoped to the reported issue? Not blocking, just curious about the longer-term direction.

Thanks for the review and the thoughtful questions!

Regarding your first concern (mutation of err.stack):
The stack trace should be entirely stable here. getRequireESMError is thrown, immediately caught within loadSource (or its caller), and then reconstructErrorStack is called synchronously right after. There is no asynchronous boundary or tick cycle where userland code could intercept or mutate the stack before reconstructErrorStack executes. Even in edge cases where a process-level uncaughtException handler intercepts it, the if (errLine === undefined) return; acts as a solid fail-safe—if the stack is somehow truncated or reformatted, we just gracefully abandon the arrow annotation without throwing a secondary error.

Regarding the TODO and the scope of the fix:
Searching for the frame that matches parentPath is actually intended as the more durable, general-purpose solution compared to manually "trimming" internal frames. If we tried to trim internals by specifically ignoring node:diagnostics_channel or node:internal/modules/..., we run the risk of that list falling out of sync as the internal loader architecture evolves. By dynamically scanning for the first frame that belongs to parentPath (the requiring user module), we are effectively skipping all internal execution wrappers and landing exactly on the caller's call site. In a sense, this achieves the goal of the TODO without needing to hardcode exclusions.

[om-ghante]

Added a fallback, if no frame matches the parent path, we now fall back to the first at frame (the old behavior) instead of silently returning. This way users always get error annotation, even on different Node versions or unexpected internal wrappers.

[codecov]

Codecov Report

❌ Patch coverage is 94.44444% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 89.81%. Comparing base (de5644c) to head (34d3b97).
⚠️ Report is 23 commits behind head on main.

Files with missing lines	Patch %	Lines
lib/internal/modules/cjs/loader.js	94.44%	5 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #62685      +/-   ##
==========================================
- Coverage   89.81%   89.81%   -0.01%     
==========================================
  Files         699      699              
  Lines      216235   216498     +263     
  Branches    41336    41387      +51     
==========================================
+ Hits       194216   194449     +233     
- Misses      14129    14140      +11     
- Partials     7890     7909      +19     

Files with missing lines	Coverage Δ
lib/internal/modules/cjs/loader.js	98.17% <94.44%> (+0.02%)	⬆️

... and 46 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.