From 2bfab1b4b4ca9c390f23066745c42f3ca594ba18 Mon Sep 17 00:00:00 2001 From: Alessandro Lai Date: Wed, 17 Nov 2021 23:14:06 +0100 Subject: [PATCH 1/5] Revert #406 to restore monolog handler registration --- src/DependencyInjection/Configuration.php | 22 +++++++++ src/DependencyInjection/SentryExtension.php | 27 ++++++++++- src/Resources/config/schema/sentry-1.0.xsd | 45 +++++++++++++++++++ src/Resources/config/services.xml | 4 ++ .../DependencyInjection/ConfigurationTest.php | 8 ++++ .../Fixtures/php/monolog_handler.php | 14 ++++++ .../Fixtures/php/monolog_handler_disabled.php | 12 +++++ .../Fixtures/xml/monolog_handler.xml | 12 +++++ .../Fixtures/xml/monolog_handler_disabled.xml | 14 ++++++ .../Fixtures/yml/monolog_handler.yml | 4 ++ .../Fixtures/yml/monolog_handler_disabled.yml | 3 ++ .../SentryExtensionTest.php | 18 ++++++++ 12 files changed, 182 insertions(+), 1 deletion(-) create mode 100644 tests/DependencyInjection/Fixtures/php/monolog_handler.php create mode 100644 tests/DependencyInjection/Fixtures/php/monolog_handler_disabled.php create mode 100644 tests/DependencyInjection/Fixtures/xml/monolog_handler.xml create mode 100644 tests/DependencyInjection/Fixtures/xml/monolog_handler_disabled.xml create mode 100644 tests/DependencyInjection/Fixtures/yml/monolog_handler.yml create mode 100644 tests/DependencyInjection/Fixtures/yml/monolog_handler_disabled.yml diff --git a/src/DependencyInjection/Configuration.php b/src/DependencyInjection/Configuration.php index 2fd7ae3c..004da2cb 100644 --- a/src/DependencyInjection/Configuration.php +++ b/src/DependencyInjection/Configuration.php @@ -6,6 +6,7 @@ use Doctrine\Bundle\DoctrineBundle\DoctrineBundle; use Jean85\PrettyVersions; +use Monolog\Logger; use Sentry\Options; use Sentry\SentryBundle\ErrorTypesParser; use Sentry\Transport\TransportFactoryInterface; @@ -135,6 +136,7 @@ public function getConfigTreeBuilder(): TreeBuilder ->end(); $this->addMessengerSection($rootNode); + $this->addMonologSection($rootNode); $this->addDistributedTracingSection($rootNode); return $treeBuilder; @@ -153,6 +155,26 @@ private function addMessengerSection(ArrayNodeDefinition $rootNode): void ->end(); } + private function addMonologSection(ArrayNodeDefinition $rootNode): void + { + $rootNode + ->children() + ->arrayNode('monolog') + ->addDefaultsIfNotSet() + ->children() + ->arrayNode('error_handler') + ->{class_exists(Logger::class) ? 'canBeDisabled' : 'canBeEnabled'}() + ->end() + ->scalarNode('level') + ->defaultValue(Logger::DEBUG) + ->cannotBeEmpty() + ->end() + ->booleanNode('bubble')->defaultTrue()->end() + ->end() + ->end() + ->end(); + } + private function addDistributedTracingSection(ArrayNodeDefinition $rootNode): void { $rootNode diff --git a/src/DependencyInjection/SentryExtension.php b/src/DependencyInjection/SentryExtension.php index 01655d7f..5064078b 100644 --- a/src/DependencyInjection/SentryExtension.php +++ b/src/DependencyInjection/SentryExtension.php @@ -6,7 +6,7 @@ use Doctrine\Bundle\DoctrineBundle\DoctrineBundle; use Jean85\PrettyVersions; -use LogicException; +use Monolog\Logger as MonologLogger; use Psr\Log\NullLogger; use Sentry\Client; use Sentry\ClientBuilder; @@ -14,6 +14,7 @@ use Sentry\Integration\IntegrationInterface; use Sentry\Integration\RequestFetcherInterface; use Sentry\Integration\RequestIntegration; +use Sentry\Monolog\Handler; use Sentry\Options; use Sentry\SentryBundle\EventListener\ConsoleListener; use Sentry\SentryBundle\EventListener\ErrorListener; @@ -32,6 +33,7 @@ use Symfony\Component\Config\FileLocator; use Symfony\Component\DependencyInjection\ContainerBuilder; use Symfony\Component\DependencyInjection\Definition; +use Symfony\Component\DependencyInjection\Exception\LogicException; use Symfony\Component\DependencyInjection\Loader; use Symfony\Component\DependencyInjection\Reference; use Symfony\Component\ErrorHandler\Error\FatalError; @@ -70,6 +72,7 @@ protected function loadInternal(array $mergedConfig, ContainerBuilder $container $this->registerDbalTracingConfiguration($container, $mergedConfig['tracing']); $this->registerTwigTracingConfiguration($container, $mergedConfig['tracing']); $this->registerCacheTracingConfiguration($container, $mergedConfig['tracing']); + $this->registerMonologHandlerConfiguration($container, $mergedConfig['monolog']); } /** @@ -236,6 +239,28 @@ private function registerCacheTracingConfiguration(ContainerBuilder $container, $container->setParameter('sentry.tracing.cache.enabled', $isConfigEnabled); } + /** + * @param array $config + */ + private function registerMonologHandlerConfiguration(ContainerBuilder $container, array $config): void + { + $errorHandlerConfig = $config['error_handler']; + + if (!$errorHandlerConfig['enabled']) { + $container->removeDefinition(Handler::class); + + return; + } + + if (!class_exists(MonologLogger::class)) { + throw new LogicException(sprintf('To use the "%s" class you need to require the "symfony/monolog-bundle" package.', Handler::class)); + } + + $definition = $container->getDefinition(Handler::class); + $definition->setArgument(0, MonologLogger::toMonologLevel($config['level'])); + $definition->setArgument(1, $config['bubble']); + } + /** * @param string[] $integrations * @param array $config diff --git a/src/Resources/config/schema/sentry-1.0.xsd b/src/Resources/config/schema/sentry-1.0.xsd index d5e3bbf7..e56863ad 100644 --- a/src/Resources/config/schema/sentry-1.0.xsd +++ b/src/Resources/config/schema/sentry-1.0.xsd @@ -12,6 +12,7 @@ + @@ -115,4 +116,48 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/src/Resources/config/services.xml b/src/Resources/config/services.xml index 0835929a..c1eb7aef 100644 --- a/src/Resources/config/services.xml +++ b/src/Resources/config/services.xml @@ -89,6 +89,10 @@ + + + + diff --git a/tests/DependencyInjection/ConfigurationTest.php b/tests/DependencyInjection/ConfigurationTest.php index 5f73cfde..61a01f14 100644 --- a/tests/DependencyInjection/ConfigurationTest.php +++ b/tests/DependencyInjection/ConfigurationTest.php @@ -6,6 +6,7 @@ use Doctrine\Bundle\DoctrineBundle\DoctrineBundle; use Jean85\PrettyVersions; +use Monolog\Logger; use PHPUnit\Framework\TestCase; use Sentry\SentryBundle\DependencyInjection\Configuration; use Symfony\Bundle\TwigBundle\TwigBundle; @@ -40,6 +41,13 @@ public function testProcessConfigurationWithDefaultConfiguration(): void 'enabled' => interface_exists(MessageBusInterface::class), 'capture_soft_fails' => true, ], + 'monolog' => [ + 'error_handler' => [ + 'enabled' => class_exists(Logger::class), + ], + 'level' => Logger::DEBUG, + 'bubble' => true, + ], 'tracing' => [ 'enabled' => true, 'dbal' => [ diff --git a/tests/DependencyInjection/Fixtures/php/monolog_handler.php b/tests/DependencyInjection/Fixtures/php/monolog_handler.php new file mode 100644 index 00000000..0ba7c1ca --- /dev/null +++ b/tests/DependencyInjection/Fixtures/php/monolog_handler.php @@ -0,0 +1,14 @@ +loadFromExtension('sentry', [ + 'monolog' => [ + 'level' => Logger::ERROR, + 'bubble' => false, + ], +]); diff --git a/tests/DependencyInjection/Fixtures/php/monolog_handler_disabled.php b/tests/DependencyInjection/Fixtures/php/monolog_handler_disabled.php new file mode 100644 index 00000000..0f2fa62f --- /dev/null +++ b/tests/DependencyInjection/Fixtures/php/monolog_handler_disabled.php @@ -0,0 +1,12 @@ +loadFromExtension('sentry', [ + 'monolog' => [ + 'error_handler' => false, + ], +]); diff --git a/tests/DependencyInjection/Fixtures/xml/monolog_handler.xml b/tests/DependencyInjection/Fixtures/xml/monolog_handler.xml new file mode 100644 index 00000000..fbf63d5e --- /dev/null +++ b/tests/DependencyInjection/Fixtures/xml/monolog_handler.xml @@ -0,0 +1,12 @@ + + + + + + + + diff --git a/tests/DependencyInjection/Fixtures/xml/monolog_handler_disabled.xml b/tests/DependencyInjection/Fixtures/xml/monolog_handler_disabled.xml new file mode 100644 index 00000000..8d20321d --- /dev/null +++ b/tests/DependencyInjection/Fixtures/xml/monolog_handler_disabled.xml @@ -0,0 +1,14 @@ + + + + + + + + + + diff --git a/tests/DependencyInjection/Fixtures/yml/monolog_handler.yml b/tests/DependencyInjection/Fixtures/yml/monolog_handler.yml new file mode 100644 index 00000000..9f9fbb90 --- /dev/null +++ b/tests/DependencyInjection/Fixtures/yml/monolog_handler.yml @@ -0,0 +1,4 @@ +sentry: + monolog: + level: !php/const Monolog\Logger::ERROR + bubble: false diff --git a/tests/DependencyInjection/Fixtures/yml/monolog_handler_disabled.yml b/tests/DependencyInjection/Fixtures/yml/monolog_handler_disabled.yml new file mode 100644 index 00000000..d2cae38b --- /dev/null +++ b/tests/DependencyInjection/Fixtures/yml/monolog_handler_disabled.yml @@ -0,0 +1,3 @@ +sentry: + monolog: + error_handler: false diff --git a/tests/DependencyInjection/SentryExtensionTest.php b/tests/DependencyInjection/SentryExtensionTest.php index de241ecc..8d54c28f 100644 --- a/tests/DependencyInjection/SentryExtensionTest.php +++ b/tests/DependencyInjection/SentryExtensionTest.php @@ -6,10 +6,12 @@ use Doctrine\Bundle\DoctrineBundle\DoctrineBundle; use Jean85\PrettyVersions; +use Monolog\Logger as MonologLogger; use PHPUnit\Framework\TestCase; use Psr\Log\NullLogger; use Sentry\ClientInterface; use Sentry\Integration\IgnoreErrorsIntegration; +use Sentry\Monolog\Handler; use Sentry\Options; use Sentry\SentryBundle\DependencyInjection\SentryExtension; use Sentry\SentryBundle\EventListener\ConsoleListener; @@ -178,6 +180,22 @@ public function testSubRequestListener(): void ], $definition->getTags()); } + public function testMonologHandler(): void + { + $container = $this->createContainerFromFixture('monolog_handler'); + $definition = $container->getDefinition(Handler::class); + + $this->assertSame(MonologLogger::ERROR, $definition->getArgument(0)); + $this->assertFalse($definition->getArgument(1)); + } + + public function testMonologHandlerIsRemovedWhenDisabled(): void + { + $container = $this->createContainerFromFixture('monolog_handler_disabled'); + + $this->assertFalse($container->hasDefinition(Handler::class)); + } + public function testClentIsCreatedFromOptions(): void { $container = $this->createContainerFromFixture('full'); From a84bc9a4c440f492e5275fd05393707ae0a52939 Mon Sep 17 00:00:00 2001 From: Alessandro Lai Date: Thu, 18 Nov 2021 22:47:06 +0100 Subject: [PATCH 2/5] Register the Handler automatically; duplicate the E2E tests to check the new monolog hooks --- .../Compiler/RegisterMonologHandlerPass.php | 23 +++++++++++++++++++ src/DependencyInjection/SentryExtension.php | 8 +++++-- src/SentryBundle.php | 3 +++ tests/End2End/App/KernelHookedToMonolog.php | 17 ++++++++++++++ .../End2End/App/sentry_hooked_to_monolog.yml | 9 ++++++++ tests/End2End/End2EndHookedToMonologTest.php | 18 +++++++++++++++ 6 files changed, 76 insertions(+), 2 deletions(-) create mode 100644 src/DependencyInjection/Compiler/RegisterMonologHandlerPass.php create mode 100644 tests/End2End/App/KernelHookedToMonolog.php create mode 100644 tests/End2End/App/sentry_hooked_to_monolog.yml create mode 100644 tests/End2End/End2EndHookedToMonologTest.php diff --git a/src/DependencyInjection/Compiler/RegisterMonologHandlerPass.php b/src/DependencyInjection/Compiler/RegisterMonologHandlerPass.php new file mode 100644 index 00000000..ea10c374 --- /dev/null +++ b/src/DependencyInjection/Compiler/RegisterMonologHandlerPass.php @@ -0,0 +1,23 @@ +hasDefinition(Handler::class)) { + return; + } + + $logger = $container->getDefinition('monolog.logger'); + $logger->addMethodCall('pushHandler', [new Reference(Handler::class)]); + } +} diff --git a/src/DependencyInjection/SentryExtension.php b/src/DependencyInjection/SentryExtension.php index 5064078b..7f7915ed 100644 --- a/src/DependencyInjection/SentryExtension.php +++ b/src/DependencyInjection/SentryExtension.php @@ -28,6 +28,7 @@ use Sentry\SentryBundle\Tracing\Twig\TwigTracingExtension; use Sentry\Serializer\RepresentationSerializer; use Sentry\Serializer\Serializer; +use Sentry\State\HubInterface; use Symfony\Bundle\TwigBundle\TwigBundle; use Symfony\Component\Cache\CacheItem; use Symfony\Component\Config\FileLocator; @@ -257,8 +258,11 @@ private function registerMonologHandlerConfiguration(ContainerBuilder $container } $definition = $container->getDefinition(Handler::class); - $definition->setArgument(0, MonologLogger::toMonologLevel($config['level'])); - $definition->setArgument(1, $config['bubble']); + $definition->setArguments([ + new Reference(HubInterface::class), + MonologLogger::toMonologLevel($config['level']), + $config['bubble'], + ]); } /** diff --git a/src/SentryBundle.php b/src/SentryBundle.php index 4098f8b3..552979b0 100644 --- a/src/SentryBundle.php +++ b/src/SentryBundle.php @@ -6,6 +6,8 @@ use Sentry\SentryBundle\DependencyInjection\Compiler\CacheTracingPass; use Sentry\SentryBundle\DependencyInjection\Compiler\DbalTracingPass; +use Sentry\SentryBundle\DependencyInjection\Compiler\RegisterMonologHandlerPass; +use Symfony\Component\DependencyInjection\Compiler\PassConfig; use Symfony\Component\DependencyInjection\ContainerBuilder; use Symfony\Component\HttpKernel\Bundle\Bundle; @@ -17,6 +19,7 @@ public function build(ContainerBuilder $container): void { parent::build($container); + $container->addCompilerPass(new RegisterMonologHandlerPass(), PassConfig::TYPE_BEFORE_REMOVING); $container->addCompilerPass(new DbalTracingPass()); $container->addCompilerPass(new CacheTracingPass()); } diff --git a/tests/End2End/App/KernelHookedToMonolog.php b/tests/End2End/App/KernelHookedToMonolog.php new file mode 100644 index 00000000..fa80ab22 --- /dev/null +++ b/tests/End2End/App/KernelHookedToMonolog.php @@ -0,0 +1,17 @@ +load(__DIR__ . '/sentry_hooked_to_monolog.yml'); + } +} diff --git a/tests/End2End/App/sentry_hooked_to_monolog.yml b/tests/End2End/App/sentry_hooked_to_monolog.yml new file mode 100644 index 00000000..26a510b1 --- /dev/null +++ b/tests/End2End/App/sentry_hooked_to_monolog.yml @@ -0,0 +1,9 @@ +sentry: + register_error_listener: false + monolog: + error_handler: + enabled: true + options: + # TODO: add a simple option to switch off the error/fatal integrations + default_integrations: false + integrations: [] diff --git a/tests/End2End/End2EndHookedToMonologTest.php b/tests/End2End/End2EndHookedToMonologTest.php new file mode 100644 index 00000000..b787b5f1 --- /dev/null +++ b/tests/End2End/End2EndHookedToMonologTest.php @@ -0,0 +1,18 @@ + Date: Sat, 20 Nov 2021 00:12:46 +0100 Subject: [PATCH 3/5] Use dumb way to attach handlers --- .../Compiler/RegisterMonologHandlerPass.php | 10 ++++++++-- tests/End2End/App/sentry_hooked_to_monolog.yml | 1 + 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/src/DependencyInjection/Compiler/RegisterMonologHandlerPass.php b/src/DependencyInjection/Compiler/RegisterMonologHandlerPass.php index ea10c374..7a32ca2e 100644 --- a/src/DependencyInjection/Compiler/RegisterMonologHandlerPass.php +++ b/src/DependencyInjection/Compiler/RegisterMonologHandlerPass.php @@ -17,7 +17,13 @@ public function process(ContainerBuilder $container): void return; } - $logger = $container->getDefinition('monolog.logger'); - $logger->addMethodCall('pushHandler', [new Reference(Handler::class)]); + foreach ($container->getServiceIds() as $serviceName) { + if (!str_starts_with($serviceName, 'monolog.logger.')) { + continue; + } + + $logger = $container->getDefinition($serviceName); + $logger->addMethodCall('pushHandler', [new Reference(Handler::class)]); + } } } diff --git a/tests/End2End/App/sentry_hooked_to_monolog.yml b/tests/End2End/App/sentry_hooked_to_monolog.yml index 26a510b1..d2590b79 100644 --- a/tests/End2End/App/sentry_hooked_to_monolog.yml +++ b/tests/End2End/App/sentry_hooked_to_monolog.yml @@ -3,6 +3,7 @@ sentry: monolog: error_handler: enabled: true + level: error options: # TODO: add a simple option to switch off the error/fatal integrations default_integrations: false From 9cea19bf78df088a09a0f06680d34142f420ab27 Mon Sep 17 00:00:00 2001 From: Alessandro Lai Date: Wed, 12 Jan 2022 23:36:34 +0100 Subject: [PATCH 4/5] Try to ignore wrapped FatalErrors --- .../Compiler/RegisterMonologHandlerPass.php | 18 ++++- src/DependencyInjection/SentryExtension.php | 10 ++- .../IgnoreFatalErrorExceptionsIntegration.php | 39 ++++++++++ src/Monolog/SymfonyHandler.php | 78 +++++++++++++++++++ src/Resources/config/services.xml | 2 + tests/End2End/End2EndHookedToMonologTest.php | 3 - tests/End2End/End2EndTest.php | 3 - 7 files changed, 142 insertions(+), 11 deletions(-) create mode 100644 src/Integration/IgnoreFatalErrorExceptionsIntegration.php create mode 100644 src/Monolog/SymfonyHandler.php diff --git a/src/DependencyInjection/Compiler/RegisterMonologHandlerPass.php b/src/DependencyInjection/Compiler/RegisterMonologHandlerPass.php index 7a32ca2e..7a2b6957 100644 --- a/src/DependencyInjection/Compiler/RegisterMonologHandlerPass.php +++ b/src/DependencyInjection/Compiler/RegisterMonologHandlerPass.php @@ -5,6 +5,9 @@ namespace Sentry\SentryBundle\DependencyInjection\Compiler; use Sentry\Monolog\Handler; +use Sentry\Options; +use Sentry\SentryBundle\Integration\IgnoreFatalErrorExceptionsIntegration; +use Sentry\SentryBundle\Monolog\SymfonyHandler; use Symfony\Component\DependencyInjection\Compiler\CompilerPassInterface; use Symfony\Component\DependencyInjection\ContainerBuilder; use Symfony\Component\DependencyInjection\Reference; @@ -17,13 +20,26 @@ public function process(ContainerBuilder $container): void return; } + $this->registerMainHandler($container); + } + + private function registerMainHandler(ContainerBuilder $container): void + { foreach ($container->getServiceIds() as $serviceName) { if (!str_starts_with($serviceName, 'monolog.logger.')) { continue; } $logger = $container->getDefinition($serviceName); - $logger->addMethodCall('pushHandler', [new Reference(Handler::class)]); + $logger->addMethodCall('pushHandler', [new Reference(SymfonyHandler::class)]); } } + + private function registerIntegrations(ContainerBuilder $container): void + { + $options = $container->getDefinition(Options::class); + $input = $options->getArgument(0); + $input['integrations'][] = IgnoreFatalErrorExceptionsIntegration::class; + $options->setArgument(0, $input); + } } diff --git a/src/DependencyInjection/SentryExtension.php b/src/DependencyInjection/SentryExtension.php index 7f7915ed..793a557e 100644 --- a/src/DependencyInjection/SentryExtension.php +++ b/src/DependencyInjection/SentryExtension.php @@ -14,7 +14,7 @@ use Sentry\Integration\IntegrationInterface; use Sentry\Integration\RequestFetcherInterface; use Sentry\Integration\RequestIntegration; -use Sentry\Monolog\Handler; +use Sentry\Monolog\Handler as SentryHandler; use Sentry\Options; use Sentry\SentryBundle\EventListener\ConsoleListener; use Sentry\SentryBundle\EventListener\ErrorListener; @@ -22,6 +22,7 @@ use Sentry\SentryBundle\EventListener\TracingConsoleListener; use Sentry\SentryBundle\EventListener\TracingRequestListener; use Sentry\SentryBundle\EventListener\TracingSubRequestListener; +use Sentry\SentryBundle\Monolog\SymfonyHandler; use Sentry\SentryBundle\SentryBundle; use Sentry\SentryBundle\Tracing\Doctrine\DBAL\ConnectionConfigurator; use Sentry\SentryBundle\Tracing\Doctrine\DBAL\TracingDriverMiddleware; @@ -248,16 +249,17 @@ private function registerMonologHandlerConfiguration(ContainerBuilder $container $errorHandlerConfig = $config['error_handler']; if (!$errorHandlerConfig['enabled']) { - $container->removeDefinition(Handler::class); + $container->removeDefinition(SymfonyHandler::class); + $container->removeDefinition(SentryHandler::class); return; } if (!class_exists(MonologLogger::class)) { - throw new LogicException(sprintf('To use the "%s" class you need to require the "symfony/monolog-bundle" package.', Handler::class)); + throw new LogicException(sprintf('To use the "%s" class you need to require the "symfony/monolog-bundle" package.', SymfonyHandler::class)); } - $definition = $container->getDefinition(Handler::class); + $definition = $container->getDefinition(SymfonyHandler::class); $definition->setArguments([ new Reference(HubInterface::class), MonologLogger::toMonologLevel($config['level']), diff --git a/src/Integration/IgnoreFatalErrorExceptionsIntegration.php b/src/Integration/IgnoreFatalErrorExceptionsIntegration.php new file mode 100644 index 00000000..ae96a3c2 --- /dev/null +++ b/src/Integration/IgnoreFatalErrorExceptionsIntegration.php @@ -0,0 +1,39 @@ +getExceptions()[0] ?? null; + + if ( + $exceptionDataBag instanceof ExceptionDataBag + && $exceptionDataBag->getType() === self::$previousExceptionDataBag->getType() + && $exceptionDataBag->getValue() === self::$previousExceptionDataBag->getValue() + ) { + return null; + } + + return $event; + }); + } +} diff --git a/src/Monolog/SymfonyHandler.php b/src/Monolog/SymfonyHandler.php new file mode 100644 index 00000000..e58c9125 --- /dev/null +++ b/src/Monolog/SymfonyHandler.php @@ -0,0 +1,78 @@ +decoratedHandler = $decoratedHandler; + } + + public function setFormatter(FormatterInterface $formatter): HandlerInterface + { + return $this->decoratedHandler->setFormatter($formatter); + } + + public function getFormatter(): FormatterInterface + { + return $this->decoratedHandler->getFormatter(); + } + + public function isHandling(array $record): bool + { + return $this->decoratedHandler->isHandling($record); + } + + public function handleBatch(array $records): void + { + $this->decoratedHandler->handleBatch($records); + } + + public function close(): void + { + $this->decoratedHandler->close(); + } + + public function pushProcessor(callable $callback): HandlerInterface + { + return $this->decoratedHandler->pushProcessor($callback); + } + + public function popProcessor(): callable + { + return $this->decoratedHandler->popProcessor(); + } + + public function reset(): void + { + $this->decoratedHandler->reset(); + } + + public function handle(array $record): bool + { + if ( + \array_key_exists('exception', $record) + && $record['exception'] instanceof FatalError + ) { + return false; + } + + return $this->decoratedHandler->handle($record); + } +} diff --git a/src/Resources/config/services.xml b/src/Resources/config/services.xml index c1eb7aef..dadc7b14 100644 --- a/src/Resources/config/services.xml +++ b/src/Resources/config/services.xml @@ -89,6 +89,8 @@ + + diff --git a/tests/End2End/End2EndHookedToMonologTest.php b/tests/End2End/End2EndHookedToMonologTest.php index b787b5f1..b2ed2446 100644 --- a/tests/End2End/End2EndHookedToMonologTest.php +++ b/tests/End2End/End2EndHookedToMonologTest.php @@ -6,9 +6,6 @@ use Sentry\SentryBundle\Tests\End2End\App\KernelHookedToMonolog; -/** - * @runTestsInSeparateProcesses - */ class End2EndHookedToMonologTest extends End2EndTest { protected static function getKernelClass(): string diff --git a/tests/End2End/End2EndTest.php b/tests/End2End/End2EndTest.php index 37df19e1..a2c094e2 100644 --- a/tests/End2End/End2EndTest.php +++ b/tests/End2End/End2EndTest.php @@ -22,9 +22,6 @@ class_alias(Client::class, KernelBrowser::class); } -/** - * @runTestsInSeparateProcesses - */ class End2EndTest extends WebTestCase { public const SENT_EVENTS_LOG = '/tmp/sentry_e2e_test_sent_events.log'; From 7216c6a1ab095523bb35196472abd76df9ba394e Mon Sep 17 00:00:00 2001 From: Alessandro Lai Date: Wed, 1 Jun 2022 18:13:38 +0200 Subject: [PATCH 5/5] Fix a couple of tests --- phpstan-baseline.neon | 35 +++++++++++++++++++ .../Compiler/RegisterMonologHandlerPass.php | 10 ------ src/Monolog/SymfonyHandler.php | 7 ++-- .../SentryExtensionTest.php | 4 +-- 4 files changed, 39 insertions(+), 17 deletions(-) diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index fcf117af..59629be1 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -40,6 +40,11 @@ parameters: count: 1 path: src/DependencyInjection/SentryExtension.php + - + message: "#^Cannot access offset 'enabled' on mixed\\.$#" + count: 1 + path: src/DependencyInjection/SentryExtension.php + - message: "#^Cannot access offset 'excluded_commands' on mixed\\.$#" count: 1 @@ -85,6 +90,11 @@ parameters: count: 1 path: src/DependencyInjection/SentryExtension.php + - + message: "#^Parameter \\#1 \\$level of static method Monolog\\\\Logger\\:\\:toMonologLevel\\(\\) expects 100\\|200\\|250\\|300\\|400\\|500\\|550\\|600\\|'ALERT'\\|'alert'\\|'CRITICAL'\\|'critical'\\|'DEBUG'\\|'debug'\\|'EMERGENCY'\\|'emergency'\\|'ERROR'\\|'error'\\|'INFO'\\|'info'\\|'NOTICE'\\|'notice'\\|'WARNING'\\|'warning', mixed given\\.$#" + count: 1 + path: src/DependencyInjection/SentryExtension.php + - message: "#^Parameter \\#2 \\$array of function array_map expects array, mixed given\\.$#" count: 1 @@ -110,6 +120,11 @@ parameters: count: 1 path: src/DependencyInjection/SentryExtension.php + - + message: "#^Parameter \\#2 \\$config of method Sentry\\\\SentryBundle\\\\DependencyInjection\\\\SentryExtension\\:\\:registerMonologHandlerConfiguration\\(\\) expects array\\, mixed given\\.$#" + count: 1 + path: src/DependencyInjection/SentryExtension.php + - message: "#^Parameter \\#2 \\$config of method Sentry\\\\SentryBundle\\\\DependencyInjection\\\\SentryExtension\\:\\:registerTracingConfiguration\\(\\) expects array\\, mixed given\\.$#" count: 1 @@ -175,6 +190,26 @@ parameters: count: 1 path: src/EventListener/SubRequestListener.php + - + message: "#^Cannot call method getType\\(\\) on Sentry\\\\ExceptionDataBag\\|null\\.$#" + count: 1 + path: src/Integration/IgnoreFatalErrorExceptionsIntegration.php + + - + message: "#^Cannot call method getValue\\(\\) on Sentry\\\\ExceptionDataBag\\|null\\.$#" + count: 1 + path: src/Integration/IgnoreFatalErrorExceptionsIntegration.php + + - + message: "#^Instanceof between null and Symfony\\\\Component\\\\ErrorHandler\\\\Error\\\\FatalError will always evaluate to false\\.$#" + count: 1 + path: src/Monolog/SymfonyHandler.php + + - + message: "#^Offset 'exception' on array\\{message\\: string, context\\: array, level\\: 100\\|200\\|250\\|300\\|400\\|500\\|550\\|600, level_name\\: 'ALERT'\\|'CRITICAL'\\|'DEBUG'\\|'EMERGENCY'\\|'ERROR'\\|'INFO'\\|'NOTICE'\\|'WARNING', channel\\: string, datetime\\: DateTimeImmutable, extra\\: array\\} on left side of \\?\\? does not exist\\.$#" + count: 1 + path: src/Monolog/SymfonyHandler.php + - message: "#^Parameter \\#1 \\$driver of method Sentry\\\\SentryBundle\\\\Tracing\\\\Doctrine\\\\DBAL\\\\TracingDriverMiddleware\\:\\:wrap\\(\\) expects Doctrine\\\\DBAL\\\\Driver, mixed given\\.$#" count: 1 diff --git a/src/DependencyInjection/Compiler/RegisterMonologHandlerPass.php b/src/DependencyInjection/Compiler/RegisterMonologHandlerPass.php index 7a2b6957..259e47e7 100644 --- a/src/DependencyInjection/Compiler/RegisterMonologHandlerPass.php +++ b/src/DependencyInjection/Compiler/RegisterMonologHandlerPass.php @@ -5,8 +5,6 @@ namespace Sentry\SentryBundle\DependencyInjection\Compiler; use Sentry\Monolog\Handler; -use Sentry\Options; -use Sentry\SentryBundle\Integration\IgnoreFatalErrorExceptionsIntegration; use Sentry\SentryBundle\Monolog\SymfonyHandler; use Symfony\Component\DependencyInjection\Compiler\CompilerPassInterface; use Symfony\Component\DependencyInjection\ContainerBuilder; @@ -34,12 +32,4 @@ private function registerMainHandler(ContainerBuilder $container): void $logger->addMethodCall('pushHandler', [new Reference(SymfonyHandler::class)]); } } - - private function registerIntegrations(ContainerBuilder $container): void - { - $options = $container->getDefinition(Options::class); - $input = $options->getArgument(0); - $input['integrations'][] = IgnoreFatalErrorExceptionsIntegration::class; - $options->setArgument(0, $input); - } } diff --git a/src/Monolog/SymfonyHandler.php b/src/Monolog/SymfonyHandler.php index e58c9125..ce7c1bbf 100644 --- a/src/Monolog/SymfonyHandler.php +++ b/src/Monolog/SymfonyHandler.php @@ -66,10 +66,9 @@ public function reset(): void public function handle(array $record): bool { - if ( - \array_key_exists('exception', $record) - && $record['exception'] instanceof FatalError - ) { + $exception = $record['exception'] ?? null; + + if ($exception instanceof FatalError) { return false; } diff --git a/tests/DependencyInjection/SentryExtensionTest.php b/tests/DependencyInjection/SentryExtensionTest.php index 56551e95..a923d70d 100644 --- a/tests/DependencyInjection/SentryExtensionTest.php +++ b/tests/DependencyInjection/SentryExtensionTest.php @@ -6,7 +6,6 @@ use Doctrine\Bundle\DoctrineBundle\DoctrineBundle; use Jean85\PrettyVersions; -use Monolog\Logger as MonologLogger; use PHPUnit\Framework\TestCase; use Psr\Log\NullLogger; use Sentry\ClientInterface; @@ -187,8 +186,7 @@ public function testMonologHandler(): void $container = $this->createContainerFromFixture('monolog_handler'); $definition = $container->getDefinition(Handler::class); - $this->assertSame(MonologLogger::ERROR, $definition->getArgument(0)); - $this->assertFalse($definition->getArgument(1)); + $this->assertCount(1, $definition->getArguments(), 'Arguments are not default ones'); } public function testMonologHandlerIsRemovedWhenDisabled(): void