util: highlight stack frames

[BridgeAR]

Using util.inspect on errors is going to highlight userland and
node_module stack frames from now on. This is done by marking Node.js
core frames grey and frames that contain node_modules in their path
yellow.

That way it's easy to grasp what frames belong to what code.

Checklist

• make -j4 test (UNIX), or vcbuild test (Windows) passes
• tests and/or benchmarks are included
• documentation is changed or added
• commit message follows commit guidelines

[nodejs-github-bot]

Lite-CI: https://ci.nodejs.org/job/node-test-pull-request-lite-pipeline/3142

[mscdex]

I'm guessing those added tests do not run on Windows? I think we'd need to test there as well.

[addaleax]

That way it's easy to grasp what frames belong to what code.

Looking at your screenshot, it feels like the node_modules lines are highlighted over the “regular” ones. I’d either reverse that, so that “own” code is being highlighted, or just mix the two together as one set of unstyled stack frames?

[BridgeAR]

@addaleax

I’d either reverse that, so that “own” code is being highlighted, or just mix the two together as one set of unstyled stack frames?

I played around with it and I originally implemented it the other way around. I choose this in the end since yellow is darker than the white frames and the last frame in the stack will likely be a userland frame and that works well with the error message that is not highlighted.
I also tried to highlight only parts of the frame, used underscores and other colors but at least for me highlighting the whole frame in these colors seemed best (so far).
I have no strong opinion on how exactly it should be highlighted and I'll play around with it a bit further and attach a couple of screenshots for comparison. I'll also add some screenshots with a white background color.

I am also fine to land this in two PRs: first grey out Node.js core frames and later distinguish node_modules from userland code?

[addaleax]

I choose this in the end since yellow is darker than the white frames and the last frame in the stack will likely be a userland frame and that works well with the error message that is not highlighted.

That’s really dependent on your terminal and terminal settings, and in particular, we’ve had reports of yellow being hard to read on a bright background.

Maybe it would help if you could explain why it is important to distinguish node_modules frames from other userland frames?

[BridgeAR]

@addaleax

That’s really dependent on your terminal and terminal settings

Definitely. Here is an overview for different terminals and how their colors would look like: https://en.wikipedia.org/wiki/ANSI_escape_code#3/4_bit

Yellow is indeed relatively dark. I originally choose that on purpose as I expect people to mainly focus on their own code and that errors hopefully come mainly from their own code and not from modules that they use.
But I agree that it might be too much in this case. So I changed to be significantly less intrusive by just using an underline for node_modules names.

[...] why [is it] important to distinguish node_modules frames from other userland frames?

I would like to immediately see what node_module was used without having to look more closely at the stack. That's why I differentiated both.

---

Here a couple of screenshots from alternatives that I checked as comparison. I do not believe that this solution is the best or that there is "a best" solution only that this is an improvement over the current situation. There's still significant room for improvement.

If anyone would rather have a different solution, please let me know! I guess we could just set up a small poll and vote in that case?

I added another safe guard against detecting non core frames as such by checking if the file is indeed a NativeModule.

I also added cross platform tests and updated the documentation.

@nodejs/util @bnoordhuis @devsnek @addaleax PTAL

[BridgeAR]

I marked this as semver-minor as it feels like a feature addition.

[devsnek]

i like the white bold one

[BridgeAR]

@devsnek

i like the white bold one

It's a bit hard to actually identify the differences with it, don't you think so? And are you also fine with the PR as it is?

[BridgeAR]

@nodejs/util PTAL. I think this quite a nice feature and it would be great if this could land soonish :)

[jasnell]

I like the general idea but would make it easier for the user to modify the styles l for each. I know users can already modify the color scheme, but allowing them to select the specific style allows for more variation

[BridgeAR]

I [...] would make it easier for the user to modify the styles l for each. [...] allowing them to select the specific style allows for more variation

Can you elaborate? The color (ANSI code) also stands for e.g., underscores, background color, bold etc.

Do you want the user to also modify the default part (the regular white part)?

[BridgeAR]

Ping @jasnell. I am not sure what exactly you'd like the user to modify.