Merge pull request #204 from getsentry/rework-to-use-only-listeners

Rework to use only listeners

Author: Jean85

CHANGELOG.md

@@ -6,6 +6,9 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
 
 ## [Unreleased]
 
+## 3.0.0-beta2 - 2019-03-22
+ - Disable Sentry's ErrorHandler, and report all errors using Symfony's events (#204)
+
 ## 3.0.0-beta1 - 2019-03-06
 The 3.0 major release has multiple breaking changes. The most notable one is the upgrade to the 2.0 base SDK client.
 Refer to the [UPGRADE-3.0.md](https://github.com/getsentry/sentry-symfony/blob/master/UPGRADE-3.0.md) document for a

README.md

@@ -11,9 +11,7 @@ Symfony integration for [Sentry](https://getsentry.com/).
 
 ## Notice 3.0
 > The current master branch contains the 3.0 version of this bundle, which is currently under development. This version
-> will support the newest 2.0 version of the underlying Sentry SDK version.
->
-> A beta version will be tagged as soon as possible, in the meantime you can continue to use the previous versions.
+> will support the newest 2.0 version of the underlying Sentry SDK version. A beta version is already available.
 > 
 > To know more about the progress of this version see [the relative milestone](https://github.com/getsentry/sentry-symfony/milestone/3)
 
@@ -22,22 +20,20 @@ Symfony integration for [Sentry](https://getsentry.com/).
 Use sentry-symfony for:
 
  * A fast sentry setup
- * Access to the `sentry.client` through the container
+ * Easy configuration in your Symfony app
  * Automatic wiring in your app. Each event has the following things added automatically to it:
    - user
    - Symfony environment
    - app path
-   - hostname
    - excluded paths (cache and vendor)
 
-
 ## Installation
 
 ### Step 1: Download the Bundle
 You can install this bundle using Composer: 
 
 ```bash
-composer require sentry/sentry-symfony:^3.0
+composer require sentry/sentry-symfony:^3.0-beta2
 ```
 
 #### Optional: use custom HTTP factory/transport
@@ -87,12 +83,13 @@ class AppKernel extends Kernel
     // ...
 }
 ```
-Note that, unlike before in version 3, the bundle will be enabled in all environments.
+Note that, unlike before in version 3, the bundle will be enabled in all environments; event reporting, instead, is enabled
+only when providing a DSN (see the next step).
 
 ### Step 3: Configure the SDK
 
-Add your [Sentry DSN](https://docs.sentry.io/quickstart/#configure-the-dsn) value of your project to ``app/config/config.yml``.
-Leaving this value empty will effectively disable Sentry reporting.
+Add your [Sentry DSN](https://docs.sentry.io/quickstart/#configure-the-dsn) value of your project to ``app/config/config_prod.yml``.
+Leaving this value empty (or undeclared) in other environments will effectively disable Sentry reporting.
 
 ```yaml
 sentry:
@@ -112,9 +109,11 @@ TODO
 ## Customization
 
 The Sentry 2.0 SDK uses the Unified API, hence it uses the concept of `Scope`s to hold information about the current 
-state of the app, and attach it to any event that is reported. This bundle has two listeners (`RequestListener` and 
-`ConsoleListener`) that adds some easy default information. Those listeners normally are executed with a priority of `1`
-to allow easier customization with custom listener, that by default run with a lower priority of `0`.
+state of the app, and attach it to any event that is reported. This bundle has three listeners (`RequestListener`, 
+`SubRequestListener` and `ConsoleListener`) that adds some easy default information. 
+
+Those listeners normally are executed with a priority of `1` to allow easier customization with custom listener, that by 
+default run with a lower priority of `0`.
 
 Those listeners are `final` so not extendable, but you can look at those to know how to add more information to the 
 current `Scope` and enrich you Sentry events.

UPGRADE-3.0.md

@@ -31,13 +31,13 @@ changed:
 
  * All services are now private; declare public aliases to access them if needed; you can still use the Sentry SDK global
    functions if you want just to capture messages manually without injecting Sentry services in your code
- * All services uses the full qualified name of the interfaces to name them
- * The `ExceptionListener` has been splitted in two: `RequestListener` and `ConsoleListener`
+ * All services uses the full qualified name of their interfaces to name them
+ * The `ExceptionListener` has been splitted and renamed: we now have a simpler `ErrorListener`, and three other listeners
+ dedicated to enriching events of data (`RequestListener`, `SubRequestListener` and `ConsoleListener`)
  * The listeners are now `final`; append your own listeners to override their behavior
  * The listeners are registered with a priority of `1`; they will run just before the default priority of `0`, to ease
    the registration of custom listener that will change `Scope` data
  * Configuration options of the bundle are now aligned with the new ones of the 2.0 SDK
- * Listeners are no longer used to capture exceptions, it's all demanded to the error handler
 
 ## New services
 This is a brief description of the services registered by this bundle:
@@ -47,7 +47,7 @@ This is a brief description of the services registered by this bundle:
  * `Sentry\ClientInterface`: this is the proper client; compared to the 1.x SDK version it's a lot more slimmed down,
  since a lot of the stuff has been splitted in separated components, so you probably will not interact with it as much as
  in the past. You also have to remind you that the client is bound to the `Hub`, and has to be changed there if you want 
- to use it automatically in error reporting
+ to use a different one automatically in error reporting
  * `Sentry\ClientBuilderInterface`: this is the factory that builds the client; you can call its methods to change all
  the settings and dependencies that will be injected in the latter created client. You can use this service to obtain more
  customized clients for your needs

phpstan.neon

@@ -6,6 +6,9 @@ parameters:
     ignoreErrors:
          - "/Call to function method_exists.. with 'Symfony.+' and 'getProjectDir' will always evaluate to false./"
          - "/Call to function method_exists.. with 'Symfony.+' and 'getRootNode' will always evaluate to false./"
+         - '/Parameter \$.+ of method Sentry\\SentryBundle\\EventListener\\ErrorListener::onConsoleException\(\) has invalid typehint type Symfony\\Component\\Console\\Event\\ConsoleExceptionEvent./'
+         - '/Call to method getException\(\) on an unknown class Symfony\\Component\\Console\\Event\\ConsoleExceptionEvent./'
+         - '/Access to undefined constant Symfony\\Component\\Console\\ConsoleEvents::EXCEPTION./'
 
 includes:
 	- vendor/jangregor/phpstan-prophecy/src/extension.neon

src/DependencyInjection/ClientBuilderConfigurator.php

@@ -0,0 +1,36 @@
+<?php
+
+namespace Sentry\SentryBundle\DependencyInjection;
+
+use Sentry\ClientBuilderInterface;
+use Sentry\Integration\ErrorListenerIntegration;
+use Sentry\Integration\ExceptionListenerIntegration;
+use Sentry\Integration\IntegrationInterface;
+use Sentry\SentryBundle\SentryBundle;
+
+class ClientBuilderConfigurator
+{
+    public static function configure(ClientBuilderInterface $clientBuilder): void
+    {
+        $clientBuilder->setSdkIdentifier(SentryBundle::SDK_IDENTIFIER);
+        $clientBuilder->setSdkVersion(SentryBundle::getSdkVersion());
+
+        $options = $clientBuilder->getOptions();
+        if (! $options->hasDefaultIntegrations()) {
+            return;
+        }
+
+        $integrations = $options->getIntegrations();
+        $options->setIntegrations(array_filter($integrations, static function (IntegrationInterface $integration): bool {
+            if ($integration instanceof ErrorListenerIntegration) {
+                return false;
+            }
+
+            if ($integration instanceof ExceptionListenerIntegration) {
+                return false;
+            }
+
+            return true;
+        }));
+    }
+}

src/DependencyInjection/Configuration.php

@@ -120,6 +120,10 @@ public function getConfigTreeBuilder()
             ->defaultValue(1);
         $listenerPriorities->scalarNode('console')
             ->defaultValue(1);
+        $listenerPriorities->scalarNode('request_error')
+            ->defaultValue(128);
+        $listenerPriorities->scalarNode('console_error')
+            ->defaultValue(128);
 
         return $treeBuilder;
     }

src/DependencyInjection/SentryExtension.php

@@ -5,9 +5,10 @@
 use Sentry\ClientBuilderInterface;
 use Sentry\Options;
 use Sentry\SentryBundle\ErrorTypesParser;
-use Sentry\SentryBundle\SentryBundle;
+use Sentry\SentryBundle\EventListener\ErrorListener;
 use Symfony\Component\Config\Definition\Exception\InvalidConfigurationException;
 use Symfony\Component\Config\FileLocator;
+use Symfony\Component\Console\ConsoleEvents;
 use Symfony\Component\DependencyInjection\ContainerBuilder;
 use Symfony\Component\DependencyInjection\Definition;
 use Symfony\Component\DependencyInjection\Loader;
@@ -36,12 +37,13 @@ public function load(array $configs, ContainerBuilder $container)
         $this->passConfigurationToOptions($container, $processedConfiguration);
 
         $container->getDefinition(ClientBuilderInterface::class)
-            ->addMethodCall('setSdkIdentifier', [SentryBundle::SDK_IDENTIFIER])
-            ->addMethodCall('setSdkVersion', [SentryBundle::getSdkVersion()]);
+            ->setConfigurator([ClientBuilderConfigurator::class, 'configure']);
 
         foreach ($processedConfiguration['listener_priorities'] as $key => $priority) {
             $container->setParameter('sentry.listener_priorities.' . $key, $priority);
         }
+
+        $this->tagConsoleErrorListener($container);
     }
 
     private function passConfigurationToOptions(ContainerBuilder $container, array $processedConfiguration): void
@@ -121,4 +123,28 @@ private function mapCallableValue(Definition $options, string $method, $optionVa
 
         $options->addMethodCall($method, [$beforeSend]);
     }
+
+    /**
+     * BC layer for Symfony < 3.3; see https://symfony.com/blog/new-in-symfony-3-3-better-handling-of-command-exceptions
+     */
+    private function tagConsoleErrorListener(ContainerBuilder $container): void
+    {
+        $listener = $container->getDefinition(ErrorListener::class);
+
+        if (class_exists('Symfony\Component\Console\Event\ConsoleErrorEvent')) {
+            $tagAttributes = [
+                'event' => ConsoleEvents::ERROR,
+                'method' => 'onConsoleError',
+                'priority' => '%sentry.listener_priorities.console_error%',
+            ];
+        } else {
+            $tagAttributes = [
+                'event' => ConsoleEvents::EXCEPTION,
+                'method' => 'onConsoleException',
+                'priority' => '%sentry.listener_priorities.console_error%',
+            ];
+        }
+
+        $listener->addTag('kernel.event_listener', $tagAttributes);
+    }
 }

src/EventListener/ErrorListener.php

@@ -0,0 +1,28 @@
+<?php
+
+namespace Sentry\SentryBundle\EventListener;
+
+use Symfony\Component\Console\Event\ConsoleErrorEvent;
+use Symfony\Component\Console\Event\ConsoleExceptionEvent;
+use Symfony\Component\HttpKernel\Event\GetResponseForExceptionEvent;
+
+final class ErrorListener
+{
+    public function onKernelException(GetResponseForExceptionEvent $event): void
+    {
+        \Sentry\captureException($event->getException());
+    }
+
+    public function onConsoleError(ConsoleErrorEvent $event): void
+    {
+        \Sentry\captureException($event->getError());
+    }
+
+    /**
+     * BC layer for Symfony < 3.3; see https://symfony.com/blog/new-in-symfony-3-3-better-handling-of-command-exceptions
+     */
+    public function onConsoleException(ConsoleExceptionEvent $event): void
+    {
+        \Sentry\captureException($event->getException());
+    }
+}

src/Resources/config/services.xml

@@ -30,6 +30,12 @@
             <tag name="kernel.event_listener" event="console.command" method="onConsoleCommand" priority="%sentry.listener_priorities.console%" />
         </service>
 
+        <service id="Sentry\SentryBundle\EventListener\ErrorListener" class="Sentry\SentryBundle\EventListener\ErrorListener" public="false">
+            <tag name="kernel.event_listener" event="kernel.exception" method="onKernelException" priority="%sentry.listener_priorities.request_error%" />
+            <!-- The following tag is done manually in PHP for BC with Symfony < 3.3 -->
+<!--            <tag name="kernel.event_listener" event="console.error" method="onConsoleError" priority="%sentry.listener_priorities.console_error%" />-->
+        </service>
+
         <service id="Sentry\SentryBundle\EventListener\RequestListener" class="Sentry\SentryBundle\EventListener\RequestListener" public="false">
             <argument type="service" id="Sentry\State\HubInterface" />
             <argument type="service" id="security.token_storage" on-invalid="ignore" />

test/DependencyInjection/ConfigurationTest.php

@@ -54,6 +54,8 @@ public function testConfigurationDefaults(): void
                 'request' => 1,
                 'sub_request' => 1,
                 'console' => 1,
+                'request_error' => 128,
+                'console_error' => 128,
             ],
             'options' => [
                 'environment' => '%kernel.environment%',

test/EventListener/ConsoleListenerTest.php

@@ -2,6 +2,7 @@
 
 namespace Sentry\SentryBundle\Test\EventListener;
 
+use PHPUnit\Framework\TestCase;
 use Prophecy\Argument;
 use Sentry\SentryBundle\EventListener\ConsoleListener;
 use Sentry\State\Hub;
@@ -10,7 +11,7 @@
 use Symfony\Component\Console\Command\Command;
 use Symfony\Component\Console\Event\ConsoleCommandEvent;
 
-class ConsoleListenerTest extends \PHPUnit\Framework\TestCase
+class ConsoleListenerTest extends TestCase
 {
     private $currentHub;
     private $currentScope;

test/SentryBundleTest.php

@@ -3,10 +3,16 @@
 namespace Sentry\SentryBundle\Test;
 
 use PHPUnit\Framework\TestCase;
+use Sentry\Integration\ErrorListenerIntegration;
+use Sentry\Integration\ExceptionListenerIntegration;
+use Sentry\Integration\IntegrationInterface;
+use Sentry\Integration\RequestIntegration;
 use Sentry\SentryBundle\DependencyInjection\SentryExtension;
 use Sentry\SentryBundle\EventListener\ConsoleListener;
+use Sentry\SentryBundle\EventListener\ErrorListener;
 use Sentry\SentryBundle\EventListener\RequestListener;
 use Sentry\SentryBundle\EventListener\SubRequestListener;
+use Sentry\State\HubInterface;
 use Symfony\Component\Console\ConsoleEvents;
 use Symfony\Component\DependencyInjection\ContainerBuilder;
 use Symfony\Component\EventDispatcher\EventDispatcherInterface;
@@ -82,10 +88,56 @@ public function testContainerHasSubRequestListenerConfiguredCorrectly(): void
         $this->assertSame($expectedTag, $consoleListener->getTags());
     }
 
+    public function testContainerHasErrorListenerConfiguredCorrectly(): void
+    {
+        $container = $this->getContainer();
+
+        $consoleListener = $container->getDefinition(ErrorListener::class);
+
+        $expectedTag = [
+            'kernel.event_listener' => [
+                [
+                    'event' => KernelEvents::EXCEPTION,
+                    'method' => 'onKernelException',
+                    'priority' => '%sentry.listener_priorities.request_error%',
+                ],
+            ],
+        ];
+
+        if (class_exists('Symfony\Component\Console\Event\ConsoleErrorEvent')) {
+            $expectedTag['kernel.event_listener'][] = [
+                'event' => ConsoleEvents::ERROR,
+                'method' => 'onConsoleError',
+                'priority' => '%sentry.listener_priorities.console_error%',
+            ];
+        } else {
+            $expectedTag['kernel.event_listener'][] = [
+                'event' => ConsoleEvents::EXCEPTION,
+                'method' => 'onConsoleException',
+                'priority' => '%sentry.listener_priorities.console_error%',
+            ];
+        }
+
+        $this->assertSame($expectedTag, $consoleListener->getTags());
+    }
+
+    public function testIntegrationsListenersAreDisabledByDefault(): 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));
+    }
+
     private function getContainer(array $configuration = []): ContainerBuilder
     {
         $containerBuilder = new ContainerBuilder();
         $containerBuilder->setParameter('kernel.root_dir', 'kernel/root');
+        $containerBuilder->setParameter('kernel.cache_dir', 'var/cache');
         if (method_exists(Kernel::class, 'getProjectDir')) {
             $containerBuilder->setParameter('kernel.project_dir', '/dir/project/root');
         }