Skip to content

Commit

Permalink
bug #47878 [HttpKernel] Remove EOL when using error_log() in HttpKern…
Browse files Browse the repository at this point in the history
…el Logger (cyve)

This PR was squashed before being merged into the 4.4 branch.

Discussion
----------

[HttpKernel] Remove EOL when using error_log() in HttpKernel Logger

| Q             | A
| ------------- | ---
| Branch?       | 4.4
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | maybe
| Tickets       |
| License       | MIT
| Doc PR        |

Hello there !

Since PR symfony/symfony#36855, when using `Symfony\Component\HttpKernel\Log\Logger`  with argument `$output = null`. The logger use the `error_log()` method to log messages. But in the log file, messages are separated by an empty line because the default formatter (`Logger::format()`) adds an extra EOL char at the end of the message.

This fix prevents the logger to add an extra EOL char when it uses `error_log()`.

Possible BC : if someone use the logger with a custom formatter that already add a EOL char at the end of the message, there will be a empty line between messages in the log file. In my opinion, this is minor enough not to worry about it ;-)

Thanks for your review :-)

Commits
-------

69cf83ea1a [HttpKernel] Remove EOL when using error_log() in HttpKernel Logger
  • Loading branch information
nicolas-grekas committed Oct 18, 2022
2 parents d84ce93 + abc1357 commit 9a34f1a
Show file tree
Hide file tree
Showing 2 changed files with 32 additions and 8 deletions.
16 changes: 10 additions & 6 deletions Log/Logger.php
Original file line number Diff line number Diff line change
Expand Up @@ -44,10 +44,14 @@ public function __construct(string $minLevel = null, $output = null, callable $f

if (isset($_ENV['SHELL_VERBOSITY']) || isset($_SERVER['SHELL_VERBOSITY'])) {
switch ((int) ($_ENV['SHELL_VERBOSITY'] ?? $_SERVER['SHELL_VERBOSITY'])) {
case -1: $minLevel = LogLevel::ERROR; break;
case 1: $minLevel = LogLevel::NOTICE; break;
case 2: $minLevel = LogLevel::INFO; break;
case 3: $minLevel = LogLevel::DEBUG; break;
case -1: $minLevel = LogLevel::ERROR;
break;
case 1: $minLevel = LogLevel::NOTICE;
break;
case 2: $minLevel = LogLevel::INFO;
break;
case 3: $minLevel = LogLevel::DEBUG;
break;
}
}
}
Expand Down Expand Up @@ -80,7 +84,7 @@ public function log($level, $message, array $context = [])

$formatter = $this->formatter;
if ($this->handle) {
@fwrite($this->handle, $formatter($level, $message, $context));
@fwrite($this->handle, $formatter($level, $message, $context).\PHP_EOL);
} else {
error_log($formatter($level, $message, $context, false));
}
Expand All @@ -105,7 +109,7 @@ private function format(string $level, string $message, array $context, bool $pr
$message = strtr($message, $replacements);
}

$log = sprintf('[%s] %s', $level, $message).\PHP_EOL;
$log = sprintf('[%s] %s', $level, $message);
if ($prefixDate) {
$log = date(\DateTime::RFC3339).' '.$log;
}
Expand Down
24 changes: 22 additions & 2 deletions Tests/Log/LoggerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ protected function tearDown(): void
public static function assertLogsMatch(array $expected, array $given)
{
foreach ($given as $k => $line) {
self::assertThat(1 === preg_match('/[0-9]{4}-[0-9]{2}-[0-9]{2}T[0-9]{2}:[0-9]{2}:[0-9]{2}[\+-][0-9]{2}:[0-9]{2} '.preg_quote($expected[$k]).'/', $line), self::isTrue(), "\"$line\" do not match expected pattern \"$expected[$k]\"");
self::assertSame(1, preg_match('/[0-9]{4}-[0-9]{2}-[0-9]{2}T[0-9]{2}:[0-9]{2}:[0-9]{2}[\+-][0-9]{2}:[0-9]{2} '.preg_quote($expected[$k]).'/', $line), "\"$line\" do not match expected pattern \"$expected[$k]\"");
}
}

Expand Down Expand Up @@ -186,7 +186,7 @@ public function testContextExceptionKeyCanBeExceptionOrOtherValues()
public function testFormatter()
{
$this->logger = new Logger(LogLevel::DEBUG, $this->tmpFile, function ($level, $message, $context) {
return json_encode(['level' => $level, 'message' => $message, 'context' => $context]).\PHP_EOL;
return json_encode(['level' => $level, 'message' => $message, 'context' => $context]);
});

$this->logger->error('An error', ['foo' => 'bar']);
Expand All @@ -196,6 +196,26 @@ public function testFormatter()
'{"level":"warning","message":"A warning","context":{"baz":"bar"}}',
], $this->getLogs());
}

public function testLogsWithoutOutput()
{
$oldErrorLog = ini_set('error_log', $this->tmpFile);

$logger = new Logger();
$logger->error('test');
$logger->critical('test');

$expected = [
'[error] test',
'[critical] test',
];

foreach ($this->getLogs() as $k => $line) {
$this->assertSame(1, preg_match('/\[[\w\/\-: ]+\] '.preg_quote($expected[$k]).'/', $line), "\"$line\" do not match expected pattern \"$expected[$k]\"");
}

ini_set('error_log', $oldErrorLog);
}
}

class DummyTest
Expand Down

0 comments on commit 9a34f1a

Please sign in to comment.