Enable error handlers back

CHANGELOG.md

@@ -5,6 +5,7 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/)
 and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.html).
 
 ## Unreleased
+ - Enable back all error listeners from base SDK integration (#322)
  - Capture and flush messages in a Messenger Worker context (#326, thanks to @emarref)
  - ...
 

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",
@@ -43,6 +44,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"
     },
     "suggest": {

phpstan.neon

@@ -3,29 +3,30 @@ parameters:
     paths:
         - src/
         - test/
+    excludes_analyse:
+        - test/End2End/App/Controller/MainController.php 
     ignoreErrors:
-         - "/Call to function method_exists.. with 'Symfony.+' and 'getRootNode' will always evaluate to false./"
-         - "/Call to function method_exists.. with 'Symfony.+' and 'getThrowable' will always evaluate to false./"
-         - '/Class PHPUnit_Framework_TestCase not found/'
-         - '/Symfony\\Component\\HttpKernel\\Event\\(GetResponse|FilterController)Event not found.$/'
-         -
-             message: '/Symfony\\Bundle\\FrameworkBundle\\Client/'
-             path: test/End2End/End2EndTest.php
-         -
-             message: '/^Call to an undefined method Symfony\\Component\\Config\\Definition\\Builder\\TreeBuilder::root...$/'
-             path: src/DependencyInjection/Configuration.php
-         -
-             message: '/Symfony\\Component\\HttpKernel\\Event\\GetResponseForExceptionEvent.$/'
-             path: src/EventListener/ErrorListener.php
-         -
-             message: '/Sentry\\SentryBundle\\EventListener\\RequestListener(Request|Controller)Event( not found)?.$/'
-             path: src/EventListener/RequestListener.php
-         -
-             message: '/Sentry\\SentryBundle\\EventListener\\SubRequestListenerRequestEvent( not found)?.$/'
-             path: src/EventListener/SubRequestListener.php
-         -
-             message: '/Class Symfony\\Component\\Messenger\\Event\\WorkerMessage[a-zA-Z]*Event constructor invoked with [0-9]+ parameters, [0-9]+ required\.$/'
-             path: test/EventListener/MessengerListenerTest.php
+        - "/Call to function method_exists.. with 'Symfony.+' and 'getRootNode' will always evaluate to false./"
+        - "/Call to function method_exists.. with 'Symfony.+' and 'getThrowable' will always evaluate to false./"
+        - '/Symfony\\Component\\HttpKernel\\Event\\(GetResponse|FilterController)Event not found.$/'
+        - 
+            message: '/Symfony\\Bundle\\FrameworkBundle\\Client/'
+            path: test/End2End/End2EndTest.php
+        - 
+            message: '/^Call to an undefined method Symfony\\Component\\Config\\Definition\\Builder\\TreeBuilder::root...$/'
+            path: src/DependencyInjection/Configuration.php
+        -
+            message: '/Symfony\\Component\\HttpKernel\\Event\\GetResponseForExceptionEvent.$/'
+            path: src/EventListener/ErrorListener.php
+        -
+            message: '/Sentry\\SentryBundle\\EventListener\\RequestListener(Request|Controller)Event( not found)?.$/'
+            path: src/EventListener/RequestListener.php
+        -
+            message: '/Sentry\\SentryBundle\\EventListener\\SubRequestListenerRequestEvent( not found)?.$/'
+            path: src/EventListener/SubRequestListener.php
+        -
+            message: '/Class Symfony\\Component\\Messenger\\Event\\WorkerMessage[a-zA-Z]*Event constructor invoked with [0-9]+ parameters, [0-9]+ required\.$/'
+            path: test/EventListener/MessengerListenerTest.php
 
 includes:
 	- vendor/jangregor/phpstan-prophecy/src/extension.neon

src/DependencyInjection/SentryExtension.php

@@ -17,6 +17,7 @@
 use Symfony\Component\DependencyInjection\Exception\LogicException;
 use Symfony\Component\DependencyInjection\Loader;
 use Symfony\Component\DependencyInjection\Reference;
+use Symfony\Component\ErrorHandler\Error\FatalError;
 use Symfony\Component\HttpKernel\DependencyInjection\Extension;
 use Symfony\Component\HttpKernel\Event\ExceptionEvent;
 use Symfony\Component\HttpKernel\KernelEvents;
@@ -128,18 +129,15 @@ private function passConfigurationToOptions(ContainerBuilder $container, array $
             }
         }
 
-        if (\array_key_exists('excluded_exceptions', $processedOptions) && $processedOptions['excluded_exceptions']) {
-            $ignoreOptions = [
-                'ignore_exceptions' => $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]);
     }
 
     private function valueToCallable($value)

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;
@@ -338,14 +336,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;
             }

test/End2End/App/Controller/MainController.php

@@ -31,13 +31,28 @@ public function exception(): Response
         throw new \RuntimeException('This is an intentional error');
     }
 
+    public function fatal(): Response
+    {
+        $foo = new class() implements \Serializable {
+        };
+
+        return new Response('This response should not happen: ' . json_encode($foo));
+    }
+
     public function index(): Response
     {
         $this->sentry->captureMessage('Hello there');
 
         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();

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:

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' }

test/End2End/End2EndTest.php

@@ -2,7 +2,7 @@
 
 namespace Sentry\SentryBundle\Test\End2End;
 
-use PHPUnit\Framework\TestCase;
+use Sentry\SentryBundle\Test\End2End\App\Controller\MainController;
 use Sentry\SentryBundle\Test\End2End\App\Kernel;
 use Sentry\State\HubInterface;
 use Symfony\Bundle\FrameworkBundle\Client;
@@ -11,7 +11,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 +20,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 +46,7 @@ public function testGet200(): void
         $this->assertSame(200, $response->getStatusCode());
 
         $this->assertLastEventIdIsNotNull($client);
+        $this->assertEventCount(1);
     }
 
     public function testGet200BehindFirewall(): void
@@ -52,6 +61,7 @@ public function testGet200BehindFirewall(): void
         $this->assertSame(200, $response->getStatusCode());
 
         $this->assertLastEventIdIsNotNull($client);
+        $this->assertEventCount(1);
     }
 
     public function testGet200WithSubrequest(): void
@@ -66,6 +76,7 @@ public function testGet200WithSubrequest(): void
         $this->assertSame(200, $response->getStatusCode());
 
         $this->assertLastEventIdIsNotNull($client);
+        $this->assertEventCount(1);
     }
 
     public function testGet404(): void
@@ -88,6 +99,7 @@ public function testGet404(): void
         }
 
         $this->assertLastEventIdIsNotNull($client);
+        $this->assertEventCount(1);
     }
 
     public function testGet500(): void
@@ -111,6 +123,47 @@ 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 (\Throwable $exception) {
+            if (! $exception instanceof \RuntimeException) {
+                throw $exception;
+            }
+
+            $this->assertStringContainsStringIgnoringCase('error', $exception->getMessage());
+            $this->assertStringContainsStringIgnoringCase('contains 2 abstract methods', $exception->getMessage());
+            $this->assertStringContainsStringIgnoringCase(MainController::class, $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 +176,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);
+    }
 }