Repository: Seldaek/monolog (php)

Issue:
WhatFailureGroupHandler does not catch errors from method close
Monolog version 2

`\Monolog\Handler\WhatFailureGroupHandler` does not catch errors from method `\Monolog\Handler\HandlerInterface::close`.
That means when close is called on handler and handler tries for example send remaining buffer to remote service but fails, then failure propagates though WhatFailureGroupHandler and execution ends with fatal error.


Pull request:
WhatFailureGroupHandler catches failures in method close
Fixes #1790 

Patch:
diff --git a/.github/workflows/continuous-integration.yml b/.github/workflows/continuous-integration.yml
index cf50545a..efa84bda 100644
--- a/.github/workflows/continuous-integration.yml
+++ b/.github/workflows/continuous-integration.yml
@@ -4,6 +4,9 @@ on:
   - push
   - pull_request
 
+permissions:
+  contents: read # to fetch code (actions/checkout)
+
 jobs:
   tests:
     name: "CI (PHP ${{ matrix.php-version }}, ${{ matrix.dependencies }} deps)"
diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml
index 9e20a03e..a7482ace 100644
--- a/.github/workflows/lint.yml
+++ b/.github/workflows/lint.yml
@@ -4,6 +4,9 @@ on:
   push:
   pull_request:
 
+permissions:
+  contents: read # to fetch code (actions/checkout)
+
 jobs:
   tests:
     name: "Lint"
diff --git a/.github/workflows/phpstan.yml b/.github/workflows/phpstan.yml
index 434324ce..fcf5007e 100644
--- a/.github/workflows/phpstan.yml
+++ b/.github/workflows/phpstan.yml
@@ -7,6 +7,9 @@ on:
 env:
   COMPOSER_FLAGS: "--ansi --no-interaction --no-progress --prefer-dist"
 
+permissions:
+  contents: read # to fetch code (actions/checkout)
+
 jobs:
   tests:
     name: "PHPStan"
diff --git a/phpstan.neon.dist b/phpstan.neon.dist
index eb126d4a..6011baa8 100644
--- a/phpstan.neon.dist
+++ b/phpstan.neon.dist
@@ -44,3 +44,11 @@ parameters:
 
         # legacy GelfPHP 1.x failures
         - '# Gelf\\Message::set(Facility|Line|File)\(#'
+
+        # php 8.1 Fiber failures
+        - '#unknown class Fiber#'
+        - '#Offset mixed does not exist on WeakMap<Fiber, int>\|null\.#'
+
+        # php 8.1 Enum failures
+        - '#Class (UnitEnum|BackedEnum) not found#'
+        - '#unknown class (UnitEnum|BackedEnum)#'
diff --git a/src/Monolog/Formatter/GoogleCloudLoggingFormatter.php b/src/Monolog/Formatter/GoogleCloudLoggingFormatter.php
index 0cd287f5..ca52ebf4 100644
--- a/src/Monolog/Formatter/GoogleCloudLoggingFormatter.php
+++ b/src/Monolog/Formatter/GoogleCloudLoggingFormatter.php
@@ -17,6 +17,7 @@ use Monolog\LogRecord;
 /**
  * Encodes message information into JSON in a format compatible with Cloud logging.
  *
+ * @see https://cloud.google.com/logging/docs/structured-logging
  * @see https://cloud.google.com/logging/docs/reference/v2/rest/v2/LogEntry
  *
  * @author Luís Cobucci <lcobucci@gmail.com>
@@ -28,7 +29,7 @@ final class GoogleCloudLoggingFormatter extends JsonFormatter
     {
         // Re-key level for GCP logging
         $record['severity'] = $record['level_name'];
-        $record['timestamp'] = $record['datetime']->format(DateTimeInterface::RFC3339_EXTENDED);
+        $record['time'] = $record['datetime']->format(DateTimeInterface::RFC3339_EXTENDED);
 
         // Remove keys that are not used by GCP
         unset($record['level'], $record['level_name'], $record['datetime']);
diff --git a/src/Monolog/Handler/AmqpHandler.php b/src/Monolog/Handler/AmqpHandler.php
index c4997482..994872ce 100644
--- a/src/Monolog/Handler/AmqpHandler.php
+++ b/src/Monolog/Handler/AmqpHandler.php
@@ -151,13 +151,14 @@ class AmqpHandler extends AbstractProcessingHandler
 
     private function createAmqpMessage(string $data): AMQPMessage
     {
-        return new AMQPMessage(
-            $data,
-            [
-                'delivery_mode' => 2,
-                'content_type' => 'application/json',
-            ]
-        );
+        $attributes = [
+            'delivery_mode' => 2,
+            'content_type' => 'application/json',
+        ];
+        if ($this->extraAttributes) {
+            $attributes = array_merge($attributes, $this->extraAttributes);
+        }
+        return new AMQPMessage($data, $attributes);
     }
 
     /**
diff --git a/src/Monolog/Handler/WhatFailureGroupHandler.php b/src/Monolog/Handler/WhatFailureGroupHandler.php
index 2dd13672..b6d3d3b1 100644
--- a/src/Monolog/Handler/WhatFailureGroupHandler.php
+++ b/src/Monolog/Handler/WhatFailureGroupHandler.php
@@ -64,4 +64,18 @@ class WhatFailureGroupHandler extends GroupHandler
             }
         }
     }
+
+    /**
+     * {@inheritDoc}
+     */
+    public function close(): void
+    {
+        foreach ($this->handlers as $handler) {
+            try {
+                $handler->close();
+            } catch (\Throwable $e) {
+                // What failure?
+            }
+        }
+    }
 }
diff --git a/src/Monolog/Logger.php b/src/Monolog/Logger.php
index 1ab75b9e..775d7cf1 100644
--- a/src/Monolog/Logger.php
+++ b/src/Monolog/Logger.php
@@ -168,6 +168,11 @@ class Logger implements LoggerInterface, ResettableInterface
      */
     private $logDepth = 0;
 
+    /**
+     * @var \WeakMap<\Fiber, int>|null Keeps track of depth inside fibers to prevent infinite logging loops
+     */
+    private $fiberLogDepth;
+
     /**
      * @var bool Whether to detect infinite logging loops
      *
@@ -189,6 +194,13 @@ class Logger implements LoggerInterface, ResettableInterface
         $this->setHandlers($handlers);
         $this->processors = $processors;
         $this->timezone = $timezone ?: new DateTimeZone(date_default_timezone_get() ?: 'UTC');
+
+        if (\PHP_VERSION_ID >= 80100) {
+            // Local variable for phpstan, see https://github.com/phpstan/phpstan/issues/6732#issuecomment-1111118412
+            /** @var \WeakMap<\Fiber, int> $fiberLogDepth */
+            $fiberLogDepth = new \WeakMap();
+            $this->fiberLogDepth = $fiberLogDepth;
+        }
     }
 
     public function getName(): string
@@ -332,12 +344,20 @@ class Logger implements LoggerInterface, ResettableInterface
         }
 
         if ($this->detectCycles) {
-            $this->logDepth += 1;
+            if (\PHP_VERSION_ID >= 80100 && $fiber = \Fiber::getCurrent()) {
+                $this->fiberLogDepth[$fiber] = $this->fiberLogDepth[$fiber] ?? 0;
+                $logDepth = ++$this->fiberLogDepth[$fiber];
+            } else {
+                $logDepth = ++$this->logDepth;
+            }
+        } else {
+            $logDepth = 0;
         }
-        if ($this->logDepth === 3) {
+
+        if ($logDepth === 3) {
             $this->warning('A possible infinite logging loop was detected and aborted. It appears some of your handler code is triggering logging, see the previous log record for a hint as to what may be the cause.');
             return false;
-        } elseif ($this->logDepth >= 5) { // log depth 4 is let through so we can log the warning above
+        } elseif ($logDepth >= 5) { // log depth 4 is let through, so we can log the warning above
             return false;
         }
 
@@ -387,7 +407,11 @@ class Logger implements LoggerInterface, ResettableInterface
             }
         } finally {
             if ($this->detectCycles) {
-                $this->logDepth--;
+                if (isset($fiber)) {
+                    $this->fiberLogDepth[$fiber]--;
+                } else {
+                    $this->logDepth--;
+                }
             }
         }
 
diff --git a/src/Monolog/Processor/PsrLogMessageProcessor.php b/src/Monolog/Processor/PsrLogMessageProcessor.php
index 2c2a00e7..e7c12176 100644
--- a/src/Monolog/Processor/PsrLogMessageProcessor.php
+++ b/src/Monolog/Processor/PsrLogMessageProcessor.php
@@ -66,6 +66,8 @@ class PsrLogMessageProcessor implements ProcessorInterface
                 } else {
                     $replacements[$placeholder] = $val->format($this->dateFormat ?: static::SIMPLE_DATE);
                 }
+            } elseif ($val instanceof \UnitEnum) {
+                $replacements[$placeholder] = $val instanceof \BackedEnum ? $val->value : $val->name;
             } elseif (is_object($val)) {
                 $replacements[$placeholder] = '[object '.Utils::getClass($val).']';
             } elseif (is_array($val)) {


Test patch:
diff --git a/tests/Monolog/Formatter/GoogleCloudLoggingFormatterTest.php b/tests/Monolog/Formatter/GoogleCloudLoggingFormatterTest.php
index 27aea2ca..55dfc76d 100644
--- a/tests/Monolog/Formatter/GoogleCloudLoggingFormatterTest.php
+++ b/tests/Monolog/Formatter/GoogleCloudLoggingFormatterTest.php
@@ -30,8 +30,8 @@ class GoogleCloudLoggingFormatterTest extends TestCase
 
         $formatted_decoded = json_decode($formatter->format($record), true);
         $this->assertArrayNotHasKey("datetime", $formatted_decoded);
-        $this->assertArrayHasKey("timestamp", $formatted_decoded);
-        $this->assertSame($record['datetime']->format(DateTimeInterface::RFC3339_EXTENDED), $formatted_decoded["timestamp"]);
+        $this->assertArrayHasKey("time", $formatted_decoded);
+        $this->assertSame($record['datetime']->format(DateTimeInterface::RFC3339_EXTENDED), $formatted_decoded["time"]);
     }
 
     /**
diff --git a/tests/Monolog/LoggerTest.php b/tests/Monolog/LoggerTest.php
index fd99df20..a0ec7405 100644
--- a/tests/Monolog/LoggerTest.php
+++ b/tests/Monolog/LoggerTest.php
@@ -792,6 +792,58 @@ class LoggerTest extends \PHPUnit\Framework\TestCase
             $this->assertEquals($datetime->format('Y-m-d H:i:s'), $record['datetime']->format('Y-m-d H:i:s'));
         }
     }
+
+    /**
+     * @requires PHP 8.1
+     */
+    public function testLogCycleDetectionWithFibersWithoutCycle()
+    {
+        $logger = new Logger(__METHOD__);
+
+        $fiberSuspendHandler = new FiberSuspendHandler();
+        $testHandler = new TestHandler();
+
+        $logger->pushHandler($fiberSuspendHandler);
+        $logger->pushHandler($testHandler);
+
+        $fibers = [];
+        for ($i = 0; $i < 10; $i++) {
+            $fiber = new \Fiber(static function () use ($logger) {
+                $logger->info('test');
+            });
+
+            $fiber->start();
+
+            // We need to keep a reference here, because otherwise the fiber gets automatically cleaned up
+            $fibers[] = $fiber;
+        }
+
+        self::assertCount(10, $testHandler->getRecords());
+    }
+
+    /**
+     * @requires PHP 8.1
+     */
+    public function testLogCycleDetectionWithFibersWithCycle()
+    {
+        $logger = new Logger(__METHOD__);
+
+        $fiberSuspendHandler = new FiberSuspendHandler();
+        $loggingHandler = new LoggingHandler($logger);
+        $testHandler = new TestHandler();
+
+        $logger->pushHandler($fiberSuspendHandler);
+        $logger->pushHandler($loggingHandler);
+        $logger->pushHandler($testHandler);
+
+        $fiber = new \Fiber(static function () use ($logger) {
+            $logger->info('test');
+        });
+
+        $fiber->start();
+
+        self::assertCount(3, $testHandler->getRecords());
+    }
 }
 
 class LoggingHandler implements HandlerInterface
@@ -826,3 +878,27 @@ class LoggingHandler implements HandlerInterface
     {
     }
 }
+
+
+class FiberSuspendHandler implements HandlerInterface
+{
+    public function isHandling(array $record): bool
+    {
+        return true;
+    }
+
+    public function handle(array $record): bool
+    {
+        \Fiber::suspend();
+
+        return true;
+    }
+
+    public function handleBatch(array $records): void
+    {
+    }
+
+    public function close(): void
+    {
+    }
+}
\ No newline at end of file