module: fix error reporting

[geeksilva97]

Refs: #55350
Fixes: #55350

The error is incorrectly reported because the traceSync call is on the stack frame. The code that computes the message was getting the first frame expecting it to have the information needed. Something like

at Object.<anonymous> (/Users/edysilva/test-node/issue-55350/test.cjs:1:1)

With traceSync it's like

 at TracingChannel.traceSync (node:diagnostics_channel:322:14)
 at Object.<anonymous> (/Users/edysilva/test-node/issue-55350/test.cjs:1:1)

This PR fixes this behavior by skipping cutting frames about the user's frame.

[nodejs-github-bot]

Review requested:

• @nodejs/loaders

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 87.99%. Comparing base (4ee87b8) to head (7a97799).
Report is 71 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #55561      +/-   ##
==========================================
- Coverage   88.00%   87.99%   -0.01%     
==========================================
  Files         656      656              
  Lines      189000   189002       +2     
  Branches    35995    35991       -4     
==========================================
- Hits       166320   166313       -7     
- Misses      15840    15847       +7     
- Partials     6840     6842       +2     

Files with missing lines	Coverage Δ
lib/internal/modules/cjs/loader.js	94.31% <100.00%> (+<0.01%)	⬆️

... and 23 files with indirect coverage changes

[mcollina]

Thanks for opening a PR! Can you please add a unit test?

[geeksilva97]

Thanks for opening a PR! Can you please add a unit test?

Sure! It's done 🫡

[RafaelGSS]

The PR changes this behavior by making the error reporting to get the last frame on the stack since it will be where all calls were triggered from.

I think this isn't what the error reporting should do? Let me investigate

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/63354/

[RafaelGSS]

See:

function main() {
  func1(func2(func3()))
}

function func1() {
  require('./app.js')
}
function func2() {}
function func3() {}

main()

Correct output:

➜  undefined git:(55350-issue) ✗ node -v
v22.3.0
➜  undefined git:(55350-issue) ✗ node test.cjs
/Users/rafaelgss/repos/os/node2/undefined/test.cjs:6
  require('./app.js')
  ^

Error [ERR_REQUIRE_ESM]: require() of ES Module /Users/rafaelgss/repos/os/node2/undefined/app.js from /Users/rafaelgss/repos/os/node2/undefined/test.cjs not supported.
Instead change the require of app.js in /Users/rafaelgss/repos/os/node2/undefined/test.cjs to a dynamic import() which is available in all CommonJS modules.
    at func1 (/Users/rafaelgss/repos/os/node2/undefined/test.cjs:6:3)
    at main (/Users/rafaelgss/repos/os/node2/undefined/test.cjs:2:3)
    at Object.<anonymous> (/Users/rafaelgss/repos/os/node2/undefined/test.cjs:11:1) {
  code: 'ERR_REQUIRE_ESM'
}

Node.js v22.3.0

After #44340:

➜  undefined git:(55350-issue) ✗ node test.cjs
/Users/rafaelgss/repos/os/node2/undefined/test.cjs:315
undefined
             ^

Error [ERR_REQUIRE_ESM]: require() of ES Module /Users/rafaelgss/repos/os/node2/undefined/app.js from /Users/rafaelgss/repos/os/node2/undefined/test.cjs not supported.
Instead change the require of app.js in /Users/rafaelgss/repos/os/node2/undefined/test.cjs to a dynamic import() which is available in all CommonJS modules.
    at TracingChannel.traceSync (node:diagnostics_channel:315:14)
    at func1 (/Users/rafaelgss/repos/os/node2/undefined/test.cjs:6:3)
    at main (/Users/rafaelgss/repos/os/node2/undefined/test.cjs:2:3)
    at Object.<anonymous> (/Users/rafaelgss/repos/os/node2/undefined/test.cjs:11:1) {
  code: 'ERR_REQUIRE_ESM'
}

Node.js v22.4.0

Your PR:

➜  undefined git:(55350-issue) ✗ ../node --no-experimental-require-module test.cjs
/Users/rafaelgss/repos/os/node2/undefined/test.cjs:11
main()
^

Error [ERR_REQUIRE_ESM]: require() of ES Module /Users/rafaelgss/repos/os/node2/undefined/app.js from /Users/rafaelgss/repos/os/node2/undefined/test.cjs not supported.
Instead change the require of app.js in /Users/rafaelgss/repos/os/node2/undefined/test.cjs to a dynamic import() which is available in all CommonJS modules.
    at TracingChannel.traceSync (node:diagnostics_channel:322:14)
    at func1 (/Users/rafaelgss/repos/os/node2/undefined/test.cjs:6:3)
    at main (/Users/rafaelgss/repos/os/node2/undefined/test.cjs:2:3)
    at Object.<anonymous> (/Users/rafaelgss/repos/os/node2/undefined/test.cjs:11:1) {
  code: 'ERR_REQUIRE_ESM'
}

Node.js v24.0.0-pre

I apologize; I haven’t had the bandwidth to look into how the fix should be structured yet. However, this change appears to be inaccurate

[geeksilva97]

The PR changes this behavior by making the error reporting to get the last frame on the stack since it will be where all calls were triggered from.

I think this isn't what the error reporting should do? Let me investigate

Wrong assumption from my side. Thanks for pointing that out. I just pushed a fix.

It takes the first frame after TraseSync. I will also get this case you brought into a test.

[RafaelGSS]

Wrong assumption from my side. Thanks for pointing that out. I just pushed a fix.

It takes the first frame after TraseSync. I will also get this case you brought into a test.

Can you try to use hideStackFrames instead? So we don't need to change the error stack trace creation. Example: https://github.com/nodejs/node/blob/main/lib/_http_outgoing.js#L667