From 672d9be4e22238209e90a54e9ea8881e1cd32f31 Mon Sep 17 00:00:00 2001 From: HypeMC Date: Sun, 7 Jan 2024 21:49:27 +0100 Subject: [PATCH] [Messenger][FrameworkBundle] Add option to exclude stack trace from `ErrorDetailsStamp` --- .../Bundle/FrameworkBundle/CHANGELOG.md | 1 + .../DependencyInjection/Configuration.php | 7 +++++++ .../FrameworkExtension.php | 7 +++++++ .../Resources/config/messenger.php | 3 +++ .../Resources/config/schema/symfony-1.0.xsd | 2 ++ .../DependencyInjection/ConfigurationTest.php | 1 + ...ssenger_with_include_stack_trace_false.php | 19 +++++++++++++++++++ ...essenger_with_include_stack_trace_true.php | 18 ++++++++++++++++++ ...ssenger_with_include_stack_trace_false.xml | 17 +++++++++++++++++ ...essenger_with_include_stack_trace_true.xml | 17 +++++++++++++++++ ...ssenger_with_include_stack_trace_false.yml | 14 ++++++++++++++ ...essenger_with_include_stack_trace_true.yml | 13 +++++++++++++ .../FrameworkExtensionTestCase.php | 11 +++++++++++ src/Symfony/Component/Messenger/CHANGELOG.md | 5 +++++ .../AddErrorDetailsStampListener.php | 10 +++++++++- .../Messenger/Stamp/ErrorDetailsStamp.php | 4 ++-- .../AddErrorDetailsStampListenerTest.php | 14 ++++++++++++++ .../Tests/Stamp/ErrorDetailsStampTest.php | 11 +++++++++++ 18 files changed, 171 insertions(+), 3 deletions(-) create mode 100644 src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/php/messenger_with_include_stack_trace_false.php create mode 100644 src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/php/messenger_with_include_stack_trace_true.php create mode 100644 src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/xml/messenger_with_include_stack_trace_false.xml create mode 100644 src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/xml/messenger_with_include_stack_trace_true.xml create mode 100644 src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/yml/messenger_with_include_stack_trace_false.yml create mode 100644 src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/yml/messenger_with_include_stack_trace_true.yml diff --git a/src/Symfony/Bundle/FrameworkBundle/CHANGELOG.md b/src/Symfony/Bundle/FrameworkBundle/CHANGELOG.md index 97fa33a3c5eb3..964d78a520a9b 100644 --- a/src/Symfony/Bundle/FrameworkBundle/CHANGELOG.md +++ b/src/Symfony/Bundle/FrameworkBundle/CHANGELOG.md @@ -9,6 +9,7 @@ CHANGELOG * Add JsonEncoder services and configuration * Add new `framework.property_info.with_constructor_extractor` option to allow enabling or disabling the constructor extractor integration * Deprecate the `--show-arguments` option of the `container:debug` command, as arguments are now always shown + * Add `include_stack_trace_in_error` option that's used in `AddErrorDetailsStampListener` 7.2 --- diff --git a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php index 265fba9fd1f2d..c29621ac19727 100644 --- a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php +++ b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php @@ -1671,6 +1671,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() @@ -1702,6 +1705,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 index 9ef918161dce4..71a98a4237b31 100644 --- a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php +++ b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php @@ -2332,6 +2332,7 @@ private function registerMessengerConfiguration(array $config, ContainerBuilder } $senderAliases = []; + $includeStackTraceInError = []; $transportRetryReferences = []; $transportRateLimiterReferences = []; foreach ($config['transports'] as $name => $transport) { @@ -2347,6 +2348,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 { @@ -2419,6 +2422,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 index 40f5b84caa2e0..212c4fc416065 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Resources/config/messenger.php +++ b/src/Symfony/Bundle/FrameworkBundle/Resources/config/messenger.php @@ -183,6 +183,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 index b0eaf1679f3d9..bd57fdb2099d2 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Resources/config/schema/symfony-1.0.xsd +++ b/src/Symfony/Bundle/FrameworkBundle/Resources/config/schema/symfony-1.0.xsd @@ -606,6 +606,7 @@ + @@ -642,6 +643,7 @@ + diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/ConfigurationTest.php b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/ConfigurationTest.php index e8ca9873f1a70..4bbdac1a86d2b 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/ConfigurationTest.php +++ b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/ConfigurationTest.php @@ -910,6 +910,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 new file mode 100644 index 0000000000000..2329955cf2161 --- /dev/null +++ b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/php/messenger_with_include_stack_trace_false.php @@ -0,0 +1,19 @@ +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 new file mode 100644 index 0000000000000..d3d9d5b6f031c --- /dev/null +++ b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/php/messenger_with_include_stack_trace_true.php @@ -0,0 +1,18 @@ +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 new file mode 100644 index 0000000000000..d3bde946371f5 --- /dev/null +++ b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/xml/messenger_with_include_stack_trace_false.xml @@ -0,0 +1,17 @@ + + + + + + + + + + + + + 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 new file mode 100644 index 0000000000000..5f76231bee4e4 --- /dev/null +++ b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/xml/messenger_with_include_stack_trace_true.xml @@ -0,0 +1,17 @@ + + + + + + + + + + + + + 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 new file mode 100644 index 0000000000000..399a4a1d84160 --- /dev/null +++ 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 new file mode 100644 index 0000000000000..7587d35bfa3c3 --- /dev/null +++ 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 index 1f4daac5d4532..9632650dc0213 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/FrameworkExtensionTestCase.php +++ b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/FrameworkExtensionTestCase.php @@ -1112,6 +1112,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 index 4d492dfd49524..ecdd284b4aabb 100644 --- a/src/Symfony/Component/Messenger/CHANGELOG.md +++ b/src/Symfony/Component/Messenger/CHANGELOG.md @@ -1,6 +1,11 @@ CHANGELOG ========= +7.3 +--- + + * Add `$includeStackTrace` parameters to `AddErrorDetailsStampListener` and `ErrorDetailsStamp` to allow excluding the stack trace from the `ErrorDetailsStamp` + 7.2 --- diff --git a/src/Symfony/Component/Messenger/EventListener/AddErrorDetailsStampListener.php b/src/Symfony/Component/Messenger/EventListener/AddErrorDetailsStampListener.php index fd5c9d6fc61c1..385265b3f5c82 100644 --- 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 $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 index 3208f00c09199..7642b538b7a56 100644 --- a/src/Symfony/Component/Messenger/Stamp/ErrorDetailsStamp.php +++ b/src/Symfony/Component/Messenger/Stamp/ErrorDetailsStamp.php @@ -27,14 +27,14 @@ public function __construct( ) { } - 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 index b99c4e81bd8b9..8d9ebe16d3e26 100644 --- 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 index 8d66537afa0c8..5407ad02fc868 100644 --- 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');