From 6e8ac67d4e86e5ae56be4339ab2a3767ba35171a Mon Sep 17 00:00:00 2001 From: Dustin Wilson Date: Tue, 8 Aug 2023 23:17:01 -0500 Subject: [PATCH] Modified supplemental error handling --- lib/Catcher.php | 19 +++++++----- lib/Catcher/Handler.php | 11 ++++--- tests/cases/TestHandler.php | 60 ++++++++++++++++++++++++++----------- tests/lib/FailLogger.php | 6 ++++ 4 files changed, 64 insertions(+), 32 deletions(-) diff --git a/lib/Catcher.php b/lib/Catcher.php index eba27c1..6586345 100644 --- a/lib/Catcher.php +++ b/lib/Catcher.php @@ -59,6 +59,14 @@ class Catcher { + + /** Checks if the error code is fatal */ + public static function isErrorFatal(int $code): bool { + return in_array($code, [ \E_ERROR, \E_PARSE, \E_CORE_ERROR, \E_COMPILE_ERROR, \E_USER_ERROR, \E_RECOVERABLE_ERROR ]); + } + + + public function getHandlers(): array { return $this->handlers; } @@ -159,7 +167,7 @@ class Catcher { public function handleError(int $code, string $message, ?string $file = null, ?int $line = null): bool { if ($code && $code & error_reporting()) { $error = new Error($message, $code, $file, $line); - if ($this->errorHandlingMethod > self::THROW_NO_ERRORS && ($this->errorHandlingMethod === self::THROW_ALL_ERRORS || $this->isErrorFatal($code)) && !$this->isShuttingDown) { + if ($this->errorHandlingMethod > self::THROW_NO_ERRORS && ($this->errorHandlingMethod === self::THROW_ALL_ERRORS || self::isErrorFatal($code)) && !$this->isShuttingDown) { $this->lastThrowable = $error; throw $error; } else { @@ -198,7 +206,7 @@ class Catcher { $exit || $this->isShuttingDown || $throwable instanceof \Exception || - ($throwable instanceof Error && $this->isErrorFatal($throwable->getCode())) || + ($throwable instanceof Error && self::isErrorFatal($throwable->getCode())) || (!$throwable instanceof Error && $throwable instanceof \Error) ) { foreach ($this->handlers as $h) { @@ -230,7 +238,7 @@ class Catcher { $this->isShuttingDown = true; if ($error = $this->getLastError()) { - if ($this->isErrorFatal($error['type'])) { + if (self::isErrorFatal($error['type'])) { $errorReporting = error_reporting(); if ($this->errorReporting !== null && $this->errorReporting === $errorReporting && ($this->errorReporting & \E_ERROR) === 0) { error_reporting($errorReporting | \E_ERROR); @@ -251,11 +259,6 @@ class Catcher { exit($status); // @codeCoverageIgnore } - /** Checks if the error code is fatal */ - protected function isErrorFatal(int $code): bool { - return in_array($code, [ \E_ERROR, \E_PARSE, \E_CORE_ERROR, \E_COMPILE_ERROR, \E_USER_ERROR, \E_RECOVERABLE_ERROR ]); - } - /** Exists so the method may be replaced when mocking in tests */ protected function getLastError(): ?array { return error_get_last(); diff --git a/lib/Catcher/Handler.php b/lib/Catcher/Handler.php index 0ec53be..a2334cc 100644 --- a/lib/Catcher/Handler.php +++ b/lib/Catcher/Handler.php @@ -153,7 +153,7 @@ abstract class Handler { set_exception_handler($exceptionHandler); // If the current exception handler happens to not be Catcher use PHP's handler - // instead; this shouldn't happen in normal operation but is here just in case + // instead. if (!is_array($exceptionHandler) || !$exceptionHandler[0] instanceof Catcher) { return false; } @@ -174,11 +174,10 @@ abstract class Handler { // If all of the handlers are silent then use PHP's handler instead; this is // because a valid use for Catcher is to have it be silent but instead have the - // logger print the errors to stderr/stdout; if there is an error in the logger - // then it wouldn't print. - if ($silentCount === $handlersCount) { - // TODO: Output an error here to state that Catcher failed? - return false; + // logger print the errors to stderr/stdout. This should only apply to fatal + // errors; this shouldn't happen in normal operation but is here just in case + if (Catcher::isErrorFatal($code) && $silentCount === $handlersCount) { + return false; //@codeCoverageIgnore } $catcher->handleError($code, $message, $file, $line); diff --git a/tests/cases/TestHandler.php b/tests/cases/TestHandler.php index 7c5b1db..460d1c2 100644 --- a/tests/cases/TestHandler.php +++ b/tests/cases/TestHandler.php @@ -8,7 +8,7 @@ declare(strict_types=1); namespace MensBeam\Catcher\Test; use MensBeam\Catcher\{ - Error, + Error as CatcherError, Handler, RangeException, ThrowableController @@ -128,9 +128,10 @@ class TestHandler extends ErrorHandlingTestCase { } - public function testFatalError(): void { - $this->expectException(RangeException::class); - $this->handler->setOption('httpCode', 42); + /** @dataProvider provideFatalErrorTests */ + public function testFatalErrors(string $throwableClassName, \Closure $closure): void { + $this->expectException($throwableClassName); + $closure($this->handler); } /** @dataProvider provideNonFatalErrorTests */ @@ -141,6 +142,29 @@ class TestHandler extends ErrorHandlingTestCase { } + public static function provideFatalErrorTests(): iterable { + $iterable = [ + [ + RangeException::class, + function (Handler $h): void { + $h->setOption('httpCode', 42); + } + ], + [ + CatcherError::class, + function (Handler $h): void { + $c = new Catcher(); + $l = new FailLogger(); + $l->error('Ook!'); + } + ] + ]; + + foreach ($iterable as $i) { + yield $i; + } + } + public static function provideArgumentSerializationTests(): iterable { $options = [ [ fn() => true ], @@ -162,7 +186,7 @@ class TestHandler extends ErrorHandlingTestCase { [ new \Exception('Ook!'), Handler::BUBBLES | Handler::OUTPUT | Handler::EXIT, [ 'forceExit' => true ] ], [ new \Error('Ook!'), Handler::BUBBLES | Handler::OUTPUT ], [ new \Exception('Ook!'), Handler::BUBBLES, [ 'silent' => true ] ], - [ new Error('Ook!', \E_ERROR, '/dev/null', 42, new \Error('Eek!')), Handler::BUBBLES | Handler::OUTPUT | Handler::NOW, [ 'forceOutputNow' => true ] ], + [ new CatcherError('Ook!', \E_ERROR, '/dev/null', 42, new \Error('Eek!')), Handler::BUBBLES | Handler::OUTPUT | Handler::NOW, [ 'forceOutputNow' => true ] ], [ new \Exception('Ook!'), Handler::BUBBLES, [ 'silent' => true, 'logger' => Phake::mock(LoggerInterface::class), 'logWhenSilent' => false ] ], [ new \Error('Ook!'), Handler::BUBBLES | Handler::OUTPUT | Handler::LOG, [ 'forceOutputNow' => true, 'logger' => Phake::mock(LoggerInterface::class) ] ] ]; @@ -178,19 +202,19 @@ class TestHandler extends ErrorHandlingTestCase { public static function provideLogTests(): iterable { $options = [ - [ new Error('Ook!', \E_NOTICE, '/dev/null', 0, new \Error('Eek!')), 'notice' ], - [ new Error('Ook!', \E_USER_NOTICE, '/dev/null', 0), 'notice' ], - [ new Error('Ook!', \E_STRICT, '/dev/null', 0, new \Error('Eek!')), 'notice' ], - [ new Error('Ook!', \E_WARNING, '/dev/null', 0), 'warning' ], - [ new Error('Ook!', \E_COMPILE_WARNING, '/dev/null', 0, new \Error('Eek!')), 'warning' ], - [ new Error('Ook!', \E_USER_WARNING, '/dev/null', 0), 'warning' ], - [ new Error('Ook!', \E_DEPRECATED, '/dev/null', 0, new \Error('Eek!')), 'warning' ], - [ new Error('Ook!', \E_USER_DEPRECATED, '/dev/null', 0), 'warning' ], - [ new Error('Ook!', \E_PARSE, '/dev/null', 0, new \Error('Eek!')), 'critical' ], - [ new Error('Ook!', \E_CORE_ERROR, '/dev/null', 0), 'critical' ], - [ new Error('Ook!', \E_COMPILE_ERROR, '/dev/null', 0, new \Error('Eek!')), 'critical' ], - [ new Error('Ook!', \E_ERROR, '/dev/null', 0), 'error' ], - [ new Error('Ook!', \E_USER_ERROR, '/dev/null', 0, new \Error('Eek!')), 'error' ], + [ new CatcherError('Ook!', \E_NOTICE, '/dev/null', 0, new \Error('Eek!')), 'notice' ], + [ new CatcherError('Ook!', \E_USER_NOTICE, '/dev/null', 0), 'notice' ], + [ new CatcherError('Ook!', \E_STRICT, '/dev/null', 0, new \Error('Eek!')), 'notice' ], + [ new CatcherError('Ook!', \E_WARNING, '/dev/null', 0), 'warning' ], + [ new CatcherError('Ook!', \E_COMPILE_WARNING, '/dev/null', 0, new \Error('Eek!')), 'warning' ], + [ new CatcherError('Ook!', \E_USER_WARNING, '/dev/null', 0), 'warning' ], + [ new CatcherError('Ook!', \E_DEPRECATED, '/dev/null', 0, new \Error('Eek!')), 'warning' ], + [ new CatcherError('Ook!', \E_USER_DEPRECATED, '/dev/null', 0), 'warning' ], + [ new CatcherError('Ook!', \E_PARSE, '/dev/null', 0, new \Error('Eek!')), 'critical' ], + [ new CatcherError('Ook!', \E_CORE_ERROR, '/dev/null', 0), 'critical' ], + [ new CatcherError('Ook!', \E_COMPILE_ERROR, '/dev/null', 0, new \Error('Eek!')), 'critical' ], + [ new CatcherError('Ook!', \E_ERROR, '/dev/null', 0), 'error' ], + [ new CatcherError('Ook!', \E_USER_ERROR, '/dev/null', 0, new \Error('Eek!')), 'error' ], [ new \PharException('Ook!'), 'alert' ], [ new \Exception('Ook!'), 'critical' ], ]; diff --git a/tests/lib/FailLogger.php b/tests/lib/FailLogger.php index f0ef3b9..f5c3f9a 100644 --- a/tests/lib/FailLogger.php +++ b/tests/lib/FailLogger.php @@ -19,5 +19,11 @@ class FailLogger implements LoggerInterface { $v = vfsStream::setup('ook'); $d = vfsStream::newDirectory('ook', 0777)->at($v); file_put_contents($d->url(), $message); + + echo "Ook!\n"; + } + + public function error(string|\Stringable $message, array $context = []): void { + trigger_error('Ook!', \E_USER_ERROR); } }