Capture messenger exceptions (#326)

* Add messenger to dev requirements

* Add subscriber to capture messages thrown in worker

* Add flushable client interface alias

* Add listener service and configuration

* Remove messenger listener if messenger isn't used

* Support messenger >4.3 when event was introduced

* Use worker_error timeout instead of handler_error

* Add worker_error to ConfigurationTest

* Remove messenger dev dependency

* Flush client on message handled event

* Capture messenger soft fails by default

* Revert "Remove messenger dev dependency"

* Add changelog entry

* Make listener final for consistency

* Flush client on failed message

* Add unit test for MessengerListener

* Test HandlerFailedException is unwrapped

* Support for failed message event bc break

* User _SERVER in test

* Check for retry method

* Keep messenger version

* Move messenger config option up a level

* Add messenger config to root child

* Get messenger option from the correct place

* Call setForRetry only in builder method

* Rename messenger soft fails parameter

* Default capture soft fails to true

* Remove capture soft fails from parameters

* Expand messenger configuration

* Prophecy 1.7 compatibility

* Remove messenger from 3.4 install

* Skip tests when no messenger available

* Dynamic configuration test for messenger

* Run CS fixer

* Pass configuration as argument

Co-Authored-By: Alessandro Lai <[email protected]>

* PHPStan tidy

* Do not mark for a specific release yet

* Add thank you note

* Add docs to the readme

Co-authored-by: Alessandro Lai <[email protected]>

Author: emarref

.travis.yml

@@ -14,23 +14,27 @@ cache:
 before_install:
   - |
     if [ "$SYMFONY_VERSION" != "" ]; then
-      sed -ri '/symfony\/monolog-bundle/! s/"symfony\/(.+)": "(.+)"/"symfony\/\1": "'$SYMFONY_VERSION'"/' composer.json;
+      sed -ri '/symfony\/(monolog-bundle|messenger)/! s/"symfony\/(.+)": "(.+)"/"symfony\/\1": "'$SYMFONY_VERSION'"/' composer.json;
+    fi;
+  - |
+    if [ "$SYMFONY_VERSION" == "3.4.*" ]; then
+      composer remove --dev symfony/messenger
     fi;
   - composer self-update
   - composer global require hirak/prestissimo
 
-install: 
+install:
   - travis_retry travis_wait composer update --no-interaction --prefer-dist --prefer-stable
 
 script: >-
-  vendor/bin/phpunit -v --coverage-clover=build/coverage-report.xml 
+  vendor/bin/phpunit -v --coverage-clover=build/coverage-report.xml
   && bash <(curl -s https://codecov.io/bash) -f build/coverage-report.xml
 
 jobs:
   include:
     - stage: Test
       php: 7.1
-      env: 
+      env:
       - SYMFONY_VERSION: 3.4.*
       - SYMFONY_DEPRECATIONS_HELPER: disabled
     - php: 7.3
@@ -48,7 +52,7 @@ jobs:
       name: PHPStan
       script:
         - composer phpstan
-    - script: 
+    - script:
         - composer cs-check
       name: PHP-CS-Fixer
   allow_failures:

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
+ - Capture and flush messages in a Messenger Worker context (#326, thanks to @emarref)
  - ...
 
 ## 3.4.4 (2020-03-16)

README.md

@@ -80,6 +80,9 @@ Keep in mind that leaving the `dsn` value empty (or undeclared) in other environ
 ```yaml
 sentry:
     dsn: "https://public:[email protected]/1"
+    messenger: 
+        enabled: true # flushes Sentry messages at the end of each message handling
+        capture_soft_fails: true # captures exceptions marked for retry too
     options:
         environment: '%kernel.environment%'
         release: '%env(VERSION)%' #your app version
@@ -171,7 +174,7 @@ The 3.0 version of the bundle uses the newest version (2.x) of the underlying Se
 
 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 three listeners (`RequestListener`, 
-`SubRequestListener` and `ConsoleListener`) that adds some easy default information. 
+`SubRequestListener` and `ConsoleListener`) that adds some easy default information. Since 3.5, a fourth listener has been added to handle the case of Messanger Workers: `MessengerListener`.
 
 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`.

composer.json

@@ -34,12 +34,13 @@
         "jangregor/phpstan-prophecy": "^0.3.0",
         "monolog/monolog": "^1.11||^2.0",
         "php-http/mock-client": "^1.0",
-        "phpstan/phpstan-shim": "^0.11",
         "phpstan/phpstan-phpunit": "^0.11",
+        "phpstan/phpstan-shim": "^0.11",
         "phpunit/phpunit": "^7.5||^8.5",
         "symfony/browser-kit": "^3.4||^4.0||^5.0",
         "symfony/expression-language": "^3.4||^4.0||^5.0",
         "symfony/framework-bundle": "^3.4||^4.0||^5.0",
+        "symfony/messenger": "^4.3||^5.0",
         "symfony/monolog-bundle": "^3.4",
         "symfony/phpunit-bridge": "^5.0",
         "symfony/yaml": "^3.4||^4.0||^5.0"

phpstan.neon

@@ -8,10 +8,10 @@ parameters:
          - "/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
          -
@@ -23,6 +23,9 @@ parameters:
          -
              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/Configuration.php

@@ -8,6 +8,7 @@
 use Symfony\Component\Config\Definition\Builder\ArrayNodeDefinition;
 use Symfony\Component\Config\Definition\Builder\TreeBuilder;
 use Symfony\Component\Config\Definition\ConfigurationInterface;
+use Symfony\Component\Messenger\MessageBusInterface;
 
 /**
  * This is the class that validates and merges configuration from your app/config files
@@ -40,7 +41,6 @@ public function getConfigTreeBuilder(): TreeBuilder
         $rootNode->children()
             ->booleanNode('register_error_listener')
             ->defaultTrue();
-
         // Options array (to be passed to Sentry\Options constructor) -- please keep alphabetical order!
         $optionsNode = $rootNode->children()
             ->arrayNode('options')
@@ -141,6 +141,8 @@ public function getConfigTreeBuilder(): TreeBuilder
             ->defaultValue(128);
         $listenerPriorities->scalarNode('console_error')
             ->defaultValue(128);
+        $listenerPriorities->scalarNode('worker_error')
+            ->defaultValue(128);
 
         // Monolog handler configuration
         $monologConfiguration = $rootNode->children()
@@ -160,6 +162,17 @@ public function getConfigTreeBuilder(): TreeBuilder
         $errorHandler->booleanNode('bubble')
             ->defaultTrue();
 
+        // Messenger configuration
+        $messengerConfiguration = $rootNode->children()
+            ->arrayNode('messenger')
+            ->addDefaultsIfNotSet()
+            ->children();
+
+        $messengerConfiguration->booleanNode('enabled')
+            ->defaultValue(interface_exists(MessageBusInterface::class));
+        $messengerConfiguration->booleanNode('capture_soft_fails')
+            ->defaultTrue();
+
         return $treeBuilder;
     }
 

src/DependencyInjection/SentryExtension.php

@@ -9,6 +9,7 @@
 use Sentry\Options;
 use Sentry\SentryBundle\ErrorTypesParser;
 use Sentry\SentryBundle\EventListener\ErrorListener;
+use Sentry\SentryBundle\EventListener\MessengerListener;
 use Symfony\Component\Config\Definition\Exception\InvalidConfigurationException;
 use Symfony\Component\Config\FileLocator;
 use Symfony\Component\DependencyInjection\ContainerBuilder;
@@ -49,6 +50,7 @@ public function load(array $configs, ContainerBuilder $container): void
         }
 
         $this->configureErrorListener($container, $processedConfiguration);
+        $this->configureMessengerListener($container, $processedConfiguration['messenger']);
         $this->configureMonologHandler($container, $processedConfiguration['monolog']);
     }
 
@@ -160,6 +162,17 @@ private function configureErrorListener(ContainerBuilder $container, array $proc
         $this->tagExceptionListener($container);
     }
 
+    private function configureMessengerListener(ContainerBuilder $container, array $processedConfiguration): void
+    {
+        if (! $processedConfiguration['enabled']) {
+            $container->removeDefinition(MessengerListener::class);
+
+            return;
+        }
+
+        $container->getDefinition(MessengerListener::class)->setArgument(1, $processedConfiguration['capture_soft_fails']);
+    }
+
     /**
      * BC layer for Symfony < 4.3
      */

src/EventListener/MessengerListener.php

@@ -0,0 +1,62 @@
+<?php
+
+namespace Sentry\SentryBundle\EventListener;
+
+use Sentry\FlushableClientInterface;
+use Symfony\Component\Messenger\Event\WorkerMessageFailedEvent;
+use Symfony\Component\Messenger\Event\WorkerMessageHandledEvent;
+use Symfony\Component\Messenger\Exception\HandlerFailedException;
+
+final class MessengerListener
+{
+    /**
+     * @var FlushableClientInterface
+     */
+    private $client;
+
+    /**
+     * @var bool
+     */
+    private $captureSoftFails;
+
+    /**
+     * @param FlushableClientInterface $client
+     * @param bool                     $captureSoftFails
+     */
+    public function __construct(FlushableClientInterface $client, bool $captureSoftFails = true)
+    {
+        $this->client = $client;
+        $this->captureSoftFails = $captureSoftFails;
+    }
+
+    /**
+     * @param WorkerMessageFailedEvent $event
+     */
+    public function onWorkerMessageFailed(WorkerMessageFailedEvent $event): void
+    {
+        if (! $this->captureSoftFails && $event->willRetry()) {
+            // Don't capture soft fails. I.e. those that will be scheduled for retry.
+            return;
+        }
+
+        $error = $event->getThrowable();
+
+        if ($error instanceof HandlerFailedException && null !== $error->getPrevious()) {
+            // Unwrap the messenger exception to get the original error
+            $error = $error->getPrevious();
+        }
+
+        $this->client->captureException($error);
+        $this->client->flush();
+    }
+
+    /**
+     * @param WorkerMessageHandledEvent $event
+     */
+    public function onWorkerMessageHandled(WorkerMessageHandledEvent $event): void
+    {
+        // Flush normally happens at shutdown... which only happens in the worker if it is run with a lifecycle limit
+        // such as --time=X or --limit=Y. Flush immediately in a background worker.
+        $this->client->flush();
+    }
+}

src/Resources/config/services.xml

@@ -15,6 +15,8 @@
             <factory service="Sentry\ClientBuilderInterface" method="getClient" />
         </service>
 
+        <service id="Sentry\FlushableClientInterface" alias="Sentry\ClientInterface" public="false"/>
+
         <service id="Sentry\State\HubInterface" class="Sentry\State\Hub" public="false">
             <factory class="Sentry\SentryBundle\SentryBundle" method="getCurrentHub" />
             <call method="bindClient">
@@ -50,6 +52,12 @@
             <tag name="kernel.event_listener" event="kernel.finish_request" method="onKernelFinishRequest" priority="%sentry.listener_priorities.sub_request%" />
         </service>
 
+        <service id="Sentry\SentryBundle\EventListener\MessengerListener" class="Sentry\SentryBundle\EventListener\MessengerListener" public="false">
+            <argument type="service" id="Sentry\FlushableClientInterface" />
+            <tag name="kernel.event_listener" event="Symfony\Component\Messenger\Event\WorkerMessageFailedEvent" method="onWorkerMessageFailed" priority="%sentry.listener_priorities.worker_error%" />
+            <tag name="kernel.event_listener" event="Symfony\Component\Messenger\Event\WorkerMessageHandledEvent" method="onWorkerMessageHandled" priority="%sentry.listener_priorities.worker_error%" />
+        </service>
+
         <service id="Sentry\SentryBundle\Command\SentryTestCommand" class="Sentry\SentryBundle\Command\SentryTestCommand" public="false">
             <tag name="console.command" />
         </service>

test/DependencyInjection/ConfigurationTest.php

@@ -8,6 +8,7 @@
 use Sentry\SentryBundle\Test\BaseTestCase;
 use Symfony\Component\Config\Definition\Exception\InvalidConfigurationException;
 use Symfony\Component\Config\Definition\Processor;
+use Symfony\Component\Messenger\MessageBusInterface;
 
 class ConfigurationTest extends BaseTestCase
 {
@@ -50,6 +51,7 @@ public function testConfigurationDefaults(): void
                 'console' => 1,
                 'request_error' => 128,
                 'console_error' => 128,
+                'worker_error' => 128,
             ],
             'options' => [
                 'class_serializers' => [],
@@ -72,6 +74,10 @@ public function testConfigurationDefaults(): void
                     'bubble' => true,
                 ],
             ],
+            'messenger' => [
+                'enabled' => interface_exists(MessageBusInterface::class),
+                'capture_soft_fails' => true,
+            ],
         ];
 
         $this->assertEquals($expectedDefaults, $processed);

test/DependencyInjection/SentryExtensionTest.php

@@ -15,6 +15,7 @@
 use Sentry\Options;
 use Sentry\SentryBundle\DependencyInjection\SentryExtension;
 use Sentry\SentryBundle\EventListener\ErrorListener;
+use Sentry\SentryBundle\EventListener\MessengerListener;
 use Sentry\SentryBundle\Test\BaseTestCase;
 use Sentry\SentrySdk;
 use Sentry\Severity;
@@ -423,6 +424,17 @@ public function testMonologHandlerIsNotRegistered(): void
         $this->assertFalse($container->has(self::MONOLOG_HANDLER_TEST_PUBLIC_ALIAS));
     }
 
+    public function testMessengerHandlerIsNotRegistered(): void
+    {
+        $container = $this->getContainer([
+            'messenger' => [
+                'enabled' => false,
+            ],
+        ]);
+
+        $this->assertFalse($container->has(MessengerListener::class));
+    }
+
     private function getContainer(array $configuration = []): Container
     {
         $containerBuilder = new ContainerBuilder();