From 052dcdebe8e2c287933caaac379f56423b1ab660 Mon Sep 17 00:00:00 2001 From: Christoph Wurst Date: Tue, 24 May 2022 08:39:20 +0200 Subject: Refactor the ErrorHandler into a dynamic class Signed-off-by: Christoph Wurst --- lib/base.php | 17 ++++++-- lib/private/Log/ErrorHandler.php | 83 ++++++++++++++++++---------------------- tests/lib/ErrorHandlerTest.php | 49 ++++++++++++++++-------- 3 files changed, 85 insertions(+), 64 deletions(-) diff --git a/lib/base.php b/lib/base.php index 04c456a12d8..fa69a7d6076 100644 --- a/lib/base.php +++ b/lib/base.php @@ -662,9 +662,20 @@ class OC { $config = \OC::$server->get(\OCP\IConfig::class); if (!defined('PHPUNIT_RUN')) { - OC\Log\ErrorHandler::setLogger(\OC::$server->getLogger()); - $debug = $config->getSystemValue('debug', false); - OC\Log\ErrorHandler::register($debug); + $errorHandler = new OC\Log\ErrorHandler( + \OCP\Server::get(\Psr\Log\LoggerInterface::class), + ); + $exceptionHandler = [$errorHandler, 'onException']; + if ($config->getSystemValue('debug', false)) { + set_error_handler([$errorHandler, 'onAll'], E_ALL); + if (\OC::$CLI) { + $exceptionHandler = ['OC_Template', 'printExceptionErrorPage']; + } + } else { + set_error_handler([$errorHandler, 'onError']); + } + register_shutdown_function([$errorHandler, 'onShutdown']); + set_exception_handler($exceptionHandler); } /** @var \OC\AppFramework\Bootstrap\Coordinator $bootstrapCoordinator */ diff --git a/lib/private/Log/ErrorHandler.php b/lib/private/Log/ErrorHandler.php index d56fecb1ecb..5f90c86c24e 100644 --- a/lib/private/Log/ErrorHandler.php +++ b/lib/private/Log/ErrorHandler.php @@ -1,4 +1,7 @@ logger = $logger; + } /** - * remove password in URLs - * @param string $msg - * @return string + * Remove password in URLs */ - protected static function removePassword($msg) { + private static function removePassword(string $msg): string { return preg_replace('#//(.*):(.*)@#', '//xxx:xxx@', $msg); } - public static function register($debug = false) { - $handler = new ErrorHandler(); - - if ($debug) { - set_error_handler([$handler, 'onAll'], E_ALL); - if (\OC::$CLI) { - set_exception_handler(['OC_Template', 'printExceptionErrorPage']); - } - } else { - set_error_handler([$handler, 'onError']); - } - register_shutdown_function([$handler, 'onShutdown']); - set_exception_handler([$handler, 'onException']); - } - - public static function setLogger(ILogger $logger) { - self::$logger = $logger; - } - - //Fatal errors handler - public static function onShutdown() { + /** + * Fatal errors handler + */ + public function onShutdown(): void { $error = error_get_last(); - if ($error && self::$logger) { - //ob_end_clean(); + if ($error) { $msg = $error['message'] . ' at ' . $error['file'] . '#' . $error['line']; - self::$logger->critical(self::removePassword($msg), ['app' => 'PHP']); + $this->logger->critical(self::removePassword($msg), ['app' => 'PHP']); } } /** - * Uncaught exception handler - * - * @param \Exception $exception + * Uncaught exception handler */ - public static function onException($exception) { + public function onException(Throwable $exception): void { $class = get_class($exception); $msg = $exception->getMessage(); $msg = "$class: $msg at " . $exception->getFile() . '#' . $exception->getLine(); - self::$logger->critical(self::removePassword($msg), ['app' => 'PHP']); + $this->logger->critical(self::removePassword($msg), ['app' => 'PHP']); } - //Recoverable errors handler - public static function onError($number, $message, $file, $line) { + /** + * Recoverable errors handler + */ + public function onError(int $number, string $message, string $file, int $line): bool { if (!(error_reporting() & $number)) { - return; + return true; } $msg = $message . ' at ' . $file . '#' . $line; - $e = new \Error(self::removePassword($msg)); - self::$logger->logException($e, ['app' => 'PHP', 'level' => self::errnoToLogLevel($number)]); + $e = new Error(self::removePassword($msg)); + $this->logger->log(self::errnoToLogLevel($number), $e->getMessage(), ['app' => 'PHP']); + return true; } - //Recoverable handler which catch all errors, warnings and notices - public static function onAll($number, $message, $file, $line) { + /** + * Recoverable handler which catch all errors, warnings and notices + */ + public function onAll(int $number, string $message, string $file, int $line): bool { $msg = $message . ' at ' . $file . '#' . $line; - $e = new \Error(self::removePassword($msg)); - self::$logger->logException($e, ['app' => 'PHP', 'level' => self::errnoToLogLevel($number)]); + $e = new Error(self::removePassword($msg)); + $this->logger->log(self::errnoToLogLevel($number), $e->getMessage(), ['app' => 'PHP']); + return true; } - public static function errnoToLogLevel(int $errno): int { + private static function errnoToLogLevel(int $errno): int { switch ($errno) { case E_USER_WARNING: return ILogger::WARN; diff --git a/tests/lib/ErrorHandlerTest.php b/tests/lib/ErrorHandlerTest.php index ea53e67005c..f6bf850f0b5 100644 --- a/tests/lib/ErrorHandlerTest.php +++ b/tests/lib/ErrorHandlerTest.php @@ -1,4 +1,7 @@ logger = $this->createMock(LoggerInterface::class); + $this->errorHandler = new ErrorHandler( + $this->logger + ); + } /** * provide username, password combinations for testRemovePassword @@ -47,24 +69,19 @@ class ErrorHandlerTest extends \Test\TestCase { * @param string $username * @param string $password */ - public function testRemovePassword($username, $password) { + public function testRemovePasswordFromError($username, $password) { $url = 'http://'.$username.':'.$password.'@owncloud.org'; $expectedResult = 'http://xxx:xxx@owncloud.org'; - $result = TestableErrorHandler::testRemovePassword($url); + $this->logger->expects(self::once()) + ->method('log') + ->with( + ILogger::ERROR, + 'Could not reach ' . $expectedResult . ' at file#4', + ['app' => 'PHP'], + ); - $this->assertEquals($expectedResult, $result); - } -} + $result = $this->errorHandler->onError(E_USER_ERROR, 'Could not reach ' . $url, 'file', 4); -/** - * dummy class to access protected methods of \OC\Log\ErrorHandler - */ -class TestableErrorHandler extends \OC\Log\ErrorHandler { - - /** - * @param string $msg - */ - public static function testRemovePassword($msg) { - return self::removePassword($msg); + self::assertTrue($result); } } -- cgit v1.2.3