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[bot]]

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

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%)	:arrow_up:

... and 23 files with indirect coverage changes

[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/

[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

[geeksilva97]

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

Would you guide me how I can do that? I tried to add it in a few places:

• wrapping traseSync
• wrapping `Module._extensions['.js']
• wrapping Module._load

None worked. It ended up messing with the stack.

at TracingChannel.traceSync (node:diagnostics_channel:322:14)
    at wrapModuleLoad (node:internal/modules/cjs/loader:220:24)
    at Function.executeUserEntryPoint [as runMain] (node:internal/modules/run_main:170:5)
    at node:internal/main/run_main_module:36:49 {

It adds this wrapModuleLoad and some more frames.

[nodejs-github-bot]

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

[nodejs-github-bot]

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

[nodejs-github-bot]

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

[aduh95]

@RafaelGSS are you still blocking?

[geeksilva97]

@RafaelGSS are you still blocking?

he wanted to check if all tests would pass before. Tests were failing on windows though. A friend of mine helped me out and I realized it was due to backslashes.

I made some changes - and added some debugging stuff - hopefully, the next CI will pass. If not, at least, it will be clear what is missing. I will then remove this commit

[nodejs-github-bot]

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