[feat] [test] Check Error stack for **all** node builtin

[loynoir]

Search Keyword

• Errors without stack trace

• swallow error stack trace

• no stack trace

• incomplete stack trace

• stack trace without script itself

What is the problem this feature will solve?

Actual

Error stack not includes /tmp/reproduce/reproduce.mjs

import { readFile } from 'node:fs/promises'

await readFile('/tmp/notexists')

node:internal/process/esm_loader:46
      internalBinding('errors').triggerUncaughtException(
                                ^

[Error: ENOENT: no such file or directory, open '/tmp/notexists'] {
  errno: -2,
  code: 'ENOENT',
  syscall: 'open',
  path: '/tmp/notexists'
}

Node.js v20.4.0

Expected

Error stack includes /tmp/reproduce/reproduce.mjs

import { readFileSync } from 'node:fs'

await readFileSync('/tmp/notexists')

node:fs:593
  handleErrorFromBinding(ctx);
  ^

Error: ENOENT: no such file or directory, open '/tmp/notexists'
    at Object.openSync (node:fs:593:3)
    at readFileSync (node:fs:461:35)
    at file:///tmp/reproduce/reproduce.mjs:3:7
    at ModuleJob.run (node:internal/modules/esm/module_job:192:25)
    at async DefaultModuleLoader.import (node:internal/modules/esm/loader:228:24)
    at async loadESM (node:internal/process/esm_loader:40:7)
    at async handleMainPromise (node:internal/modules/run_main:66:12) {
  errno: -2,
  syscall: 'open',
  code: 'ENOENT',
  path: '/tmp/notexists'
}

Node.js v20.4.0

What is the feature you are proposing to solve the problem?

Given script /tmp/reproduce/reproduce.mjs.

If it throw, expect its error stack should includes /tmp/reproduce/reproduce.mjs

However, seems some node functions not obey that.

Some thoughts.

1. Is there a full list of these kind of function, which need to be careful?

2. Would be nice to have test to check error stack include the script itself.

3. Expect error stack include script itself. Is this by spec? Or is it a convention? Or neither, just a very common case?

What alternatives have you considered?

No response

Related

#30944

[aduh95]

/cc @nodejs/fs

3. Is this by spec? Or is it a convention? Or neither, just a very common case?

AFAIK there's no spec regarding error stack traces, it's at best a convention, a common case would be a more precise way to put it.

Related

#30944

Isn't that more than just related? It looks like a duplicate to me. What's the difference between the two issues?

[loynoir]

AFAIK there's no spec regarding error stack traces, it's at best a convention, a common case would be a more precise way to put it.

The spec should describe such import thing for debug, else, very hard to figure out where goes wrong. 😢

It looks like a duplicate to me. What's the difference between the two issues?

1. I suggest to check error-stack-include-script-itself to all node builtin function test

2. Thus, gather a full list, which function user should use carefully (hard to debug stack-trace-missing-script-itself where goes wrong within codebase).

[joyeecheung]

This is specifically an issue with promise rejected from C++ land, so it's a duplicate of #30944 - the JS spec can't do anything with it, the fact that a JS land readFile() invocation can result in the later creation of an unhandled rejection somewhere (in C++) is a Node.js thing, so it's up to Node.js to thread the JS call site and the C++ rejection site together to create a meaningful stack trace for that rejection. Node.js does not normally keep track of the JS call sites in this case (because if there isn't an error, which is what usually happens, all the bookkeeping would result in a significant overhead for nothing). Although it does keep track of the information when the inspector is enabled - I think the issue described in OP still reproduces when the inspector is enabled, so I think in that case at least something can be done to put the missing pieces together and enhance the stack trace using the existing tracked information.

[github-actions]

There has been no activity on this feature request for 5 months and it is unlikely to be implemented. It will be closed 6 months after the last non-automated comment.

For more information on how the project manages feature requests, please consult the feature request management document.

[Gonz-coding-co]

Oh dang

…

On Mon, Feb 5, 2024 at 5:25 PM github-actions[bot] ***@***.***> wrote: There has been no activity on this feature request for 5 months and it is unlikely to be implemented. It will be closed 6 months after the last non-automated comment. For more information on how the project manages feature requests, please consult the feature request management document <https://github.com/nodejs/node/blob/HEAD/doc/contributing/feature-request-management.md> . — Reply to this email directly, view it on GitHub <#49045 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/A7QBBDGMAKY62G52LFRWGIDYSGBA5AVCNFSM6AAAAAA3GBA5LWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSMRYGYYDGOBYGY> . You are receiving this because you are subscribed to this thread.Message ID: ***@***.***>

[legendecas]

Closed in duplicated of #30944.