diff --git a/CHANGELOG.md b/CHANGELOG.md index 51cba1fc..c51edb9d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,13 @@ Changelog ========= +## 2.22.2 (2021-09-06) + +### Bug Fixes + +* Fix events in Lumen always being handled + [#452](https://github.com/bugsnag/bugsnag-laravel/pull/452) + ## 2.22.1 (2021-04-29) ### Bug Fixes diff --git a/src/BugsnagServiceProvider.php b/src/BugsnagServiceProvider.php index cd9102d7..dfd1ea60 100644 --- a/src/BugsnagServiceProvider.php +++ b/src/BugsnagServiceProvider.php @@ -36,7 +36,7 @@ class BugsnagServiceProvider extends ServiceProvider * * @var string */ - const VERSION = '2.22.1'; + const VERSION = '2.22.2'; /** * Boot the service provider. diff --git a/src/Internal/BacktraceProcessor.php b/src/Internal/BacktraceProcessor.php new file mode 100644 index 00000000..6fc11bce --- /dev/null +++ b/src/Internal/BacktraceProcessor.php @@ -0,0 +1,186 @@ +backtrace = $backtrace; + } + + /** + * Determine if the backtrace was from an unhandled error. + * + * @return bool + */ + public function isUnhandled() + { + foreach ($this->backtrace as $frame) { + $this->processFrame($frame); + + // stop iterating early if we know we're done + if ($this->state === self::STATE_DONE) { + break; + } + } + + return $this->unhandled; + } + + /** + * @param array $frame + * + * @return void + */ + private function processFrame(array $frame) + { + if (!isset($frame['class'])) { + return; + } + + $class = $frame['class']; + + switch ($this->state) { + case self::STATE_FRAMEWORK_HANDLER: + // if this class is a framework exception handler and the function + // matches self::HANDLER_METHOD, we can move on to searching for + // the caller + if (($class === self::LARAVEL_HANDLER_CLASS || $class === self::LUMEN_HANDLER_CLASS) + && isset($frame['function']) + && $frame['function'] === self::HANDLER_METHOD + ) { + $this->state = self::STATE_HANDLER_CALLER; + } + + break; + + case self::STATE_HANDLER_CALLER: + // if this is an app exception handler or a framework class, we + // can move on to determine if this was unhandled or not + if ($class === self::LARAVEL_APP_EXCEPTION_HANDLER + || $class === self::LUMEN_APP_EXCEPTION_HANDLER + || $this->isVendor($class) + ) { + $this->state = self::STATE_IS_UNHANDLED; + } + + break; + + case self::STATE_IS_UNHANDLED: + // we are only interested in running this once so move immediately + // into the "done" state. This ensures we only check the frame + // immediately before the caller of the exception handler + $this->state = self::STATE_DONE; + + // if this class is internal to the framework then the exception + // was unhandled + if ($this->isVendor($class)) { + $this->unhandled = true; + } + + break; + } + } + + /** + * Does the given class belong to a vendor namespace? + * + * @see self::VENDOR_NAMESPACES + * + * @param string $class + * + * @return bool + */ + private function isVendor($class) + { + return substr($class, 0, strlen(self::LARAVEL_VENDOR_NAMESPACE)) === self::LARAVEL_VENDOR_NAMESPACE + || substr($class, 0, strlen(self::LUMEN_VENDOR_NAMESPACE)) === self::LUMEN_VENDOR_NAMESPACE; + } +} diff --git a/src/Middleware/UnhandledState.php b/src/Middleware/UnhandledState.php index 40cf7c0c..d237f2e7 100644 --- a/src/Middleware/UnhandledState.php +++ b/src/Middleware/UnhandledState.php @@ -2,30 +2,11 @@ namespace Bugsnag\BugsnagLaravel\Middleware; +use Bugsnag\BugsnagLaravel\Internal\BacktraceProcessor; use Bugsnag\Report; class UnhandledState { - /** - * Unhandled state middleware implementation details. - * - * This middleware functions on the basis of three things: - * 1. All unhandled exceptions must pass through the `HANDLER_CLASS` report - * method - * 2. Unhandled exceptions will have had a caller from inside the Illuminate - * namespace or App exception handler - * 3. The above exception handler must have originally been called from - * within the Illuminate namespace - * - * This middleware calls the inbuilt PHP backtrace, and traverses each frame - * to determine if the above conditions are met. If they are, the report is - * marked as unhandled. - */ - const HANDLER_CLASS = 'Illuminate\\Foundation\\Exceptions\\Handler'; - const HANDLER_METHOD = 'report'; - const ILLUMINATE_NAMESPACE = 'Illuminate'; - const APP_EXCEPTION_HANDLER = 'App\\Exception\\Handler'; - /** * Execute the unhandled state middleware. * @@ -37,50 +18,21 @@ class UnhandledState public function __invoke(Report $report, callable $next) { $stackFrames = debug_backtrace(DEBUG_BACKTRACE_IGNORE_ARGS); - $reportFrame = false; - $callerFrame = false; - $unhandled = false; - if (is_null($stackFrames)) { - return; - } - foreach ($stackFrames as $frame) { - if (!array_key_exists('class', $frame)) { - continue; - } - $class = $frame['class']; - if (!$reportFrame) { - if (!array_key_exists('function', $frame)) { - continue; - } elseif (($class === $this::HANDLER_CLASS) && ($frame['function'] === $this::HANDLER_METHOD)) { - $reportFrame = true; - } - } elseif (!$callerFrame) { - $startsWithIlluminate = substr($class, 0, strlen($this::ILLUMINATE_NAMESPACE)) === $this::ILLUMINATE_NAMESPACE; - if ($startsWithIlluminate || ($class == $this::APP_EXCEPTION_HANDLER)) { - $callerFrame = true; - } - } elseif (!$unhandled) { - $startsWithIlluminate = substr($class, 0, strlen($this::ILLUMINATE_NAMESPACE)) === $this::ILLUMINATE_NAMESPACE; - if ($startsWithIlluminate) { - $unhandled = true; - } - break; - } + + if (!is_array($stackFrames)) { + $stackFrames = []; } - if ($unhandled) { + + $backtraceProcessor = new BacktraceProcessor($stackFrames); + + if ($backtraceProcessor->isUnhandled()) { $report->setUnhandled(true); $report->setSeverityReason([ 'type' => 'unhandledExceptionMiddleware', - 'attributes' => [ - 'framework' => 'Laravel', - ], + 'attributes' => ['framework' => 'Laravel'], ]); } - $next($report); - } - protected function stringStartsWith($haystack, $needle) - { - return substr($haystack, 0, strlen($needle)) === $needle; + $next($report); } } diff --git a/tests/Middleware/UnhandledStateTest.php b/tests/Middleware/UnhandledStateTest.php new file mode 100644 index 00000000..79b045a6 --- /dev/null +++ b/tests/Middleware/UnhandledStateTest.php @@ -0,0 +1,346 @@ +report = Report::fromPHPThrowable( + new Configuration('api-key'), + new Exception('abc') + ); + + $this->nextWasCalled = false; + $this->next = function (Report $report) { + $this->assertSame($this->report, $report); + $this->nextWasCalled = true; + }; + } + + /** + * @after + */ + protected function afterEach() + { + DebugBacktraceStub::clear(); + + $this->assertTrue($this->nextWasCalled); + } + + /** + * @dataProvider unhandledBacktraceProviderLaravel + * @dataProvider unhandledBacktraceProviderLumen + */ + public function testReportIsUnhandled(array $backtrace) + { + DebugBacktraceStub::set($backtrace); + + $this->assertFalse($this->report->getUnhandled()); + $this->assertSame(['type' => 'handledException'], $this->report->getSeverityReason()); + + $unhandledState = new UnhandledState(); + $unhandledState->__invoke($this->report, $this->next); + + $this->assertTrue( + $this->report->getUnhandled(), + 'Expected the report to be unhandled but it was handled!' + ); + + $this->assertSame( + [ + 'type' => 'unhandledExceptionMiddleware', + 'attributes' => ['framework' => 'Laravel'], + ], + $this->report->getSeverityReason() + ); + } + + public function unhandledBacktraceProviderLaravel() + { + yield 'minimal backtrace' => [[ + // the backtrace must go through the Handler::report method + ['class' => \Illuminate\Foundation\Exceptions\Handler::class, 'function' => 'report'], + // then any class in the Illuminate namespace + ['class' => \Illuminate\Something::class], + // followed by any other class in the Illuminate namespace + ['class' => \Illuminate\SomethingElse::class], + ]]; + + yield 'minimal backtrace (App exception handler)' => [[ + // the backtrace must go through the Handler::report method + ['class' => \Illuminate\Foundation\Exceptions\Handler::class, 'function' => 'report'], + // then through the app exception handler + ['class' => \App\Exception\Handler::class], + // followed by any other class in the Illuminate namespace + ['class' => \Illuminate\SomethingElse::class], + ]]; + + yield 'backtrace with other classes' => [[ + ['class' => \SomeClass::class], + ['class' => \Illuminate\Foundation\Exceptions\Handler::class, 'function' => 'report'], + ['class' => \Some\OtherClass::class], + ['class' => \Illuminate\Abc::class], + ['class' => \Illuminate\AbcElse::class], + ['class' => \Yet\AnotherClass::class], + ]]; + + yield 'backtrace with other classes (App exception handler)' => [[ + ['class' => \SomeClass::class], + ['class' => \Illuminate\Foundation\Exceptions\Handler::class, 'function' => 'report'], + ['class' => \Some\OtherClass::class], + ['class' => \App\Exception\Handler::class], + ['class' => \Illuminate\AbcElse::class], + ['class' => \Yet\AnotherClass::class], + ]]; + + yield 'backtrace with non-class frames' => [[ + ['class' => \SomeClass::class], + ['function' => 'a'], + ['class' => \Illuminate\Foundation\Exceptions\Handler::class, 'function' => 'report'], + ['function' => 'b'], + ['function' => 'c'], + ['class' => \Some\OtherClass::class], + ['class' => \Illuminate\Abc::class], + ['function' => 'x'], + ['function' => 'y'], + ['class' => \Illuminate\AbcElse::class], + ['function' => 'z'], + ['class' => \Yet\AnotherClass::class], + ]]; + + yield 'backtrace with non-class frames (App exception handler)' => [[ + ['class' => \SomeClass::class], + ['function' => 'a'], + ['class' => \Illuminate\Foundation\Exceptions\Handler::class, 'function' => 'report'], + ['function' => 'b'], + ['function' => 'c'], + ['class' => \Some\OtherClass::class], + ['class' => \App\Exception\Handler::class], + ['function' => 'x'], + ['function' => 'y'], + ['class' => \Illuminate\AbcElse::class], + ['function' => 'z'], + ['class' => \Yet\AnotherClass::class], + ]]; + } + + public function unhandledBacktraceProviderLumen() + { + yield 'Lumen (App exception handler)' => [[ + ['class' => \Bugsnag\PsrLogger\AbstractLogger::class, 'function' => 'error'], + ['class' => \Laravel\Lumen\Exceptions\Handler::class, 'function' => 'report'], + ['class' => \App\Exceptions\Handler::class, 'function' => 'report'], + ['class' => \Laravel\Lumen\Application::class, 'function' => 'sendExceptionToHandler'], + ['class' => \Laravel\Lumen\Application::class, 'function' => 'dispatch'], + ['class' => \Laravel\Lumen\Application::class, 'function' => 'run'], + ]]; + + yield 'Lumen' => [[ + ['class' => \SomeClass::class], + ['class' => \Laravel\Lumen\Exceptions\Handler::class, 'function' => 'report'], + ['class' => \Some\OtherClass::class], + ['class' => \Laravel\Lumen\Application::class, 'function' => 'sendExceptionToHandler'], + ['class' => \Laravel\Lumen\Application::class, 'function' => 'dispatch'], + ['class' => \Laravel\Lumen\Application::class, 'function' => 'run'], + ]]; + + yield 'Lumen with other classes (App exception handler)' => [[ + ['class' => \SomeClass::class], + ['class' => \Laravel\Lumen\Exceptions\Handler::class, 'function' => 'report'], + ['class' => \Some\OtherClass::class], + ['class' => \App\Exceptions\Handler::class, 'function' => 'report'], + ['class' => \Laravel\Lumen\Application::class, 'function' => 'sendExceptionToHandler'], + ['class' => \Laravel\Lumen\Application::class, 'function' => 'dispatch'], + ['class' => \Laravel\Lumen\Application::class, 'function' => 'run'], + ]]; + + yield 'Lumen with other classes' => [[ + ['class' => \Bugsnag\PsrLogger\AbstractLogger::class, 'function' => 'error'], + ['class' => \SomeClass::class], + ['class' => \Laravel\Lumen\Exceptions\Handler::class, 'function' => 'report'], + ['class' => \Some\OtherClass::class], + ['class' => \Laravel\Lumen\Application::class, 'function' => 'sendExceptionToHandler'], + ['class' => \Laravel\Lumen\Application::class, 'function' => 'dispatch'], + ['class' => \Laravel\Lumen\Application::class, 'function' => 'run'], + ]]; + + yield 'Lumen with non-class frames (App exception handler)' => [[ + ['class' => \Bugsnag\PsrLogger\AbstractLogger::class, 'function' => 'error'], + ['function' => 'abc'], + ['class' => \Laravel\Lumen\Exceptions\Handler::class, 'function' => 'report'], + ['function' => 'xyz'], + ['class' => \App\Exceptions\Handler::class, 'function' => 'report'], + ['class' => \Laravel\Lumen\Application::class, 'function' => 'sendExceptionToHandler'], + ['class' => \Laravel\Lumen\Application::class, 'function' => 'dispatch'], + ['class' => \Laravel\Lumen\Application::class, 'function' => 'run'], + ]]; + + yield 'Lumen with non-class frames' => [[ + ['class' => \Bugsnag\PsrLogger\AbstractLogger::class, 'function' => 'error'], + ['function' => 'abc'], + ['class' => \Laravel\Lumen\Exceptions\Handler::class, 'function' => 'report'], + ['function' => 'xyz'], + ['class' => \Laravel\Lumen\Application::class, 'function' => 'sendExceptionToHandler'], + ['class' => \Laravel\Lumen\Application::class, 'function' => 'dispatch'], + ['class' => \Laravel\Lumen\Application::class, 'function' => 'run'], + ]]; + } + + /** + * @dataProvider handledBacktraceProviderLaravel + * @dataProvider handledBacktraceProviderLumen + */ + public function testReportIsHandled(array $backtrace) + { + DebugBacktraceStub::set($backtrace); + + $this->assertFalse($this->report->getUnhandled()); + $this->assertSame(['type' => 'handledException'], $this->report->getSeverityReason()); + + $unhandledState = new UnhandledState(); + $unhandledState->__invoke($this->report, $this->next); + + $this->assertFalse($this->report->getUnhandled()); + $this->assertSame(['type' => 'handledException'], $this->report->getSeverityReason()); + } + + public function handledBacktraceProviderLaravel() + { + yield 'empty backtrace' => [[[]]]; + + yield 'no illuminate exception handler' => [[ + ['class' => \NotIlluminate\Foundation\Exceptions\Handler::class, 'function' => 'report'], + ['class' => \Illuminate\Something::class], + ['class' => \Illuminate\SomethingElse::class], + ]]; + + yield 'illuminate exception handler but wrong method' => [[ + ['class' => \Illuminate\Foundation\Exceptions\Handler::class, 'function' => 'notReport'], + ['class' => \Illuminate\Something::class], + ['class' => \Illuminate\SomethingElse::class], + ]]; + + yield 'no illuminate exception handler (App exception handler)' => [[ + ['class' => \NotIlluminate\Foundation\Exceptions\Handler::class, 'function' => 'report'], + ['class' => \App\Exception\Handler::class], + ['class' => \Illuminate\SomethingElse::class], + ]]; + + yield 'illuminate exception handler but wrong method (App exception handler)' => [[ + ['class' => \Illuminate\Foundation\Exceptions\Handler::class, 'function' => 'notReport'], + ['class' => \App\Exception\Handler::class], + ['class' => \Illuminate\SomethingElse::class], + ]]; + + yield 'illuminate namespace not at the beginning' => [[ + ['class' => \Illuminate\Foundation\Exceptions\Handler::class, 'function' => 'report'], + ['class' => \Abc\Illuminate\Something::class], + ['class' => \Abc\Illuminate\SomethingElse::class], + ]]; + + yield 'illuminate namespace not at the beginning (App exception handler)' => [[ + ['class' => \Illuminate\Foundation\Exceptions\Handler::class, 'function' => 'report'], + ['class' => \App\Exception\Handler::class], + ['class' => \Abc\Illuminate\SomethingElse::class], + ]]; + + yield 'no consecutive Illuminate classes' => [[ + ['class' => \SomeClass::class], + ['class' => \Illuminate\Foundation\Exceptions\Handler::class, 'function' => 'report'], + ['class' => \Some\OtherClass::class], + ['class' => \Illuminate\Abc::class], + // this ensures the report is handled as there must be two + // consecutive Illuminate frames + ['class' => \AnotherClass::class], + ['class' => \Illuminate\AbcElse::class], + ['class' => \Yet\AnotherClass::class], + ]]; + + yield 'no consecutive Illuminate classes (App exception handler)' => [[ + ['class' => \SomeClass::class], + ['class' => \Illuminate\Foundation\Exceptions\Handler::class, 'function' => 'report'], + ['class' => \Some\OtherClass::class], + ['class' => \App\Exception\Handler::class], + // this ensures the report is handled as there must be two + // consecutive Illuminate frames + ['class' => \AnotherClass::class], + ['class' => \Illuminate\AbcElse::class], + ['class' => \Yet\AnotherClass::class], + ]]; + } + + public function handledBacktraceProviderLumen() + { + yield 'Lumen no exception handler' => [[ + ['class' => \Bugsnag\PsrLogger\AbstractLogger::class, 'function' => 'error'], + ['class' => \Not\Laravel\Lumen\Exceptions\Handler::class, 'function' => 'report'], + ['class' => \App\Exceptions\Handler::class, 'function' => 'report'], + ['class' => \Laravel\Lumen\Application::class, 'function' => 'sendExceptionToHandler'], + ['class' => \Laravel\Lumen\Application::class, 'function' => 'dispatch'], + ['class' => \Laravel\Lumen\Application::class, 'function' => 'run'], + ]]; + + yield 'Lumen exception handler but wrong method' => [[ + ['class' => \Bugsnag\PsrLogger\AbstractLogger::class, 'function' => 'error'], + ['class' => \Laravel\Lumen\Exceptions\Handler::class, 'function' => 'notReport'], + ['class' => \App\Exceptions\Handler::class, 'function' => 'report'], + ['class' => \Laravel\Lumen\Application::class, 'function' => 'sendExceptionToHandler'], + ['class' => \Laravel\Lumen\Application::class, 'function' => 'dispatch'], + ['class' => \Laravel\Lumen\Application::class, 'function' => 'run'], + ]]; + + yield 'Lumen non-consecutive frames after App exception handler' => [[ + ['class' => \Bugsnag\PsrLogger\AbstractLogger::class, 'function' => 'error'], + ['class' => \Laravel\Lumen\Exceptions\Handler::class, 'function' => 'report'], + ['class' => \App\Exceptions\Handler::class, 'function' => 'report'], + ['class' => \Not\Laravel\Lumen\Application::class, 'function' => 'sendExceptionToHandler'], + ['class' => \Laravel\Lumen\Application::class, 'function' => 'dispatch'], + ['class' => \Laravel\Lumen\Application::class, 'function' => 'run'], + ]]; + + yield 'Lumen non-consecutive frames after Lumen exception handler' => [[ + ['class' => \Bugsnag\PsrLogger\AbstractLogger::class, 'function' => 'error'], + ['class' => \Laravel\Lumen\Exceptions\Handler::class, 'function' => 'report'], + ['class' => \Laravel\Lumen\Application::class, 'function' => 'sendExceptionToHandler'], + ['class' => \Not\Laravel\Lumen\Application::class, 'function' => 'dispatch'], + ['class' => \Laravel\Lumen\Application::class, 'function' => 'run'], + ]]; + } +} diff --git a/tests/Stubs/DebugBacktraceStub.php b/tests/Stubs/DebugBacktraceStub.php new file mode 100644 index 00000000..df5bb9e7 --- /dev/null +++ b/tests/Stubs/DebugBacktraceStub.php @@ -0,0 +1,32 @@ +