Skip to content

[Console] Console errors are displayed twice on screen #27510

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.

Sign up for GitHub

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

Jump to bottom
Closed
yceruto opened this issue Jun 6, 2018 · 3 comments
Closed

[Console] Console errors are displayed twice on screen #27510

yceruto opened this issue Jun 6, 2018 · 3 comments
Labels
Bug Console Status: Reviewed

Comments

@yceruto
Copy link
Member

yceruto commented Jun 6, 2018 •
edited
Loading

Symfony version(s) affected: 3.4+

Description
This happens when you install a fresh Symfony app.

How to reproduce

$ composer create-project symfony/skeleton foo
$ cd foo
$ bin/console cache:

Additional context
At this moment, the kernel's logger is the one activated in the system, showing error logs on stderr stream by default.

I'm not sure if it's expected, but it looks like a visual issue for CLI apps:
logger-outerr

symfony/src/Symfony/Component/HttpKernel/Log/Logger.php

Line 40 in 9a077ca

public function __construct($minLevel = null, $output = 'php://stderr', callable $formatter = null)

symfony/src/Symfony/Component/Console/EventListener/ErrorListener.php

Line 46 in 9a077ca

$this->logger->error('Error thrown while running command "{command}". Message: "{message}"', array('exception' => $error, 'command' => $inputString, 'message' => $error->getMessage()));

@chalasr
Copy link
Member

chalasr commented Jun 6, 2018

Confirmed and I agree it is not desirable.
We already fixed that for the monolog logger on the standard edition symfony/symfony-standard#1044 (flex recipe settings are the same).

@modiamir
Copy link

modiamir commented Jun 7, 2018

Hello, should the ErrorListener prevent logging if the logger handler is stream?

@yceruto
Copy link
Member Author

yceruto commented Jun 7, 2018 •
edited
Loading

@modiamir I don't think so. The ErrorListener is not aware of the logger handler as it doesn't belong to the public API.

This affects the most CLI app that don't need a log file. So what about to configure the minLevel to LogLevel::CRITICAL by default here?:

symfony/src/Symfony/Component/HttpKernel/DependencyInjection/LoggerPass.php

Lines 38 to 39 in 4435144

$container->register('logger', Logger::class)
->setPublic(false);

This would avoid to display twice each console error by default. Later, if necessary we can set the min level by configuration:

services:
    logger:
        class: Symfony\Component\HttpKernel\Log\Logger
        arguments: ['warning']

@yceruto yceruto mentioned this issue Mar 9, 2019
Merged
@fabpot fabpot closed this as completed Mar 9, 2019
fabpot added a commit that referenced this issue Mar 9, 2019
@fabpot
bug #30497 [HttpKernel] Change default log level for output streams (…
cf728a5 
…yceruto)

This PR was merged into the 3.4 branch.

Discussion
----------

[HttpKernel] Change default log level for output streams

| Q             | A
| ------------- | ---
| Branch?       | 3.4
| Bug fix?      | yes (visual)
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #27510
| License       | MIT

**BEFORE**
![default-logger-level-before](https://user-images.githubusercontent.com/2028198/54072209-94e93980-4245-11e9-855a-2d32de797072.png)

**AFTER**
![default-logger-level-after](https://user-images.githubusercontent.com/2028198/54072210-9a468400-4245-11e9-96ec-ce5334aba72c.png)

This should improve the first DX/UX for console-based apps.

Commits
-------

d69d571 Change default log level for output streams
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Bug Console Status: Reviewed
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
6 participants
@fabpot @xabbuh @yceruto @chalasr @carsonbot @modiamir