ENGCOM-4104: #20773: Do not throw exception during autoload #20950

Author: sivaschenko

dev/tests/integration/testsuite/Magento/Framework/Code/Generator/AutoloaderTest.php

@@ -0,0 +1,85 @@
+<?php declare(strict_types=1);
+/**
+ * Copyright © Magento, Inc. All rights reserved.
+ * See COPYING.txt for license details.
+ */
+
+namespace Magento\Framework\Code\Generator;
+
+use Magento\Framework\Code\Generator;
+use Magento\Framework\Logger\Monolog as MagentoMonologLogger;
+use Magento\TestFramework\ObjectManager;
+use PHPUnit\Framework\TestCase;
+use PHPUnit_Framework_MockObject_MockObject as MockObject;
+use Psr\Log\LoggerInterface;
+
+class AutoloaderTest extends TestCase
+{
+    /**
+     * This method exists to fix the wrong return type hint on \Magento\Framework\App\ObjectManager::getInstance.
+     * This way the IDE knows it's dealing with an instance of \Magento\TestFramework\ObjectManager and
+     * not \Magento\Framework\App\ObjectManager. The former has the method addSharedInstance, the latter does not.
+     *
+     * @return ObjectManager|\Magento\Framework\App\ObjectManager
+     * @SuppressWarnings(PHPMD.StaticAccess)
+     */
+    private function getTestFrameworkObjectManager()
+    {
+        return ObjectManager::getInstance();
+    }
+
+    /**
+     * @before
+     */
+    public function setupLoggerTestDouble(): void
+    {
+        $loggerTestDouble = $this->createMock(LoggerInterface::class);
+        $this->getTestFrameworkObjectManager()->addSharedInstance($loggerTestDouble, MagentoMonologLogger::class);
+    }
+
+    /**
+     * @after
+     */
+    public function removeLoggerTestDouble(): void
+    {
+        $this->getTestFrameworkObjectManager()->removeSharedInstance(MagentoMonologLogger::class);
+    }
+
+    /**
+     * @param \RuntimeException $testException
+     * @return Generator|MockObject
+     */
+    private function createExceptionThrowingGeneratorTestDouble(\RuntimeException $testException)
+    {
+        /** @var Generator|MockObject $generatorStub */
+        $generatorStub = $this->createMock(Generator::class);
+        $generatorStub->method('generateClass')->willThrowException($testException);
+
+        return $generatorStub;
+    }
+
+    public function testLogsExceptionDuringGeneration(): void
+    {
+        $exceptionMessage = 'Test exception thrown during generation';
+        $testException = new \RuntimeException($exceptionMessage);
+
+        $loggerMock = ObjectManager::getInstance()->get(LoggerInterface::class);
+        $loggerMock->expects($this->once())->method('debug')->with($exceptionMessage, ['exception' => $testException]);
+
+        $autoloader = new Autoloader($this->createExceptionThrowingGeneratorTestDouble($testException));
+        $this->assertNull($autoloader->load(NonExistingClassName::class));
+    }
+
+    public function testFiltersDuplicateExceptionMessages(): void
+    {
+        $exceptionMessage = 'Test exception thrown during generation';
+        $testException = new \RuntimeException($exceptionMessage);
+
+        $loggerMock = ObjectManager::getInstance()->get(LoggerInterface::class);
+        $loggerMock->expects($this->once())->method('debug')->with($exceptionMessage, ['exception' => $testException]);
+
+        $autoloader = new Autoloader($this->createExceptionThrowingGeneratorTestDouble($testException));
+        $autoloader->load(OneNonExistingClassName::class);
+        $autoloader->load(AnotherNonExistingClassName::class);
+    }
+}

lib/internal/Magento/Framework/Code/Generator/Autoloader.php

@@ -3,37 +3,94 @@
  * Copyright © Magento, Inc. All rights reserved.
  * See COPYING.txt for license details.
  */
+
 namespace Magento\Framework\Code\Generator;
 
+use Magento\Framework\App\ObjectManager;
 use Magento\Framework\Code\Generator;
+use Psr\Log\LoggerInterface;
 
+/**
+ * Class loader and generator.
+ */
 class Autoloader
 {
     /**
-     * @var \Magento\Framework\Code\Generator
+     * @var Generator
      */
     protected $_generator;
 
     /**
-     * @param \Magento\Framework\Code\Generator $generator
+     * Enables guarding against spamming the debug log with duplicate messages, as
+     * the generation exception will be thrown multiple times within a single request.
+     *
+     * @var string
+     */
+    private $lastGenerationErrorMessage;
+
+    /**
+     * @param Generator $generator
      */
-    public function __construct(
-        \Magento\Framework\Code\Generator $generator
-    ) {
+    public function __construct(Generator $generator)
+    {
         $this->_generator = $generator;
     }
 
     /**
      * Load specified class name and generate it if necessary
      *
+     * According to PSR-4 section 2.4 an autoloader MUST NOT throw an exception and SHOULD NOT return a value.
+     *
+     * @see https://www.php-fig.org/psr/psr-4/
+     *
      * @param string $className
-     * @return bool True if class was loaded
+     * @return void
      */
     public function load($className)
     {
-        if (!class_exists($className)) {
-            return Generator::GENERATION_ERROR != $this->_generator->generateClass($className);
+        if (! class_exists($className)) {
+            try {
+                $this->_generator->generateClass($className);
+            } catch (\Exception $exception) {
+                $this->tryToLogExceptionMessageIfNotDuplicate($exception);
+            }
+        }
+    }
+
+    /**
+     * Log exception.
+     *
+     * @param \Exception $exception
+     */
+    private function tryToLogExceptionMessageIfNotDuplicate(\Exception $exception): void
+    {
+        if ($this->lastGenerationErrorMessage !== $exception->getMessage()) {
+            $this->lastGenerationErrorMessage = $exception->getMessage();
+            $this->tryToLogException($exception);
+        }
+    }
+
+    /**
+     * Try to capture the exception message.
+     *
+     * The Autoloader is instantiated before the ObjectManager, so the LoggerInterface can not be injected.
+     * The Logger is instantiated in the try/catch block because ObjectManager might still not be initialized.
+     * In that case the exception message can not be captured.
+     *
+     * The debug level is used for logging in case class generation fails for a common class, but a custom
+     * autoloader is used later in the stack. A more severe log level would fill the logs with messages on production.
+     * The exception message now can be accessed in developer mode if debug logging is enabled.
+     *
+     * @param \Exception $exception
+     * @return void
+     */
+    private function tryToLogException(\Exception $exception): void
+    {
+        try {
+            $logger = ObjectManager::getInstance()->get(LoggerInterface::class);
+            $logger->debug($exception->getMessage(), ['exception' => $exception]);
+        } catch (\Exception $ignoreThisException) {
+            // Do not take an action here, since the original exception might have been caused by logger
         }
-        return true;
     }
 }