Node: Long stack traces in core

[addaleax]

With all the work currently being done around async_hooks and stack trace enhancement through e.g. source maps, I’m wondering whether we should bring official long stack trace support into core.

I’d personally love to see it – I’m certainly biased, but for debugging flaky tests alone it would make things a lot easier to not to have to pull in an external utility for that (or to have to write an ad-hoc one).

Alternatives would be to keep these utilities in userland and/or to just vendor one in but keep developing it outside of core. In particular the latter also seems good to me.

@nodejs/async_hooks @bcoe @AndreasMadsen

[benjamingr]

I'm not sure what that would entail.

Also - do you mean for things other than async functions?

I was under the impression that we already do have long stack traces in core (for async functions) with --async-stack-traces

[benjamingr]

I am obviously a big +1 on anything that makes debugging async code easier :)

[addaleax]

@benjamingr I’m talking about any async resources, not just async functions here :) So including nextTicks, timers, etc.

[cjihrig]

I'm +1 on the idea in general.

Alternatives would be to keep these utilities in userland and/or to just vendor one in but keep developing it outside of core.

I'd be OK with that if the vendored module behaved like any other core module. For example, when we added rimraf support, the userland module didn't really have the same developer experience, mostly around things like option names and error handling.

[gireeshpunathil]

sorry for asking a basic question: what is a long stack trace? a simulated chain of stack traces that belong to one logical call unit but were never available together in the thread stack due to the presences of async calls? or is it something else?

[addaleax]

@gireeshpunathil Yes, exactly.

[joyeecheung]

I guess the confusion comes from that the concept is usually referred to as "async stack trace"? (e.g. the asyncStackTrace implemented by our inspector agent when Debugger.setAsyncCallStackDepth is sent, which is implemented using async hooks). So essentially this means we are going to always do this instead of reserving it for handling Debugger.setAsyncCallStackDepth in the inspector?

[addaleax]

@joyeecheung Not always – I’m thinking that this would be enabled through e.g. a command line flag.

[joyeecheung]

@addaleax I see. I would be inclined to maintaining this in core instead of vendoring in one in considering we already have an implementation in Node.js core for the inspector. We could essentially just refactor that code so that it reports to something that can be fetched by e.g. our stack trace preparation callback. (this again brings up the issue about making it play along with user-land Error.prepareStackTrace though)

[addaleax]

(this again brings up the issue about making it play along with user-land Error.prepareStackTrace though)

I, personally, would be consider a solution along the lines of https://github.com/nodejs/node/pull/30984 to be perfect (e.g. adding callFrame.asyncDepth()), but I’m afraid there’s going to be opposition against that like there is for the other PR.

[bcoe]

@addaleax @joyeecheung just seeing this. I liked @joyeecheung's suggestion of potentially adding an additional parameter to prepareStackTrace, which could be used to lookup additional interesting information about the stack frame. I can see why folks were squeamish about simply packing this information on to the stack frame objects that originate in (I believe) V8?

Perhaps we could hammer out a little RFC document for this proposal?

[joyeecheung]

@bcoe To clarify I was just suggesting fetching the information in our internal stack trace preparation callback, which calls the user-land Error.prepareStackTrace. To also expose something to the user-land hook could be a nice-to-have, but it seems complicated so I would not consider that a blocker (first of all, be aware that we currently always respect the user-landError.prepareStackTrace from the main context so it's easy to mess up across contexts if we are not careful).

[reklatsmasters]

It also should have an option to return stack trace only for userland code.