diff --git a/lib/internal/Magento/Framework/Amqp/Test/Unit/TopologyInstallerTest.php b/lib/internal/Magento/Framework/Amqp/Test/Unit/TopologyInstallerTest.php index a2bdcb19f2504..25e190172afaa 100644 --- a/lib/internal/Magento/Framework/Amqp/Test/Unit/TopologyInstallerTest.php +++ b/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(); } diff --git a/lib/internal/Magento/Framework/Amqp/TopologyInstaller.php b/lib/internal/Magento/Framework/Amqp/TopologyInstaller.php index e1fa29cedf1f5..4f5ca2188de83 100644 --- a/lib/internal/Magento/Framework/Amqp/TopologyInstaller.php +++ b/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]); } } } diff --git a/lib/internal/Magento/Framework/Api/ImageProcessor.php b/lib/internal/Magento/Framework/Api/ImageProcessor.php index 35504e1b2489e..298246f2a917a 100644 --- a/lib/internal/Magento/Framework/Api/ImageProcessor.php +++ b/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 ''; diff --git a/lib/internal/Magento/Framework/App/Area.php b/lib/internal/Magento/Framework/App/Area.php index 4f82b8b9bf935..59e31e70487e6 100644 --- a/lib/internal/Magento/Framework/App/Area.php +++ b/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; } diff --git a/lib/internal/Magento/Framework/App/ExceptionHandler.php b/lib/internal/Magento/Framework/App/ExceptionHandler.php index a4f3ac3015606..d16759773643e 100644 --- a/lib/internal/Magento/Framework/App/ExceptionHandler.php +++ b/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) diff --git a/lib/internal/Magento/Framework/App/FeedFactory.php b/lib/internal/Magento/Framework/App/FeedFactory.php index 9384bfb646d6c..c9f69674834d2 100644 --- a/lib/internal/Magento/Framework/App/FeedFactory.php +++ b/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 diff --git a/lib/internal/Magento/Framework/App/Test/Unit/AreaTest.php b/lib/internal/Magento/Framework/App/Test/Unit/AreaTest.php index fc1e898056208..967bdeb6f97a1 100644 --- a/lib/internal/Magento/Framework/App/Test/Unit/AreaTest.php +++ b/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); } } diff --git a/lib/internal/Magento/Framework/App/Test/Unit/ExceptionHandlerTest.php b/lib/internal/Magento/Framework/App/Test/Unit/ExceptionHandlerTest.php index 045a3acb6b361..2e6db49020246 100644 --- a/lib/internal/Magento/Framework/App/Test/Unit/ExceptionHandlerTest.php +++ b/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) diff --git a/lib/internal/Magento/Framework/Css/PreProcessor/ErrorHandler.php b/lib/internal/Magento/Framework/Css/PreProcessor/ErrorHandler.php index 4171ec43ff925..0f61221f093ce 100644 --- a/lib/internal/Magento/Framework/Css/PreProcessor/ErrorHandler.php +++ b/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]); } } diff --git a/lib/internal/Magento/Framework/GraphQl/Query/ErrorHandler.php b/lib/internal/Magento/Framework/GraphQl/Query/ErrorHandler.php index 6effa1e972193..1c2e4028a7855 100644 --- a/lib/internal/Magento/Framework/GraphQl/Query/ErrorHandler.php +++ b/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] + ); } } diff --git a/lib/internal/Magento/Framework/Image/Adapter/AbstractAdapter.php b/lib/internal/Magento/Framework/Image/Adapter/AbstractAdapter.php index ba938c420dd14..744aa30cd8d3b 100644 --- a/lib/internal/Magento/Framework/Image/Adapter/AbstractAdapter.php +++ b/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.' diff --git a/lib/internal/Magento/Framework/Image/Test/Unit/Adapter/ImageMagickTest.php b/lib/internal/Magento/Framework/Image/Test/Unit/Adapter/ImageMagickTest.php index 4af0ee5151858..aaba3293f636d 100644 --- a/lib/internal/Magento/Framework/Image/Test/Unit/Adapter/ImageMagickTest.php +++ b/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'); } diff --git a/lib/internal/Magento/Framework/Logger/Handler/Base.php b/lib/internal/Magento/Framework/Logger/Handler/Base.php index 24279eaa2523d..885a398e3c10d 100644 --- a/lib/internal/Magento/Framework/Logger/Handler/Base.php +++ b/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 + ); } /** diff --git a/lib/internal/Magento/Framework/Logger/Handler/System.php b/lib/internal/Magento/Framework/Logger/Handler/System.php index 1417752ce3974..ccbae9aaf9bd6 100644 --- a/lib/internal/Magento/Framework/Logger/Handler/System.php +++ b/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); } /** diff --git a/lib/internal/Magento/Framework/Logger/README.md b/lib/internal/Magento/Framework/Logger/README.md index 2d2b0410de1d1..21f3045566d0d 100644 --- a/lib/internal/Magento/Framework/Logger/README.md +++ b/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 + + + true + + + + + jsonLogFormatter + + + + + jsonLogFormatter + + +``` + +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. diff --git a/lib/internal/Magento/Framework/Logger/Test/Unit/Handler/BaseTest.php b/lib/internal/Magento/Framework/Logger/Test/Unit/Handler/BaseTest.php index c94f4a5b3c359..9c4be3c015812 100644 --- a/lib/internal/Magento/Framework/Logger/Test/Unit/Handler/BaseTest.php +++ b/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')] + ); + } } diff --git a/lib/internal/Magento/Framework/Mail/Test/Unit/TransportTest.php b/lib/internal/Magento/Framework/Mail/Test/Unit/TransportTest.php index 8851b1d5b4f5d..611e229199a3f 100644 --- a/lib/internal/Magento/Framework/Mail/Test/Unit/TransportTest.php +++ b/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.'); diff --git a/lib/internal/Magento/Framework/Mail/Transport.php b/lib/internal/Magento/Framework/Mail/Transport.php index 531b073cdee4c..29f0deeefce30 100644 --- a/lib/internal/Magento/Framework/Mail/Transport.php +++ b/lib/internal/Magento/Framework/Mail/Transport.php @@ -56,13 +56,16 @@ public function sendMessage(): void $mailer = new Mailer($this->symfonyTransport); $mailer->send($email); } catch (TransportExceptionInterface $transportException) { - $this->logger->error('Transport error while sending email: ' . $transportException->getMessage()); + $this->logger->error( + 'Transport error while sending an email message', + ['exception' => $transportException] + ); throw new MailException( new Phrase('Transport error: Unable to send mail at this time.'), $transportException ); } catch (\Exception $e) { - $this->logger->error($e); + $this->logger->error('Unable to send an email message', ['exception' => $e]); throw new MailException(new Phrase('Unable to send mail. Please try again later.'), $e); } } diff --git a/lib/internal/Magento/Framework/MessageQueue/Consumer.php b/lib/internal/Magento/Framework/MessageQueue/Consumer.php index fe31a769665bb..4fd6a95c902ac 100644 --- a/lib/internal/Magento/Framework/MessageQueue/Consumer.php +++ b/lib/internal/Magento/Framework/MessageQueue/Consumer.php @@ -224,6 +224,7 @@ private function getTransactionCallback(QueueInterface $queue) return function (EnvelopeInterface $message) use ($queue) { /** @var LockInterface $lock */ $lock = null; + $topicName = null; try { $topicName = $message->getProperties()['topic_name']; $topicConfig = $this->communicationConfig->getTopic($topicName); @@ -256,7 +257,10 @@ private function getTransactionCallback(QueueInterface $queue) } } catch (NotFoundException $exception) { $queue->acknowledge($message); - $this->logger->warning($exception->getMessage()); + $this->logger->warning( + 'Message of the {topicName} topic was acknowledged without being processed', + ['topicName' => $topicName, 'exception' => $exception] + ); } catch (Exception $exception) { $queue->reject($message, false, $exception->getMessage()); if ($lock) { diff --git a/lib/internal/Magento/Framework/MessageQueue/Test/Unit/ConsumerTest.php b/lib/internal/Magento/Framework/MessageQueue/Test/Unit/ConsumerTest.php index cc7db290d7b02..b162c3e362781 100644 --- a/lib/internal/Magento/Framework/MessageQueue/Test/Unit/ConsumerTest.php +++ b/lib/internal/Magento/Framework/MessageQueue/Test/Unit/ConsumerTest.php @@ -178,6 +178,7 @@ public function testProcessWithNotFoundException() $numberOfMessages = 1; $consumerName = 'consumer.name'; $exceptionPhrase = new Phrase('Exception successfully thrown'); + $notFoundException = new NotFoundException($exceptionPhrase); $this->poisonPillRead->expects($this->atLeastOnce())->method('getLatestVersion')->willReturn('version-1'); $this->poisonPillCompare->expects($this->atLeastOnce())->method('isLatestVersion')->willReturn(true); $this->deploymentConfig->expects($this->any())->method('get') @@ -191,13 +192,12 @@ public function testProcessWithNotFoundException() ->willReturn($topicConfig); $this->configuration->expects($this->atLeastOnce())->method('getConsumerName')->willReturn($consumerName); $this->messageController->expects($this->once())->method('lock')->with($envelope, $consumerName) - ->willThrowException( - new NotFoundException( - $exceptionPhrase - ) - ); + ->willThrowException($notFoundException); $queue->expects($this->once())->method('acknowledge')->with($envelope); - $this->logger->expects($this->once())->method('warning')->with($exceptionPhrase->render()); + $this->logger->expects($this->once())->method('warning')->with( + 'Message of the {topicName} topic was acknowledged without being processed', + ['topicName' => $properties['topic_name'], 'exception' => $notFoundException] + ); $this->consumer->process($numberOfMessages); } diff --git a/lib/internal/Magento/Framework/Model/ExecuteCommitCallbacks.php b/lib/internal/Magento/Framework/Model/ExecuteCommitCallbacks.php index b310a5f37fef6..3cdaad7208e2a 100644 --- a/lib/internal/Magento/Framework/Model/ExecuteCommitCallbacks.php +++ b/lib/internal/Magento/Framework/Model/ExecuteCommitCallbacks.php @@ -43,7 +43,7 @@ public function afterCommit(AdapterInterface $subject, AdapterInterface $result) try { call_user_func($callback); } catch (\Throwable $e) { - $this->logger->critical($e); + $this->logger->critical('Transaction commit callback failed', ['exception' => $e]); } } } diff --git a/lib/internal/Magento/Framework/Phrase/Renderer/Inline.php b/lib/internal/Magento/Framework/Phrase/Renderer/Inline.php index 0297ed93f05a4..09ed165719023 100644 --- a/lib/internal/Magento/Framework/Phrase/Renderer/Inline.php +++ b/lib/internal/Magento/Framework/Phrase/Renderer/Inline.php @@ -73,7 +73,7 @@ public function render(array $source, array $arguments) . '}}{{' . $this->translator->getTheme() . '}}}'; } } catch (\Exception $e) { - $this->logger->critical($e->getMessage()); + $this->logger->critical('Unable to render an inline translation', ['exception' => $e]); throw $e; } diff --git a/lib/internal/Magento/Framework/Phrase/Renderer/Translate.php b/lib/internal/Magento/Framework/Phrase/Renderer/Translate.php index de44896b76632..b4535cabf9cf4 100644 --- a/lib/internal/Magento/Framework/Phrase/Renderer/Translate.php +++ b/lib/internal/Magento/Framework/Phrase/Renderer/Translate.php @@ -64,7 +64,7 @@ public function render(array $source, array $arguments) try { $data = $this->translator->getData(); } catch (\Exception $e) { - $this->logger->critical($e->getMessage()); + $this->logger->critical('Unable to load translation data', ['exception' => $e]); throw $e; } diff --git a/lib/internal/Magento/Framework/Session/SaveHandler/Redis/Logger.php b/lib/internal/Magento/Framework/Session/SaveHandler/Redis/Logger.php index 82e5f27463c86..3b61a4ea25dfc 100644 --- a/lib/internal/Magento/Framework/Session/SaveHandler/Redis/Logger.php +++ b/lib/internal/Magento/Framework/Session/SaveHandler/Redis/Logger.php @@ -41,7 +41,10 @@ public function __construct(ConfigInterface $config, LoggerInterface $logger, Re } /** - * {@inheritdoc} + * @inheritdoc + * + * @param int $level + * @return void */ public function setLogLevel($level) { @@ -49,7 +52,11 @@ public function setLogLevel($level) } /** - * {@inheritdoc} + * @inheritdoc + * + * @param string $message + * @param int $level + * @return void */ public function log($message, $level) { @@ -84,10 +91,13 @@ public function log($message, $level) } /** - * {@inheritdoc} + * @inheritdoc + * + * @param \Exception $e + * @return void */ public function logException(\Exception $e) { - $this->logger->critical($e->getMessage()); + $this->logger->critical('Redis session handler failure', ['exception' => $e]); } } diff --git a/lib/internal/Magento/Framework/Session/Test/Unit/SaveHandler/Redis/LoggerTest.php b/lib/internal/Magento/Framework/Session/Test/Unit/SaveHandler/Redis/LoggerTest.php index 887b4a0c3f396..331be3b4be3f2 100644 --- a/lib/internal/Magento/Framework/Session/Test/Unit/SaveHandler/Redis/LoggerTest.php +++ b/lib/internal/Magento/Framework/Session/Test/Unit/SaveHandler/Redis/LoggerTest.php @@ -98,7 +98,7 @@ public function testLogException() $exception = new \Exception('Error message'); $this->psrLogger->expects($this->once()) ->method('critical') - ->with($exception->getMessage()); + ->with('Redis session handler failure', ['exception' => $exception]); $this->logger->logException($exception); } } diff --git a/lib/internal/Magento/Framework/Stomp/Jolokia/ArtemisClient.php b/lib/internal/Magento/Framework/Stomp/Jolokia/ArtemisClient.php index 0eb1624ce7115..ee742212042ab 100644 --- a/lib/internal/Magento/Framework/Stomp/Jolokia/ArtemisClient.php +++ b/lib/internal/Magento/Framework/Stomp/Jolokia/ArtemisClient.php @@ -39,7 +39,7 @@ public function isAvailable(): bool $data = $this->executeRequest($params); $result = isset($data['value']['agent']); } catch (RequestFailedException $e) { - $this->logger->notice($e); + $this->logger->notice('Artemis broker is not available', ['exception' => $e]); $result = false; } diff --git a/lib/internal/Magento/Framework/Stomp/Test/Unit/TopologyInstallerTest.php b/lib/internal/Magento/Framework/Stomp/Test/Unit/TopologyInstallerTest.php index db66a5ebc349a..ab5fbcdc4902c 100644 --- a/lib/internal/Magento/Framework/Stomp/Test/Unit/TopologyInstallerTest.php +++ b/lib/internal/Magento/Framework/Stomp/Test/Unit/TopologyInstallerTest.php @@ -60,17 +60,17 @@ protected function setUp(): void */ public function testInstallException() { - $exceptionMessage = "Exception message"; + $exception = new StompException('Exception message'); $this->topologyConfigMock ->expects($this->once()) ->method('getQueues') - ->willThrowException(new StompException($exceptionMessage)); + ->willThrowException($exception); $this->loggerMock ->expects($this->once()) ->method('error') - ->with($this->stringContains("STOMP topology installation failed: {$exceptionMessage}")); + ->with('STOMP topology installation failed', ['exception' => $exception]); $this->topologyInstaller->install(); } diff --git a/lib/internal/Magento/Framework/Stomp/TopologyInstaller.php b/lib/internal/Magento/Framework/Stomp/TopologyInstaller.php index 1fc7878a3eb28..63f529057598a 100644 --- a/lib/internal/Magento/Framework/Stomp/TopologyInstaller.php +++ b/lib/internal/Magento/Framework/Stomp/TopologyInstaller.php @@ -70,7 +70,7 @@ public function install(): void $this->queueInstaller->install($queue); } } catch (\Exception $e) { - $this->logger->error("STOMP topology installation failed: {$e->getMessage()}\n{$e->getTraceAsString()}"); + $this->logger->error('STOMP topology installation failed', ['exception' => $e]); } } } diff --git a/lib/internal/Magento/Framework/View/Asset/Merged.php b/lib/internal/Magento/Framework/View/Asset/Merged.php index 82be529b1994f..e0e8c98b55629 100644 --- a/lib/internal/Magento/Framework/View/Asset/Merged.php +++ b/lib/internal/Magento/Framework/View/Asset/Merged.php @@ -111,7 +111,7 @@ protected function initialize() $this->mergeStrategy->merge($this->assets, $mergedAsset); $this->assets = [$mergedAsset]; } catch (\Exception $e) { - $this->logger->critical($e); + $this->logger->critical('Unable to merge assets', ['exception' => $e]); } } } diff --git a/lib/internal/Magento/Framework/View/Design/Theme/Image.php b/lib/internal/Magento/Framework/View/Design/Theme/Image.php index bcb2330cd4783..96b800543b6d6 100644 --- a/lib/internal/Magento/Framework/View/Design/Theme/Image.php +++ b/lib/internal/Magento/Framework/View/Design/Theme/Image.php @@ -17,60 +17,46 @@ class Image { /** - * Preview image width + * Width in pixels a preview image is scaled down to */ - const PREVIEW_IMAGE_WIDTH = 800; + public const PREVIEW_IMAGE_WIDTH = 800; /** - * Preview image height + * Height in pixels a preview image is scaled down to */ - const PREVIEW_IMAGE_HEIGHT = 800; + public const PREVIEW_IMAGE_HEIGHT = 800; /** - * Media directory - * * @var WriteInterface */ protected $mediaDirectory; /** - * Root directory - * * @var WriteInterface */ protected $rootDirectory; /** - * Image factory - * * @var \Magento\Framework\Image\Factory */ protected $imageFactory; /** - * Image uploader - * * @var Image\Uploader */ protected $uploader; /** - * Theme image path - * * @var Image\PathInterface */ protected $themeImagePath; /** - * Logger - * * @var \Psr\Log\LoggerInterface */ protected $logger; /** - * Theme - * * @var ThemeInterface */ protected $theme; @@ -160,7 +146,10 @@ public function createPreviewImageCopy(ThemeInterface $theme) $this->theme->setPreviewImage($destinationFileName); } catch (\Magento\Framework\Exception\FileSystemException $e) { $this->theme->setPreviewImage(null); - $this->logger->critical($e); + $this->logger->critical( + 'Unable to create a theme preview image from {sourcePath}', + ['sourcePath' => $sourcePath, 'exception' => $e] + ); } return $isCopied; } diff --git a/lib/internal/Magento/Framework/View/Element/AbstractBlock.php b/lib/internal/Magento/Framework/View/Element/AbstractBlock.php index 06f7b66033d5c..fb57a0c857312 100644 --- a/lib/internal/Magento/Framework/View/Element/AbstractBlock.php +++ b/lib/internal/Magento/Framework/View/Element/AbstractBlock.php @@ -793,7 +793,10 @@ public function getViewFileUrl($fileId, array $params = []) $params = array_merge(['_secure' => $this->getRequest()->isSecure()], $params); return $this->_assetRepo->getUrlWithParams($fileId, $params); } catch (\Magento\Framework\Exception\LocalizedException $e) { - $this->_logger->critical($e); + $this->_logger->critical( + 'Unable to resolve the URL of the {fileId} view file', + ['fileId' => $fileId, 'exception' => $e] + ); return $this->_getNotFoundUrl(); } } diff --git a/lib/internal/Magento/Framework/View/Layout.php b/lib/internal/Magento/Framework/View/Layout.php index 8d4c66fdf5d1d..bef45a8b50131 100644 --- a/lib/internal/Magento/Framework/View/Layout.php +++ b/lib/internal/Magento/Framework/View/Layout.php @@ -611,7 +611,10 @@ public function renderNonCachedElement($name) if ($this->appState->getMode() === AppState::MODE_DEVELOPER) { throw $e; } - $this->logger->critical($e); + $this->logger->critical( + 'Unable to render the {elementName} layout element', + ['elementName' => $name, 'exception' => $e] + ); } return $result; } @@ -798,8 +801,7 @@ public function isManipulationAllowed($name) */ public function setBlock($name, $block) { - $name = $name ?? ''; - $this->_blocks[$name] = $block; + $this->_blocks[(string)$name] = $block; return $this; } diff --git a/lib/internal/Magento/Framework/View/Layout/Generator/Block.php b/lib/internal/Magento/Framework/View/Layout/Generator/Block.php index 1c020ffab1429..5398a73fe300f 100644 --- a/lib/internal/Magento/Framework/View/Layout/Generator/Block.php +++ b/lib/internal/Magento/Framework/View/Layout/Generator/Block.php @@ -278,7 +278,10 @@ protected function getBlockInstance($block, array $arguments = []) try { $block = $this->blockFactory->createBlock($block, $arguments); } catch (\ReflectionException $e) { - $this->logger->critical($e->getMessage()); + $this->logger->critical( + 'Unable to instantiate the {blockClass} block', + ['blockClass' => $block, 'exception' => $e] + ); } } if (!$block instanceof \Magento\Framework\View\Element\AbstractBlock) { diff --git a/lib/internal/Magento/Framework/View/Layout/ScheduledStructure/Helper.php b/lib/internal/Magento/Framework/View/Layout/ScheduledStructure/Helper.php index cbb279b47a3ef..e6b2f032b9a7c 100644 --- a/lib/internal/Magento/Framework/View/Layout/ScheduledStructure/Helper.php +++ b/lib/internal/Magento/Framework/View/Layout/ScheduledStructure/Helper.php @@ -192,7 +192,10 @@ public function scheduleElement( try { $structure->setAsChild($name, $parentName, $alias); } catch (\Exception $e) { - $this->logger->critical($e); + $this->logger->critical( + 'Unable to set the {elementName} element as a child of {parentName}', + ['elementName' => $name, 'parentName' => $parentName, 'exception' => $e] + ); } } else { $scheduledStructure->setElementToBrokenParentList($key); diff --git a/lib/internal/Magento/Framework/View/Page/Config/Renderer.php b/lib/internal/Magento/Framework/View/Page/Config/Renderer.php index 5ee1a1bcceddc..2e12b444569eb 100644 --- a/lib/internal/Magento/Framework/View/Page/Config/Renderer.php +++ b/lib/internal/Magento/Framework/View/Page/Config/Renderer.php @@ -476,7 +476,13 @@ protected function renderAssetHtml(\Magento\Framework\View\Asset\PropertyGroup $ $result .= sprintf($template, $asset->getUrl()); } } catch (LocalizedException $e) { - $this->logger->critical($e); + $this->logger->critical( + 'Unable to render assets of the {contentType} content type', + [ + 'contentType' => $group->getProperty(GroupedCollection::PROPERTY_CONTENT_TYPE), + 'exception' => $e, + ] + ); $template = $this->getAssetTemplate( $group->getProperty(GroupedCollection::PROPERTY_CONTENT_TYPE), $defaultAttributes diff --git a/lib/internal/Magento/Framework/View/Result/Page.php b/lib/internal/Magento/Framework/View/Result/Page.php index 642c94a533656..c159791a90ce8 100644 --- a/lib/internal/Magento/Framework/View/Result/Page.php +++ b/lib/internal/Magento/Framework/View/Result/Page.php @@ -374,7 +374,10 @@ protected function getViewFileUrl($fileId, array $params = []) $params = array_merge(['_secure' => $this->request->isSecure()], $params); return $this->assetRepo->getUrlWithParams($fileId, $params); } catch (\Magento\Framework\Exception\LocalizedException $e) { - $this->logger->critical($e); + $this->logger->critical( + 'Unable to resolve the URL of the {fileId} view file', + ['fileId' => $fileId, 'exception' => $e] + ); return $this->urlBuilder->getUrl('', ['_direct' => 'core/index/notFound']); } } diff --git a/lib/internal/Magento/Framework/View/TemplateEngine/Xhtml/Template.php b/lib/internal/Magento/Framework/View/TemplateEngine/Xhtml/Template.php index b3c4c219d8d87..2578c87da45f4 100644 --- a/lib/internal/Magento/Framework/View/TemplateEngine/Xhtml/Template.php +++ b/lib/internal/Magento/Framework/View/TemplateEngine/Xhtml/Template.php @@ -10,9 +10,9 @@ */ class Template { - const XML_VERSION = '1.0'; + public const XML_VERSION = '1.0'; - const XML_ENCODING = 'UTF-8'; + public const XML_ENCODING = 'UTF-8'; /** * @var \Psr\Log\LoggerInterface @@ -75,7 +75,7 @@ public function __toString() $this->templateNode->ownerDocument->normalizeDocument(); $result = $this->templateNode->ownerDocument->saveHTML(); } catch (\Exception $e) { - $this->logger->critical($e->getMessage()); + $this->logger->critical('Unable to render an XHTML template', ['exception' => $e]); $result = ''; } return $result; diff --git a/lib/internal/Magento/Framework/View/Test/Unit/Asset/MergedTest.php b/lib/internal/Magento/Framework/View/Test/Unit/Asset/MergedTest.php index a1c1922d3df4f..8a409974ef509 100644 --- a/lib/internal/Magento/Framework/View/Test/Unit/Asset/MergedTest.php +++ b/lib/internal/Magento/Framework/View/Test/Unit/Asset/MergedTest.php @@ -173,7 +173,8 @@ public function testIteratorInterfaceMergeFailure() 'versionStorage' => $this->versionStorage, ]); - $this->logger->expects($this->once())->method('critical')->with($this->identicalTo($mergeError)); + $this->logger->expects($this->once())->method('critical') + ->with('Unable to merge assets', $this->identicalTo(['exception' => $mergeError])); $expectedResult = [$this->assetJsOne, $this->assetJsTwo, $assetBroken]; $this->assertIteratorEquals($expectedResult, $merged); diff --git a/lib/internal/Magento/Framework/View/Test/Unit/LayoutTest.php b/lib/internal/Magento/Framework/View/Test/Unit/LayoutTest.php index 81a8e41c4da4a..a1f3cc3849c44 100644 --- a/lib/internal/Magento/Framework/View/Test/Unit/LayoutTest.php +++ b/lib/internal/Magento/Framework/View/Test/Unit/LayoutTest.php @@ -1248,7 +1248,10 @@ public function testRenderNonCachedElementWithException(): void $this->loggerMock->expects($this->once()) ->method('critical') - ->with($exception); + ->with( + 'Unable to render the {elementName} layout element', + ['elementName' => 'test_container', 'exception' => $exception] + ); $this->response->expects($this->once())->method('setNoCacheHeaders'); $model = clone $this->model; diff --git a/lib/internal/Magento/Framework/View/Test/Unit/Page/Config/RendererTest.php b/lib/internal/Magento/Framework/View/Test/Unit/Page/Config/RendererTest.php index 50051d3173171..8fb5118ea21a0 100644 --- a/lib/internal/Magento/Framework/View/Test/Unit/Page/Config/RendererTest.php +++ b/lib/internal/Magento/Framework/View/Test/Unit/Page/Config/RendererTest.php @@ -403,7 +403,10 @@ public function testRenderAssets($groupOne, $groupTwo, $expectedResult): void $this->loggerMock->expects($this->once()) ->method('critical') - ->with($exception); + ->with( + 'Unable to render assets of the {contentType} content type', + ['contentType' => $groupTwo['type'], 'exception' => $exception] + ); $this->urlBuilderMock->expects($this->once()) ->method('getUrl') diff --git a/lib/internal/Magento/Framework/Webapi/ErrorProcessor.php b/lib/internal/Magento/Framework/Webapi/ErrorProcessor.php index 36248b73776af..c6ecc6cac87d2 100644 --- a/lib/internal/Magento/Framework/Webapi/ErrorProcessor.php +++ b/lib/internal/Magento/Framework/Webapi/ErrorProcessor.php @@ -306,10 +306,10 @@ public function renderException(\Exception $exception, $httpCode = self::DEFAULT protected function _critical(\Exception $exception) { $reportId = uniqid("webapi-"); - $message = "Report ID: {$reportId}; Message: {$exception->getMessage()}"; - $code = $exception->getCode(); - $exception = new \Exception($message, $code, $exception); - $this->_logger->critical($exception); + $this->_logger->critical( + 'Web API request failed. Report ID: {reportId}', + ['reportId' => $reportId, 'exception' => $exception] + ); return $reportId; } diff --git a/lib/internal/Magento/Framework/Webapi/Test/Unit/ErrorProcessorTest.php b/lib/internal/Magento/Framework/Webapi/Test/Unit/ErrorProcessorTest.php index 57464be880139..65ab47321e464 100644 --- a/lib/internal/Magento/Framework/Webapi/Test/Unit/ErrorProcessorTest.php +++ b/lib/internal/Magento/Framework/Webapi/Test/Unit/ErrorProcessorTest.php @@ -257,8 +257,9 @@ public function testCriticalExceptionStackTrace() $this->_loggerMock->expects($this->once()) ->method('critical') ->willReturnCallback( - function (\Exception $loggedException) use ($thrownException) { - $this->assertSame($thrownException, $loggedException->getPrevious()); + function (string $message, array $context) use ($thrownException) { + $this->assertStringContainsString('Report ID:', $message); + $this->assertSame($thrownException, $context['exception']); } ); $this->_errorProcessor->maskException($thrownException);