diff --git a/CHANGELOG.md b/CHANGELOG.md index 10a8a473..8ef89332 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,9 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. ## Unreleased - ... +## 4.0.0 (TBA) + - Enable back all error listeners from base SDK integration (#322) + ## 3.5.1 (2020-05-07) - Capture events using the `Hub` in the `MessengerListener` to avoid loosing `Scope` data (#339, thanks to @sh41) - Capture multiple events if multiple exceptions are generated in a Messenger Worker context (#340, thanks to @emarref) diff --git a/composer.json b/composer.json index 31847e41..cc219f96 100644 --- a/composer.json +++ b/composer.json @@ -21,6 +21,7 @@ "require": { "php": "^7.1", "jean85/pretty-package-versions": "^1.0", + "ocramius/package-versions": "^1.3.0", "sentry/sdk": "^2.1", "symfony/config": "^3.4||^4.0||^5.0", "symfony/console": "^3.4||^4.0||^5.0", @@ -44,6 +45,7 @@ "symfony/messenger": "^4.3||^5.0", "symfony/monolog-bundle": "^3.4", "symfony/phpunit-bridge": "^5.0", + "symfony/process": "^3.4||^4.0||^5.0", "symfony/yaml": "^3.4||^4.0||^5.0" }, "conflict": { diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index 00067d1e..4bc8e729 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -235,11 +235,6 @@ parameters: count: 1 path: test/End2End/App/Kernel.php - - - message: "#^Class PHPUnit_Framework_TestCase not found\\.$#" - count: 1 - path: test/End2End/End2EndTest.php - - message: "#^Class Symfony\\\\Bundle\\\\FrameworkBundle\\\\Client not found\\.$#" count: 1 diff --git a/src/DependencyInjection/IntegrationFilterFactory.php b/src/DependencyInjection/IntegrationFilterFactory.php deleted file mode 100644 index e33d6c1f..00000000 --- a/src/DependencyInjection/IntegrationFilterFactory.php +++ /dev/null @@ -1,35 +0,0 @@ - $processedOptions['excluded_exceptions'], - ]; - - $integrations[] = new Definition(IgnoreErrorsIntegration::class, [$ignoreOptions]); - } + // we ignore fatal errors wrapped by Symfony because they produce double event reporting + $processedOptions['excluded_exceptions'][] = FatalError::class; + $ignoreOptions = [ + 'ignore_exceptions' => $processedOptions['excluded_exceptions'], + ]; - $integrationsCallable = new Definition('callable', [$integrations]); - $integrationsCallable->setFactory([IntegrationFilterFactory::class, 'create']); + $integrations[] = new Definition(IgnoreErrorsIntegration::class, [$ignoreOptions]); - $options->addMethodCall('setIntegrations', [$integrationsCallable]); + $options->addMethodCall('setIntegrations', [$integrations]); } /** diff --git a/test/DependencyInjection/SentryExtensionTest.php b/test/DependencyInjection/SentryExtensionTest.php index 0e68b747..b84efb6a 100644 --- a/test/DependencyInjection/SentryExtensionTest.php +++ b/test/DependencyInjection/SentryExtensionTest.php @@ -8,8 +8,6 @@ use Sentry\Breadcrumb; use Sentry\ClientInterface; use Sentry\Event; -use Sentry\Integration\ErrorListenerIntegration; -use Sentry\Integration\ExceptionListenerIntegration; use Sentry\Integration\IntegrationInterface; use Sentry\Monolog\Handler; use Sentry\Options; @@ -339,14 +337,6 @@ public function testIntegrations(): void $found = false; foreach ($integrations as $integration) { - if ($integration instanceof ErrorListenerIntegration) { - $this->fail('Should not have ErrorListenerIntegration registered'); - } - - if ($integration instanceof ExceptionListenerIntegration) { - $this->fail('Should not have ExceptionListenerIntegration registered'); - } - if ($integration instanceof IntegrationMock) { $found = true; } diff --git a/test/End2End/App/Controller/MainController.php b/test/End2End/App/Controller/MainController.php index 6fdb9c23..500cccd1 100644 --- a/test/End2End/App/Controller/MainController.php +++ b/test/End2End/App/Controller/MainController.php @@ -31,6 +31,13 @@ public function exception(): Response throw new \RuntimeException('This is an intentional error'); } + public function fatal(): Response + { + $foo = eval("return new class() implements \Serializable {};"); + + return new Response('This response should not happen: ' . json_encode($foo)); + } + public function index(): Response { $this->sentry->captureMessage('Hello there'); @@ -38,6 +45,13 @@ public function index(): Response return new Response('Hello there'); } + public function notice(): Response + { + @trigger_error('This is an intentional notice', E_USER_NOTICE); + + return new Response('Hello there'); + } + public function subrequest(): Response { $request = $this->requestStack->getCurrentRequest(); diff --git a/test/End2End/App/config.yml b/test/End2End/App/config.yml index 7f966599..684c93cc 100644 --- a/test/End2End/App/config.yml +++ b/test/End2End/App/config.yml @@ -1,3 +1,8 @@ +sentry: + options: + capture_silenced_errors: true + error_types: E_ALL & ~E_USER_DEPRECATED + framework: router: { resource: "%routing_config_dir%/routing.yml" } secret: secret @@ -8,6 +13,17 @@ services: alias: Sentry\State\HubInterface public: true + Sentry\ClientBuilderInterface: + class: Sentry\ClientBuilder + arguments: + - '@Sentry\Options' + calls: + - method: setTransportFactory + arguments: + - '@Sentry\SentryBundle\Test\End2End\StubTransportFactory' + + Sentry\SentryBundle\Test\End2End\StubTransportFactory: ~ + Sentry\SentryBundle\Test\End2End\App\Controller\MainController: autowire: true tags: diff --git a/test/End2End/App/routing.yml b/test/End2End/App/routing.yml index 651fd22c..fd0fd0ac 100644 --- a/test/End2End/App/routing.yml +++ b/test/End2End/App/routing.yml @@ -2,6 +2,10 @@ exception: path: /exception defaults: { _controller: 'Sentry\SentryBundle\Test\End2End\App\Controller\MainController::exception' } +fatal: + path: /fatal + defaults: { _controller: 'Sentry\SentryBundle\Test\End2End\App\Controller\MainController::fatal' } + 200: path: /200 defaults: { _controller: 'Sentry\SentryBundle\Test\End2End\App\Controller\MainController::index' } @@ -13,3 +17,7 @@ secured200: subrequest: path: /subrequest defaults: { _controller: 'Sentry\SentryBundle\Test\End2End\App\Controller\MainController::subrequest' } + +notice: + path: /notice + defaults: { _controller: 'Sentry\SentryBundle\Test\End2End\App\Controller\MainController::notice' } diff --git a/test/End2End/End2EndTest.php b/test/End2End/End2EndTest.php index b99b9e31..bc78da3a 100644 --- a/test/End2End/End2EndTest.php +++ b/test/End2End/End2EndTest.php @@ -2,7 +2,6 @@ namespace Sentry\SentryBundle\Test\End2End; -use PHPUnit\Framework\TestCase; use Sentry\SentryBundle\Test\End2End\App\Kernel; use Sentry\State\HubInterface; use Symfony\Bundle\FrameworkBundle\Client; @@ -11,7 +10,6 @@ use Symfony\Component\HttpFoundation\Response; use Symfony\Component\HttpKernel\Exception\NotFoundHttpException; -class_alias(TestCase::class, \PHPUnit_Framework_TestCase::class); if (! class_exists(KernelBrowser::class)) { class_alias(Client::class, KernelBrowser::class); } @@ -21,11 +19,20 @@ class_alias(Client::class, KernelBrowser::class); */ class End2EndTest extends WebTestCase { + public const SENT_EVENTS_LOG = '/tmp/sentry_e2e_test_sent_events.log'; + protected static function getKernelClass(): string { return Kernel::class; } + protected function setUp(): void + { + parent::setUp(); + + file_put_contents(self::SENT_EVENTS_LOG, ''); + } + public function testGet200(): void { $client = static::createClient(['debug' => false]); @@ -38,6 +45,7 @@ public function testGet200(): void $this->assertSame(200, $response->getStatusCode()); $this->assertLastEventIdIsNotNull($client); + $this->assertEventCount(1); } public function testGet200BehindFirewall(): void @@ -52,6 +60,7 @@ public function testGet200BehindFirewall(): void $this->assertSame(200, $response->getStatusCode()); $this->assertLastEventIdIsNotNull($client); + $this->assertEventCount(1); } public function testGet200WithSubrequest(): void @@ -66,6 +75,7 @@ public function testGet200WithSubrequest(): void $this->assertSame(200, $response->getStatusCode()); $this->assertLastEventIdIsNotNull($client); + $this->assertEventCount(1); } public function testGet404(): void @@ -88,6 +98,7 @@ public function testGet404(): void } $this->assertLastEventIdIsNotNull($client); + $this->assertEventCount(1); } public function testGet500(): void @@ -111,6 +122,44 @@ public function testGet500(): void } $this->assertLastEventIdIsNotNull($client); + $this->assertEventCount(1); + } + + public function testGetFatal(): void + { + $client = static::createClient(); + + try { + $client->insulate(true); + $client->request('GET', '/fatal'); + + $response = $client->getResponse(); + + $this->assertInstanceOf(Response::class, $response); + $this->assertSame(500, $response->getStatusCode()); + $this->assertStringNotContainsString('not happen', $response->getContent() ?: ''); + } catch (\RuntimeException $exception) { + $this->assertStringContainsStringIgnoringCase('error', $exception->getMessage()); + $this->assertStringContainsStringIgnoringCase('contains 2 abstract methods', $exception->getMessage()); + $this->assertStringContainsStringIgnoringCase('MainController.php', $exception->getMessage()); + $this->assertStringContainsStringIgnoringCase('eval()\'d code on line', $exception->getMessage()); + } + + $this->assertEventCount(1); + } + + public function testNotice(): void + { + $client = static::createClient(); + $client->request('GET', '/notice'); + + $response = $client->getResponse(); + + $this->assertInstanceOf(Response::class, $response); + $this->assertSame(200, $response->getStatusCode()); + + $this->assertLastEventIdIsNotNull($client); + $this->assertEventCount(1); } private function assertLastEventIdIsNotNull(KernelBrowser $client): void @@ -123,4 +172,12 @@ private function assertLastEventIdIsNotNull(KernelBrowser $client): void $this->assertNotNull($hub->getLastEventId(), 'Last error not captured'); } + + private function assertEventCount(int $expectedCount): void + { + $events = file_get_contents(self::SENT_EVENTS_LOG); + $this->assertNotFalse($events, 'Cannot read sent events log'); + $listOfEvents = array_filter(explode(StubTransportFactory::SEPARATOR, trim($events))); + $this->assertCount($expectedCount, $listOfEvents, 'Wrong number of events sent: ' . PHP_EOL . $events); + } } diff --git a/test/End2End/StubTransportFactory.php b/test/End2End/StubTransportFactory.php new file mode 100644 index 00000000..0e375505 --- /dev/null +++ b/test/End2End/StubTransportFactory.php @@ -0,0 +1,39 @@ +getMessage()) { + $message = $event->getMessage(); + } elseif ($event->getExceptions()) { + $message = $event->getExceptions()[0]['value']; + } else { + $message = 'NO MESSAGE NOR EXCEPTIONS'; + } + + file_put_contents( + End2EndTest::SENT_EVENTS_LOG, + $event->getId() . ': ' . $message . PHP_EOL . StubTransportFactory::SEPARATOR . PHP_EOL, + FILE_APPEND + ); + + return $event->getId(); + } + }; + } +} diff --git a/test/SentryBundleTest.php b/test/SentryBundleTest.php index aa91657b..87ad492b 100644 --- a/test/SentryBundleTest.php +++ b/test/SentryBundleTest.php @@ -127,7 +127,7 @@ public function testContainerHasTestCommandRegisteredCorrectly(): void $this->assertArrayHasKey('console.command', $consoleListener->getTags()); } - public function testIntegrationsListenersAreDisabledByDefault(): void + public function testIntegrationsListenersAreEnabled(): void { $container = $this->getContainer(); @@ -135,8 +135,8 @@ public function testIntegrationsListenersAreDisabledByDefault(): void $this->assertInstanceOf(HubInterface::class, $hub); $this->assertInstanceOf(IntegrationInterface::class, $hub->getIntegration(RequestIntegration::class)); - $this->assertNull($hub->getIntegration(ErrorListenerIntegration::class)); - $this->assertNull($hub->getIntegration(ExceptionListenerIntegration::class)); + $this->assertNotNull($hub->getIntegration(ErrorListenerIntegration::class)); + $this->assertNotNull($hub->getIntegration(ExceptionListenerIntegration::class)); } private function getContainer(): ContainerBuilder