Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

incomplete stack #31297

Closed
CTimmerman opened this issue Jan 10, 2020 · 7 comments
Closed

incomplete stack #31297

CTimmerman opened this issue Jan 10, 2020 · 7 comments
Labels
invalid Issues and PRs that are invalid.

Comments

@CTimmerman
Copy link

CTimmerman commented Jan 10, 2020

  • Version: v10.15.2
  • Platform: Linux cees-XPS-13-9380 4.15.0-1065-oem Deprecate domains #75-Ubuntu SMP Wed Nov 20 10:51:26 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux
  • Subsystem:
const http = require('http')

function includemeplz() {
    //throw new Error()  // works fine
    http.request({}).on('error', err => console.log('onerror', err.stack))  // not in stack!
}

includemeplz()

Maybe a duplicate of #11865 but that's closed and the issue still occurs with Node 13 on https://npm.runkit.com/

@bnoordhuis
Copy link
Member

The Error object includes a stack trace up to but not including your callback. That's working as expected.

Think about it, if f calls g as infunction f() { var e = new Error(); g(e) }, how could g show up in the stack trace? It isn't on the stack yet when the stack trace is captured.

@bnoordhuis bnoordhuis added the invalid Issues and PRs that are invalid. label Jan 11, 2020
@Hakerh400
Copy link
Contributor

I think OP is talking about includemeplz function missing from the stack, not the callback itself.

@CTimmerman The function is not in the stack trace because the error is thrown asynchronously, so the context where the error is constructed is outside your function. If you replace it with any asynchronous function that takes a callback (for example setTimeout) and throw an error directly from the callback, you will notice that the wrapper function is elided from the stack.

Theoretically, Node.js can internally save the call stack info upon an asynchronous function scheduling and, if an error occurs, throw the error with reconstructed stack trace, or use async stack traces. However, it may have a large performance impact (see #30944 for example).

Maybe a duplicate of #11865

No, that issue is unrelated.

@CTimmerman
Copy link
Author

I don't see how remembering the place where the async function was started would cause a significant performance issue.

@bnoordhuis
Copy link
Member

Capturing call stacks isn't free: it takes time and memory linear to the depth of the stack. That memory is the sticking point: it needs to stay around until the promise isn't observable anymore; effectively, until the promise is garbage-collected.

@CTimmerman
Copy link
Author

Capturing call stacks isn't free: it takes time and memory linear to the depth of the stack. That memory is the sticking point: it needs to stay around until the promise isn't observable anymore; effectively, until the promise is garbage-collected.

True, hence adding a single line to the stack, and maybe a "..." for the millions of skipped ones you seem to imply, should make the stack useful again.

Also the same problem happens when the containing function is async:

const axios = require("axios")

process.on('unhandledRejection', (err, p) => console.log(err.stack))

;(async function includemeplz() {
	await axios.get('/')
})()

@bnoordhuis
Copy link
Member

Even a single-frame call stack isn't cheap (enough). If it was, we would have done it a long time ago.

There's plenty of infrastructure for long stack traces (async_hooks, the inspector) and several npm modules exist that utilize it but it's not enabled by default. See #31080 for a proposal.

@CTimmerman
Copy link
Author

Even a single-frame call stack isn't cheap (enough). If it was, we would have done it a long time ago.

Error: connect ECONNREFUSED 127.0.0.1:80
    at TCPConnectWrap.afterConnect [as oncomplete] (net.js:1104:14)

That's a stack frame i don't care about so would prefer to see this:

Error: connect ECONNREFUSED 127.0.0.1:80
    at includemeplz (/home/cees/code/tester/test.js:17:8)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
invalid Issues and PRs that are invalid.
Projects
None yet
Development

No branches or pull requests

3 participants