From 8ae546b2b98fe0278520d07e5216a65e97a0a5e2 Mon Sep 17 00:00:00 2001 From: Pedro Peixoto Date: Mon, 11 Nov 2024 06:26:15 -0300 Subject: [PATCH 1/3] Allow the user to define a custom mode when `fopen`-ing a log file (#1913) * Allow the user to define a custom mode when `fopen`-ing a log file * Undo unnecessary changes --------- Co-authored-by: Jordi Boggiano --- src/Monolog/Handler/StreamHandler.php | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/Monolog/Handler/StreamHandler.php b/src/Monolog/Handler/StreamHandler.php index 561d9c755..64aaa3441 100644 --- a/src/Monolog/Handler/StreamHandler.php +++ b/src/Monolog/Handler/StreamHandler.php @@ -41,6 +41,8 @@ class StreamHandler extends AbstractProcessingHandler protected $filePermission; /** @var bool */ protected $useLocking; + /** @var string */ + protected $fileOpenMode; /** @var true|null */ private $dirCreated = null; @@ -48,10 +50,11 @@ class StreamHandler extends AbstractProcessingHandler * @param resource|string $stream If a missing path can't be created, an UnexpectedValueException will be thrown on first write * @param int|null $filePermission Optional file permissions (default (0644) are only for owner read/write) * @param bool $useLocking Try to lock log file before doing any writes + * @param string $fileOpenMode The fopen() mode used when opening a file, if $stream is a file path * * @throws \InvalidArgumentException If stream is not a resource or string */ - public function __construct($stream, $level = Logger::DEBUG, bool $bubble = true, ?int $filePermission = null, bool $useLocking = false) + public function __construct($stream, $level = Logger::DEBUG, bool $bubble = true, ?int $filePermission = null, bool $useLocking = false, $fileOpenMode = 'a') { parent::__construct($level, $bubble); @@ -78,6 +81,7 @@ public function __construct($stream, $level = Logger::DEBUG, bool $bubble = true throw new \InvalidArgumentException('A stream must either be a resource or a string.'); } + $this->fileOpenMode = $fileOpenMode; $this->filePermission = $filePermission; $this->useLocking = $useLocking; } @@ -138,7 +142,7 @@ protected function write(array $record): void return $this->customErrorHandler(...$args); }); try { - $stream = fopen($url, 'a'); + $stream = fopen($url, $this->fileOpenMode); if ($this->filePermission !== null) { @chmod($url, $this->filePermission); } From b92508d0d7ef94534d3080ba831785cf7d3a0742 Mon Sep 17 00:00:00 2001 From: Jordi Boggiano Date: Mon, 11 Nov 2024 14:06:20 +0100 Subject: [PATCH 2/3] Fix build --- src/Monolog/Utils.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Monolog/Utils.php b/src/Monolog/Utils.php index 360c42199..d4ff4c040 100644 --- a/src/Monolog/Utils.php +++ b/src/Monolog/Utils.php @@ -249,7 +249,7 @@ public static function expandIniShorthandBytes($val) } $val = (int) $match['val']; - switch (strtolower($match['unit'] ?? '')) { + switch (strtolower($match['unit'])) { case 'g': $val *= 1024; case 'm': From 0779fb91e524d6a1eea42643e06aefdda18c3b97 Mon Sep 17 00:00:00 2001 From: Jordi Boggiano Date: Mon, 11 Nov 2024 20:08:58 +0100 Subject: [PATCH 3/3] Close and reopen file handles if a write fails (#1882) * Close file handle after each write, refs #1862, refs #1860, refs #1634 * Modify patch to retry writes once if they fail, then throw if not * Fix php7.2 build --- src/Monolog/Handler/StreamHandler.php | 28 ++++++++- tests/Monolog/Handler/StreamHandlerTest.php | 68 ++++++++++++++++++++- 2 files changed, 92 insertions(+), 4 deletions(-) diff --git a/src/Monolog/Handler/StreamHandler.php b/src/Monolog/Handler/StreamHandler.php index 64aaa3441..218d43847 100644 --- a/src/Monolog/Handler/StreamHandler.php +++ b/src/Monolog/Handler/StreamHandler.php @@ -45,6 +45,8 @@ class StreamHandler extends AbstractProcessingHandler protected $fileOpenMode; /** @var true|null */ private $dirCreated = null; + /** @var bool */ + private $retrying = false; /** * @param resource|string $stream If a missing path can't be created, an UnexpectedValueException will be thrown on first write @@ -168,8 +170,30 @@ protected function write(array $record): void flock($stream, LOCK_EX); } - $this->streamWrite($stream, $record); + $this->errorMessage = null; + set_error_handler(function (...$args) { + return $this->customErrorHandler(...$args); + }); + try { + $this->streamWrite($stream, $record); + } finally { + restore_error_handler(); + } + if ($this->errorMessage !== null) { + $error = $this->errorMessage; + // close the resource if possible to reopen it, and retry the failed write + if (!$this->retrying && $this->url !== null && $this->url !== 'php://memory') { + $this->retrying = true; + $this->close(); + $this->write($record); + + return; + } + + throw new \UnexpectedValueException('Writing to the log file failed: '.$error . Utils::getRecordMessageForException($record)); + } + $this->retrying = false; if ($this->useLocking) { flock($stream, LOCK_UN); } @@ -189,7 +213,7 @@ protected function streamWrite($stream, array $record): void private function customErrorHandler(int $code, string $msg): bool { - $this->errorMessage = preg_replace('{^(fopen|mkdir)\(.*?\): }', '', $msg); + $this->errorMessage = preg_replace('{^(fopen|mkdir|fwrite)\(.*?\): }', '', $msg); return true; } diff --git a/tests/Monolog/Handler/StreamHandlerTest.php b/tests/Monolog/Handler/StreamHandlerTest.php index 8c69cc58e..4a339d5f7 100644 --- a/tests/Monolog/Handler/StreamHandlerTest.php +++ b/tests/Monolog/Handler/StreamHandlerTest.php @@ -17,6 +17,13 @@ class StreamHandlerTest extends TestCase { + public function tearDown(): void + { + parent::tearDown(); + + @unlink(__DIR__.'/test.log'); + } + /** * @covers Monolog\Handler\StreamHandler::__construct * @covers Monolog\Handler\StreamHandler::write @@ -54,9 +61,9 @@ public function testClose() $handler->handle($this->getRecord(Logger::WARNING, 'test')); $stream = $handler->getStream(); - $this->assertTrue(is_resource($stream)); + $this->assertTrue(\is_resource($stream)); $handler->close(); - $this->assertFalse(is_resource($stream)); + $this->assertFalse(\is_resource($stream)); } /** @@ -204,6 +211,63 @@ public function testWriteNonExistingFileResource() $handler->handle($this->getRecord()); } + /** + * @covers Monolog\Handler\StreamHandler::write + */ + public function testWriteErrorDuringWriteRetriesWithClose() + { + $handler = $this->getMockBuilder(StreamHandler::class) + ->onlyMethods(['streamWrite']) + ->setConstructorArgs(['file://'.sys_get_temp_dir().'/bar/'.rand(0, 10000).DIRECTORY_SEPARATOR.rand(0, 10000)]) + ->getMock(); + + $refs = []; + $handler->expects($this->exactly(2)) + ->method('streamWrite') + ->willReturnCallback(function ($stream) use (&$refs) { + $refs[] = $stream; + if (\count($refs) === 2) { + self::assertNotSame($stream, $refs[0]); + } + if (\count($refs) === 1) { + trigger_error('fwrite(): Write of 378 bytes failed with errno=32 Broken pipe', E_USER_ERROR); + } + }); + + $handler->handle($this->getRecord()); + if (method_exists($this, 'assertIsClosedResource')) { + self::assertIsClosedResource($refs[0]); + self::assertIsResource($refs[1]); + } + } + + /** + * @covers Monolog\Handler\StreamHandler::write + */ + public function testWriteErrorDuringWriteRetriesButThrowsIfStillFails() + { + $handler = $this->getMockBuilder(StreamHandler::class) + ->onlyMethods(['streamWrite']) + ->setConstructorArgs(['file://'.sys_get_temp_dir().'/bar/'.rand(0, 10000).DIRECTORY_SEPARATOR.rand(0, 10000)]) + ->getMock(); + + $refs = []; + $handler->expects($this->exactly(2)) + ->method('streamWrite') + ->willReturnCallback(function ($stream) use (&$refs) { + $refs[] = $stream; + if (\count($refs) === 2) { + self::assertNotSame($stream, $refs[0]); + } + trigger_error('fwrite(): Write of 378 bytes failed with errno=32 Broken pipe', E_USER_ERROR); + }); + + self::expectException(\UnexpectedValueException::class); + self::expectExceptionMessage('Writing to the log file failed: Write of 378 bytes failed with errno=32 Broken pipe +The exception occurred while attempting to log: test'); + $handler->handle($this->getRecord()); + } + /** * @covers Monolog\Handler\StreamHandler::__construct * @covers Monolog\Handler\StreamHandler::write