Skip to content

RFC Added stdout to console handler #17449

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

Closed
wants to merge 1 commit into from
Closed

RFC Added stdout to console handler #17449

wants to merge 1 commit into from

Conversation

gonzalovilaseca
Copy link
Contributor

Q A
Bug fix? yes
New feature? yes
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets
License MIT
Doc PR

Please DON'T review this PR, it's just an PoC!

Is there any reason why the ConsoleHandler is redirecting all output to stderr?

In this code ConsoleHandler writes on the error output if the level is equal or greater than ERROR, otherwise it writes into standard output.

If the community like this change I'll rework naming, interfaces etc.

@stof
Copy link
Member

stof commented Jan 19, 2016

The reason is simple: log messages are not part of the command output, but of extra information provided to humans running it.

Your issue comes from a misunderstanding of what stdout and stderr are about (naming them stdout and stderr creates this confusion btw. They are just the first and the second pipes). stderr is not only about reporting errors (reporting errors is the job of the exit code)

@@ -122,7 +124,8 @@ public function onCommand(ConsoleCommandEvent $event)
{
$output = $event->getOutput();
if ($output instanceof ConsoleOutputInterface) {
$output = $output->getErrorOutput();
$this->stdoutput = $output->getStdOutput();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think getStdOutput is a member of Symfony\Component\Console\Output\ConsoleOutputInterface or its parent Symfony\Component\Console\Output\OutputInterface

@gonzalovilaseca
Copy link
Contributor Author

Hi @stof,

Thanks for getting back to me so quickly.

I don't fully understand your point here, although I certainly acknowledge the difference between detecting command success or failure through the exit code.

However in our use-case we would like to distinguish not only between a command that failed entirely, but also between commands that had some kind of non-fatal error indicated by output sent to stderr (as opposed to stdout). It doesn't mean that the command failed as such, but the underlying application events had something negative that we would like to treat differently.

Part of the guidelines we have been using here is from Symfony documentation on http://symfony.com/doc/current/components/process.html, the section 'Getting real-time Process Output' where it's checking for Process::ERR === $type.

If we run a standard Symfony console command it sends errors (uncaught exceptions) to stderr and general output from the OutputInterface ($output->writeln) to stdout.

This seems at odds to the Monolog ConsoleHandler which is dumping everything to stderr. Although maybe I don't understand the true nature of underlying pipes, surely there is an inconsistency here?

@Tobion
Copy link
Contributor

Tobion commented Jan 19, 2016

See discussion in #15728. You can reconfigure it to use stdout by calling setOutput manually.

@wouterj
Copy link
Member

wouterj commented Jan 20, 2016

As Stof explained, stderr doesn't mean error output, it means not-important output. Anything that shouldn't be pipped through should be put to stderr. Logging shouldn't be piped and thus has to be send to stderr. Detecting "non-fatal failure" through checking if output was sent to stderr is wrong, stderr hasn't anything to do with failure or success.

If anything, either stderr or stdout should be used for all log levels imo (only directing log messages to stderr or stdout based on the log level seems to be incorrect, as it is based on the assumption that stderr means error).

@fabpot
Copy link
Member

fabpot commented Jan 21, 2016

Just have a look at Git output for instance, it does use stderr for non-error output.

@fabpot fabpot closed this Jan 21, 2016
@gonzalovilaseca
Copy link
Contributor Author

Actually git status is outputted to stdout, git -k (throws error) is outputted to sdterr.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants