nodejs / node

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

console.group and console.groupCollapsed should write to stderr #55511

Open kriskowal opened 1 month ago

kriskowal commented 1 month ago

Version

all

Platform

all

Subsystem

console

What steps will reproduce the bug?

Given the program console-group.js

console.group('group');
console.groupCollapsed('group collapsed');

run

node console-group.js 2>/dev/null

How often does it reproduce? Is there a required condition?

consistently, all versions

What is the expected behavior? Why is that the expected behavior?

The above command should produce no output. This prevents a diagnostic from inadvertently interleaving text in parsable stdout.

One could argue the same should apply to console.log and console.info and that all machine readable program output should be written to process.stdout explicitly, but that ship has sailed.

What do you see instead?

Send group labels to stderr.

Additional information

No response

RedYetiDev commented 1 month ago

This is not a bug. console.group and similar commands log to stdout, not stderr.

kriskowal commented 1 month ago

I agree that console.group and console.groupCollapsed write to stdout. Is that behavior defensible on any other grounds beyond Hyrum’s Law?

RedYetiDev commented 1 month ago

The spec leaves it up to the implementers to decide these things (https://console.spec.whatwg.org/#group), but it makes sense for it to go to stdout. It's not logging an error, so why go to stderr?

Hyrum's law is always in effect, and this behavior is compatible with all the major runtimes, so changing it (which IMO isn't necessary at all) wouldn't be an easy task

kriskowal commented 1 month ago

Your position is grounded on the assumption that console is an API for generating output as opposed to generating diagnostic information for a debugger, which I will grant is one valid interpretation. This is made explicit by the early design choice to borrow console for console.log("Hello, World!") and conflating the purpose of console for both debugging and generating output, in addition to direct manipulation of process.stdout and the omission of print.

So, to use the console object purely as an instrument of debugging, with the expectation that it will compose well with other portions of the program that write to stdout and also be portable to the web where console is purely an instrument for debugging, one must currently create a console adapter that detects whether the underlying platform is Node.js or the web, and shunt the whole console API through console.error. I would argue we can do better.

We do have the option of importing console and making one that does shunt to stderr, but importing console is not web portable.

RedYetiDev commented 1 month ago

CC @nodejs/console

kriskowal commented 1 month ago

To Hyrum’s Law, a program will break assuming that the output of group or groupCollapsed is expected to be interleaved with two-space indented log and warn and machine-readable on stdout, in Node.js, and not merely a human-readable interleaving of stdout and stderr. I believe it’s worth testing the hypothesis that this is a possible and worthwhile change. Provided that folks here come to agree and align on a design, I can propose the necessary changes.

RedYetiDev commented 1 month ago

I, for one, am -2 on this change. This will break thousands of existing programs, and break the expected behavior of logging to stdout.

Although, there are many who may have differing opinions