-
Notifications
You must be signed in to change notification settings - Fork 30k
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
errors: skip fatal error highlighting on windows #33132
Conversation
Hmm... I understand with and agree with the general nature of this change but it still bothers me. Presumptively, this is an issue for more than just printing the error stack and I really dislike being forced to spread platform specific checks throughout the code as those become harder to maintain (or even keep track of) over time. I'm not sure I have a better suggestion right now but I'm reluctant to +1 this. |
I am also reluctant to completely disable colors on fatal errors in Windows. The issue about the Windows version seems to be around differentiating Windows Server 2019 from regular Windows 10. If there's any other possibility to detect for example if |
It's interesting how all tests passed, while none of them is modified. I actually see why - this change cannot be detected programmatically on Windows (or at least very hard).
@jasnell. I hope we all realize this change is temporary. The actual fix is described in #29387 (comment), but it requires a lot of refactoring and also requires some changes in libuv. Given that this issue has been making troubles for a lot of Windows users for more than a year, wouldn't it be better to disable highlighting while we discuss how to actually resolve the issue?
@BridgeAR. I added additional check, PTAL. In general, there are no official Microsoft documentation regarding ANSI escapes support. There is a script for detecting the ANSI support, but it is not reliable. I tested it on different virtual machines with various console emulators and it does not work in all cases. |
Any change requests here? If not, can we have this landed before 14.2.0 release (or at least before 14.3.0)? |
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. However, fatal error are handled differently and we cannot easily highlight them.
Four tests were failing due to the fact that I updated the code and rebased onto master. |
friendly ping |
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. However, fatal error are handled differently and we cannot easily highlight them. PR-URL: nodejs#33132 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]>
Landed in 1c61914 🎉 |
Glad this landed! I may be a bit late in asking, but would a |
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. However, fatal error are handled differently and we cannot easily highlight them. PR-URL: #33132 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]>
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. However, fatal error are handled differently and we cannot easily highlight them. PR-URL: #33132 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]>
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. However, fatal error are handled differently and we cannot easily highlight them. PR-URL: #33132 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]>
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.
However, fatal error are handled differently and we cannot easily
highlight them.
Fixes #29387
According to this, Windows fully supports ANSI escapes starting from v.1607 ("Anniversery Update", OS build 14393), so we could add additional check:
but given that #29387 (comment) uses build 17763, seems like that this test is not reliable.
Checklist
make -j4 test
(UNIX), orvcbuild test
(Windows) passes