Enable error handlers back (getsentry#322)

* Reinforce E2E tests with log of sent events

* Add failing test for notices

* Fix wrong setup in test

* Improve E2E tests; add case for fatals

* Try to revert the integration disabling to have the full error reporting back

* Fix test after last modification

* Require --dev symfony/process for client isolation

* Do not capture deprecations in E2E tests

* Fix CS

* Fix PHPStan

* Fix last PHPStan error

* Remove unneeded alias

* Remove unused class

* Try to avoid double reporting of fatal errors

* Add changelog entry

* Fix after-merge issues

Author: Jean85

phpstan-baseline.neon

@@ -230,11 +230,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

src/DependencyInjection/IntegrationFilterFactory.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]);
     }
 
     /**

src/DependencyInjection/SentryExtension.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;
             }

test/DependencyInjection/SentryExtensionTest.php

@@ -31,13 +31,27 @@ 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');
 
         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/Controller/MainController.php

@@ -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/config.yml

@@ -0,0 +1,39 @@
+<?php
+
+namespace Sentry\SentryBundle\Test\End2End;
+
+use Sentry\Event;
+use Sentry\Options;
+use Sentry\Transport\TransportFactoryInterface;
+use Sentry\Transport\TransportInterface;
+
+class StubTransportFactory implements TransportFactoryInterface
+{
+    public const SEPARATOR = '###';
+
+    public function create(Options $options): TransportInterface
+    {
+        return new class() implements TransportInterface {
+            public function send(Event $event): ?string
+            {
+                touch(End2EndTest::SENT_EVENTS_LOG);
+
+                if ($event->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();
+            }
+        };
+    }
+}

test/End2End/StubTransportFactory.php

@@ -127,16 +127,16 @@ public function testContainerHasTestCommandRegisteredCorrectly(): void
         $this->assertArrayHasKey('console.command', $consoleListener->getTags());
     }
 
-    public function testIntegrationsListenersAreDisabledByDefault(): void
+    public function testIntegrationsListenersAreEnabled(): void
     {
         $container = $this->getContainer();
 
         $hub = $container->get(HubInterface::class);
 
         $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