Jordi Boggiano преди 1 година
родител
ревизия
24a1110715
променени са 2 файла, в които са добавени 93 реда и са изтрити 7 реда
  1. 29 7
      src/Monolog/Handler/StreamHandler.php
  2. 64 0
      tests/Monolog/Handler/StreamHandlerTest.php

+ 29 - 7
src/Monolog/Handler/StreamHandler.php

@@ -34,17 +34,20 @@ class StreamHandler extends AbstractProcessingHandler
     private string|null $errorMessage = null;
     protected int|null $filePermission;
     protected bool $useLocking;
+    protected string $fileOpenMode;
     /** @var true|null */
     private bool|null $dirCreated = null;
+    private bool $retrying = false;
 
     /**
      * @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, int|string|Level $level = Level::Debug, bool $bubble = true, ?int $filePermission = null, bool $useLocking = false)
+    public function __construct($stream, int|string|Level $level = Level::Debug, bool $bubble = true, ?int $filePermission = null, bool $useLocking = false, string $fileOpenMode = 'a')
     {
         parent::__construct($level, $bubble);
 
@@ -71,6 +74,7 @@ class StreamHandler extends AbstractProcessingHandler
             throw new \InvalidArgumentException('A stream must either be a resource or a string.');
         }
 
+        $this->fileOpenMode = $fileOpenMode;
         $this->filePermission = $filePermission;
         $this->useLocking = $useLocking;
     }
@@ -122,12 +126,10 @@ class StreamHandler extends AbstractProcessingHandler
             }
             $this->createDir($url);
             $this->errorMessage = null;
-            set_error_handler(function (...$args) {
-                return $this->customErrorHandler(...$args);
-            });
+            set_error_handler($this->customErrorHandler(...));
 
             try {
-                $stream = fopen($url, 'a');
+                $stream = fopen($url, $this->fileOpenMode);
                 if ($this->filePermission !== null) {
                     @chmod($url, $this->filePermission);
                 }
@@ -149,8 +151,28 @@ class StreamHandler extends AbstractProcessingHandler
             flock($stream, LOCK_EX);
         }
 
-        $this->streamWrite($stream, $record);
+        $this->errorMessage = null;
+        set_error_handler($this->customErrorHandler(...));
+        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);
         }
@@ -167,7 +189,7 @@ class StreamHandler extends AbstractProcessingHandler
 
     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;
     }

+ 64 - 0
tests/Monolog/Handler/StreamHandlerTest.php

@@ -17,6 +17,13 @@ use PHPUnit\Framework\Attributes\DataProvider;
 
 class StreamHandlerTest extends TestCase
 {
+    public function tearDown(): void
+    {
+        parent::tearDown();
+
+        @unlink(__DIR__.'/test.log');
+    }
+
     /**
      * @covers Monolog\Handler\StreamHandler::__construct
      * @covers Monolog\Handler\StreamHandler::write
@@ -205,6 +212,63 @@ STRING;
         $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