From 55d47c30d00546f28a93a68c94ff0ec96c0f79c4 Mon Sep 17 00:00:00 2001 From: Nuno Maduro Date: Fri, 15 Oct 2021 15:50:35 +0100 Subject: [PATCH 1/4] Logs deprecations instead of treating them as exceptions --- phpunit.xml.dist | 1 + .../Foundation/Bootstrap/HandleExceptions.php | 71 +++++++- .../Bootstrap/HandleExceptionsTest.php | 168 ++++++++++++++++++ 3 files changed, 239 insertions(+), 1 deletion(-) create mode 100644 tests/Foundation/Bootstrap/HandleExceptionsTest.php diff --git a/phpunit.xml.dist b/phpunit.xml.dist index 05294e99751a..fda9396657f2 100644 --- a/phpunit.xml.dist +++ b/phpunit.xml.dist @@ -6,6 +6,7 @@ convertErrorsToExceptions="true" convertNoticesToExceptions="true" convertWarningsToExceptions="true" + convertDeprecationsToExceptions="false" processIsolation="false" stopOnError="false" stopOnFailure="false" diff --git a/src/Illuminate/Foundation/Bootstrap/HandleExceptions.php b/src/Illuminate/Foundation/Bootstrap/HandleExceptions.php index 280efaa87158..61a58d43b22b 100644 --- a/src/Illuminate/Foundation/Bootstrap/HandleExceptions.php +++ b/src/Illuminate/Foundation/Bootstrap/HandleExceptions.php @@ -6,6 +6,7 @@ use Exception; use Illuminate\Contracts\Debug\ExceptionHandler; use Illuminate\Contracts\Foundation\Application; +use Illuminate\Log\LogManager; use Symfony\Component\Console\Output\ConsoleOutput; use Symfony\Component\ErrorHandler\Error\FatalError; use Throwable; @@ -52,7 +53,50 @@ public function bootstrap(Application $app) } /** - * Convert PHP errors to ErrorException instances. + * Reports an deprecation to the "deprecations" logger. + * + * @param string $message + * @param string $file + * @param int $line + * @return void + */ + public function handleDeprecation($message, $file, $line) + { + try { + $logger = $this->getLogger(); + } catch (Exception $e) { + return; + } + + $this->ensureDeprecationsLoggerIsConfigured(); + + with($logger->channel('deprecations'), function ($log) use ($message, $file, $line) { + $log->warning(sprintf('%s in %s on line %s', + $message, $file, $line + )); + }); + } + + /** + * Ensures the "deprecations" logger is configured. + * + * @return void + */ + public function ensureDeprecationsLoggerIsConfigured() + { + with($this->app['config'], function ($config) { + if ($config->get('logging.channels.deprecations')) { + return; + } + + $driver = $config->get('logging.deprecations') ?? 'null'; + + $config->set('logging.channels.deprecations', $config->get("logging.channels.{$driver}")); + }); + } + + /** + * Report PHP deprecations, or convert PHP errors to ErrorException instances. * * @param int $level * @param string $message @@ -66,6 +110,10 @@ public function bootstrap(Application $app) public function handleError($level, $message, $file = '', $line = 0, $context = []) { if (error_reporting() & $level) { + if ($this->isDeprecation($level)) { + return $this->handleDeprecation($message, $file, $line); + } + throw new ErrorException($message, 0, $level, $file, $line); } } @@ -143,6 +191,17 @@ protected function fatalErrorFromPhpError(array $error, $traceOffset = null) return new FatalError($error['message'], 0, $error, $traceOffset); } + /** + * Determine if the error level is a deprecation. + * + * @param int $level + * @return bool + */ + protected function isDeprecation($level) + { + return in_array($level, [E_DEPRECATED, E_USER_DEPRECATED]); + } + /** * Determine if the error type is fatal. * @@ -163,4 +222,14 @@ protected function getExceptionHandler() { return $this->app->make(ExceptionHandler::class); } + + /** + * Get an instance of the logger implementation. + * + * @return \Illuminate\Log\LogManager + */ + public function getLogger() + { + return $this->app->make(LogManager::class); + } } diff --git a/tests/Foundation/Bootstrap/HandleExceptionsTest.php b/tests/Foundation/Bootstrap/HandleExceptionsTest.php new file mode 100644 index 000000000000..32c98a3f8ce1 --- /dev/null +++ b/tests/Foundation/Bootstrap/HandleExceptionsTest.php @@ -0,0 +1,168 @@ +container = Container::setInstance(new Container); + + $this->config = new Config(); + + $this->container->singleton('config', function () { + return $this->config; + }); + + $this->handleExceptions = new HandleExceptions(); + + with(new ReflectionClass($this->handleExceptions), function ($reflection) { + $reflection->getProperty('app')->setValue( + $this->handleExceptions, + $this->container + ); + }); + } + + protected function tearDown(): void + { + Container::setInstance(null); + } + + public function testPhpDeprecations() + { + $logger = m::mock(LogManager::class); + $this->container->instance(LogManager::class, $logger); + $logger->shouldReceive('channel')->with('deprecations')->andReturnSelf(); + $logger->shouldReceive('warning')->with(sprintf('%s in %s on line %s', + 'str_contains(): Passing null to parameter #2 ($needle) of type string is deprecated', + '/home/user/laravel/routes/web.php', + 17 + )); + + $this->handleExceptions->handleError( + E_DEPRECATED, + 'str_contains(): Passing null to parameter #2 ($needle) of type string is deprecated', + '/home/user/laravel/routes/web.php', + 17 + ); + } + + public function testUserDeprecations() + { + $logger = m::mock(LogManager::class); + $this->container->instance(LogManager::class, $logger); + $logger->shouldReceive('channel')->with('deprecations')->andReturnSelf(); + $logger->shouldReceive('warning')->with(sprintf('%s in %s on line %s', + 'str_contains(): Passing null to parameter #2 ($needle) of type string is deprecated', + '/home/user/laravel/routes/web.php', + 17 + )); + + $this->handleExceptions->handleError( + E_USER_DEPRECATED, + 'str_contains(): Passing null to parameter #2 ($needle) of type string is deprecated', + '/home/user/laravel/routes/web.php', + 17 + ); + } + + public function testErrors() + { + $logger = m::mock(LogManager::class); + $this->container->instance(LogManager::class, $logger); + $logger->shouldNotReceive('channel'); + $logger->shouldNotReceive('warning'); + + $this->expectException(ErrorException::class); + $this->expectExceptionMessage('Something went wrong'); + + $this->handleExceptions->handleError( + E_ERROR, + 'Something went wrong', + '/home/user/laravel/src/Providers/AppServiceProvider.php', + 17 + ); + } + + public function testEnsuresDeprecationsDriver() + { + $logger = m::mock(LogManager::class); + $this->container->instance(LogManager::class, $logger); + $logger->shouldReceive('channel')->andReturnSelf(); + $logger->shouldReceive('warning'); + + $this->config->set('logging.channels.stack', [ + 'driver' => 'stack', + 'channels' => ['single'], + 'ignore_exceptions' => false, + ]); + $this->config->set('logging.deprecations', 'stack'); + + $this->handleExceptions->handleError( + E_USER_DEPRECATED, + 'str_contains(): Passing null to parameter #2 ($needle) of type string is deprecated', + '/home/user/laravel/routes/web.php', + 17 + ); + + $this->assertEquals( + [ + 'driver' => 'stack', + 'channels' => ['single'], + 'ignore_exceptions' => false, + ], + $this->config->get('logging.channels.deprecations') + ); + } + + public function testEnsuresNullDeprecationsDriver() + { + $logger = m::mock(LogManager::class); + $this->container->instance(LogManager::class, $logger); + $logger->shouldReceive('channel')->andReturnSelf(); + $logger->shouldReceive('warning'); + + $this->config->set('logging.channels.null', [ + 'driver' => 'monolog', + 'handler' => NullHandler::class, + ]); + + $this->handleExceptions->handleError( + E_USER_DEPRECATED, + 'str_contains(): Passing null to parameter #2 ($needle) of type string is deprecated', + '/home/user/laravel/routes/web.php', + 17 + ); + + $this->assertEquals( + NullHandler::class, + $this->config->get('logging.channels.deprecations.handler') + ); + } + + public function testNoDeprecationsDriverIfNoDeprecationsHereSend() + { + $this->assertEquals(null, $this->config->get('logging.deprecations')); + $this->assertEquals(null, $this->config->get('logging.channels.deprecations')); + } + + public function testIgnoreDeprecationIfLoggerUnresolvable() + { + $this->handleExceptions->handleError( + E_DEPRECATED, + 'str_contains(): Passing null to parameter #2 ($needle) of type string is deprecated', + '/home/user/laravel/routes/web.php', + 17 + ); + } +} From ed9df3ba2b337c645de20e65b6e4360298c2eef7 Mon Sep 17 00:00:00 2001 From: Nuno Maduro Date: Fri, 15 Oct 2021 16:09:40 +0100 Subject: [PATCH 2/4] Fixes property access --- tests/Foundation/Bootstrap/HandleExceptionsTest.php | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/Foundation/Bootstrap/HandleExceptionsTest.php b/tests/Foundation/Bootstrap/HandleExceptionsTest.php index 32c98a3f8ce1..14a314322268 100644 --- a/tests/Foundation/Bootstrap/HandleExceptionsTest.php +++ b/tests/Foundation/Bootstrap/HandleExceptionsTest.php @@ -26,7 +26,9 @@ protected function setUp(): void $this->handleExceptions = new HandleExceptions(); with(new ReflectionClass($this->handleExceptions), function ($reflection) { - $reflection->getProperty('app')->setValue( + $property = tap($reflection->getProperty('app'))->setAccessible(true); + + $property->setValue( $this->handleExceptions, $this->container ); From c781ec16c19fd0cfb4fb77c2a43ad57495b67c11 Mon Sep 17 00:00:00 2001 From: Nuno Maduro Date: Fri, 15 Oct 2021 17:10:52 +0100 Subject: [PATCH 3/4] No need PHPUnit changes --- phpunit.xml.dist | 1 - 1 file changed, 1 deletion(-) diff --git a/phpunit.xml.dist b/phpunit.xml.dist index fda9396657f2..05294e99751a 100644 --- a/phpunit.xml.dist +++ b/phpunit.xml.dist @@ -6,7 +6,6 @@ convertErrorsToExceptions="true" convertNoticesToExceptions="true" convertWarningsToExceptions="true" - convertDeprecationsToExceptions="false" processIsolation="false" stopOnError="false" stopOnFailure="false" From 359ae4c46fbe85352d416c74ec895b536134cb68 Mon Sep 17 00:00:00 2001 From: Taylor Otwell Date: Mon, 18 Oct 2021 15:34:41 -0500 Subject: [PATCH 4/4] formatting --- .../Foundation/Bootstrap/HandleExceptions.php | 66 ++++++++----------- 1 file changed, 28 insertions(+), 38 deletions(-) diff --git a/src/Illuminate/Foundation/Bootstrap/HandleExceptions.php b/src/Illuminate/Foundation/Bootstrap/HandleExceptions.php index 61a58d43b22b..7914695678c7 100644 --- a/src/Illuminate/Foundation/Bootstrap/HandleExceptions.php +++ b/src/Illuminate/Foundation/Bootstrap/HandleExceptions.php @@ -53,7 +53,30 @@ public function bootstrap(Application $app) } /** - * Reports an deprecation to the "deprecations" logger. + * Report PHP deprecations, or convert PHP errors to ErrorException instances. + * + * @param int $level + * @param string $message + * @param string $file + * @param int $line + * @param array $context + * @return void + * + * @throws \ErrorException + */ + public function handleError($level, $message, $file = '', $line = 0, $context = []) + { + if (error_reporting() & $level) { + if ($this->isDeprecation($level)) { + return $this->handleDeprecation($message, $file, $line); + } + + throw new ErrorException($message, 0, $level, $file, $line); + } + } + + /** + * Reports a deprecation to the "deprecations" logger. * * @param string $message * @param string $file @@ -63,12 +86,12 @@ public function bootstrap(Application $app) public function handleDeprecation($message, $file, $line) { try { - $logger = $this->getLogger(); + $logger = $this->app->make(LogManager::class); } catch (Exception $e) { return; } - $this->ensureDeprecationsLoggerIsConfigured(); + $this->ensureDeprecationLoggerIsConfigured(); with($logger->channel('deprecations'), function ($log) use ($message, $file, $line) { $log->warning(sprintf('%s in %s on line %s', @@ -78,11 +101,11 @@ public function handleDeprecation($message, $file, $line) } /** - * Ensures the "deprecations" logger is configured. + * Ensure the "deprecations" logger is configured. * * @return void */ - public function ensureDeprecationsLoggerIsConfigured() + protected function ensureDeprecationLoggerIsConfigured() { with($this->app['config'], function ($config) { if ($config->get('logging.channels.deprecations')) { @@ -95,29 +118,6 @@ public function ensureDeprecationsLoggerIsConfigured() }); } - /** - * Report PHP deprecations, or convert PHP errors to ErrorException instances. - * - * @param int $level - * @param string $message - * @param string $file - * @param int $line - * @param array $context - * @return void - * - * @throws \ErrorException - */ - public function handleError($level, $message, $file = '', $line = 0, $context = []) - { - if (error_reporting() & $level) { - if ($this->isDeprecation($level)) { - return $this->handleDeprecation($message, $file, $line); - } - - throw new ErrorException($message, 0, $level, $file, $line); - } - } - /** * Handle an uncaught exception from the application. * @@ -222,14 +222,4 @@ protected function getExceptionHandler() { return $this->app->make(ExceptionHandler::class); } - - /** - * Get an instance of the logger implementation. - * - * @return \Illuminate\Log\LogManager - */ - public function getLogger() - { - return $this->app->make(LogManager::class); - } }