Add BC trick to avoid issues with Symfony 3.x

Jean85 · Jean85 · commit 9681cb634c8e · 2017-12-12T13:05:51.000+01:00
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -11,11 +11,8 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
  - Add support for Symfony 4.x
 ### Changed
  - The `SentryBundle::VERSION` constant has been replaced with the `SentryBundle::getVersion(): string` method, to get a more accurate result
- - Due to a deprecation in `symfony/console`, we require it at at least version 3.3, and we made this change to the `SentryExceptionListenerInterface`:
+ - Due to a deprecation in `symfony/console`, we require it at at least version 3.3, and we added a method to `SentryExceptionListenerInterface`:
 ```php
-    // before
-    public function onConsoleException(ConsoleExceptionEvent $event);
-    // after
     public function onConsoleException(ConsoleErrorEvent $event);
 ```
 ### Removed
diff --git a/src/EventListener/ExceptionListener.php b/src/EventListener/ExceptionListener.php
@@ -6,6 +6,8 @@
 use Sentry\SentryBundle\SentrySymfonyEvents;
 use Symfony\Component\Console\Event\ConsoleCommandEvent;
 use Symfony\Component\Console\Event\ConsoleErrorEvent;
+use Symfony\Component\Console\Event\ConsoleEvent;
+use Symfony\Component\Console\Event\ConsoleExceptionEvent;
 use Symfony\Component\EventDispatcher\EventDispatcherInterface;
 use Symfony\Component\HttpKernel\Event\GetResponseEvent;
 use Symfony\Component\HttpKernel\Event\GetResponseForExceptionEvent;
@@ -119,14 +121,29 @@ public function onConsoleCommand(ConsoleCommandEvent $event): void
         // only triggers loading of client, does not need to do anything.
     }
 
-    /**
-     * @param ConsoleErrorEvent $event
-     */
-    public function onConsoleException(ConsoleErrorEvent $event): void
+    public function onConsoleError(ConsoleErrorEvent $event): void
+    {
+        $this->handleConsoleError($event);
+    }
+
+    public function onConsoleException(ConsoleExceptionEvent $event): void
+    {
+        $this->handleConsoleError($event);
+    }
+
+    private function handleConsoleError(ConsoleEvent $event): void
     {
         $command = $event->getCommand();
-        /** @var \Exception $exception to avoid issues with PHPStan */
-        $exception = $event->getError();
+        switch (true) {
+            case $event instanceof ConsoleErrorEvent:
+                $exception = $event->getError();
+                break;
+            case $event instanceof ConsoleExceptionEvent:
+                $exception = $event->getException();
+                break;
+            default:
+                throw new \InvalidArgumentException('Event not recognized: ' . \get_class($event));
+        }
 
         if ($this->shouldExceptionCaptureBeSkipped($exception)) {
             return;
diff --git a/src/EventListener/SentryExceptionListenerInterface.php b/src/EventListener/SentryExceptionListenerInterface.php
@@ -3,6 +3,7 @@
 namespace Sentry\SentryBundle\EventListener;
 
 use Symfony\Component\Console\Event\ConsoleErrorEvent;
+use Symfony\Component\Console\Event\ConsoleExceptionEvent;
 use Symfony\Component\HttpKernel\Event\GetResponseEvent;
 use Symfony\Component\HttpKernel\Event\GetResponseForExceptionEvent;
 
@@ -33,5 +34,14 @@ public function onKernelException(GetResponseForExceptionEvent $event): void;
      *
      * @param ConsoleErrorEvent $event
      */
-    public function onConsoleException(ConsoleErrorEvent $event): void;
+    public function onConsoleError(ConsoleErrorEvent $event): void;
+
+    /**
+     * When an exception occurs on the command line, this method will be
+     * triggered for capturing the error.
+     *
+     * @param ConsoleErrorEvent $event
+     * @deprecated This method exists for BC with Symfony 3.x
+     */
+    public function onConsoleException(ConsoleExceptionEvent $event): void;
 }
diff --git a/src/Resources/config/services.yml b/src/Resources/config/services.yml
@@ -1,21 +1,22 @@
 services:
-    sentry.client:
-        class: '%sentry.client%'
-        arguments: ['%sentry.dsn%', '%sentry.options%']
-        public: true
-        calls:
-            - [install, []]
+  sentry.client:
+    class: '%sentry.client%'
+    arguments: ['%sentry.dsn%', '%sentry.options%']
+    public: true
+    calls:
+      - [install, []]
 
-    sentry.exception_listener:
-        class: '%sentry.exception_listener%'
-        arguments:
-          - '@sentry.client'
-          - '@event_dispatcher'
-          - '%sentry.skip_capture%'
-          - '@?security.token_storage'
-          - '@?security.authorization_checker'
-        tags:
-            - { name: kernel.event_listener, event: kernel.request,    method: onKernelRequest, priority: '%sentry.listener_priorities.request%'}
-            - { name: kernel.event_listener, event: kernel.exception,  method: onKernelException, priority: '%sentry.listener_priorities.kernel_exception%' }
-            - { name: kernel.event_listener, event: console.command,   method: onConsoleCommand }
-            - { name: kernel.event_listener, event: console.exception, method: onConsoleException, priority: '%sentry.listener_priorities.console_exception%' }
+  sentry.exception_listener:
+    class: '%sentry.exception_listener%'
+    arguments:
+      - '@sentry.client'
+      - '@event_dispatcher'
+      - '%sentry.skip_capture%'
+      - '@?security.token_storage'
+      - '@?security.authorization_checker'
+    tags:
+      - { name: kernel.event_listener, event: kernel.request,    method: onKernelRequest, priority: '%sentry.listener_priorities.request%'}
+      - { name: kernel.event_listener, event: kernel.exception,  method: onKernelException, priority: '%sentry.listener_priorities.kernel_exception%' }
+      - { name: kernel.event_listener, event: console.command,   method: onConsoleCommand }
+      - { name: kernel.event_listener, event: console.exception, method: onConsoleException, priority: '%sentry.listener_priorities.console_exception%' }
+      - { name: kernel.event_listener, event: console.error,     method: onConsoleError, priority: '%sentry.listener_priorities.console_exception%' }
diff --git a/test/DependencyInjection/SentryExtensionTest.php b/test/DependencyInjection/SentryExtensionTest.php
@@ -292,6 +292,11 @@ public function test_that_it_has_proper_event_listener_tags_for_exception_listen
                     'method' => 'onConsoleException',
                     'priority' => '%sentry.listener_priorities.console_exception%',
                 ],
+                [
+                    'event' => 'console.error',
+                    'method' => 'onConsoleError',
+                    'priority' => '%sentry.listener_priorities.console_exception%',
+                ],
             ],
             $tags
         );
diff --git a/test/EventListener/ExceptionListenerTest.php b/test/EventListener/ExceptionListenerTest.php
@@ -10,6 +10,7 @@
 use Sentry\SentryBundle\SentrySymfonyEvents;
 use Symfony\Component\Console\Command\Command;
 use Symfony\Component\Console\Event\ConsoleErrorEvent;
+use Symfony\Component\Console\Event\ConsoleExceptionEvent;
 use Symfony\Component\Console\Input\InputInterface;
 use Symfony\Component\Console\Output\OutputInterface;
 use Symfony\Component\DependencyInjection\Alias;
@@ -459,6 +460,47 @@ public function test_that_it_captures_exception()
      * @dataProvider mockCommandProvider
      */
     public function test_that_it_captures_console_exception(?Command $mockCommand, string $expectedCommandName)
+    {
+        if (null === $mockCommand) {
+            $this->markTestSkipped('Command missing is not possibile with ConsoleExceptionEvent');
+        }
+
+        $exception = $this->createMock(\Exception::class);
+        /** @var InputInterface $input */
+        $input = $this->createMock(InputInterface::class);
+        /** @var OutputInterface $output */
+        $output = $this->createMock(OutputInterface::class);
+
+        $event = new ConsoleExceptionEvent($mockCommand, $input, $output, $exception, 10);
+
+        $this->mockEventDispatcher
+            ->expects($this->once())
+            ->method('dispatch')
+            ->with($this->identicalTo(SentrySymfonyEvents::PRE_CAPTURE), $this->identicalTo($event));
+
+        $this->mockSentryClient
+            ->expects($this->once())
+            ->method('captureException')
+            ->with(
+                $this->identicalTo($exception),
+                $this->identicalTo([
+                    'tags' => [
+                        'command' => $expectedCommandName,
+                        'status_code' => 10,
+                    ],
+                ])
+            );
+
+        $this->containerBuilder->compile();
+        /** @var SentryExceptionListenerInterface $listener */
+        $listener = $this->getListener();
+        $listener->onConsoleException($event);
+    }
+
+    /**
+     * @dataProvider mockCommandProvider
+     */
+    public function test_that_it_captures_console_error(?Command $mockCommand, string $expectedCommandName)
     {
         $error = $this->createMock(\Error::class);
         /** @var InputInterface $input */
@@ -490,7 +532,7 @@ public function test_that_it_captures_console_exception(?Command $mockCommand, s
         $this->containerBuilder->compile();
         /** @var SentryExceptionListenerInterface $listener */
         $listener = $this->getListener();
-        $listener->onConsoleException($event);
+        $listener->onConsoleError($event);
     }
 
     public function mockCommandProvider()
