nodejs / node

Node.js JavaScript runtime ✨🐢🚀✨
https://nodejs.org
Other
107.35k stars 29.48k forks source link

Uncaught error stack trace is misformatted on some platforms #29387

Open Hakerh400 opened 5 years ago

Hakerh400 commented 5 years ago

Some consoles do not convert ANSI escape sequences to colors, rather display them directly to the stdout. On those consoles, libuv emulates colors by intercepting stdout stream and calling corresponding Windows API functions for setting console colors, if needed.

PR https://github.com/nodejs/node/pull/27052 introduced stack trace highlighting, but it bypasses libuv and prints the raw string. It works on Linux and on some Windows console emulators that support ANSI colors, but does not work on most Windows consoles, including the default console (cmd.exe) on Windows 8.1

As pointed out in https://github.com/nodejs/node/pull/28308#issuecomment-503790444, a possible fix is to modify PrintErrorString() to use uv_write. However, I tried different approaches, but can't get all tests to pass. I'm opening this thread so that we at least have a tracking issue for this old bug.

joyeecheung commented 5 years ago

For reference https://github.com/nodejs/node/pull/28451 is a failing attempted fix for this - I guess the proper fix should be exposing the color translation from libuv somehow and handle the color translation by ourselves. We can't use a uv_stream_t with fd 2 when there is another active stream for stderr (e.g. when process.stderr is accessed)

DerekNonGeneric commented 4 years ago

What is the status of this issue? Is anyone currently working on it? This is a really nasty bug.

BridgeAR commented 4 years ago

@DerekNonGeneric I doubt that anyone is actively working on it currently. Do you know if this is still an issue on Windows 10? We might just deactivate colors for Windows 8.1 / could try to detect what terminal is used to deactivate colors there. That might be a good intermediate solution.

Hakerh400 commented 4 years ago

Windows Command Prompt supports ANSI sequences starting from Windows 10 v.1607. [1] If we agree that temporarily disabling this on Win < v.1607 is the best solution at the moment, I would like to open a PR.

[1]: https://api.dart.dev/stable/1.24.3/dart-io/Stdout/supportsAnsiEscapes.html

DerekNonGeneric commented 4 years ago

Do you know if this is still an issue on Windows 10?

I can't really comment on Windows 10 at the moment since I am currently testing on Windows Server 2019. I can provide more detailed OS information if necessary (let me know). Here's what cmd.exe is looking like for me.

image

DerekNonGeneric commented 4 years ago

/re https://github.com/nodejs/node/pull/33132#issuecomment-620798072

This problem does not appear to be exclusive to cmd.exe (also happens on PowerShell).

image

wesgarland commented 4 years ago

ANSI control codes used to be a built-in in DOS environments...you just had to load ansi.sys from your config.sys file...whatever happened to that?

Derek - maybe a temp workaround for you? Found this on stack overflow --

FYI, in latest Windows 10, you can enable ANSI in conhost via the following reghack -- in HKCU\Console create a DWORD named VirtualTerminalLevel and set it to 0x1; then restart cmd.exe. -- You can test it with the following powershell "?[1;31mele ?[32mct ?[33mroni ?[35mX ?[36mtar ?[m".Replace('?', [char]27);. – BrainSlugs83 Oct 27 '18 at 21:17

DerekNonGeneric commented 4 years ago

@wesgarland, thanks for looking into this! I was able to confirm that the versions of cmd.exe and PowerShell on my system do indeed support ANSI. I used the supports-ansi module to first check and confirmed this as well by using the actual escape sequences.

The ANSI escape sequences are working as expected in cmd.exe and PS when using console.log (the text is being colorized). However, this does not explain why the stack trace errors do not work as expected. I suspect those errors not being processed by libuv prior to being printed.

joyeecheung commented 4 years ago

The issue is libuv only provides color code translation via uv_try_write/uv_write (when the fd is for tty) which we cannot be used when there's another active stream on the stderr file descriptor (e.g. when process.stderr has been accessed) : see https://github.com/nodejs/node/pull/28451#discussion_r298370185 - we currently directly write to stderr without going through the libuv translation so that's why you will just see the codes on Windows.

The ANSI escape sequences are working as expected in cmd.exe and PS when using console.log (the text is being colorized)

This is because console.log() goes through process.stdout which does uv_try_write underneath. I don't think we use process.stderr to write the uncaught errors, however, because..well uncaught errors could very well happen in process.stderr.write again since it's JS.

We could either expose the color translation code from libuv and do that before we write to stderr directly (without using libuv streams) - the code is quite entangled in the tty write routine so it's not easy to refactor it out though - or use a different color translation facility on Windows. Maybe we could also just close the stream with fd 2 held by process.stderr and then do a uv_try_write again (like what's done in https://github.com/nodejs/node/pull/28451) in the uncaught exception callback though I don't know if that can be done synchronously on Windows. Or maybe we can try to identify and reuse that stream somehow.

matthew-e-brown commented 2 years ago

I have started running into what I believe is the same issue using Git Bash (also on Windows). My Git Bash is configured to use MinTTY, not conhost.

node-stack-print

If this is not the same issue, let me know and I'll open a new one.

aqocyrale commented 5 months ago

this is still not fixed. when will this be fixed? the fix is really simple, in the windows C code just add this:

#include <windows.h>

void ansi_codes_enable() {
    HANDLE hOut = GetStdHandle(STD_OUTPUT_HANDLE);
    if (hOut == INVALID_HANDLE_VALUE) {
        return;
    }

    DWORD dwMode = 0;
    if (!GetConsoleMode(hOut, &dwMode)) {
        return;
    }

    dwMode |= ENABLE_VIRTUAL_TERMINAL_PROCESSING;
    SetConsoleMode(hOut, dwMode);
}

in case the problem is that ansi codes are not enabled in nodejs for some reason. if that's not the issue, just fix whatever it is. it's really hard to read error messages right now when they don't display correctly and you see the literal ansi codes instead of the effects of what they're supposed to do.