[Messenger][FrameworkBundle] Add option to exclude stack trace from ErrorDetailsStamp

HypeMC · HypeMC · commit c61849d79c27 · 2024-01-07T22:24:45.000+01:00
diff --git a/src/Symfony/Bundle/FrameworkBundle/CHANGELOG.md b/src/Symfony/Bundle/FrameworkBundle/CHANGELOG.md
@@ -9,6 +9,7 @@ CHANGELOG
  * Move the Router `cache_dir` to `kernel.build_dir`
  * Deprecate the `router.cache_dir` config option
  * Add `rate_limiter` tags to rate limiter services
+ * Add `include_stack_trace_in_error` option that's used in `AddErrorDetailsStampListener`
 
 7.0
 ---
diff --git a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php
@@ -1575,6 +1575,9 @@ function ($a) {
                                         ->defaultNull()
                                         ->info('Transport name to send failed messages to (after all retries have failed).')
                                     ->end()
+                                    ->booleanNode('include_stack_trace_in_error')
+                                        ->info('Whether to include a stack trace in the error details stamp.')
+                                    ->end()
                                     ->arrayNode('retry_strategy')
                                         ->addDefaultsIfNotSet()
                                         ->beforeNormalization()
@@ -1606,6 +1609,10 @@ function ($a) {
                             ->defaultNull()
                             ->info('Transport name to send failed messages to (after all retries have failed).')
                         ->end()
+                        ->booleanNode('include_stack_trace_in_error')
+                            ->defaultTrue()
+                            ->info('Whether to include a stack trace in the error details stamp.')
+                        ->end()
                         ->arrayNode('stop_worker_on_signals')
                             ->defaultValue([])
                             ->info('A list of signals that should stop the worker; defaults to SIGTERM and SIGINT.')
diff --git a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php
@@ -2170,6 +2170,7 @@ private function registerMessengerConfiguration(array $config, ContainerBuilder
         }
 
         $senderAliases = [];
+        $includeStackTraceInError = [];
         $transportRetryReferences = [];
         $transportRateLimiterReferences = [];
         foreach ($config['transports'] as $name => $transport) {
@@ -2186,6 +2187,8 @@ private function registerMessengerConfiguration(array $config, ContainerBuilder
             $container->setDefinition($transportId = 'messenger.transport.'.$name, $transportDefinition);
             $senderAliases[$name] = $transportId;
 
+            $includeStackTraceInError[$name] = $transport['include_stack_trace_in_error'] ?? $config['include_stack_trace_in_error'];
+
             if (null !== $transport['retry_strategy']['service']) {
                 $transportRetryReferences[$name] = new Reference($transport['retry_strategy']['service']);
             } else {
@@ -2258,6 +2261,10 @@ private function registerMessengerConfiguration(array $config, ContainerBuilder
             ->replaceArgument(1, $sendersServiceLocator)
         ;
 
+        $container->getDefinition('messenger.failure.add_error_details_stamp_listener')
+            ->replaceArgument(0, $includeStackTraceInError)
+        ;
+
         $container->getDefinition('messenger.retry.send_failed_message_for_retry_listener')
             ->replaceArgument(0, $sendersServiceLocator)
         ;
diff --git a/src/Symfony/Bundle/FrameworkBundle/Resources/config/messenger.php b/src/Symfony/Bundle/FrameworkBundle/Resources/config/messenger.php
@@ -180,6 +180,9 @@
             ->tag('monolog.logger', ['channel' => 'messenger'])
 
         ->set('messenger.failure.add_error_details_stamp_listener', AddErrorDetailsStampListener::class)
+            ->args([
+                abstract_arg('include_stack_trace_in_error'),
+            ])
             ->tag('kernel.event_subscriber')
 
         ->set('messenger.failure.send_failed_message_to_failure_transport_listener', SendFailedMessageToFailureTransportListener::class)
diff --git a/src/Symfony/Bundle/FrameworkBundle/Resources/config/schema/symfony-1.0.xsd b/src/Symfony/Bundle/FrameworkBundle/Resources/config/schema/symfony-1.0.xsd
@@ -564,6 +564,7 @@
         <xsd:attribute name="default-bus" type="xsd:string" />
         <xsd:attribute name="enabled" type="xsd:boolean" />
         <xsd:attribute name="failure-transport" type="xsd:string" />
+        <xsd:attribute name="include-stack-trace-in-error" type="xsd:boolean" />
     </xsd:complexType>
 
     <xsd:complexType name="messenger_serializer">
@@ -600,6 +601,7 @@
         <xsd:attribute name="serializer" type="xsd:string" />
         <xsd:attribute name="dsn" type="xsd:string" />
         <xsd:attribute name="failure-transport" type="xsd:string" />
+        <xsd:attribute name="include-stack-trace-in-error" type="xsd:boolean" />
         <xsd:attribute name="rate-limiter" type="xsd:string" />
     </xsd:complexType>
 
diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/ConfigurationTest.php b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/ConfigurationTest.php
@@ -724,6 +724,7 @@ class_exists(SemaphoreStore::class) && SemaphoreStore::isSupported() ? 'semaphor
                 'default_bus' => null,
                 'buses' => ['messenger.bus.default' => ['default_middleware' => ['enabled' => true, 'allow_no_handlers' => false, 'allow_no_senders' => true], 'middleware' => []]],
                 'stop_worker_on_signals' => [],
+                'include_stack_trace_in_error' => true,
             ],
             'disallow_search_engine_index' => true,
             'http_client' => [
diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/php/messenger_with_include_stack_trace_false.php b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/php/messenger_with_include_stack_trace_false.php
@@ -0,0 +1,19 @@
+<?php
+
+$container->loadFromExtension('framework', [
+    'annotations' => false,
+    'http_method_override' => false,
+    'handle_all_throwables' => true,
+    'php_errors' => ['log' => true],
+    'messenger' => [
+        'include_stack_trace_in_error' => false,
+        'transports' => [
+            'sender.biz' => 'null://',
+            'sender.bar' => [
+                'dsn' => 'null://',
+                'include_stack_trace_in_error' => true,
+            ],
+            'sender.foo' => 'null://',
+        ],
+    ],
+]);
diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/php/messenger_with_include_stack_trace_true.php b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/php/messenger_with_include_stack_trace_true.php
@@ -0,0 +1,18 @@
+<?php
+
+$container->loadFromExtension('framework', [
+    'annotations' => false,
+    'http_method_override' => false,
+    'handle_all_throwables' => true,
+    'php_errors' => ['log' => true],
+    'messenger' => [
+        'transports' => [
+            'sender.biz' => 'null://',
+            'sender.bar' => [
+                'dsn' => 'null://',
+                'include_stack_trace_in_error' => false,
+            ],
+            'sender.foo' => 'null://',
+        ],
+    ],
+]);
diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/xml/messenger_with_include_stack_trace_false.xml b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/xml/messenger_with_include_stack_trace_false.xml
@@ -0,0 +1,17 @@
+<?xml version="1.0" encoding="utf-8" ?>
+<container xmlns="http://symfony.com/schema/dic/services"
+    xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+    xmlns:framework="http://symfony.com/schema/dic/symfony"
+    xsi:schemaLocation="http://symfony.com/schema/dic/services https://symfony.com/schema/dic/services/services-1.0.xsd
+        http://symfony.com/schema/dic/symfony https://symfony.com/schema/dic/symfony/symfony-1.0.xsd">
+
+    <framework:config http-method-override="false" handle-all-throwables="true">
+        <framework:annotations enabled="false" />
+        <framework:php-errors log="true" />
+        <framework:messenger include-stack-trace-in-error="false">
+            <framework:transport name="sender.biz" dsn="null://" />
+            <framework:transport name="sender.bar" dsn="null://" include-stack-trace-in-error="true" />
+            <framework:transport name="sender.foo" dsn="null://" />
+        </framework:messenger>
+    </framework:config>
+</container>
diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/xml/messenger_with_include_stack_trace_true.xml b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/xml/messenger_with_include_stack_trace_true.xml
@@ -0,0 +1,17 @@
+<?xml version="1.0" encoding="utf-8" ?>
+<container xmlns="http://symfony.com/schema/dic/services"
+    xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+    xmlns:framework="http://symfony.com/schema/dic/symfony"
+    xsi:schemaLocation="http://symfony.com/schema/dic/services https://symfony.com/schema/dic/services/services-1.0.xsd
+        http://symfony.com/schema/dic/symfony https://symfony.com/schema/dic/symfony/symfony-1.0.xsd">
+
+    <framework:config http-method-override="false" handle-all-throwables="true">
+        <framework:annotations enabled="false" />
+        <framework:php-errors log="true" />
+        <framework:messenger>
+            <framework:transport name="sender.biz" dsn="null://" />
+            <framework:transport name="sender.bar" dsn="null://" include-stack-trace-in-error="false" />
+            <framework:transport name="sender.foo" dsn="null://" />
+        </framework:messenger>
+    </framework:config>
+</container>
diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/yml/messenger_with_include_stack_trace_false.yml b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/yml/messenger_with_include_stack_trace_false.yml
@@ -0,0 +1,14 @@
+framework:
+    annotations: false
+    http_method_override: false
+    handle_all_throwables: true
+    php_errors:
+        log: true
+    messenger:
+        include_stack_trace_in_error: false
+        transports:
+            sender.biz: 'null://'
+            sender.bar:
+                dsn: 'null://'
+                include_stack_trace_in_error: true
+            sender.foo: 'null://'
diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/yml/messenger_with_include_stack_trace_true.yml b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/yml/messenger_with_include_stack_trace_true.yml
@@ -0,0 +1,13 @@
+framework:
+    annotations: false
+    http_method_override: false
+    handle_all_throwables: true
+    php_errors:
+        log: true
+    messenger:
+        transports:
+            sender.biz: 'null://'
+            sender.bar:
+                dsn: 'null://'
+                include_stack_trace_in_error: false
+            sender.foo: 'null://'
diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/FrameworkExtensionTestCase.php b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/FrameworkExtensionTestCase.php
@@ -1096,6 +1096,17 @@ public function testMessengerInvalidWildcardRouting()
         $this->createContainerFromFile('messenger_routing_invalid_transport');
     }
 
+    /**
+     * @testWith ["messenger_with_include_stack_trace_true", {"sender.biz": true, "sender.bar": false, "sender.foo": true}]
+     *           ["messenger_with_include_stack_trace_false", {"sender.biz": false, "sender.bar": true, "sender.foo": false}]
+     */
+    public function testMessengerWithIncludeStackTrace(string $file, array $expected)
+    {
+        $container = $this->createContainerFromFile($file);
+
+        $this->assertSame($expected, $container->getDefinition('messenger.failure.add_error_details_stamp_listener')->getArgument(0));
+    }
+
     public function testTranslator()
     {
         $container = $this->createContainerFromFile('full');
diff --git a/src/Symfony/Component/Messenger/CHANGELOG.md b/src/Symfony/Component/Messenger/CHANGELOG.md
@@ -8,6 +8,7 @@ CHANGELOG
  * Add `--all` option to the `messenger:consume` command
  * Make `#[AsMessageHandler]` final
  * Add parameter `$jitter` to `MultiplierRetryStrategy` in order to randomize delay and prevent the thundering herd effect
+ * Add `$includeStackTrace` parameters to `AddErrorDetailsStampListener` and `ErrorDetailsStamp` to allow excluding the stack trace from the `ErrorDetailsStamp`
 
 7.0
 ---
diff --git a/src/Symfony/Component/Messenger/EventListener/AddErrorDetailsStampListener.php b/src/Symfony/Component/Messenger/EventListener/AddErrorDetailsStampListener.php
@@ -17,9 +17,17 @@
 
 final class AddErrorDetailsStampListener implements EventSubscriberInterface
 {
+    /**
+     * @param array<string, bool> $includeStackTrace
+     */
+    public function __construct(
+        private readonly array $includeStackTrace = [],
+    ) {
+    }
+
     public function onMessageFailed(WorkerMessageFailedEvent $event): void
     {
-        $stamp = ErrorDetailsStamp::create($event->getThrowable());
+        $stamp = ErrorDetailsStamp::create($event->getThrowable(), $this->includeStackTrace[$event->getReceiverName()] ?? true);
         $previousStamp = $event->getEnvelope()->last(ErrorDetailsStamp::class);
 
         // Do not append duplicate information
diff --git a/src/Symfony/Component/Messenger/Stamp/ErrorDetailsStamp.php b/src/Symfony/Component/Messenger/Stamp/ErrorDetailsStamp.php
@@ -32,14 +32,14 @@ public function __construct(string $exceptionClass, int|string $exceptionCode, s
         $this->flattenException = $flattenException;
     }
 
-    public static function create(\Throwable $throwable): self
+    public static function create(\Throwable $throwable, bool $includeStackTrace = true): self
     {
         if ($throwable instanceof HandlerFailedException) {
             $throwable = $throwable->getPrevious();
         }
 
         $flattenException = null;
-        if (class_exists(FlattenException::class)) {
+        if ($includeStackTrace && class_exists(FlattenException::class)) {
             $flattenException = FlattenException::createFromThrowable($throwable);
         }
 
diff --git a/src/Symfony/Component/Messenger/Tests/EventListener/AddErrorDetailsStampListenerTest.php b/src/Symfony/Component/Messenger/Tests/EventListener/AddErrorDetailsStampListenerTest.php
@@ -33,6 +33,20 @@ public function testExceptionDetailsAreAdded()
         $this->assertEquals($expectedStamp, $event->getEnvelope()->last(ErrorDetailsStamp::class));
     }
 
+    public function testExceptionDetailsWithoutStackTraceAreAdded()
+    {
+        $listener = new AddErrorDetailsStampListener(['my_receiver' => false]);
+
+        $envelope = new Envelope(new \stdClass());
+        $exception = new \Exception('It failed!');
+        $event = new WorkerMessageFailedEvent($envelope, 'my_receiver', $exception);
+        $expectedStamp = ErrorDetailsStamp::create($exception, false);
+
+        $listener->onMessageFailed($event);
+
+        $this->assertEquals($expectedStamp, $event->getEnvelope()->last(ErrorDetailsStamp::class));
+    }
+
     public function testWorkerAddsNewErrorDetailsStampOnFailure()
     {
         $listener = new AddErrorDetailsStampListener();
diff --git a/src/Symfony/Component/Messenger/Tests/Stamp/ErrorDetailsStampTest.php b/src/Symfony/Component/Messenger/Tests/Stamp/ErrorDetailsStampTest.php
@@ -52,6 +52,17 @@ public function testUnwrappingHandlerFailedException()
         $this->assertEquals($flattenException, $stamp->getFlattenException());
     }
 
+    public function testFlattenExceptionIsNotIncluded()
+    {
+        $exception = new \Exception('exception message');
+
+        $stamp = ErrorDetailsStamp::create($exception, false);
+
+        $this->assertSame(\Exception::class, $stamp->getExceptionClass());
+        $this->assertSame('exception message', $stamp->getExceptionMessage());
+        $this->assertNull($stamp->getFlattenException());
+    }
+
     public function testDeserialization()
     {
         $exception = new \Exception('exception message');
