bug #36855 [HttpKernel] Fix error logger when stderr is redirected to…

… /dev/null (fabpot)

This PR was merged into the 3.4 branch.

Discussion
----------

[HttpKernel] Fix error logger when stderr is redirected to /dev/null

| Q             | A
| ------------- | ---
| Branch?       | 3.4
| Bug fix?      | yes
| New feature?  | no <!-- please update src/**/CHANGELOG.md files -->
| Deprecations? | no <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files -->
| Tickets       | n/a <!-- prefix each issue number with "Fix #", if any -->
| License       | MIT
| Doc PR        | n/a

The HttpKernel Logger is meant to be used as a last resort logging mechanism when no logger has been explicitly configured (Monolog is not a dependency for instance).

For small apps, that can be more than enough.

But under some circumstances, it does not work. When you are using PHP-FPM, `stderr` is ignored by default (`catch_workers_output` is `false`) and so, logs are ignored as well. There is no issue with the official PHP Docker image as the setting has been explicitly set to `true`. Not an issue with Symfony CLI as well, as we also change the setting. Not a problem either with the PHP built-in server as it does not use PHP FPM anyway.

But, in many other places, where the setting has its default value, logs are lost (as you can imagine, it happened to me). As this feature is meant to be a fallback, I think it should always work, or at least, we need to make everything possible to make it work out of the box; that's why I've considered it a bug and hence a PR on 3.4.

This PR changes the default value for the output to `null`, which uses `error_log()` instead of `stderr` to log errors. Why is it better? The output of `error_log()` is controllable by the `error_logs` PHP ini setting and it is well understood by everyone (the default configuration should always work well); so it should work in most/more cases.

The other change (to be discussed) is to also log messages at the `ERROR` level and not just the `CRITICAL` ones.

/cc @dunglas

Commits
-------

5f829bd [HttpKernel] Fix error logger when stderr is redirected to /dev/null (FPM)

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

@@ -37,10 +37,10 @@ class Logger extends AbstractLogger
     private $formatter;
     private $handle;
 
-    public function __construct($minLevel = null, $output = 'php://stderr', callable $formatter = null)
+    public function __construct($minLevel = null, $output = null, callable $formatter = null)
     {
         if (null === $minLevel) {
-            $minLevel = 'php://stdout' === $output || 'php://stderr' === $output ? LogLevel::CRITICAL : LogLevel::WARNING;
+            $minLevel = null === $output || 'php://stdout' === $output || 'php://stderr' === $output ? LogLevel::ERROR : LogLevel::WARNING;
 
             if (isset($_ENV['SHELL_VERBOSITY']) || isset($_SERVER['SHELL_VERBOSITY'])) {
                 switch ((int) (isset($_ENV['SHELL_VERBOSITY']) ? $_ENV['SHELL_VERBOSITY'] : $_SERVER['SHELL_VERBOSITY'])) {
@@ -58,7 +58,7 @@ public function __construct($minLevel = null, $output = 'php://stderr', callable
 
         $this->minLevelIndex = self::$levels[$minLevel];
         $this->formatter = $formatter ?: [$this, 'format'];
-        if (false === $this->handle = \is_resource($output) ? $output : @fopen($output, 'a')) {
+        if ($output && false === $this->handle = \is_resource($output) ? $output : @fopen($output, 'a')) {
             throw new InvalidArgumentException(sprintf('Unable to open "%s".', $output));
         }
     }
@@ -77,7 +77,11 @@ public function log($level, $message, array $context = [])
         }
 
         $formatter = $this->formatter;
-        @fwrite($this->handle, $formatter($level, $message, $context));
+        if ($this->handle) {
+            @fwrite($this->handle, $formatter($level, $message, $context));
+        } else {
+            error_log($formatter($level, $message, $context, false));
+        }
     }
 
     /**
@@ -86,7 +90,7 @@ public function log($level, $message, array $context = [])
      *
      * @return string
      */
-    private function format($level, $message, array $context)
+    private function format($level, $message, array $context, $prefixDate = true)
     {
         if (false !== strpos($message, '{')) {
             $replacements = [];
@@ -105,6 +109,11 @@ private function format($level, $message, array $context)
             $message = strtr($message, $replacements);
         }
 
-        return sprintf('%s [%s] %s', date(\DateTime::RFC3339), $level, $message).PHP_EOL;
+        $log = sprintf('[%s] %s', $level, $message).PHP_EOL;
+        if ($prefixDate) {
+            $log = date(\DateTime::RFC3339).' '.$log;
+        }
+
+        return $log;
     }
 }