Log stack traces and report exceptions as PSR-3 context (#41068, #41069)

lib/internal/Magento/Framework/Amqp/Test/Unit/TopologyInstallerTest.php

@@ -56,21 +56,21 @@ protected function setUp(): void
     }
 
     /**
-     * Make sure that topology creation errors in log contain actual error message.
+     * Make sure that topology creation errors are reported with the exception in the log context.
      */
     public function testInstallException()
     {
-        $exceptionMessage = "Exception message";
+        $exception = new AMQPLogicException('Exception message');
 
         $this->topologyConfigMock
             ->expects($this->once())
             ->method('getQueues')
-            ->willThrowException(new AMQPLogicException($exceptionMessage));
+            ->willThrowException($exception);
 
         $this->loggerMock
             ->expects($this->once())
             ->method('error')
-            ->with($this->stringContains("AMQP topology installation failed: {$exceptionMessage}"));
+            ->with('AMQP topology installation failed', ['exception' => $exception]);
 
         $this->topologyInstaller->install();
     }

lib/internal/Magento/Framework/Amqp/TopologyInstaller.php

@@ -93,7 +93,7 @@ public function install()
                 $this->exchangeInstaller->install($amqpConfig->getChannel(), $exchange);
             }
         } catch (\Exception $e) {
-            $this->logger->error("AMQP topology installation failed: {$e->getMessage()}\n{$e->getTraceAsString()}");
+            $this->logger->error('AMQP topology installation failed', ['exception' => $e]);
         }
     }
 }

lib/internal/Magento/Framework/Api/ImageProcessor.php

@@ -146,7 +146,10 @@ public function processImageContent($entityType, $imageContent)
                 (string) $entityType,
             );
         } catch (\Exception $e) {
-            $this->logger->critical($e);
+            $this->logger->critical(
+                'Unable to process the image content of the {entityType} entity',
+                ['entityType' => (string)$entityType, 'exception' => $e]
+            );
         }
 
         return '';

lib/internal/Magento/Framework/App/Area.php

@@ -177,7 +177,10 @@ protected function _applyUserAgentDesignException($request)
                 return true;
             }
         } catch (\Exception $e) {
-            $this->_logger->critical($e);
+            $this->_logger->critical(
+                'Unable to apply the user agent design exception for the {areaCode} area',
+                ['areaCode' => $this->_code, 'exception' => $e]
+            );
         }
         return false;
     }

lib/internal/Magento/Framework/App/ExceptionHandler.php

@@ -213,7 +213,7 @@ private function handleSessionException(
     private function handleInitException(\Exception $exception): bool
     {
         if ($exception instanceof InitException) {
-            $this->logger->critical($exception);
+            $this->logger->critical($exception->getMessage(), ['exception' => $exception]);
             // phpcs:ignore Magento2.Security.IncludeFile
             require $this->filesystem
                 ->getDirectoryRead(DirectoryList::PUB)
@@ -249,7 +249,10 @@ private function handleGenericReport(Bootstrap $bootstrap, \Exception $exception
             $reportData['script_name'] = $params['SCRIPT_NAME'];
         }
         $reportData['report_id'] = $this->encryptor->getHash(implode('', $reportData));
-        $this->logger->critical($exception, ['report_id' => $reportData['report_id']]);
+        $this->logger->critical(
+            $exception->getMessage(),
+            ['report_id' => $reportData['report_id'], 'exception' => $exception]
+        );
         // phpcs:ignore Magento2.Security.IncludeFile
         require $this->filesystem
             ->getDirectoryRead(DirectoryList::PUB)

lib/internal/Magento/Framework/App/FeedFactory.php

@@ -11,7 +11,7 @@
 use Psr\Log\LoggerInterface;
 
 /**
- * Feed factory
+ * Creates a feed of the requested format from the given feed data
  */
 class FeedFactory implements FeedFactoryInterface
 {
@@ -46,7 +46,11 @@ public function __construct(
     }
 
     /**
-     * {@inheritdoc}
+     * @inheritdoc
+     *
+     * @param array $data
+     * @param string $format
+     * @return FeedInterface
      */
     public function create(array $data, string $format = FeedFactoryInterface::FORMAT_RSS) : FeedInterface
     {
@@ -68,7 +72,10 @@ public function create(array $data, string $format = FeedFactoryInterface::FORMA
                 ['data' => $data]
             );
         } catch (\Exception $e) {
-            $this->logger->error($e->getMessage());
+            $this->logger->error(
+                'Unable to create a feed of the {feedFormat} format',
+                ['feedFormat' => $format, 'exception' => $e]
+            );
             throw new \Magento\Framework\Exception\RuntimeException(
                 new \Magento\Framework\Phrase('There has been an error with import'),
                 $e

lib/internal/Magento/Framework/App/Test/Unit/AreaTest.php

@@ -30,7 +30,7 @@
  */
 class AreaTest extends TestCase
 {
-    const SCOPE_ID = '1';
+    public const SCOPE_ID = '1';
 
     /**
      * @var ObjectManager
@@ -327,7 +327,10 @@ public function testDetectDesignByRequestWithException()
             ->getMock();
         $this->loggerMock->expects($this->once())
             ->method('critical')
-            ->with($exception);
+            ->with(
+                'Unable to apply the user agent design exception for the {areaCode} area',
+                ['areaCode' => $this->areaCode, 'exception' => $exception]
+            );
         $this->object->detectDesign($requestMock);
     }
 }

lib/internal/Magento/Framework/App/Test/Unit/ExceptionHandlerTest.php

@@ -180,7 +180,7 @@ public function testHandleInitException()
             ->willReturn(__DIR__ . '/_files/pub/errors/404.php');
         $this->loggerMock->expects($this->once())
             ->method('critical')
-            ->with($exception);
+            ->with($exception->getMessage(), ['exception' => $exception]);
         $this->filesystemMock->expects($this->once())
             ->method('getDirectoryRead')
             ->with(DirectoryList::PUB)
@@ -225,7 +225,10 @@ public function testHandleGenericReport()
             ->willReturn('some-sha256-hash');
         $this->loggerMock->expects($this->once())
             ->method('critical')
-            ->with($exception, ['report_id' => 'some-sha256-hash']);
+            ->with(
+                $exception->getMessage(),
+                ['report_id' => 'some-sha256-hash', 'exception' => $exception]
+            );
         $this->filesystemMock->expects($this->once())
             ->method('getDirectoryRead')
             ->with(DirectoryList::PUB)

lib/internal/Magento/Framework/Css/PreProcessor/ErrorHandler.php

@@ -24,10 +24,13 @@ public function __construct(\Psr\Log\LoggerInterface $logger)
     }
 
     /**
-     * {@inheritdoc}
+     * @inheritdoc
+     *
+     * @param \Exception $e
+     * @return void
      */
     public function processException(\Exception $e)
     {
-        $this->logger->critical($e);
+        $this->logger->critical('Error while pre-processing CSS', ['exception' => $e]);
     }
 }

lib/internal/Magento/Framework/GraphQl/Query/ErrorHandler.php

@@ -82,6 +82,9 @@ private function log(Error $error): void
             return;
         }
 
-        $this->logger->error($error);
+        $this->logger->error(
+            'GraphQL request failed with an error of the {errorCategory} category',
+            ['errorCategory' => $category, 'exception' => $error]
+        );
     }
 }

lib/internal/Magento/Framework/Image/Adapter/AbstractAdapter.php

@@ -699,7 +699,10 @@ protected function _prepareDestination($destination = null, $newName = null)
             try {
                 $this->directoryWrite->create($this->directoryWrite->getRelativePath($destination));
             } catch (FileSystemException $e) {
-                $this->logger->critical($e);
+                $this->logger->critical(
+                    'Unable to create the {destination} image directory',
+                    ['destination' => $destination, 'exception' => $e]
+                );
                 //phpcs:ignore Magento2.Exceptions.DirectThrow
                 throw new \DomainException(
                     'Unable to write file into directory ' . $destination . '. Access forbidden.'

lib/internal/Magento/Framework/Image/Test/Unit/Adapter/ImageMagickTest.php

@@ -96,7 +96,11 @@ public function testSaveWithException()
             new Phrase('Unable to write file into directory product/cache. Access forbidden.')
         );
         $this->writeMock->method('create')->willThrowException($exception);
-        $this->loggerMock->expects($this->once())->method('critical')->with($exception);
+        $this->loggerMock->expects($this->once())->method('critical')
+            ->with(
+                'Unable to create the {destination} image directory',
+                ['destination' => 'product/cache', 'exception' => $exception]
+            );
         $this->imageMagic->save('product/cache', 'sample.jpg');
     }
 

lib/internal/Magento/Framework/Logger/Handler/Base.php

@@ -10,6 +10,7 @@
 use InvalidArgumentException;
 use Magento\Framework\Filesystem\DriverInterface;
 use Magento\Framework\ObjectManager\ResetAfterRequestInterface;
+use Monolog\Formatter\FormatterInterface;
 use Monolog\Formatter\LineFormatter;
 use Monolog\Handler\StreamHandler;
 use Monolog\Logger;
@@ -41,11 +42,13 @@ class Base extends StreamHandler implements ResetAfterRequestInterface
      * @param DriverInterface $filesystem
      * @param string|null $filePath
      * @param string|null $fileName
+     * @param FormatterInterface|null $formatter Defaults to a LineFormatter that includes stack traces
      */
     public function __construct(
         DriverInterface $filesystem,
         ?string $filePath = null,
-        ?string $fileName = null
+        ?string $fileName = null,
+        ?FormatterInterface $formatter = null
     ) {
         $this->filesystem = $filesystem;
 
@@ -58,7 +61,26 @@ public function __construct(
             $this->loggerType
         );
 
-        $this->setFormatter(new LineFormatter(null, null, true));
+        $this->setFormatter($formatter ?? $this->createDefaultFormatter());
+    }
+
+    /**
+     * Create the formatter used when none was injected
+     *
+     * Stack traces are included so that a Throwable reported through the PSR-3 reserved
+     * $context['exception'] key stays diagnosable, instead of being reduced to its throw site.
+     *
+     * @return FormatterInterface
+     */
+    private function createDefaultFormatter(): FormatterInterface
+    {
+        return new LineFormatter(
+            format: null,
+            dateFormat: null,
+            allowInlineLineBreaks: true,
+            ignoreEmptyContextAndExtra: false,
+            includeStacktraces: true
+        );
     }
 
     /**

lib/internal/Magento/Framework/Logger/Handler/System.php

@@ -10,6 +10,7 @@
 use Exception;
 use Magento\Framework\Filesystem\DriverInterface;
 use Magento\Framework\Logger\Handler\Exception as ExceptionHandler;
+use Monolog\Formatter\FormatterInterface;
 use Monolog\Logger;
 use Monolog\LogRecord;
 
@@ -37,15 +38,17 @@ class System extends Base
      * @param DriverInterface $filesystem
      * @param ExceptionHandler $exceptionHandler
      * @param string|null $filePath
+     * @param FormatterInterface|null $formatter
      * @throws Exception
      */
     public function __construct(
         DriverInterface $filesystem,
         ExceptionHandler $exceptionHandler,
-        ?string $filePath = null
+        ?string $filePath = null,
+        ?FormatterInterface $formatter = null
     ) {
         $this->exceptionHandler = $exceptionHandler;
-        parent::__construct($filesystem, $filePath);
+        parent::__construct($filesystem, $filePath, null, $formatter);
     }
 
     /**

lib/internal/Magento/Framework/Logger/README.md

@@ -1,3 +1,48 @@
 # Logger
 
 **Logger** provides a standard mechanism to log to system and error logs.
+
+## Reporting exceptions
+
+Pass the `Throwable` in the PSR-3 reserved `exception` context key, and keep the message a
+constant template so that occurrences of the same fault aggregate together:
+
+```php
+$this->logger->critical('Unable to process image for product {productId}', [
+    'productId' => $productId,
+    'exception' => $e,
+]);
+```
+
+Do not stringify the exception into the message (`$this->logger->critical($e)`) — the trace ends
+up in the message field as free text, the message becomes unique per occurrence, and the record
+is not routed to `exception.log`. Do not put `$e->getTrace()` in the context either: trace frames
+carry call arguments, which may contain personal data or credentials.
+
+## Log formatting
+
+`Magento\Framework\Logger\Handler\Base` formats records with a Monolog `LineFormatter` that
+includes stack traces. A different formatter can be injected without extending the handler, for
+example to emit one valid JSON document per record for log aggregation:
+
+```xml
+<virtualType name="jsonLogFormatter" type="Monolog\Formatter\JsonFormatter">
+    <arguments>
+        <argument name="includeStacktraces" xsi:type="boolean">true</argument>
+    </arguments>
+</virtualType>
+<type name="Magento\Framework\Logger\Handler\System">
+    <arguments>
+        <argument name="formatter" xsi:type="object">jsonLogFormatter</argument>
+    </arguments>
+</type>
+<type name="Magento\Framework\Logger\Handler\Exception">
+    <arguments>
+        <argument name="formatter" xsi:type="object">jsonLogFormatter</argument>
+    </arguments>
+</type>
+```
+
+Each handler owns its formatter, so `system.log` and `exception.log` have to be configured
+separately — `Handler\System` delegates records that carry `context['exception']` to
+`Handler\Exception`, which formats them itself.

lib/internal/Magento/Framework/Logger/Test/Unit/Handler/BaseTest.php

@@ -9,6 +9,9 @@
 
 use Magento\Framework\Filesystem\DriverInterface;
 use Magento\Framework\Logger\Handler\Base;
+use Monolog\Formatter\FormatterInterface;
+use Monolog\Level;
+use Monolog\LogRecord;
 use PHPUnit\Framework\MockObject\MockObject;
 use PHPUnit\Framework\TestCase;
 
@@ -58,4 +61,42 @@ public function testSanitizeParentLevelFolder()
             $this->sanitizeMethod->invokeArgs($this->model, ['../../../var/hack/custom.log'])
         );
     }
+
+    /**
+     * A Throwable reported through the PSR-3 reserved context key must keep its stack trace.
+     */
+    public function testDefaultFormatterIncludesStackTraces(): void
+    {
+        $formatted = $this->model->getFormatter()->format($this->createRecordWithException());
+
+        $this->assertStringContainsString('[stacktrace]', $formatted);
+        $this->assertStringContainsString(__FUNCTION__, $formatted);
+    }
+
+    public function testDefaultFormatterKeepsMessageAndContext(): void
+    {
+        $formatted = $this->model->getFormatter()->format($this->createRecordWithException());
+
+        $this->assertStringContainsString('Something failed while processing {orderId}', $formatted);
+        $this->assertStringContainsString('"orderId":1234', $formatted);
+    }
+
+    public function testInjectedFormatterIsUsed(): void
+    {
+        $formatter = $this->createMock(FormatterInterface::class);
+        $handler = new Base($this->createMock(DriverInterface::class), null, null, $formatter);
+
+        $this->assertSame($formatter, $handler->getFormatter());
+    }
+
+    private function createRecordWithException(): LogRecord
+    {
+        return new LogRecord(
+            new \DateTimeImmutable('2026-01-01 00:00:00'),
+            'main',
+            Level::Critical,
+            'Something failed while processing {orderId}',
+            ['orderId' => 1234, 'exception' => new \RuntimeException('Something failed')]
+        );
+    }
 }

lib/internal/Magento/Framework/Mail/Test/Unit/TransportTest.php

@@ -72,7 +72,8 @@ protected function setUp(): void
     public function testSendMessageBrokenMessage(): void
     {
         $exception = new RfcComplianceException('Email "" does not comply with addr-spec of RFC 2822.');
-        $this->loggerMock->expects(self::once())->method('error')->with($exception);
+        $this->loggerMock->expects(self::once())->method('error')
+            ->with('Unable to send an email message', ['exception' => $exception]);
         $this->expectException('Magento\Framework\Exception\MailException');
         $this->expectExceptionMessage('Unable to send mail. Please try again later.');