From b6e13cba01630b97b15528a996e8215b7b5a0974 Mon Sep 17 00:00:00 2001 From: Alessandro Lai Date: Tue, 8 Jan 2019 12:53:22 +0100 Subject: [PATCH 01/49] Require Sentry client 2.0 --- composer.json | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/composer.json b/composer.json index 310d9209..9083abe8 100644 --- a/composer.json +++ b/composer.json @@ -21,7 +21,7 @@ "require": { "php": "^7.1", "jean85/pretty-package-versions": "^1.0", - "sentry/sentry": "^1.9", + "sentry/sentry": "^2.0-beta", "symfony/config": "^3.0||^4.0", "symfony/console": "^3.3||^4.0", "symfony/dependency-injection": "^3.0||^4.0", @@ -31,6 +31,7 @@ }, "require-dev": { "friendsofphp/php-cs-fixer": "^2.8", + "php-http/mock-client": "^1.0", "phpstan/phpstan": "^0.10", "phpstan/phpstan-phpunit": "^0.10", "phpunit/phpunit": "^7", From 59d98a8673dac766b228e25094789988c7f19ff3 Mon Sep 17 00:00:00 2001 From: Alessandro Lai Date: Mon, 21 Jan 2019 15:05:18 +0100 Subject: [PATCH 02/49] Draft and migrate the services config --- src/Resources/config/services.xml | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/src/Resources/config/services.xml b/src/Resources/config/services.xml index c322f116..19bc3edc 100644 --- a/src/Resources/config/services.xml +++ b/src/Resources/config/services.xml @@ -5,26 +5,24 @@ xsi:schemaLocation="http://symfony.com/schema/dic/services http://symfony.com/schema/dic/services/services-1.0.xsd"> - - %sentry.dsn% - %sentry.options% - + + - + + + + + + + - - - - %sentry.skip_capture% - - From 6239a63867ee86a76ab317124d6cdf2102adce9a Mon Sep 17 00:00:00 2001 From: Alessandro Lai Date: Mon, 21 Jan 2019 16:27:32 +0100 Subject: [PATCH 03/49] Rework and simplify the listener --- src/DependencyInjection/Configuration.php | 119 ------------ src/DependencyInjection/SentryExtension.php | 4 - src/Event/SentryUserContextEvent.php | 22 --- src/EventListener/ExceptionListener.php | 180 +++++------------- .../SentryExceptionListenerInterface.php | 47 ----- src/Resources/config/services.xml | 4 + src/SentrySymfonyClient.php | 22 --- src/SentrySymfonyEvents.php | 30 --- 8 files changed, 52 insertions(+), 376 deletions(-) delete mode 100644 src/Event/SentryUserContextEvent.php delete mode 100644 src/EventListener/SentryExceptionListenerInterface.php delete mode 100644 src/SentrySymfonyClient.php delete mode 100644 src/SentrySymfonyEvents.php diff --git a/src/DependencyInjection/Configuration.php b/src/DependencyInjection/Configuration.php index 3e6bcce0..0d682599 100644 --- a/src/DependencyInjection/Configuration.php +++ b/src/DependencyInjection/Configuration.php @@ -2,14 +2,9 @@ namespace Sentry\SentryBundle\DependencyInjection; -use Raven_Compat; -use Sentry\SentryBundle\EventListener\ExceptionListener; -use Sentry\SentryBundle\EventListener\SentryExceptionListenerInterface; -use Sentry\SentryBundle\SentrySymfonyClient; use Symfony\Component\Config\Definition\Builder\ArrayNodeDefinition; use Symfony\Component\Config\Definition\Builder\TreeBuilder; use Symfony\Component\Config\Definition\ConfigurationInterface; -use Symfony\Component\HttpKernel\Exception\HttpExceptionInterface; /** * This is the class that validates and merges configuration from your app/config files @@ -32,12 +27,6 @@ public function getConfigTreeBuilder() // Basic Sentry configuration $rootNode ->children() - ->scalarNode('app_path') - ->defaultValue('%kernel.root_dir%/..') - ->end() - ->scalarNode('client') - ->defaultValue(SentrySymfonyClient::class) - ->end() ->scalarNode('dsn') ->beforeNormalization() ->ifString() @@ -46,117 +35,9 @@ public function getConfigTreeBuilder() ->defaultNull() ->end(); - // Sentry client options - $rootNode - ->children() - ->arrayNode('options') - ->addDefaultsIfNotSet() - ->children() - ->scalarNode('logger')->defaultValue('php')->end() - ->scalarNode('server')->defaultNull()->end() - ->scalarNode('secret_key')->defaultNull()->end() - ->scalarNode('public_key')->defaultNull()->end() - ->scalarNode('project')->defaultValue(1)->end() - ->booleanNode('auto_log_stacks')->defaultFalse()->end() - ->scalarNode('name')->defaultValue(Raven_Compat::gethostname())->end() - ->scalarNode('site')->defaultNull()->end() - ->arrayNode('tags') - ->prototype('scalar')->end() - ->end() - ->scalarNode('release')->defaultNull()->end() - ->scalarNode('environment')->defaultValue('%kernel.environment%')->end() - ->scalarNode('sample_rate')->defaultValue(1)->end() - ->booleanNode('trace')->defaultTrue()->end() - ->scalarNode('timeout')->defaultValue(2)->end() - ->scalarNode('message_limit')->defaultValue(\Raven_Client::MESSAGE_LIMIT)->end() - ->arrayNode('exclude') - ->prototype('scalar')->end() - ->end() - ->arrayNode('excluded_exceptions') - ->prototype('scalar')->end() - ->end() - ->scalarNode('http_proxy')->defaultNull()->end() - ->arrayNode('extra') - ->prototype('scalar')->end() - ->end() - ->scalarNode('curl_method')->defaultValue('sync')->end() - ->scalarNode('curl_path')->defaultValue('curl')->end() - ->booleanNode('curl_ipv4')->defaultTrue()->end() - ->scalarNode('ca_cert')->defaultNull()->end() - ->booleanNode('verify_ssl')->defaultTrue()->end() - ->scalarNode('curl_ssl_version')->defaultNull()->end() - ->scalarNode('trust_x_forwarded_proto')->defaultFalse()->end() - ->scalarNode('mb_detect_order')->defaultNull()->end() - ->scalarNode('error_types')->defaultNull()->end() - ->scalarNode('app_path')->defaultValue('%kernel.root_dir%/..')->end() - ->arrayNode('excluded_app_paths') - ->defaultValue( - [ - '%kernel.root_dir%/../vendor', - '%kernel.root_dir%/../app/cache', - '%kernel.root_dir%/../var/cache', - ] - ) - ->prototype('scalar')->end() - ->end() - ->arrayNode('prefixes') - ->defaultValue(['%kernel.root_dir%/..']) - ->prototype('scalar')->end() - ->end() - ->booleanNode('install_default_breadcrumb_handlers')->defaultTrue()->end() - ->booleanNode('install_shutdown_handler')->defaultTrue()->end() - ->arrayNode('processors') - ->defaultValue([\Raven_Processor_SanitizeDataProcessor::class]) - ->prototype('scalar')->end() - ->end() - ->arrayNode('processorOptions') - ->arrayPrototype() - ->prototype('scalar')->end() - ->end() - ->end() - ->end() - ->end(); - - // Bundle-specific configuration - $rootNode - ->children() - ->scalarNode('exception_listener') - ->defaultValue(ExceptionListener::class) - ->validate() - ->ifTrue($this->getExceptionListenerInvalidationClosure()) - ->thenInvalid('The "sentry.exception_listener" parameter should be a FQCN of a class implementing the SentryExceptionListenerInterface interface') - ->end() - ->end() - ->arrayNode('skip_capture') - ->prototype('scalar')->end() - ->defaultValue([HttpExceptionInterface::class]) - ->end() - ->arrayNode('listener_priorities') - ->addDefaultsIfNotSet() - ->children() - ->scalarNode('request')->defaultValue(0)->end() - ->scalarNode('kernel_exception')->defaultValue(0)->end() - ->scalarNode('console_exception')->defaultValue(0)->end() - ->end() - ->end() - ->end() - ; - return $treeBuilder; } - private function getExceptionListenerInvalidationClosure(): callable - { - return function ($value) { - $implements = class_implements($value); - if ($implements === false) { - return true; - } - - return ! in_array(SentryExceptionListenerInterface::class, $implements, true); - }; - } - private function getTrimClosure(): callable { return function ($str) { diff --git a/src/DependencyInjection/SentryExtension.php b/src/DependencyInjection/SentryExtension.php index 30948c94..4be87abb 100644 --- a/src/DependencyInjection/SentryExtension.php +++ b/src/DependencyInjection/SentryExtension.php @@ -27,10 +27,6 @@ public function load(array $configs, ContainerBuilder $container) $loader = new Loader\XmlFileLoader($container, new FileLocator(__DIR__ . '/../Resources/config')); $loader->load('services.xml'); - foreach ($config as $key => $value) { - $container->setParameter('sentry.' . $key, $value); - } - foreach ($config['listener_priorities'] as $key => $priority) { $container->setParameter('sentry.listener_priorities.' . $key, $priority); } diff --git a/src/Event/SentryUserContextEvent.php b/src/Event/SentryUserContextEvent.php deleted file mode 100644 index 5369908b..00000000 --- a/src/Event/SentryUserContextEvent.php +++ /dev/null @@ -1,22 +0,0 @@ -authenticationToken = $authenticationToken; - } - - public function getAuthenticationToken(): TokenInterface - { - return $this->authenticationToken; - } -} diff --git a/src/EventListener/ExceptionListener.php b/src/EventListener/ExceptionListener.php index aaf69586..fa32b357 100644 --- a/src/EventListener/ExceptionListener.php +++ b/src/EventListener/ExceptionListener.php @@ -2,17 +2,11 @@ namespace Sentry\SentryBundle\EventListener; -use Sentry\SentryBundle\Event\SentryUserContextEvent; -use Sentry\SentryBundle\SentrySymfonyEvents; +use Sentry\State\Hub; +use Sentry\State\HubInterface; 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\HttpFoundation\Request; -use Symfony\Component\HttpFoundation\RequestStack; +use Symfony\Component\HttpKernel\Event\FilterControllerEvent; use Symfony\Component\HttpKernel\Event\GetResponseEvent; -use Symfony\Component\HttpKernel\Event\GetResponseForExceptionEvent; use Symfony\Component\HttpKernel\HttpKernelInterface; use Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorageInterface; use Symfony\Component\Security\Core\Authorization\AuthorizationCheckerInterface; @@ -23,58 +17,33 @@ * Class ExceptionListener * @package Sentry\SentryBundle\EventListener */ -class ExceptionListener implements SentryExceptionListenerInterface +final class ExceptionListener { + /** @var HubInterface */ + private $hub; + /** @var TokenStorageInterface|null */ - protected $tokenStorage; + private $tokenStorage; /** @var AuthorizationCheckerInterface|null */ - protected $authorizationChecker; - - /** @var \Raven_Client */ - protected $client; - - /** @var EventDispatcherInterface */ - protected $eventDispatcher; - - /** @var RequestStack */ - protected $requestStack; - - /** @var string[] */ - protected $skipCapture; + private $authorizationChecker; /** * ExceptionListener constructor. - * @param \Raven_Client $client - * @param EventDispatcherInterface $dispatcher - * @param array $skipCapture + * @param HubInterface $hub * @param TokenStorageInterface|null $tokenStorage * @param AuthorizationCheckerInterface|null $authorizationChecker */ public function __construct( - \Raven_Client $client, - EventDispatcherInterface $dispatcher, - RequestStack $requestStack, - array $skipCapture, - TokenStorageInterface $tokenStorage = null, - AuthorizationCheckerInterface $authorizationChecker = null + HubInterface $hub, + ?TokenStorageInterface $tokenStorage, + ?AuthorizationCheckerInterface $authorizationChecker ) { - $this->client = $client; - $this->eventDispatcher = $dispatcher; - $this->requestStack = $requestStack; - $this->skipCapture = $skipCapture; + $this->hub = $hub; // not used, needed to trigger instantiation $this->tokenStorage = $tokenStorage; $this->authorizationChecker = $authorizationChecker; } - /** - * @param \Raven_Client $client - */ - public function setClient(\Raven_Client $client) - { - $this->client = $client; - } - /** * Set the username from the security context by listening on core.request * @@ -92,37 +61,36 @@ public function onKernelRequest(GetResponseEvent $event): void $token = $this->tokenStorage->getToken(); - if (null !== $token && $token->isAuthenticated() && $this->authorizationChecker->isGranted(AuthenticatedVoter::IS_AUTHENTICATED_REMEMBERED)) { - $this->setUserValue($token->getUser()); - - $contextEvent = new SentryUserContextEvent($token); - $this->eventDispatcher->dispatch(SentrySymfonyEvents::SET_USER_CONTEXT, $contextEvent); + if ( + null !== $token + && $token->isAuthenticated() + && $this->authorizationChecker->isGranted(AuthenticatedVoter::IS_AUTHENTICATED_REMEMBERED) + ) { + $userData = $this->getUserData($token->getUser()); } + + $userData['ip_address'] = $event->getRequest()->getClientIp(); + + Hub::getCurrent() + ->getScope() + ->setUser($userData); } - /** - * @param GetResponseForExceptionEvent $event - */ - public function onKernelException(GetResponseForExceptionEvent $event): void + public function onKernelController(FilterControllerEvent $event): void { - $exception = $event->getException(); - - if ($this->shouldExceptionCaptureBeSkipped($exception)) { + if (HttpKernelInterface::MASTER_REQUEST !== $event->getRequestType()) { return; } - $data = ['tags' => []]; - $request = $this->requestStack->getCurrentRequest(); - if ($request instanceof Request) { - $data['tags']['route'] = $request->attributes->get('_route'); - } + $matchedRoute = $event->getRequest()->attributes->get('_route'); - $this->eventDispatcher->dispatch(SentrySymfonyEvents::PRE_CAPTURE, $event); - $this->client->captureException($exception, $data); + Hub::getCurrent() + ->getScope() + ->setTag('route', $matchedRoute); } /** - * This method only ensures that the client and error handlers are registered at the start of the command + * This method ensures that the client and error handlers are registered at the start of the command * execution cycle, and not only on exceptions * * @param ConsoleCommandEvent $event @@ -130,89 +98,37 @@ public function onKernelException(GetResponseForExceptionEvent $event): void * @return void */ public function onConsoleCommand(ConsoleCommandEvent $event): void - { - // only triggers loading of client, does not need to do anything. - } - - public function onConsoleError(ConsoleErrorEvent $event): void - { - $this->handleConsoleError($event); - } - - public function onConsoleException(ConsoleExceptionEvent $event): void - { - $this->handleConsoleError($event); - } - - /** - * @param ConsoleExceptionEvent|ConsoleErrorEvent $event - */ - protected function handleConsoleError(ConsoleEvent $event): void { $command = $event->getCommand(); - 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; - } - - $data = [ - 'tags' => [ - 'command' => $command ? $command->getName() : 'N/A', - 'status_code' => $event->getExitCode(), - ], - ]; - - $this->eventDispatcher->dispatch(SentrySymfonyEvents::PRE_CAPTURE, $event); - $this->client->captureException($exception, $data); - } - - protected function shouldExceptionCaptureBeSkipped(\Throwable $exception): bool - { - foreach ($this->skipCapture as $className) { - if ($exception instanceof $className) { - return true; - } - } - - return false; + Hub::getCurrent() + ->getScope() + ->setTag('command', $command ? $command->getName() : 'N/A'); } /** * @param UserInterface | object | string $user */ - protected function setUserValue($user) + private function getUserData($user): array { - $data = []; - - $request = $this->requestStack->getCurrentRequest(); - if ($request instanceof Request) { - $data['ip_address'] = $request->getClientIp(); - } - if ($user instanceof UserInterface) { - $this->client->set_user_data($user->getUsername(), null, $data); - - return; + return [ + 'username' => $user->getUsername(), + ]; } if (is_string($user)) { - $this->client->set_user_data($user, null, $data); - - return; + return [ + 'username' => $user, + ]; } if (is_object($user) && method_exists($user, '__toString')) { - $this->client->set_user_data((string)$user, null, $data); + return [ + 'username' => $user->__toString(), + ]; } + + return []; } } diff --git a/src/EventListener/SentryExceptionListenerInterface.php b/src/EventListener/SentryExceptionListenerInterface.php deleted file mode 100644 index 9d972074..00000000 --- a/src/EventListener/SentryExceptionListenerInterface.php +++ /dev/null @@ -1,47 +0,0 @@ - + + + + diff --git a/src/SentrySymfonyClient.php b/src/SentrySymfonyClient.php deleted file mode 100644 index 3568a115..00000000 --- a/src/SentrySymfonyClient.php +++ /dev/null @@ -1,22 +0,0 @@ -parse(); - } - - $options['sdk'] = [ - 'name' => 'sentry-symfony', - 'version' => SentryBundle::getVersion(), - ]; - $options['tags']['symfony_version'] = \Symfony\Component\HttpKernel\Kernel::VERSION; - - parent::__construct($dsn, $options); - } -} diff --git a/src/SentrySymfonyEvents.php b/src/SentrySymfonyEvents.php deleted file mode 100644 index 6c71666d..00000000 --- a/src/SentrySymfonyEvents.php +++ /dev/null @@ -1,30 +0,0 @@ - Date: Mon, 21 Jan 2019 16:34:25 +0100 Subject: [PATCH 04/49] Split the listeners in two --- CHANGELOG.md | 6 +-- README.md | 12 +++--- src/EventListener/ConsoleListener.php | 39 +++++++++++++++++++ ...eptionListener.php => RequestListener.php} | 24 ++---------- src/Resources/config/services.xml | 16 +++++--- .../SentryExtensionTest.php | 10 ++--- test/EventListener/ExceptionListenerTest.php | 8 ++-- ...Listener.php => CustomRequestListener.php} | 4 +- test/Fixtures/InvalidExceptionListener.php | 2 +- 9 files changed, 74 insertions(+), 47 deletions(-) create mode 100644 src/EventListener/ConsoleListener.php rename src/EventListener/{ExceptionListener.php => RequestListener.php} (83%) rename test/Fixtures/{CustomExceptionListener.php => CustomRequestListener.php} (50%) diff --git a/CHANGELOG.md b/CHANGELOG.md index 455c5162..822c9504 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,7 +9,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. ### Added - Add the `route` tag automatically to any event generated by a request (#167, thanks @ruudk) ### Changed - - Make the `ExceptionListener` more extendable by making all members at least `protected` (#176, thanks @bouland) + - Make the `RequestListener` more extendable by making all members at least `protected` (#176, thanks @bouland) ## 2.1.0 - 2018-11-05 ### Added @@ -66,8 +66,8 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. - Add official support to PHP 7.2 (#71) ### Changed - Changed source folder from `src/Sentry/SentryBundle` to just `src/` (thanks to PSR-4 and Composer this doesn't affect you) - - Re-sort the constructor's arguments of `ExceptionListener` - - The `SentrySymfonyClient` is no longer an optional argument of `ExceptionListener`; it's now required + - Re-sort the constructor's arguments of `RequestListener` + - The `SentrySymfonyClient` is no longer an optional argument of `RequestListener`; it's now required ### Fixed - Remove usage of `create_function()` to avoid deprecations (#71) - Fix a possible bug that could make Sentry crash if an error is triggered before loading a console command diff --git a/README.md b/README.md index 62a68e3c..5d0fe965 100644 --- a/README.md +++ b/README.md @@ -144,7 +144,7 @@ sentry: #### exception_listener -This is used to replace the default exception listener that this bundle uses. The value must be a FQCN of a class implementing the SentryExceptionListenerInterface interface. See [Create a Custom ExceptionListener](#create-a-custom-exceptionlistener) for more details. +This is used to replace the default exception listener that this bundle uses. The value must be a FQCN of a class implementing the SentryExceptionListenerInterface interface. See [Create a Custom RequestListener](#create-a-custom-exceptionlistener) for more details. ```yaml sentry: @@ -200,18 +200,18 @@ sentry: ## Customization -It is possible to customize the configuration of the user context, as well as modify the client immediately before an exception is captured by wiring up an event subscriber to the events that are emitted by the default configured `ExceptionListener` (alternatively, you can also just define your own custom exception listener). +It is possible to customize the configuration of the user context, as well as modify the client immediately before an exception is captured by wiring up an event subscriber to the events that are emitted by the default configured `RequestListener` (alternatively, you can also just define your own custom exception listener). -### Create a custom ExceptionListener +### Create a custom RequestListener -You can always replace the default `ExceptionListener` with your own custom listener. To do this, assign a different class to the `exception_listener` property in your Sentry configuration, e.g.: +You can always replace the default `RequestListener` with your own custom listener. To do this, assign a different class to the `exception_listener` property in your Sentry configuration, e.g.: ```yaml sentry: exception_listener: AppBundle\EventListener\MySentryExceptionListener ``` -... and then define the custom `ExceptionListener` that implements the `SentryExceptionListenerInterface`, e.g.: +... and then define the custom `RequestListener` that implements the `SentryExceptionListenerInterface`, e.g.: ```php // src/AppBundle/EventSubscriber/MySentryEventListener.php @@ -253,7 +253,7 @@ class MySentryExceptionListener implements SentryExceptionListenerInterface As a side note, while the above demonstrates a custom exception listener that does not extend anything you could choose to extend the default -`ExceptionListener` and only override the functionality that you want to. +`RequestListener` and only override the functionality that you want to. ### Add an EventSubscriber for Sentry Events diff --git a/src/EventListener/ConsoleListener.php b/src/EventListener/ConsoleListener.php new file mode 100644 index 00000000..621be56d --- /dev/null +++ b/src/EventListener/ConsoleListener.php @@ -0,0 +1,39 @@ +hub = $hub; // not used, needed to trigger instantiation + } + + /** + * This method ensures that the client and error handlers are registered at the start of the command + * execution cycle, and not only on exceptions + * + * @param ConsoleCommandEvent $event + * + * @return void + */ + public function onConsoleCommand(ConsoleCommandEvent $event): void + { + $command = $event->getCommand(); + + Hub::getCurrent() + ->getScope() + ->setTag('command', $command ? $command->getName() : 'N/A'); + } +} diff --git a/src/EventListener/ExceptionListener.php b/src/EventListener/RequestListener.php similarity index 83% rename from src/EventListener/ExceptionListener.php rename to src/EventListener/RequestListener.php index fa32b357..b0cea15e 100644 --- a/src/EventListener/ExceptionListener.php +++ b/src/EventListener/RequestListener.php @@ -4,7 +4,6 @@ use Sentry\State\Hub; use Sentry\State\HubInterface; -use Symfony\Component\Console\Event\ConsoleCommandEvent; use Symfony\Component\HttpKernel\Event\FilterControllerEvent; use Symfony\Component\HttpKernel\Event\GetResponseEvent; use Symfony\Component\HttpKernel\HttpKernelInterface; @@ -14,10 +13,10 @@ use Symfony\Component\Security\Core\User\UserInterface; /** - * Class ExceptionListener + * Class RequestListener * @package Sentry\SentryBundle\EventListener */ -final class ExceptionListener +final class RequestListener { /** @var HubInterface */ private $hub; @@ -29,7 +28,7 @@ final class ExceptionListener private $authorizationChecker; /** - * ExceptionListener constructor. + * RequestListener constructor. * @param HubInterface $hub * @param TokenStorageInterface|null $tokenStorage * @param AuthorizationCheckerInterface|null $authorizationChecker @@ -89,23 +88,6 @@ public function onKernelController(FilterControllerEvent $event): void ->setTag('route', $matchedRoute); } - /** - * This method ensures that the client and error handlers are registered at the start of the command - * execution cycle, and not only on exceptions - * - * @param ConsoleCommandEvent $event - * - * @return void - */ - public function onConsoleCommand(ConsoleCommandEvent $event): void - { - $command = $event->getCommand(); - - Hub::getCurrent() - ->getScope() - ->setTag('command', $command ? $command->getName() : 'N/A'); - } - /** * @param UserInterface | object | string $user */ diff --git a/src/Resources/config/services.xml b/src/Resources/config/services.xml index 3d462d19..4126749a 100644 --- a/src/Resources/config/services.xml +++ b/src/Resources/config/services.xml @@ -9,24 +9,30 @@ - + - + - + - + + - + + + + + + diff --git a/test/DependencyInjection/SentryExtensionTest.php b/test/DependencyInjection/SentryExtensionTest.php index fccd1495..828bd3c1 100644 --- a/test/DependencyInjection/SentryExtensionTest.php +++ b/test/DependencyInjection/SentryExtensionTest.php @@ -4,9 +4,9 @@ use PHPUnit\Framework\TestCase; use Sentry\SentryBundle\DependencyInjection\SentryExtension; -use Sentry\SentryBundle\EventListener\ExceptionListener; +use Sentry\SentryBundle\EventListener\RequestListener; use Sentry\SentryBundle\SentrySymfonyClient; -use Sentry\SentryBundle\Test\Fixtures\CustomExceptionListener; +use Sentry\SentryBundle\Test\Fixtures\CustomRequestListener; use Symfony\Component\Config\Definition\Exception\InvalidConfigurationException; use Symfony\Component\DependencyInjection\Alias; use Symfony\Component\DependencyInjection\Container; @@ -27,7 +27,7 @@ public function test_that_configuration_uses_the_right_default_values() $this->assertSame('kernel/root/..', $container->getParameter('sentry.app_path')); $this->assertSame(SentrySymfonyClient::class, $container->getParameter('sentry.client')); $this->assertNull($container->getParameter('sentry.dsn')); - $this->assertSame(ExceptionListener::class, $container->getParameter('sentry.exception_listener')); + $this->assertSame(RequestListener::class, $container->getParameter('sentry.exception_listener')); $this->assertSame([HttpExceptionInterface::class], $container->getParameter('sentry.skip_capture')); $priorities = $container->getParameter('sentry.listener_priorities'); @@ -189,7 +189,7 @@ public function test_that_it_is_invalid_if_exception_listener_fails_to_implement public function test_that_it_uses_exception_listener_value() { - $class = CustomExceptionListener::class; + $class = CustomRequestListener::class; $container = $this->getContainer( [ 'exception_listener' => $class, @@ -263,7 +263,7 @@ public function test_that_it_has_sentry_client_service_and_it_defaults_to_symfon public function test_that_it_has_sentry_exception_listener_and_it_defaults_to_default_exception_listener() { $listener = $this->getContainer()->get(self::LISTENER_TEST_PUBLIC_ALIAS); - $this->assertInstanceOf(ExceptionListener::class, $listener); + $this->assertInstanceOf(RequestListener::class, $listener); } public function test_that_it_has_proper_event_listener_tags_for_exception_listener() diff --git a/test/EventListener/ExceptionListenerTest.php b/test/EventListener/ExceptionListenerTest.php index 933f7bc0..4bf2ae65 100644 --- a/test/EventListener/ExceptionListenerTest.php +++ b/test/EventListener/ExceptionListenerTest.php @@ -5,7 +5,7 @@ use PHPUnit\Framework\TestCase; use Sentry\SentryBundle\DependencyInjection\SentryExtension; use Sentry\SentryBundle\Event\SentryUserContextEvent; -use Sentry\SentryBundle\EventListener\ExceptionListener; +use Sentry\SentryBundle\EventListener\RequestListener; use Sentry\SentryBundle\EventListener\SentryExceptionListenerInterface; use Sentry\SentryBundle\SentrySymfonyEvents; use Symfony\Component\Console\Command\Command; @@ -77,7 +77,7 @@ public function test_that_it_is_an_instance_of_sentry_exception_listener() $this->containerBuilder->compile(); $listener = $this->getListener(); - $this->assertInstanceOf(ExceptionListener::class, $listener); + $this->assertInstanceOf(RequestListener::class, $listener); } public function test_that_user_data_is_not_set_on_subrequest() @@ -666,9 +666,9 @@ public function test_that_it_can_replace_client() ->with($this->identicalTo($reportableException)); $this->containerBuilder->compile(); - /** @var ExceptionListener $listener */ + /** @var RequestListener $listener */ $listener = $this->getListener(); - $this->assertInstanceOf(ExceptionListener::class, $listener); + $this->assertInstanceOf(RequestListener::class, $listener); $listener->setClient($replacementClient); diff --git a/test/Fixtures/CustomExceptionListener.php b/test/Fixtures/CustomRequestListener.php similarity index 50% rename from test/Fixtures/CustomExceptionListener.php rename to test/Fixtures/CustomRequestListener.php index 31f285e3..908e6121 100644 --- a/test/Fixtures/CustomExceptionListener.php +++ b/test/Fixtures/CustomRequestListener.php @@ -2,11 +2,11 @@ namespace Sentry\SentryBundle\Test\Fixtures; -use Sentry\SentryBundle\EventListener\ExceptionListener; +use Sentry\SentryBundle\EventListener\RequestListener; /** * @package Sentry\SentryBundle\Tests\Fixtures */ -class CustomExceptionListener extends ExceptionListener +class CustomRequestListener extends RequestListener { } diff --git a/test/Fixtures/InvalidExceptionListener.php b/test/Fixtures/InvalidExceptionListener.php index 864ce417..ae023e57 100644 --- a/test/Fixtures/InvalidExceptionListener.php +++ b/test/Fixtures/InvalidExceptionListener.php @@ -3,7 +3,7 @@ namespace Sentry\SentryBundle\Test\Fixtures; /** - * An invalid ExceptionListener for testing. + * An invalid RequestListener for testing. * * This exception listener does not implement, or extend a class that * implements, the appropriate interface that the dependency injection From a6c8a9a91ee0cb293c62ce42607d7912a5912ac5 Mon Sep 17 00:00:00 2001 From: Alessandro Lai Date: Tue, 22 Jan 2019 09:25:06 +0100 Subject: [PATCH 05/49] First working version --- src/DependencyInjection/Configuration.php | 9 ++++++++ src/DependencyInjection/SentryExtension.php | 4 ++++ src/Resources/config/services.xml | 23 ++++++++++++--------- 3 files changed, 26 insertions(+), 10 deletions(-) diff --git a/src/DependencyInjection/Configuration.php b/src/DependencyInjection/Configuration.php index 0d682599..a28ed3a3 100644 --- a/src/DependencyInjection/Configuration.php +++ b/src/DependencyInjection/Configuration.php @@ -35,6 +35,15 @@ public function getConfigTreeBuilder() ->defaultNull() ->end(); + $rootNode->children() + ->arrayNode('listener_priorities') + ->addDefaultsIfNotSet() + ->children() + ->scalarNode('request')->defaultValue(1)->end() + ->scalarNode('console')->defaultValue(1)->end() + ->end() + ->end(); + return $treeBuilder; } diff --git a/src/DependencyInjection/SentryExtension.php b/src/DependencyInjection/SentryExtension.php index 4be87abb..ef358686 100644 --- a/src/DependencyInjection/SentryExtension.php +++ b/src/DependencyInjection/SentryExtension.php @@ -2,6 +2,7 @@ namespace Sentry\SentryBundle\DependencyInjection; +use Sentry\Options; use Symfony\Component\Config\Definition\Exception\InvalidConfigurationException; use Symfony\Component\Config\FileLocator; use Symfony\Component\DependencyInjection\ContainerBuilder; @@ -27,6 +28,9 @@ public function load(array $configs, ContainerBuilder $container) $loader = new Loader\XmlFileLoader($container, new FileLocator(__DIR__ . '/../Resources/config')); $loader->load('services.xml'); + $container->getDefinition(Options::class) + ->addArgument(['dsn' => $config['dsn']]); + foreach ($config['listener_priorities'] as $key => $priority) { $container->setParameter('sentry.listener_priorities.' . $key, $priority); } diff --git a/src/Resources/config/services.xml b/src/Resources/config/services.xml index 4126749a..bf62e007 100644 --- a/src/Resources/config/services.xml +++ b/src/Resources/config/services.xml @@ -5,9 +5,7 @@ xsi:schemaLocation="http://symfony.com/schema/dic/services http://symfony.com/schema/dic/services/services-1.0.xsd"> - - - + @@ -18,21 +16,26 @@ - + + + + - - + + - + - - + + + + - + From 2bf512e405ba633f45fe3209ab8878a47fafd0d3 Mon Sep 17 00:00:00 2001 From: Alessandro Lai Date: Tue, 22 Jan 2019 15:13:16 +0100 Subject: [PATCH 06/49] Fix CS; delete unextendable classes from fixtures --- src/EventListener/RequestListener.php | 4 ++-- test/Fixtures/CustomRequestListener.php | 12 ------------ test/Fixtures/InvalidExceptionListener.php | 16 ---------------- 3 files changed, 2 insertions(+), 30 deletions(-) delete mode 100644 test/Fixtures/CustomRequestListener.php delete mode 100644 test/Fixtures/InvalidExceptionListener.php diff --git a/src/EventListener/RequestListener.php b/src/EventListener/RequestListener.php index b0cea15e..527d9461 100644 --- a/src/EventListener/RequestListener.php +++ b/src/EventListener/RequestListener.php @@ -61,8 +61,8 @@ public function onKernelRequest(GetResponseEvent $event): void $token = $this->tokenStorage->getToken(); if ( - null !== $token - && $token->isAuthenticated() + null !== $token + && $token->isAuthenticated() && $this->authorizationChecker->isGranted(AuthenticatedVoter::IS_AUTHENTICATED_REMEMBERED) ) { $userData = $this->getUserData($token->getUser()); diff --git a/test/Fixtures/CustomRequestListener.php b/test/Fixtures/CustomRequestListener.php deleted file mode 100644 index 908e6121..00000000 --- a/test/Fixtures/CustomRequestListener.php +++ /dev/null @@ -1,12 +0,0 @@ -RequestListener for testing. - * - * This exception listener does not implement, or extend a class that - * implements, the appropriate interface that the dependency injection - * container requires. - * - * @package Sentry\SentryBundle\Tests\Fixtures - */ -class InvalidExceptionListener -{ -} From 495353ea47104c120468a988b4a4f51517f952fc Mon Sep 17 00:00:00 2001 From: Alessandro Lai Date: Tue, 22 Jan 2019 15:25:23 +0100 Subject: [PATCH 07/49] Add SDK identifier and version --- src/DependencyInjection/SentryExtension.php | 6 ++++++ src/SentryBundle.php | 4 +++- test/SentrySymfonyClientTest.php | 2 +- 3 files changed, 10 insertions(+), 2 deletions(-) diff --git a/src/DependencyInjection/SentryExtension.php b/src/DependencyInjection/SentryExtension.php index ef358686..8ca7cfbd 100644 --- a/src/DependencyInjection/SentryExtension.php +++ b/src/DependencyInjection/SentryExtension.php @@ -2,7 +2,9 @@ namespace Sentry\SentryBundle\DependencyInjection; +use Sentry\ClientBuilderInterface; use Sentry\Options; +use Sentry\SentryBundle\SentryBundle; use Symfony\Component\Config\Definition\Exception\InvalidConfigurationException; use Symfony\Component\Config\FileLocator; use Symfony\Component\DependencyInjection\ContainerBuilder; @@ -31,6 +33,10 @@ public function load(array $configs, ContainerBuilder $container) $container->getDefinition(Options::class) ->addArgument(['dsn' => $config['dsn']]); + $container->getDefinition(ClientBuilderInterface::class) + ->addMethodCall('setSdkIdentifier', [SentryBundle::SDK_IDENTIFIER]) + ->addMethodCall('setSdkVersion', [SentryBundle::getSdkVersion()]); + foreach ($config['listener_priorities'] as $key => $priority) { $container->setParameter('sentry.listener_priorities.' . $key, $priority); } diff --git a/src/SentryBundle.php b/src/SentryBundle.php index fa563a96..acf470c9 100644 --- a/src/SentryBundle.php +++ b/src/SentryBundle.php @@ -7,7 +7,9 @@ class SentryBundle extends Bundle { - public static function getVersion(): string + public const SDK_IDENTIFIER = 'sentry.php.symfony'; + + public static function getSdkVersion(): string { return PrettyVersions::getVersion('sentry/sentry-symfony') ->getPrettyVersion(); diff --git a/test/SentrySymfonyClientTest.php b/test/SentrySymfonyClientTest.php index d7679885..f073122e 100644 --- a/test/SentrySymfonyClientTest.php +++ b/test/SentrySymfonyClientTest.php @@ -15,7 +15,7 @@ public function test_that_it_sets_sdk_name_and_version() $data = $client->get_default_data(); $this->assertEquals('sentry-symfony', $data['sdk']['name']); - $this->assertEquals(SentryBundle::getVersion(), $data['sdk']['version']); + $this->assertEquals(SentryBundle::getSdkVersion(), $data['sdk']['version']); } public function test_that_it_forwards_options() From 285defe653627fe3f7d0e02b5da3fe07919caccc Mon Sep 17 00:00:00 2001 From: Alessandro Lai Date: Tue, 22 Jan 2019 16:44:27 +0100 Subject: [PATCH 08/49] WIP: add first options to config --- src/DependencyInjection/Configuration.php | 55 +++++++++++++++++++---- 1 file changed, 46 insertions(+), 9 deletions(-) diff --git a/src/DependencyInjection/Configuration.php b/src/DependencyInjection/Configuration.php index a28ed3a3..3e9282ab 100644 --- a/src/DependencyInjection/Configuration.php +++ b/src/DependencyInjection/Configuration.php @@ -5,6 +5,7 @@ use Symfony\Component\Config\Definition\Builder\ArrayNodeDefinition; use Symfony\Component\Config\Definition\Builder\TreeBuilder; use Symfony\Component\Config\Definition\ConfigurationInterface; +use Symfony\Component\HttpKernel\Exception\HttpExceptionInterface; /** * This is the class that validates and merges configuration from your app/config files @@ -25,15 +26,51 @@ public function getConfigTreeBuilder() $rootNode = \method_exists(TreeBuilder::class, 'getRootNode') ? $treeBuilder->getRootNode() : $treeBuilder->root('sentry'); // Basic Sentry configuration - $rootNode - ->children() - ->scalarNode('dsn') - ->beforeNormalization() - ->ifString() - ->then($this->getTrimClosure()) - ->end() - ->defaultNull() - ->end(); + $rootNode->children() + ->scalarNode('dsn') + ->beforeNormalization() + ->ifString() + ->then($this->getTrimClosure()) + ->end() + ->defaultNull() + ->end(); + + // Options array (to be passed to Sentry\Options constructor) -- please keep alphabetical order! + $optionsNode = $rootNode->children()->arrayNode('options'); + $optionsNode->addDefaultsIfNotSet(); + + $optionsNode->children() + ->booleanNode('default_integrations')->end() + // TODO -- integrations + ->arrayNode('prefixes') + ->scalarPrototype()->end() + ->end() + ->scalarNode('project_root') + ->defaultValue('%kernel.project_root%') + ->end() + ->floatNode('sample_rate') + ->validate() + ->min(0.0) + ->max(1.0) + ->end() + ->end() + ->integerNode('send_attempts') + ->validate() + ->min(1) + ->end() + ->end() + ->booleanNode('serialize_all_object')->end() + + ; + + // Bundle-specific configuration + $rootNode->children() + ->arrayNode('excluded_exceptions') + ->addDefaultsIfNotSet() + ->prototype('scalar')->end() + ->defaultValue([HttpExceptionInterface::class]) + ->end() + ->end(); $rootNode->children() ->arrayNode('listener_priorities') From 783a1abafb8fce9ac1b1feaa7313f8e548abe219 Mon Sep 17 00:00:00 2001 From: Alessandro Lai Date: Tue, 22 Jan 2019 23:44:18 +0100 Subject: [PATCH 09/49] Add, fix and improve DI tests --- src/DependencyInjection/Configuration.php | 22 +- src/DependencyInjection/SentryExtension.php | 4 +- .../DependencyInjection/ConfigurationTest.php | 78 ++++ .../SentryExtensionTest.php | 333 +++--------------- 4 files changed, 141 insertions(+), 296 deletions(-) create mode 100644 test/DependencyInjection/ConfigurationTest.php diff --git a/src/DependencyInjection/Configuration.php b/src/DependencyInjection/Configuration.php index 3e9282ab..1bfd0e3b 100644 --- a/src/DependencyInjection/Configuration.php +++ b/src/DependencyInjection/Configuration.php @@ -5,7 +5,6 @@ use Symfony\Component\Config\Definition\Builder\ArrayNodeDefinition; use Symfony\Component\Config\Definition\Builder\TreeBuilder; use Symfony\Component\Config\Definition\ConfigurationInterface; -use Symfony\Component\HttpKernel\Exception\HttpExceptionInterface; /** * This is the class that validates and merges configuration from your app/config files @@ -36,28 +35,26 @@ public function getConfigTreeBuilder() ->end(); // Options array (to be passed to Sentry\Options constructor) -- please keep alphabetical order! - $optionsNode = $rootNode->children()->arrayNode('options'); + $optionsNode = $rootNode->children() + ->arrayNode('options'); $optionsNode->addDefaultsIfNotSet(); - $optionsNode->children() + $optionsNode + ->children() ->booleanNode('default_integrations')->end() // TODO -- integrations ->arrayNode('prefixes') ->scalarPrototype()->end() ->end() ->scalarNode('project_root') - ->defaultValue('%kernel.project_root%') + ->defaultValue('%kernel.project_dir%') ->end() ->floatNode('sample_rate') - ->validate() - ->min(0.0) - ->max(1.0) - ->end() + ->min(0.0) + ->max(1.0) ->end() ->integerNode('send_attempts') - ->validate() - ->min(1) - ->end() + ->min(1) ->end() ->booleanNode('serialize_all_object')->end() @@ -66,10 +63,7 @@ public function getConfigTreeBuilder() // Bundle-specific configuration $rootNode->children() ->arrayNode('excluded_exceptions') - ->addDefaultsIfNotSet() ->prototype('scalar')->end() - ->defaultValue([HttpExceptionInterface::class]) - ->end() ->end(); $rootNode->children() diff --git a/src/DependencyInjection/SentryExtension.php b/src/DependencyInjection/SentryExtension.php index 8ca7cfbd..cbdb7efd 100644 --- a/src/DependencyInjection/SentryExtension.php +++ b/src/DependencyInjection/SentryExtension.php @@ -30,8 +30,10 @@ public function load(array $configs, ContainerBuilder $container) $loader = new Loader\XmlFileLoader($container, new FileLocator(__DIR__ . '/../Resources/config')); $loader->load('services.xml'); + $arrayOptions = $config['options']; + $arrayOptions['dsn'] = $config['dsn']; $container->getDefinition(Options::class) - ->addArgument(['dsn' => $config['dsn']]); + ->addArgument($arrayOptions); $container->getDefinition(ClientBuilderInterface::class) ->addMethodCall('setSdkIdentifier', [SentryBundle::SDK_IDENTIFIER]) diff --git a/test/DependencyInjection/ConfigurationTest.php b/test/DependencyInjection/ConfigurationTest.php new file mode 100644 index 00000000..488f3b90 --- /dev/null +++ b/test/DependencyInjection/ConfigurationTest.php @@ -0,0 +1,78 @@ +getContainer(['options' => [$option => $value]]); + + $this->addToAssertionCount(1); + } + + public function optionValuesProvider(): array + { + return [ + ['default_integrations', true], + ['default_integrations', false], + ['prefixes', ['some-string']], + ['project_root', '/some/dir'], + ['sample_rate', 0], + ['sample_rate', 1], + ['send_attempts', 1], + ['send_attempts', 999], + ['serialize_all_object', true], + ['serialize_all_object', false], + ]; + } + + /** + * @dataProvider invalidValuesProvider + */ + public function testInvalidValues(string $option, $value): void + { + $this->expectException(InvalidConfigurationException::class); + + $this->getContainer(['options' => [$option => $value]]); + } + + public function invalidValuesProvider(): array + { + return [ + ['default_integrations', 'true'], + ['default_integrations', 1], + ['prefixes', 'string'], + ['project_root', []], + ['sample_rate', 1.1], + ['sample_rate', -1], + ['send_attempts', 1.5], + ['send_attempts', 0], + ['send_attempts', -1], + ['serialize_all_object', 'true'], + ]; + } + + private function getContainer(array $configuration = []): Container + { + $containerBuilder = new ContainerBuilder(); + $containerBuilder->setParameter('kernel.project_dir', '/dir/project/root'); + $containerBuilder->setParameter('kernel.environment', 'test'); + + $extension = new SentryExtension(); + $extension->load(['sentry' => $configuration], $containerBuilder); + + $containerBuilder->compile(); + + return $containerBuilder; + } +} diff --git a/test/DependencyInjection/SentryExtensionTest.php b/test/DependencyInjection/SentryExtensionTest.php index 828bd3c1..6cc812ce 100644 --- a/test/DependencyInjection/SentryExtensionTest.php +++ b/test/DependencyInjection/SentryExtensionTest.php @@ -3,121 +3,71 @@ namespace Sentry\SentryBundle\Test\DependencyInjection; use PHPUnit\Framework\TestCase; +use Sentry\Options; use Sentry\SentryBundle\DependencyInjection\SentryExtension; +use Sentry\SentryBundle\EventListener\ConsoleListener; use Sentry\SentryBundle\EventListener\RequestListener; -use Sentry\SentryBundle\SentrySymfonyClient; -use Sentry\SentryBundle\Test\Fixtures\CustomRequestListener; -use Symfony\Component\Config\Definition\Exception\InvalidConfigurationException; use Symfony\Component\DependencyInjection\Alias; use Symfony\Component\DependencyInjection\Container; use Symfony\Component\DependencyInjection\ContainerBuilder; use Symfony\Component\EventDispatcher\EventDispatcherInterface; use Symfony\Component\HttpFoundation\RequestStack; -use Symfony\Component\HttpKernel\Exception\HttpExceptionInterface; +use Symfony\Component\HttpKernel\Kernel; class SentryExtensionTest extends TestCase { private const SUPPORTED_SENTRY_OPTIONS_COUNT = 35; - private const LISTENER_TEST_PUBLIC_ALIAS = 'sentry.exception_listener.public_alias'; + private const REQUEST_LISTENER_TEST_PUBLIC_ALIAS = 'sentry.request_listener.public_alias'; + private const CONSOLE_LISTENER_TEST_PUBLIC_ALIAS = 'sentry.console_listener.public_alias'; + private const OPTIONS_TEST_PUBLIC_ALIAS = 'sentry.options.public_alias'; - public function test_that_configuration_uses_the_right_default_values() + public function testOptionsDefaultValues(): void { $container = $this->getContainer(); + $options = $this->getOptionsFrom($container); - $this->assertSame('kernel/root/..', $container->getParameter('sentry.app_path')); - $this->assertSame(SentrySymfonyClient::class, $container->getParameter('sentry.client')); - $this->assertNull($container->getParameter('sentry.dsn')); - $this->assertSame(RequestListener::class, $container->getParameter('sentry.exception_listener')); - $this->assertSame([HttpExceptionInterface::class], $container->getParameter('sentry.skip_capture')); - - $priorities = $container->getParameter('sentry.listener_priorities'); - $this->assertInternalType('array', $priorities); - $this->assertSame(0, $priorities['request']); - $this->assertSame(0, $priorities['kernel_exception']); - $this->assertSame(0, $priorities['console_exception']); + if (method_exists(Kernel::class, 'getProjectDir')) { + $this->assertSame('/dir/project/root/', $options->getProjectRoot()); + } else { + $this->assertSame('kernel/root/..', $options->getProjectRoot()); + } + $this->assertNull($options->getDsn()); - $options = $container->getParameter('sentry.options'); - $this->assertCount(self::SUPPORTED_SENTRY_OPTIONS_COUNT, $options); - // same order as in Configuration class - $this->assertSame('php', $options['logger']); - $this->assertNull($options['server']); - $this->assertNull($options['secret_key']); - $this->assertNull($options['public_key']); - $this->assertSame(1, $options['project']); - $this->assertFalse($options['auto_log_stacks']); - $this->assertSame(\Raven_Compat::gethostname(), $options['name']); - $this->assertNull($options['site']); - $this->assertSame([], $options['tags']); - $this->assertNull($options['release']); - $this->assertSame('test', $options['environment']); - $this->assertSame(1, $options['sample_rate']); - $this->assertTrue($options['trace']); - $this->assertSame(\Raven_Client::MESSAGE_LIMIT, $options['message_limit']); - $this->assertSame([], $options['exclude']); - $this->assertNull($options['http_proxy']); - $this->assertSame([], $options['extra']); - $this->assertSame('sync', $options['curl_method']); - $this->assertSame('curl', $options['curl_path']); - $this->assertTrue($options['curl_ipv4']); - $this->assertNull($options['ca_cert']); - $this->assertTrue($options['verify_ssl']); - $this->assertNull($options['curl_ssl_version']); - $this->assertFalse($options['trust_x_forwarded_proto']); - $this->assertNull($options['mb_detect_order']); - $this->assertNull($options['error_types']); - $this->assertSame('kernel/root/..', $options['app_path']); - $this->assertContains('kernel/root/../vendor', $options['excluded_app_paths']); - $this->assertContains('kernel/root/../app/cache', $options['excluded_app_paths']); - $this->assertContains('kernel/root/../var/cache', $options['excluded_app_paths']); - $this->assertSame(['kernel/root/..'], $options['prefixes']); - $this->assertTrue($options['install_default_breadcrumb_handlers']); - $this->assertTrue($options['install_shutdown_handler']); - $this->assertSame([\Raven_Processor_SanitizeDataProcessor::class], $options['processors']); - $this->assertSame([], $options['processorOptions']); + $this->assertSame(1, $container->getParameter('sentry.listener_priorities.request')); + $this->assertSame(1, $container->getParameter('sentry.listener_priorities.console')); } - public function test_that_it_uses_app_path_value() + /** + * @dataProvider optionsValueProvider + */ + public function testValuesArePassedToOptions(string $name, $value, string $getter): void { + $this->assertTrue(method_exists(Options::class, $getter), 'Bad data provider: wrong getter'); + + $defaultContainer = $this->getContainer(); $container = $this->getContainer( [ - 'options' => ['app_path' => 'sentry/app/path'], + 'options' => [$name => $value], ] ); - $options = $container->getParameter('sentry.options'); - $this->assertSame( - 'sentry/app/path', - $options['app_path'] - ); - } - - public function test_that_it_uses_client_value() - { - $container = $this->getContainer( - [ - 'client' => 'clientClass', - ] + $this->assertNotEquals( + $this->getOptionsFrom($defaultContainer)->$getter(), + $this->getOptionsFrom($container)->$getter(), + 'Bad data provider: value is same as default' ); $this->assertSame( - 'clientClass', - $container->getParameter('sentry.client') + $value, + $this->getOptionsFrom($container)->$getter() ); } - public function test_that_it_uses_environment_value() + public function optionsValueProvider() { - $container = $this->getContainer( - [ - 'options' => ['environment' => 'custom_env'], - ] - ); - - $options = $container->getParameter('sentry.options'); - $this->assertSame( - 'custom_env', - $options['environment'] - ); + return [ + ['default_integrations', true, 'hasDefaultIntegrations'], + ]; } /** @@ -125,6 +75,7 @@ public function test_that_it_uses_environment_value() */ public function test_that_it_ignores_empty_dsn_value($emptyDsn) { + $this->markTestIncomplete(); $container = $this->getContainer( [ 'dsn' => $emptyDsn, @@ -144,130 +95,9 @@ public function emptyDsnValueProvider() ]; } - public function test_that_it_uses_dsn_value() - { - $container = $this->getContainer( - [ - 'dsn' => 'custom_dsn', - ] - ); - - $this->assertSame( - 'custom_dsn', - $container->getParameter('sentry.dsn') - ); - } - - public function test_that_it_uses_http_proxy_value() - { - $container = $this->getContainer( - [ - 'options' => [ - 'http_proxy' => 'http://user:password@host:port', - ], - ] - ); - - $options = $container->getParameter('sentry.options'); - - $this->assertSame( - 'http://user:password@host:port', - $options['http_proxy'] - ); - } - - public function test_that_it_is_invalid_if_exception_listener_fails_to_implement_required_interface() - { - $this->expectException(InvalidConfigurationException::class); - - $this->getContainer( - [ - 'exception_listener' => 'Some\Invalid\Class', - ] - ); - } - - public function test_that_it_uses_exception_listener_value() - { - $class = CustomRequestListener::class; - $container = $this->getContainer( - [ - 'exception_listener' => $class, - ] - ); - - $this->assertSame( - $class, - $container->getParameter('sentry.exception_listener') - ); - } - - public function test_that_it_uses_skipped_capture_value() - { - $container = $this->getContainer( - [ - 'skip_capture' => [ - 'classA', - 'classB', - ], - ] - ); - - $this->assertSame( - ['classA', 'classB'], - $container->getParameter('sentry.skip_capture') - ); - } - - public function test_that_it_uses_release_value() - { - $container = $this->getContainer( - [ - 'options' => ['release' => '1.0'], - ] - ); - - $options = $container->getParameter('sentry.options'); - $this->assertSame( - '1.0', - $options['release'] - ); - } - - public function test_that_it_uses_prefixes_value() - { - $container = $this->getContainer( - [ - 'options' => [ - 'prefixes' => [ - 'dirA', - 'dirB', - ], - ], - ] - ); - - $options = $container->getParameter('sentry.options'); - $this->assertSame( - ['dirA', 'dirB'], - $options['prefixes'] - ); - } - - public function test_that_it_has_sentry_client_service_and_it_defaults_to_symfony_client() - { - $client = $this->getContainer()->get('sentry.client'); - $this->assertInstanceOf(SentrySymfonyClient::class, $client); - } - - public function test_that_it_has_sentry_exception_listener_and_it_defaults_to_default_exception_listener() - { - $listener = $this->getContainer()->get(self::LISTENER_TEST_PUBLIC_ALIAS); - $this->assertInstanceOf(RequestListener::class, $listener); - } - public function test_that_it_has_proper_event_listener_tags_for_exception_listener() { + $this->markTestIncomplete(); $containerBuilder = new ContainerBuilder(); $extension = new SentryExtension(); $extension->load([], $containerBuilder); @@ -298,84 +128,13 @@ public function test_that_it_has_proper_event_listener_tags_for_exception_listen ); } - public function test_that_it_sets_all_sentry_options() - { - $options = [ - 'logger' => 'logger', - 'server' => 'server', - 'secret_key' => 'secret_key', - 'public_key' => 'public_key', - 'project' => 'project', - 'auto_log_stacks' => true, - 'name' => 'name', - 'site' => 'site', - 'tags' => [ - 'tag1' => 'tagname', - 'tag2' => 'tagename 2', - ], - 'release' => 'release', - 'environment' => 'environment', - 'sample_rate' => 0.9, - 'trace' => false, - 'timeout' => 1, - 'message_limit' => 512, - 'exclude' => [ - 'test1', - 'test2', - ], - 'excluded_exceptions' => [ - 'test3', - 'test4', - ], - 'http_proxy' => 'http_proxy', - 'extra' => [ - 'extra1' => 'extra1', - 'extra2' => 'extra2', - ], - 'curl_method' => 'curl_method', - 'curl_path' => 'curl_path', - 'curl_ipv4' => false, - 'ca_cert' => 'ca_cert', - 'verify_ssl' => false, - 'curl_ssl_version' => 'curl_ssl_version', - 'trust_x_forwarded_proto' => true, - 'mb_detect_order' => 'mb_detect_order', - 'error_types' => 'E_ALL & ~E_DEPRECATED & ~E_NOTICE', - 'app_path' => 'app_path', - 'excluded_app_paths' => ['excluded_app_path1', 'excluded_app_path2'], - 'prefixes' => ['prefix1', 'prefix2'], - 'install_default_breadcrumb_handlers' => false, - 'install_shutdown_handler' => false, - 'processors' => ['processor1', 'processor2'], - 'processorOptions' => [ - 'processor1' => [ - 'processorOption1' => 'asasdf', - ], - 'processor2' => [ - 'processorOption2' => 'asasdf', - ], - ], - ]; - - $this->assertCount(self::SUPPORTED_SENTRY_OPTIONS_COUNT, $options); - $defaultOptions = $this->getContainer()->getParameter('sentry.options'); - foreach ($options as $name => $value) { - $this->assertNotEquals( - $defaultOptions[$name], - $value, - 'Test precondition failed: using default value for ' . $name - ); - } - - $container = $this->getContainer(['options' => $options]); - - $this->assertSame($options, $container->getParameter('sentry.options')); - } - private function getContainer(array $configuration = []): Container { $containerBuilder = new ContainerBuilder(); $containerBuilder->setParameter('kernel.root_dir', 'kernel/root'); + if (method_exists(Kernel::class, 'getProjectDir')) { + $containerBuilder->setParameter('kernel.project_dir', '/dir/project/root'); + } $containerBuilder->setParameter('kernel.environment', 'test'); $mockEventDispatcher = $this @@ -386,7 +145,9 @@ private function getContainer(array $configuration = []): Container $containerBuilder->set('request_stack', $mockRequestStack); $containerBuilder->set('event_dispatcher', $mockEventDispatcher); - $containerBuilder->setAlias(self::LISTENER_TEST_PUBLIC_ALIAS, new Alias('sentry.exception_listener', true)); + $containerBuilder->setAlias(self::OPTIONS_TEST_PUBLIC_ALIAS, new Alias(Options::class, true)); + $containerBuilder->setAlias(self::REQUEST_LISTENER_TEST_PUBLIC_ALIAS, new Alias(RequestListener::class, true)); + $containerBuilder->setAlias(self::CONSOLE_LISTENER_TEST_PUBLIC_ALIAS, new Alias(ConsoleListener::class, true)); $extension = new SentryExtension(); $extension->load(['sentry' => $configuration], $containerBuilder); @@ -395,4 +156,14 @@ private function getContainer(array $configuration = []): Container return $containerBuilder; } + + private function getOptionsFrom(Container $container): Options + { + $this->assertTrue($container->has(self::OPTIONS_TEST_PUBLIC_ALIAS), 'Options (or public alias) missing from container!'); + + $options = $container->get(self::OPTIONS_TEST_PUBLIC_ALIAS); + $this->assertInstanceOf(Options::class, $options); + + return $options; + } } From 70fde7d9be25097f02dee615558e8f7c97d4e878 Mon Sep 17 00:00:00 2001 From: Alessandro Lai Date: Wed, 23 Jan 2019 10:05:52 +0100 Subject: [PATCH 10/49] Remove test; refactor extension --- src/DependencyInjection/SentryExtension.php | 15 ++++--- test/SentrySymfonyClientTest.php | 48 --------------------- 2 files changed, 9 insertions(+), 54 deletions(-) delete mode 100644 test/SentrySymfonyClientTest.php diff --git a/src/DependencyInjection/SentryExtension.php b/src/DependencyInjection/SentryExtension.php index cbdb7efd..1f633842 100644 --- a/src/DependencyInjection/SentryExtension.php +++ b/src/DependencyInjection/SentryExtension.php @@ -26,21 +26,24 @@ class SentryExtension extends Extension public function load(array $configs, ContainerBuilder $container) { $configuration = new Configuration(); - $config = $this->processConfiguration($configuration, $configs); + $processedConfiguration = $this->processConfiguration($configuration, $configs); $loader = new Loader\XmlFileLoader($container, new FileLocator(__DIR__ . '/../Resources/config')); $loader->load('services.xml'); - $arrayOptions = $config['options']; - $arrayOptions['dsn'] = $config['dsn']; - $container->getDefinition(Options::class) - ->addArgument($arrayOptions); + $this->passConfigurationToOptions($container, $processedConfiguration); $container->getDefinition(ClientBuilderInterface::class) ->addMethodCall('setSdkIdentifier', [SentryBundle::SDK_IDENTIFIER]) ->addMethodCall('setSdkVersion', [SentryBundle::getSdkVersion()]); - foreach ($config['listener_priorities'] as $key => $priority) { + foreach ($processedConfiguration['listener_priorities'] as $key => $priority) { $container->setParameter('sentry.listener_priorities.' . $key, $priority); } } + + private function passConfigurationToOptions(ContainerBuilder $container, array $processedConfiguration): void + { + $options = $container->getDefinition(Options::class); + $options->addArgument(['dsn' => $processedConfiguration['dsn']]); + } } diff --git a/test/SentrySymfonyClientTest.php b/test/SentrySymfonyClientTest.php deleted file mode 100644 index f073122e..00000000 --- a/test/SentrySymfonyClientTest.php +++ /dev/null @@ -1,48 +0,0 @@ -get_default_data(); - - $this->assertEquals('sentry-symfony', $data['sdk']['name']); - $this->assertEquals(SentryBundle::getSdkVersion(), $data['sdk']['version']); - } - - public function test_that_it_forwards_options() - { - $client = new SentrySymfonyClient('https://a:b@app.getsentry.com/project', [ - 'name' => 'test', - 'tags' => ['some_custom' => 'test'], - ]); - - $data = $client->get_default_data(); - - // Not a big fan of doing this kind of assertions, couples tests to external API... - // Perhaps, refactor is needed for this class? - $this->assertEquals('test', $data['server_name']); - $this->assertEquals(\Symfony\Component\HttpKernel\Kernel::VERSION, $data['tags']['symfony_version']); - $this->assertEquals('test', $data['tags']['some_custom']); - $this->assertEquals('https://app.getsentry.com/api/project/store/', $client->getServerEndpoint(null)); - $this->assertEquals('a', $client->public_key); - $this->assertEquals('b', $client->secret_key); - } - - public function test_that_it_processes_error_types_option() - { - $client = new SentrySymfonyClient('https://a:b@app.getsentry.com/project', [ - 'error_types' => 'E_ALL & ~E_WARNING', - ]); - - $this->assertAttributeSame(E_ALL & ~E_WARNING, 'error_types', $client); - } -} From 385676beaed3eaecadfde2709aa68993cee895bb Mon Sep 17 00:00:00 2001 From: Alessandro Lai Date: Wed, 23 Jan 2019 10:57:20 +0100 Subject: [PATCH 11/49] Improve test of Configuration --- src/DependencyInjection/Configuration.php | 17 +++--- .../DependencyInjection/ConfigurationTest.php | 55 +++++++++++++------ 2 files changed, 48 insertions(+), 24 deletions(-) diff --git a/src/DependencyInjection/Configuration.php b/src/DependencyInjection/Configuration.php index 1bfd0e3b..3c89868c 100644 --- a/src/DependencyInjection/Configuration.php +++ b/src/DependencyInjection/Configuration.php @@ -2,6 +2,7 @@ namespace Sentry\SentryBundle\DependencyInjection; +use Sentry\Options; use Symfony\Component\Config\Definition\Builder\ArrayNodeDefinition; use Symfony\Component\Config\Definition\Builder\TreeBuilder; use Symfony\Component\Config\Definition\ConfigurationInterface; @@ -36,14 +37,21 @@ public function getConfigTreeBuilder() // Options array (to be passed to Sentry\Options constructor) -- please keep alphabetical order! $optionsNode = $rootNode->children() - ->arrayNode('options'); - $optionsNode->addDefaultsIfNotSet(); + ->arrayNode('options') + ->addDefaultsIfNotSet(); + + $defaultValues = new Options(); $optionsNode ->children() ->booleanNode('default_integrations')->end() + ->arrayNode('excluded_exceptions') + ->defaultValue($defaultValues->getExcludedExceptions()) + ->scalarPrototype()->end() + ->end() // TODO -- integrations ->arrayNode('prefixes') + ->defaultValue($defaultValues->getPrefixes()) ->scalarPrototype()->end() ->end() ->scalarNode('project_root') @@ -61,11 +69,6 @@ public function getConfigTreeBuilder() ; // Bundle-specific configuration - $rootNode->children() - ->arrayNode('excluded_exceptions') - ->prototype('scalar')->end() - ->end(); - $rootNode->children() ->arrayNode('listener_priorities') ->addDefaultsIfNotSet() diff --git a/test/DependencyInjection/ConfigurationTest.php b/test/DependencyInjection/ConfigurationTest.php index 488f3b90..c9895c6a 100644 --- a/test/DependencyInjection/ConfigurationTest.php +++ b/test/DependencyInjection/ConfigurationTest.php @@ -3,21 +3,47 @@ namespace Sentry\SentryBundle\Test\DependencyInjection; use PHPUnit\Framework\TestCase; -use Sentry\SentryBundle\DependencyInjection\SentryExtension; +use Sentry\Options; +use Sentry\SentryBundle\DependencyInjection\Configuration; use Symfony\Component\Config\Definition\Exception\InvalidConfigurationException; -use Symfony\Component\DependencyInjection\Container; -use Symfony\Component\DependencyInjection\ContainerBuilder; +use Symfony\Component\Config\Definition\Processor; +use Symfony\Component\HttpKernel\Kernel; class ConfigurationTest extends TestCase { + public function testConfigurationDefaults(): void + { + $defaultSdkValues = new Options(); + $processed = $this->processConfiguration([]); + $expectedDefaults = [ + 'dsn' => null, + 'listener_priorities' => [ + 'request' => 1, + 'console' => 1, + ], + 'options' => [ + 'excluded_exceptions' => $defaultSdkValues->getExcludedExceptions(), + 'prefixes' => $defaultSdkValues->getPrefixes(), + 'project_root' => '%kernel.root_dir%/..', + ], + ]; + + if (method_exists(Kernel::class, 'getProjectDir')) { + $expectedDefaults['options']['project_root'] = '%kernel.project_dir%'; + } + + $this->assertEquals($expectedDefaults, $processed); + } + /** * @dataProvider optionValuesProvider */ - public function testOptionValues(string $option, $value): void + public function testOptionValuesProcessing(string $option, $value): void { - $this->getContainer(['options' => [$option => $value]]); + $input = ['options' => [$option => $value]]; + $processed = $this->processConfiguration($input); - $this->addToAssertionCount(1); + $this->assertArraySubset($input, $processed); } public function optionValuesProvider(): array @@ -41,9 +67,11 @@ public function optionValuesProvider(): array */ public function testInvalidValues(string $option, $value): void { + $input = ['options' => [$option => $value]]; + $this->expectException(InvalidConfigurationException::class); - $this->getContainer(['options' => [$option => $value]]); + $this->processConfiguration($input); } public function invalidValuesProvider(): array @@ -62,17 +90,10 @@ public function invalidValuesProvider(): array ]; } - private function getContainer(array $configuration = []): Container + private function processConfiguration(array $values): array { - $containerBuilder = new ContainerBuilder(); - $containerBuilder->setParameter('kernel.project_dir', '/dir/project/root'); - $containerBuilder->setParameter('kernel.environment', 'test'); - - $extension = new SentryExtension(); - $extension->load(['sentry' => $configuration], $containerBuilder); - - $containerBuilder->compile(); + $processor = new Processor(); - return $containerBuilder; + return $processor->processConfiguration(new Configuration(), ['sentry' => $values]); } } From da123623742543458c3915f8a239e030aad1d76f Mon Sep 17 00:00:00 2001 From: Alessandro Lai Date: Wed, 23 Jan 2019 14:30:49 +0100 Subject: [PATCH 12/49] Map the first two options from the config --- src/DependencyInjection/Configuration.php | 2 +- src/DependencyInjection/SentryExtension.php | 10 ++++++++++ test/DependencyInjection/SentryExtensionTest.php | 14 +++++++------- 3 files changed, 18 insertions(+), 8 deletions(-) diff --git a/src/DependencyInjection/Configuration.php b/src/DependencyInjection/Configuration.php index 3c89868c..70b1a54f 100644 --- a/src/DependencyInjection/Configuration.php +++ b/src/DependencyInjection/Configuration.php @@ -39,7 +39,7 @@ public function getConfigTreeBuilder() $optionsNode = $rootNode->children() ->arrayNode('options') ->addDefaultsIfNotSet(); - + $defaultValues = new Options(); $optionsNode diff --git a/src/DependencyInjection/SentryExtension.php b/src/DependencyInjection/SentryExtension.php index 1f633842..a14d055b 100644 --- a/src/DependencyInjection/SentryExtension.php +++ b/src/DependencyInjection/SentryExtension.php @@ -45,5 +45,15 @@ private function passConfigurationToOptions(ContainerBuilder $container, array $ { $options = $container->getDefinition(Options::class); $options->addArgument(['dsn' => $processedConfiguration['dsn']]); + + $processedOptions = $processedConfiguration['options']; + + if ($processedOptions['project_root'] ?? false) { + $options->addMethodCall('setProjectRoot', [$processedOptions['project_root']]); + } + + if (\array_key_exists('default_integrations', $processedOptions)) { + $options->addMethodCall('setDefaultIntegrations', [$processedOptions['default_integrations']]); + } } } diff --git a/test/DependencyInjection/SentryExtensionTest.php b/test/DependencyInjection/SentryExtensionTest.php index 6cc812ce..772c9f5f 100644 --- a/test/DependencyInjection/SentryExtensionTest.php +++ b/test/DependencyInjection/SentryExtensionTest.php @@ -44,29 +44,29 @@ public function testValuesArePassedToOptions(string $name, $value, string $gette { $this->assertTrue(method_exists(Options::class, $getter), 'Bad data provider: wrong getter'); - $defaultContainer = $this->getContainer(); $container = $this->getContainer( [ 'options' => [$name => $value], ] ); + $this->assertSame( + $value, + $this->getOptionsFrom($container)->$getter() + ); + + $defaultContainer = $this->getContainer(); $this->assertNotEquals( $this->getOptionsFrom($defaultContainer)->$getter(), $this->getOptionsFrom($container)->$getter(), 'Bad data provider: value is same as default' ); - - $this->assertSame( - $value, - $this->getOptionsFrom($container)->$getter() - ); } public function optionsValueProvider() { return [ - ['default_integrations', true, 'hasDefaultIntegrations'], + ['default_integrations', false, 'hasDefaultIntegrations'], ]; } From eb80431b96d212c3b04d91b47487b1b0a98c8996 Mon Sep 17 00:00:00 2001 From: Alessandro Lai Date: Wed, 23 Jan 2019 14:34:24 +0100 Subject: [PATCH 13/49] Change CS-Fixer commandline --- composer.json | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/composer.json b/composer.json index 9083abe8..0c66f854 100644 --- a/composer.json +++ b/composer.json @@ -50,8 +50,8 @@ }, "scripts": { "phpstan": "vendor/bin/phpstan analyse", - "cs-check": "vendor/bin/php-cs-fixer fix --verbose --diff --dry-run", - "cs-fix": "vendor/bin/php-cs-fixer fix --verbose --diff" + "cs-check": "vendor/bin/php-cs-fixer fix --ansi --verbose --dry-run", + "cs-fix": "vendor/bin/php-cs-fixer fix --ansi --verbose" }, "extra": { "branch-alias": { From 90e07c3e1c55a7ce1824e50c953e5fcb4887927d Mon Sep 17 00:00:00 2001 From: Alessandro Lai Date: Wed, 23 Jan 2019 14:34:42 +0100 Subject: [PATCH 14/49] Refactor to single private method --- src/DependencyInjection/SentryExtension.php | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/src/DependencyInjection/SentryExtension.php b/src/DependencyInjection/SentryExtension.php index a14d055b..7efa3fc8 100644 --- a/src/DependencyInjection/SentryExtension.php +++ b/src/DependencyInjection/SentryExtension.php @@ -8,6 +8,7 @@ use Symfony\Component\Config\Definition\Exception\InvalidConfigurationException; use Symfony\Component\Config\FileLocator; use Symfony\Component\DependencyInjection\ContainerBuilder; +use Symfony\Component\DependencyInjection\Definition; use Symfony\Component\DependencyInjection\Loader; use Symfony\Component\HttpKernel\DependencyInjection\Extension; @@ -48,12 +49,14 @@ private function passConfigurationToOptions(ContainerBuilder $container, array $ $processedOptions = $processedConfiguration['options']; - if ($processedOptions['project_root'] ?? false) { - $options->addMethodCall('setProjectRoot', [$processedOptions['project_root']]); - } + $this->mapValue($options, $processedOptions, 'project_root', 'setProjectRoot'); + $this->mapValue($options, $processedOptions, 'default_integrations', 'setDefaultIntegrations'); + } - if (\array_key_exists('default_integrations', $processedOptions)) { - $options->addMethodCall('setDefaultIntegrations', [$processedOptions['default_integrations']]); + private function mapValue(Definition $options, array $processedOptions, string $key, string $setterMethod): void + { + if (\array_key_exists($key, $processedOptions)) { + $options->addMethodCall($setterMethod, [$processedOptions[$key]]); } } } From 5fca96d2857538ab3d3b957e7dacf0f30c0bab59 Mon Sep 17 00:00:00 2001 From: Alessandro Lai Date: Wed, 23 Jan 2019 14:46:19 +0100 Subject: [PATCH 15/49] Map an other options; refactor to a constant map --- src/DependencyInjection/SentryExtension.php | 19 ++++++++++--------- .../SentryExtensionTest.php | 3 ++- 2 files changed, 12 insertions(+), 10 deletions(-) diff --git a/src/DependencyInjection/SentryExtension.php b/src/DependencyInjection/SentryExtension.php index 7efa3fc8..e0a38d4a 100644 --- a/src/DependencyInjection/SentryExtension.php +++ b/src/DependencyInjection/SentryExtension.php @@ -8,7 +8,6 @@ use Symfony\Component\Config\Definition\Exception\InvalidConfigurationException; use Symfony\Component\Config\FileLocator; use Symfony\Component\DependencyInjection\ContainerBuilder; -use Symfony\Component\DependencyInjection\Definition; use Symfony\Component\DependencyInjection\Loader; use Symfony\Component\HttpKernel\DependencyInjection\Extension; @@ -19,6 +18,12 @@ */ class SentryExtension extends Extension { + private const CONFIGURATION_TO_OPTIONS_MAP = [ + 'default_integrations' => 'setDefaultIntegrations', + 'excluded_exceptions' => 'setExcludedExceptions', + 'project_root' => 'setProjectRoot', + ]; + /** * {@inheritDoc} * @@ -49,14 +54,10 @@ private function passConfigurationToOptions(ContainerBuilder $container, array $ $processedOptions = $processedConfiguration['options']; - $this->mapValue($options, $processedOptions, 'project_root', 'setProjectRoot'); - $this->mapValue($options, $processedOptions, 'default_integrations', 'setDefaultIntegrations'); - } - - private function mapValue(Definition $options, array $processedOptions, string $key, string $setterMethod): void - { - if (\array_key_exists($key, $processedOptions)) { - $options->addMethodCall($setterMethod, [$processedOptions[$key]]); + foreach (self::CONFIGURATION_TO_OPTIONS_MAP as $optionName => $setterMethod) { + if (\array_key_exists($optionName, $processedOptions)) { + $options->addMethodCall($setterMethod, [$processedOptions[$optionName]]); + } } } } diff --git a/test/DependencyInjection/SentryExtensionTest.php b/test/DependencyInjection/SentryExtensionTest.php index 772c9f5f..b9a26e19 100644 --- a/test/DependencyInjection/SentryExtensionTest.php +++ b/test/DependencyInjection/SentryExtensionTest.php @@ -63,10 +63,11 @@ public function testValuesArePassedToOptions(string $name, $value, string $gette ); } - public function optionsValueProvider() + public function optionsValueProvider(): array { return [ ['default_integrations', false, 'hasDefaultIntegrations'], + ['excluded_exceptions', [\Throwable::class], 'getExcludedExceptions'], ]; } From dea56171eebddfab3707c80e7e321bd0a50ad475 Mon Sep 17 00:00:00 2001 From: Alessandro Lai Date: Wed, 23 Jan 2019 15:00:16 +0100 Subject: [PATCH 16/49] Complete the test; remove serialize_all_object --- src/DependencyInjection/Configuration.php | 2 -- src/DependencyInjection/SentryExtension.php | 3 +++ test/DependencyInjection/ConfigurationTest.php | 3 --- test/DependencyInjection/SentryExtensionTest.php | 4 ++++ 4 files changed, 7 insertions(+), 5 deletions(-) diff --git a/src/DependencyInjection/Configuration.php b/src/DependencyInjection/Configuration.php index 70b1a54f..daf9a60b 100644 --- a/src/DependencyInjection/Configuration.php +++ b/src/DependencyInjection/Configuration.php @@ -64,8 +64,6 @@ public function getConfigTreeBuilder() ->integerNode('send_attempts') ->min(1) ->end() - ->booleanNode('serialize_all_object')->end() - ; // Bundle-specific configuration diff --git a/src/DependencyInjection/SentryExtension.php b/src/DependencyInjection/SentryExtension.php index e0a38d4a..ef2f9e64 100644 --- a/src/DependencyInjection/SentryExtension.php +++ b/src/DependencyInjection/SentryExtension.php @@ -21,7 +21,10 @@ class SentryExtension extends Extension private const CONFIGURATION_TO_OPTIONS_MAP = [ 'default_integrations' => 'setDefaultIntegrations', 'excluded_exceptions' => 'setExcludedExceptions', + 'prefixes' => 'setPrefixes', 'project_root' => 'setProjectRoot', + 'sample_rate' => 'setSampleRate', + 'send_attempts' => 'setSendAttempts', ]; /** diff --git a/test/DependencyInjection/ConfigurationTest.php b/test/DependencyInjection/ConfigurationTest.php index c9895c6a..74c6a013 100644 --- a/test/DependencyInjection/ConfigurationTest.php +++ b/test/DependencyInjection/ConfigurationTest.php @@ -57,8 +57,6 @@ public function optionValuesProvider(): array ['sample_rate', 1], ['send_attempts', 1], ['send_attempts', 999], - ['serialize_all_object', true], - ['serialize_all_object', false], ]; } @@ -86,7 +84,6 @@ public function invalidValuesProvider(): array ['send_attempts', 1.5], ['send_attempts', 0], ['send_attempts', -1], - ['serialize_all_object', 'true'], ]; } diff --git a/test/DependencyInjection/SentryExtensionTest.php b/test/DependencyInjection/SentryExtensionTest.php index b9a26e19..a5f74241 100644 --- a/test/DependencyInjection/SentryExtensionTest.php +++ b/test/DependencyInjection/SentryExtensionTest.php @@ -68,6 +68,10 @@ public function optionsValueProvider(): array return [ ['default_integrations', false, 'hasDefaultIntegrations'], ['excluded_exceptions', [\Throwable::class], 'getExcludedExceptions'], + ['prefixes', ['/some/path/prefix/'], 'getPrefixes'], + ['project_root', '/some/project/', 'getProjectRoot'], + ['sample_rate', 0.5, 'getSampleRate'], + ['send_attempts', 2, 'getSendAttempts'], ]; } From 93f8aa6cc415a35bd77791c4e0693c549aa54ee6 Mon Sep 17 00:00:00 2001 From: Alessandro Lai Date: Wed, 23 Jan 2019 15:06:48 +0100 Subject: [PATCH 17/49] Refactor to generated get/set names --- src/DependencyInjection/SentryExtension.php | 20 +++++++++---------- .../SentryExtensionTest.php | 18 ++++++++++------- 2 files changed, 21 insertions(+), 17 deletions(-) diff --git a/src/DependencyInjection/SentryExtension.php b/src/DependencyInjection/SentryExtension.php index ef2f9e64..42d94d63 100644 --- a/src/DependencyInjection/SentryExtension.php +++ b/src/DependencyInjection/SentryExtension.php @@ -18,15 +18,6 @@ */ class SentryExtension extends Extension { - private const CONFIGURATION_TO_OPTIONS_MAP = [ - 'default_integrations' => 'setDefaultIntegrations', - 'excluded_exceptions' => 'setExcludedExceptions', - 'prefixes' => 'setPrefixes', - 'project_root' => 'setProjectRoot', - 'sample_rate' => 'setSampleRate', - 'send_attempts' => 'setSendAttempts', - ]; - /** * {@inheritDoc} * @@ -56,9 +47,18 @@ private function passConfigurationToOptions(ContainerBuilder $container, array $ $options->addArgument(['dsn' => $processedConfiguration['dsn']]); $processedOptions = $processedConfiguration['options']; + $mappableOptions = [ + 'default_integrations', + 'excluded_exceptions', + 'prefixes', + 'project_root', + 'sample_rate', + 'send_attempts', + ]; - foreach (self::CONFIGURATION_TO_OPTIONS_MAP as $optionName => $setterMethod) { + foreach ($mappableOptions as $optionName) { if (\array_key_exists($optionName, $processedOptions)) { + $setterMethod = 'set' . str_replace('_', '', ucwords($optionName, '_')); $options->addMethodCall($setterMethod, [$processedOptions[$optionName]]); } } diff --git a/test/DependencyInjection/SentryExtensionTest.php b/test/DependencyInjection/SentryExtensionTest.php index a5f74241..31c28071 100644 --- a/test/DependencyInjection/SentryExtensionTest.php +++ b/test/DependencyInjection/SentryExtensionTest.php @@ -40,9 +40,13 @@ public function testOptionsDefaultValues(): void /** * @dataProvider optionsValueProvider */ - public function testValuesArePassedToOptions(string $name, $value, string $getter): void + public function testValuesArePassedToOptions(string $name, $value, string $getter = null): void { - $this->assertTrue(method_exists(Options::class, $getter), 'Bad data provider: wrong getter'); + if (null === $getter) { + $getter = 'get' . str_replace('_', '', ucwords($name, '_')); + } + + $this->assertTrue(method_exists(Options::class, $getter), 'Bad data provider, wrong getter: ' . $getter); $container = $this->getContainer( [ @@ -67,11 +71,11 @@ public function optionsValueProvider(): array { return [ ['default_integrations', false, 'hasDefaultIntegrations'], - ['excluded_exceptions', [\Throwable::class], 'getExcludedExceptions'], - ['prefixes', ['/some/path/prefix/'], 'getPrefixes'], - ['project_root', '/some/project/', 'getProjectRoot'], - ['sample_rate', 0.5, 'getSampleRate'], - ['send_attempts', 2, 'getSendAttempts'], + ['excluded_exceptions', [\Throwable::class]], + ['prefixes', ['/some/path/prefix/']], + ['project_root', '/some/project/'], + ['sample_rate', 0.5], + ['send_attempts', 2], ]; } From 7def6f0a4da3256c3ecea0f297d5012b79a6b4c3 Mon Sep 17 00:00:00 2001 From: Alessandro Lai Date: Wed, 23 Jan 2019 15:40:27 +0100 Subject: [PATCH 18/49] Require PHPStan-Prophecy extension --- composer.json | 1 + phpstan.neon | 1 + 2 files changed, 2 insertions(+) diff --git a/composer.json b/composer.json index 0c66f854..949ecf5c 100644 --- a/composer.json +++ b/composer.json @@ -31,6 +31,7 @@ }, "require-dev": { "friendsofphp/php-cs-fixer": "^2.8", + "jangregor/phpstan-prophecy": "^0.3.0", "php-http/mock-client": "^1.0", "phpstan/phpstan": "^0.10", "phpstan/phpstan-phpunit": "^0.10", diff --git a/phpstan.neon b/phpstan.neon index 3d861b27..9da7d8e9 100644 --- a/phpstan.neon +++ b/phpstan.neon @@ -9,4 +9,5 @@ parameters: excludes_analyse: - '%currentWorkingDirectory%/src/DependencyInjection/Configuration.php' includes: + - vendor/jangregor/phpstan-prophecy/src/extension.neon - vendor/phpstan/phpstan-phpunit/extension.neon From 30a14a9bdabacfee5f278b3db82e61687db6e3bb Mon Sep 17 00:00:00 2001 From: Alessandro Lai Date: Wed, 23 Jan 2019 15:54:27 +0100 Subject: [PATCH 19/49] WIP: start RequestListener test --- test/EventListener/RequestListenerTest.php | 49 ++++++++++++++++++++++ 1 file changed, 49 insertions(+) create mode 100644 test/EventListener/RequestListenerTest.php diff --git a/test/EventListener/RequestListenerTest.php b/test/EventListener/RequestListenerTest.php new file mode 100644 index 00000000..0385bd56 --- /dev/null +++ b/test/EventListener/RequestListenerTest.php @@ -0,0 +1,49 @@ +currentScope = new Scope(); + $this->currentHub = $this->prophesize(HubInterface::class); + $this->currentHub->getScope() + ->shouldBeCalled() + ->willReturn($this->currentScope); + + Hub::setCurrent($this->currentHub->reveal()); + } + + public function testOnKernelRequestUserDataIsSetToScope(): void + { + $listener = new RequestListener( + $this->currentHub->reveal(), + $this->prophesize(TokenStorageInterface::class)->reveal(), + $this->prophesize(AuthorizationCheckerInterface::class)->reveal() + ); + + $event = $this->prophesize(GetResponseEvent::class); + $event->isMasterRequest() + ->willReturn(true); + + $listener->onKernelRequest($event->reveal()); + + $this->markTestIncomplete(); + $this->assertSame([], $this->currentScope->getUser()); + } +} From 102f7ca984df6136469ea8ac49edb152d3c0c8dd Mon Sep 17 00:00:00 2001 From: Alessandro Lai Date: Wed, 23 Jan 2019 15:57:51 +0100 Subject: [PATCH 20/49] Skip old test --- test/EventListener/ExceptionListenerTest.php | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/test/EventListener/ExceptionListenerTest.php b/test/EventListener/ExceptionListenerTest.php index 4bf2ae65..35106fcc 100644 --- a/test/EventListener/ExceptionListenerTest.php +++ b/test/EventListener/ExceptionListenerTest.php @@ -4,13 +4,9 @@ use PHPUnit\Framework\TestCase; use Sentry\SentryBundle\DependencyInjection\SentryExtension; -use Sentry\SentryBundle\Event\SentryUserContextEvent; use Sentry\SentryBundle\EventListener\RequestListener; -use Sentry\SentryBundle\EventListener\SentryExceptionListenerInterface; -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; @@ -34,8 +30,6 @@ class ExceptionListenerTest extends TestCase private $containerBuilder; - private $mockSentryClient; - private $mockTokenStorage; private $mockAuthorizationChecker; @@ -49,9 +43,9 @@ class ExceptionListenerTest extends TestCase public function setUp() { + $this->markTestSkipped('To be refactored'); $this->mockTokenStorage = $this->createMock(TokenStorageInterface::class); $this->mockAuthorizationChecker = $this->createMock(AuthorizationCheckerInterface::class); - $this->mockSentryClient = $this->createMock(\Raven_Client::class); $this->mockEventDispatcher = $this->createMock(EventDispatcherInterface::class); $this->requestStack = new RequestStack(); @@ -62,7 +56,6 @@ public function setUp() $containerBuilder->set('request_stack', $this->requestStack); $containerBuilder->set('security.token_storage', $this->mockTokenStorage); $containerBuilder->set('security.authorization_checker', $this->mockAuthorizationChecker); - $containerBuilder->set('sentry.client', $this->mockSentryClient); $containerBuilder->set('event_dispatcher', $this->mockEventDispatcher); $containerBuilder->setAlias(self::LISTENER_TEST_PUBLIC_ALIAS, new Alias('sentry.exception_listener', true)); From 132892b1bb5d657d11fb0fbf66dee4817a7a1fae Mon Sep 17 00:00:00 2001 From: Alessandro Lai Date: Wed, 23 Jan 2019 16:31:43 +0100 Subject: [PATCH 21/49] Test RequestListener --- src/EventListener/RequestListener.php | 5 +- test/EventListener/RequestListenerTest.php | 105 +++++++++++++++++++-- 2 files changed, 97 insertions(+), 13 deletions(-) diff --git a/src/EventListener/RequestListener.php b/src/EventListener/RequestListener.php index 527d9461..cc1a4937 100644 --- a/src/EventListener/RequestListener.php +++ b/src/EventListener/RequestListener.php @@ -6,7 +6,6 @@ use Sentry\State\HubInterface; use Symfony\Component\HttpKernel\Event\FilterControllerEvent; use Symfony\Component\HttpKernel\Event\GetResponseEvent; -use Symfony\Component\HttpKernel\HttpKernelInterface; use Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorageInterface; use Symfony\Component\Security\Core\Authorization\AuthorizationCheckerInterface; use Symfony\Component\Security\Core\Authorization\Voter\AuthenticatedVoter; @@ -50,7 +49,7 @@ public function __construct( */ public function onKernelRequest(GetResponseEvent $event): void { - if (HttpKernelInterface::MASTER_REQUEST !== $event->getRequestType()) { + if (! $event->isMasterRequest()) { return; } @@ -77,7 +76,7 @@ public function onKernelRequest(GetResponseEvent $event): void public function onKernelController(FilterControllerEvent $event): void { - if (HttpKernelInterface::MASTER_REQUEST !== $event->getRequestType()) { + if (! $event->isMasterRequest()) { return; } diff --git a/test/EventListener/RequestListenerTest.php b/test/EventListener/RequestListenerTest.php index 0385bd56..8fb68abd 100644 --- a/test/EventListener/RequestListenerTest.php +++ b/test/EventListener/RequestListenerTest.php @@ -7,9 +7,15 @@ use Sentry\State\Hub; use Sentry\State\HubInterface; use Sentry\State\Scope; +use Symfony\Component\HttpFoundation\ParameterBag; +use Symfony\Component\HttpFoundation\Request; +use Symfony\Component\HttpKernel\Event\FilterControllerEvent; use Symfony\Component\HttpKernel\Event\GetResponseEvent; use Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorageInterface; +use Symfony\Component\Security\Core\Authentication\Token\TokenInterface; use Symfony\Component\Security\Core\Authorization\AuthorizationCheckerInterface; +use Symfony\Component\Security\Core\Authorization\Voter\AuthenticatedVoter; +use Symfony\Component\Security\Core\User\UserInterface; class RequestListenerTest extends TestCase { @@ -19,7 +25,7 @@ class RequestListenerTest extends TestCase protected function setUp() { parent::setUp(); - + $this->currentScope = new Scope(); $this->currentHub = $this->prophesize(HubInterface::class); $this->currentHub->getScope() @@ -29,21 +35,100 @@ protected function setUp() Hub::setCurrent($this->currentHub->reveal()); } - public function testOnKernelRequestUserDataIsSetToScope(): void + /** + * @dataProvider userDataProvider + */ + public function testOnKernelRequestUserDataIsSetToScope($user): void { + $tokenStorage = $this->prophesize(TokenStorageInterface::class); + $authorizationChecker = $this->prophesize(AuthorizationCheckerInterface::class); + $event = $this->prophesize(GetResponseEvent::class); + $request = $this->prophesize(Request::class); + $token = $this->prophesize(TokenInterface::class); + + $event->isMasterRequest() + ->willReturn(true); + + $tokenStorage->getToken() + ->willReturn($token->reveal()); + + $token->isAuthenticated() + ->willReturn(true); + $authorizationChecker->isGranted(AuthenticatedVoter::IS_AUTHENTICATED_REMEMBERED) + ->willReturn(true); + + $token->getUser() + ->willReturn($user); + + $event->getRequest() + ->willReturn($request->reveal()); + $request->getClientIp() + ->willReturn('1.2.3.4'); + $listener = new RequestListener( $this->currentHub->reveal(), - $this->prophesize(TokenStorageInterface::class)->reveal(), - $this->prophesize(AuthorizationCheckerInterface::class)->reveal() + $tokenStorage->reveal(), + $authorizationChecker->reveal() ); - $event = $this->prophesize(GetResponseEvent::class); + $listener->onKernelRequest($event->reveal()); + + $expectedUserData = [ + 'ip_address' => '1.2.3.4', + 'username' => 'john-doe', + ]; + $this->assertEquals($expectedUserData, $this->currentScope->getUser()); + } + + public function userDataProvider(): \Generator + { + yield ['john-doe']; + + $userInterface = $this->prophesize(UserInterface::class); + $userInterface->getUsername() + ->willReturn('john-doe'); + + yield [$userInterface->reveal()]; + yield [new ToStringUser('john-doe')]; + } + + public function testOnKernelControllerAddsRouteTag(): void + { + $event = $this->prophesize(FilterControllerEvent::class); + $request = $this->prophesize(Request::class); + $attributes = new ParameterBag(); + $attributes->set('_route', 'sf-route'); + $event->isMasterRequest() ->willReturn(true); - - $listener->onKernelRequest($event->reveal()); - - $this->markTestIncomplete(); - $this->assertSame([], $this->currentScope->getUser()); + $event->getRequest() + ->willReturn($request->reveal()); + + $request->attributes = $attributes; + + $listener = new RequestListener( + $this->currentHub->reveal(), + $this->prophesize(TokenStorageInterface::class)->reveal(), + $this->prophesize(AuthorizationCheckerInterface::class)->reveal() + ); + + $listener->onKernelController($event->reveal()); + + $this->assertSame(['route' => 'sf-route'], $this->currentScope->getTags()); + } +} + +class ToStringUser +{ + private $username; + + public function __construct(string $username) + { + $this->username = $username; + } + + public function __toString(): string + { + return $this->username; } } From 27208bb34ac5484d760d83df21465bac48e2c290 Mon Sep 17 00:00:00 2001 From: Alessandro Lai Date: Wed, 23 Jan 2019 16:43:26 +0100 Subject: [PATCH 22/49] Add test for ConsoleListener --- test/EventListener/ConsoleListenerTest.php | 59 ++++++++++++++++++++++ 1 file changed, 59 insertions(+) create mode 100644 test/EventListener/ConsoleListenerTest.php diff --git a/test/EventListener/ConsoleListenerTest.php b/test/EventListener/ConsoleListenerTest.php new file mode 100644 index 00000000..71862cb7 --- /dev/null +++ b/test/EventListener/ConsoleListenerTest.php @@ -0,0 +1,59 @@ +currentScope = new Scope(); + $this->currentHub = $this->prophesize(HubInterface::class); + $this->currentHub->getScope() + ->shouldBeCalled() + ->willReturn($this->currentScope); + + Hub::setCurrent($this->currentHub->reveal()); + } + + public function testOnConsoleCommandAddsCommandName(): void + { + $command = $this->prophesize(Command::class); + $command->getName() + ->willReturn('sf:command:name'); + + $event = $this->prophesize(ConsoleCommandEvent::class); + $event->getCommand() + ->willReturn($command->reveal()); + + $listener = new ConsoleListener($this->currentHub->reveal()); + + $listener->onConsoleCommand($event->reveal()); + + $this->assertSame(['command' => 'sf:command:name'], $this->currentScope->getTags()); + } + + public function testOnConsoleCommandAddsPlaceholderCommandName(): void + { + $event = $this->prophesize(ConsoleCommandEvent::class); + $event->getCommand() + ->willReturn(null); + + $listener = new ConsoleListener($this->currentHub->reveal()); + + $listener->onConsoleCommand($event->reveal()); + + $this->assertSame(['command' => 'N/A'], $this->currentScope->getTags()); + } +} From 3c2da29b6619d40f68652fb96cba25dfc67bdf82 Mon Sep 17 00:00:00 2001 From: Alessandro Lai Date: Wed, 23 Jan 2019 16:55:04 +0100 Subject: [PATCH 23/49] Fix PHPStan errors --- phpstan.neon | 4 ++-- test/EventListener/RequestListenerTest.php | 10 +++------- 2 files changed, 5 insertions(+), 9 deletions(-) diff --git a/phpstan.neon b/phpstan.neon index 9da7d8e9..e1987177 100644 --- a/phpstan.neon +++ b/phpstan.neon @@ -4,10 +4,10 @@ parameters: - src/ - test/ ignoreErrors: - - '/Symfony\\Component\\Console\\Event\\ConsoleExceptionEvent/' - - '/Call to an undefined method Symfony\\Component\\Console\\Event\\ConsoleEvent::getExitCode\(\)\./' + - "/Call to function method_exists.. with 'Symfony.+' and 'getProjectDir' will always evaluate to false./" excludes_analyse: - '%currentWorkingDirectory%/src/DependencyInjection/Configuration.php' + - '%currentWorkingDirectory%/test/EventListener/ExceptionListenerTest.php' includes: - vendor/jangregor/phpstan-prophecy/src/extension.neon - vendor/phpstan/phpstan-phpunit/extension.neon diff --git a/test/EventListener/RequestListenerTest.php b/test/EventListener/RequestListenerTest.php index 8fb68abd..aa59c2fd 100644 --- a/test/EventListener/RequestListenerTest.php +++ b/test/EventListener/RequestListenerTest.php @@ -7,7 +7,6 @@ use Sentry\State\Hub; use Sentry\State\HubInterface; use Sentry\State\Scope; -use Symfony\Component\HttpFoundation\ParameterBag; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpKernel\Event\FilterControllerEvent; use Symfony\Component\HttpKernel\Event\GetResponseEvent; @@ -94,17 +93,14 @@ public function userDataProvider(): \Generator public function testOnKernelControllerAddsRouteTag(): void { + $request = new Request(); + $request->attributes->set('_route', 'sf-route'); $event = $this->prophesize(FilterControllerEvent::class); - $request = $this->prophesize(Request::class); - $attributes = new ParameterBag(); - $attributes->set('_route', 'sf-route'); $event->isMasterRequest() ->willReturn(true); $event->getRequest() - ->willReturn($request->reveal()); - - $request->attributes = $attributes; + ->willReturn($request); $listener = new RequestListener( $this->currentHub->reveal(), From 068eb11816a1761994c0086ea545517b979b57bd Mon Sep 17 00:00:00 2001 From: Alessandro Lai Date: Thu, 24 Jan 2019 09:35:37 +0100 Subject: [PATCH 24/49] Try to fix prefer-lowest build and add PHP 7.3 --- .travis.yml | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/.travis.yml b/.travis.yml index c7a7a657..e6996057 100644 --- a/.travis.yml +++ b/.travis.yml @@ -5,10 +5,12 @@ dist: trusty php: - 7.1 - 7.2 + - 7.3 cache: directories: - $HOME/.composer/cache/files + - composer.lock before_install: - phpenv config-rm xdebug.ini @@ -16,7 +18,7 @@ before_install: - composer global require hirak/prestissimo install: - - travis_retry travis_wait composer update --no-interaction --prefer-dist --prefer-stable $COMPOSER_OPTIONS + - travis_retry travis_wait composer update --no-interaction --prefer-dist --prefer-stable script: - vendor/bin/phpunit -v @@ -25,7 +27,9 @@ jobs: include: - stage: Test php: 7.1 - env: COMPOSER_OPTIONS="--prefer-lowest" + install: + - travis_retry travis_wait composer install --no-interaction --prefer-dist --prefer-stable + - travis_retry travis_wait composer update --no-interaction --prefer-dist --prefer-stable --prefer-lowest - stage: Code style and static analysis script: - composer phpstan From 3bd77c01183330f41e13787ae3492447484d357f Mon Sep 17 00:00:00 2001 From: Alessandro Lai Date: Thu, 24 Jan 2019 09:44:58 +0100 Subject: [PATCH 25/49] Do not cache composer.lock --- .travis.yml | 1 - 1 file changed, 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index e6996057..183a2ca6 100644 --- a/.travis.yml +++ b/.travis.yml @@ -10,7 +10,6 @@ php: cache: directories: - $HOME/.composer/cache/files - - composer.lock before_install: - phpenv config-rm xdebug.ini From 5f3452a9149201f9571db55dd4eb5801343a2ada Mon Sep 17 00:00:00 2001 From: Alessandro Lai Date: Thu, 24 Jan 2019 09:47:20 +0100 Subject: [PATCH 26/49] Fix composer install command in build --- .travis.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index 183a2ca6..0c91b705 100644 --- a/.travis.yml +++ b/.travis.yml @@ -27,7 +27,7 @@ jobs: - stage: Test php: 7.1 install: - - travis_retry travis_wait composer install --no-interaction --prefer-dist --prefer-stable + - travis_retry travis_wait composer install --no-interaction --prefer-dist - travis_retry travis_wait composer update --no-interaction --prefer-dist --prefer-stable --prefer-lowest - stage: Code style and static analysis script: From b66f262a63d9d600dc4316dba23055966e6ef976 Mon Sep 17 00:00:00 2001 From: Alessandro Lai Date: Thu, 24 Jan 2019 09:56:57 +0100 Subject: [PATCH 27/49] Fix BC with older SF versions --- src/DependencyInjection/Configuration.php | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/src/DependencyInjection/Configuration.php b/src/DependencyInjection/Configuration.php index daf9a60b..975f6886 100644 --- a/src/DependencyInjection/Configuration.php +++ b/src/DependencyInjection/Configuration.php @@ -6,6 +6,7 @@ use Symfony\Component\Config\Definition\Builder\ArrayNodeDefinition; use Symfony\Component\Config\Definition\Builder\TreeBuilder; use Symfony\Component\Config\Definition\ConfigurationInterface; +use Symfony\Component\HttpKernel\Kernel; /** * This is the class that validates and merges configuration from your app/config files @@ -55,7 +56,7 @@ public function getConfigTreeBuilder() ->scalarPrototype()->end() ->end() ->scalarNode('project_root') - ->defaultValue('%kernel.project_dir%') + ->defaultValue($this->getProjectRoot()) ->end() ->floatNode('sample_rate') ->min(0.0) @@ -90,4 +91,13 @@ private function getTrimClosure(): callable return $value; }; } + + private function getProjectRoot(): string + { + if (method_exists(Kernel::class, 'getProjectDir')) { + return '%kernel.project_dir%'; + } + + return '%kernel.root_dir%/..'; + } } From 118d5866be668353a96769deaabb810587131c90 Mon Sep 17 00:00:00 2001 From: Alessandro Lai Date: Thu, 24 Jan 2019 10:12:51 +0100 Subject: [PATCH 28/49] Test (and fix) bundle listener registration --- src/Resources/config/services.xml | 1 + test/SentryBundleTest.php | 74 +++++++++++++++++++++++++++++++ 2 files changed, 75 insertions(+) create mode 100644 test/SentryBundleTest.php diff --git a/src/Resources/config/services.xml b/src/Resources/config/services.xml index bf62e007..a4031818 100644 --- a/src/Resources/config/services.xml +++ b/src/Resources/config/services.xml @@ -36,6 +36,7 @@ + diff --git a/test/SentryBundleTest.php b/test/SentryBundleTest.php new file mode 100644 index 00000000..5b3136a5 --- /dev/null +++ b/test/SentryBundleTest.php @@ -0,0 +1,74 @@ +getContainer(); + + $consoleListener = $container->getDefinition(ConsoleListener::class); + + $expectedTag = [ + 'kernel.event_listener' => [ + [ + 'event' => 'console.command', + 'method' => 'onConsoleCommand', + 'priority' => '%sentry.listener_priorities.console%', + ], + ], + ]; + + $this->assertSame($expectedTag, $consoleListener->getTags()); + } + + public function testContainerHasRequestListenerConfiguredCorrectly(): void + { + $container = $this->getContainer(); + + $consoleListener = $container->getDefinition(RequestListener::class); + + $expectedTag = [ + 'kernel.event_listener' => [ + [ + 'event' => 'kernel.request', + 'method' => 'onKernelRequest', + 'priority' => '%sentry.listener_priorities.request%', + ], + [ + 'event' => 'kernel.controller', + 'method' => 'onKernelController', + 'priority' => '%sentry.listener_priorities.request%', + ], + ], + ]; + + $this->assertSame($expectedTag, $consoleListener->getTags()); + } + + private function getContainer(array $configuration = []): ContainerBuilder + { + $containerBuilder = new ContainerBuilder(); + $containerBuilder->setParameter('kernel.root_dir', 'kernel/root'); + if (method_exists(Kernel::class, 'getProjectDir')) { + $containerBuilder->setParameter('kernel.project_dir', '/dir/project/root'); + } + + $containerBuilder->setParameter('kernel.environment', 'test'); + $containerBuilder->set('event_dispatcher', $this->prophesize(EventDispatcherInterface::class)->reveal()); + + $extension = new SentryExtension(); + $extension->load(['sentry' => $configuration], $containerBuilder); + + return $containerBuilder; + } +} From 5e190296de1dc1614914220b848c7aa302f83f0f Mon Sep 17 00:00:00 2001 From: Alessandro Lai Date: Thu, 24 Jan 2019 11:27:32 +0100 Subject: [PATCH 29/49] Prepare for master merge adding documentation --- README.md | 9 +++++++++ UPGRADE-3.0.md | 48 ++++++++++++++++++++++++++++++++++++++++++++++++ composer.json | 5 +++-- 3 files changed, 60 insertions(+), 2 deletions(-) create mode 100644 UPGRADE-3.0.md diff --git a/README.md b/README.md index 5d0fe965..0d1f8d79 100644 --- a/README.md +++ b/README.md @@ -9,6 +9,15 @@ Symfony integration for [Sentry](https://getsentry.com/). [![Scrutinizer][Master scrutinizer image]][Master scrutinizer link] [![Coverage Status][Master coverage image]][Master scrutinizer link] +## Notice 3.0 +> The current master branch contains the 3.0 version of this bundle, which is currently under development. This version +> will support the newest 2.0 version of the underlying Sentry SDK version. +> +> A beta version will be tagged as soon as possible, in the meantime you can continue to use the previous versions. +> +> To know more about the progress of this version see [the relative +milestone](https://github.com/getsentry/sentry-symfony/milestone/3) + ## Benefits Use sentry-symfony for: diff --git a/UPGRADE-3.0.md b/UPGRADE-3.0.md new file mode 100644 index 00000000..d2569587 --- /dev/null +++ b/UPGRADE-3.0.md @@ -0,0 +1,48 @@ +# Upgrade to 3.0 +The 3.0 major release of this bundle has some major changes. This document will try to list them all, to help users +during the upgrade path. + +## Sentry SDK 2.0 +The major change is in the fact that we now require the underlying `sentry/sentry` package to have version 2. +This new version has been completely rewritten: if you use this bundle and you interact directly with the underlying SDK +and client, you should read through the [relative upgrade document](https://github.com/getsentry/sentry-php/blob/master/UPGRADE-2.0.md). + +## HTTPlug +Since SDK 2.0 uses HTTPlug to remain transport-agnostic, you need to manually require two packages that provides +[`php-http/async-client-implementation`](https://packagist.org/providers/php-http/async-client-implementation) +and [`http-message-implementation`](https://packagist.org/providers/psr/http-message-implementation). + +For example, if you want to install/upgrade using Curl as transport and the PSR-7 implementation by Guzzle, you can use: + +```bash +composer require sentry/sentry:2.0.0-beta1 php-http/curl-client guzzlehttp/psr7 +``` + +## Changes in the services +Due to the SDK changes, and to follow newer Symfony best practices, the services exposed by the bundle are completely +changed: + + * All services are now private; declare public aliases to access them if needed; you can still use the Sentry SDK global + functions if you want just to capture messages manually without injecting Sentry services in your code + * All services uses the full qualified name of the interfaces to name them + * The `ExceptionListener` has been splitted in two: `RequestListener` and `ConsoleListener` + * The listeners are now `final`; append your own listeners to override their behavior + * The listeners are registered with a priority of `1`; they will run just before the default priority of `0`, to ease + the registration of custom listener that will change `Scope` data + * Configuration options of the bundle are now aligned with the new ones of the 2.0 SDK + * Listeners are no longer used to capture exceptions, it's all demanded to the error handler + +## New services +This is a brief description of the services registered by this bundle: + + * `Sentry\State\HubInterface`: this is the central root of the SDK; it's the `Hub` that the bundle will instantiate at + startup, and the current one too if you do not change it + * `Sentry\ClientInterface`: this is the proper client; compared to the 1.x SDK version it's a lot more slimmed down, + since a lot of the stuff has been splitted in separated component, so you probably will not interact with it as much as + in the past. You also have to remind you that the client is bound to the `Hub`, and has to be changed there if you want + to use it automatically in error reporting + * `Sentry\ClientBuilderInterface`: this is the factory that builds the client; you can call its methods to change all + the settings and dependency that will be injected in the latter created client. You can use this service to obtain more + customized clients for your needs + * `Sentry\Options`: this class holds all the configuration used in the client and other SDK components; it's populated + starting from the bundle configuration diff --git a/composer.json b/composer.json index 949ecf5c..a786b245 100644 --- a/composer.json +++ b/composer.json @@ -56,8 +56,9 @@ }, "extra": { "branch-alias": { - "master": "2.1.x-dev", - "releases/1.x": "1.1.x-dev" + "master": "3.x-dev", + "releases/2.x": "2.x-dev", + "releases/1.x": "1.x-dev" } } } From 4eed8b3db46fd4833d5c118b39d58b39f97f2ce6 Mon Sep 17 00:00:00 2001 From: Alessandro Lai Date: Thu, 24 Jan 2019 12:13:45 +0100 Subject: [PATCH 30/49] Fix CS --- test/SentryBundleTest.php | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/test/SentryBundleTest.php b/test/SentryBundleTest.php index 5b3136a5..221d28c2 100644 --- a/test/SentryBundleTest.php +++ b/test/SentryBundleTest.php @@ -12,12 +12,12 @@ class SentryBundleTest extends TestCase { - public function testContainerHasConsoleListenerConfiguredCorrectly(): void + public function testContainerHasConsoleListenerConfiguredCorrectly(): void { $container = $this->getContainer(); - + $consoleListener = $container->getDefinition(ConsoleListener::class); - + $expectedTag = [ 'kernel.event_listener' => [ [ @@ -31,12 +31,12 @@ public function testContainerHasConsoleListenerConfiguredCorrectly(): void $this->assertSame($expectedTag, $consoleListener->getTags()); } - public function testContainerHasRequestListenerConfiguredCorrectly(): void + public function testContainerHasRequestListenerConfiguredCorrectly(): void { $container = $this->getContainer(); - + $consoleListener = $container->getDefinition(RequestListener::class); - + $expectedTag = [ 'kernel.event_listener' => [ [ From 6a4e11e13e45b3cc1d8dd114e8b1757b35be5f1a Mon Sep 17 00:00:00 2001 From: Alessandro Lai Date: Thu, 24 Jan 2019 12:14:13 +0100 Subject: [PATCH 31/49] Revert unwanted changes --- CHANGELOG.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 822c9504..455c5162 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,7 +9,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. ### Added - Add the `route` tag automatically to any event generated by a request (#167, thanks @ruudk) ### Changed - - Make the `RequestListener` more extendable by making all members at least `protected` (#176, thanks @bouland) + - Make the `ExceptionListener` more extendable by making all members at least `protected` (#176, thanks @bouland) ## 2.1.0 - 2018-11-05 ### Added @@ -66,8 +66,8 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. - Add official support to PHP 7.2 (#71) ### Changed - Changed source folder from `src/Sentry/SentryBundle` to just `src/` (thanks to PSR-4 and Composer this doesn't affect you) - - Re-sort the constructor's arguments of `RequestListener` - - The `SentrySymfonyClient` is no longer an optional argument of `RequestListener`; it's now required + - Re-sort the constructor's arguments of `ExceptionListener` + - The `SentrySymfonyClient` is no longer an optional argument of `ExceptionListener`; it's now required ### Fixed - Remove usage of `create_function()` to avoid deprecations (#71) - Fix a possible bug that could make Sentry crash if an error is triggered before loading a console command From 7bbdadefff21ca8136f6f2ec187c96a24430bc6e Mon Sep 17 00:00:00 2001 From: Alessandro Lai Date: Thu, 24 Jan 2019 12:24:08 +0100 Subject: [PATCH 32/49] Cleanup the readme --- README.md | 248 +++--------------------------------------------------- 1 file changed, 11 insertions(+), 237 deletions(-) diff --git a/README.md b/README.md index 0d1f8d79..7e03135f 100644 --- a/README.md +++ b/README.md @@ -87,250 +87,24 @@ sentry: ``` ## Maintained versions - * 2.x is actively maintained on the master branch, but it requires Symfony 3+ and PHP 7.1+; - * 1.x is still supported to allow Symfony 2 and PHP 5.6/7.0; it may receive backports of features from the master branch, but it's not guaranteed - * 0.8.x is no longer maintained, with the 0.8.8 release containing the latest new features; it may only receive security fixes in the future. + * 3.x is actively maintained and developed on the master branch, and uses Sentry SDK 2.0; + * 2.x is supported only for fixes; from this version onwards it requires Symfony 3+ and PHP 7.1+; + * 1.x is no longer maintained; you can use it for Symfony < 2.8 and PHP 5.6/7.0; + * 0.8.x is no longer maintained. ## Configuration -The following options can be configured via ``app/config/config.yml``. - -### Skip some exceptions - -```yaml -sentry: - skip_capture: - - 'Symfony\Component\HttpKernel\Exception\HttpExceptionInterface' -``` - -### Listeners' priority - -You can change the priority of the 3 default listeners of this bundle with the `listener_priorities` key of your config. -The default value is `0`, and here are the 3 possible sub-keys: - -```yaml -listener_priorities: - request: 0 - kernel_exception: 0 - console_exception: 0 -``` - -... respectively for the `onKernelRequest`, `onKernelException` and `onConsoleException` events. - -### Options - -In the following section you will find some of the available options you can configure, listed alphabetically. All available options and a more detailed description of each can be found [here](https://docs.sentry.io/clients/php/config/), in the Sentry documentation. - -#### app_path - -The base path to your application. Used to trim prefixes and mark frames of the stack trace as part of your application. - -```yaml -sentry: - options: - app_path: "/path/to/myapp" -``` - -#### environment - -The environment your code is running in (e.g. production). - -```yaml -sentry: - options: - environment: "%kernel.environment%" -``` - -#### error types - -Define which error types should be reported. - -```yaml -sentry: - options: - error_types: E_ALL & ~E_DEPRECATED & ~E_NOTICE -``` - -#### exception_listener - -This is used to replace the default exception listener that this bundle uses. The value must be a FQCN of a class implementing the SentryExceptionListenerInterface interface. See [Create a Custom RequestListener](#create-a-custom-exceptionlistener) for more details. - -```yaml -sentry: - exception_listener: AppBundle\EventListener\MySentryExceptionListener -``` - -#### prefixes - -A list of prefixes to strip from filenames. Often these would be vendor/include paths. - -```yaml -sentry: - options: - prefixes: - - /usr/lib/include -``` - -#### release - -The version of your application. Often this is the Git SHA hash of the commit. - -```yaml -sentry: - options: - release: "beeee2a06521a60e646bbb8fe38702e61e4929bf" -``` - -#### tags - -Define tags for the logged errors. - -```yaml -sentry: - options: - tags: - tag1: tagvalue - tag2: tagvalue -``` - -### Deprecated configuration options - -In previous releases of this bundle, up to 0.8.2, some of the previous options where set outside of the options level of the configuration file. Those still work but are deprecated, and they will be dropped in the stable 1.x release, so **you are advised to abandon them**; to provide forward compatibility, they can still be used alongside the standard syntax, but values must match. This is a list of those options: - -```yaml -sentry: - app_path: ~ - environment: ~ - error_types: ~ - excluded_app_paths: ~ - prefixes: ~ - release: ~ -``` +TODO ## Customization -It is possible to customize the configuration of the user context, as well as modify the client immediately before an exception is captured by wiring up an event subscriber to the events that are emitted by the default configured `RequestListener` (alternatively, you can also just define your own custom exception listener). - -### Create a custom RequestListener +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 two listeners (`RequestListener` and +`ConsoleListener`) that adds some easy default information. 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`. -You can always replace the default `RequestListener` with your own custom listener. To do this, assign a different class to the `exception_listener` property in your Sentry configuration, e.g.: - -```yaml -sentry: - exception_listener: AppBundle\EventListener\MySentryExceptionListener -``` - -... and then define the custom `RequestListener` that implements the `SentryExceptionListenerInterface`, e.g.: - -```php -// src/AppBundle/EventSubscriber/MySentryEventListener.php -namespace AppBundle\EventSubscriber; - -use Sentry\SentryBundle\EventListener\SentryExceptionListenerInterface; -use Symfony\Component\Console\Event\ConsoleExceptionEvent; -use Symfony\Component\EventDispatcher\EventDispatcherInterface; -use Symfony\Component\HttpKernel\Event\GetResponseEvent; -use Symfony\Component\HttpKernel\Event\GetResponseForExceptionEvent; -use Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorageInterface; -use Symfony\Component\Security\Core\Authorization\AuthorizationCheckerInterface; - -class MySentryExceptionListener implements SentryExceptionListenerInterface -{ - // ... - - public function __construct(TokenStorageInterface $tokenStorage = null, AuthorizationCheckerInterface $authorizationChecker = null, \Raven_Client $client = null, array $skipCapture, EventDispatcherInterface $dispatcher = null) - { - // ... - } - - public function onKernelRequest(GetResponseEvent $event) - { - // ... - } - - public function onKernelException(GetResponseForExceptionEvent $event) - { - // ... - } - - public function onConsoleException(ConsoleExceptionEvent $event) - { - // ... - } -} -``` - -As a side note, while the above demonstrates a custom exception listener that -does not extend anything you could choose to extend the default -`RequestListener` and only override the functionality that you want to. - -### Add an EventSubscriber for Sentry Events - -Create a new class, e.g. `MySentryEventSubscriber`: - -```php -// src/AppBundle/EventSubscriber/MySentryEventListener.php -namespace AppBundle\EventSubscriber; - -use Sentry\SentryBundle\Event\SentryUserContextEvent; -use Sentry\SentryBundle\SentrySymfonyEvents; -use Symfony\Component\EventDispatcher\EventSubscriberInterface; - -class MySentryEventSubscriber implements EventSubscriberInterface -{ - /** @var \Raven_Client */ - protected $client; - - public function __construct(\Raven_Client $client) - { - $this->client = $client; - } - - public static function getSubscribedEvents() - { - // return the subscribed events, their methods and priorities - return array( - SentrySymfonyEvents::PRE_CAPTURE => 'onPreCapture', - SentrySymfonyEvents::SET_USER_CONTEXT => 'onSetUserContext' - ); - } - - public function onSetUserContext(SentryUserContextEvent $event) - { - // ... - } - - public function onPreCapture(Event $event) - { - if ($event instanceof GetResponseForExceptionEvent) { - // ... - } - elseif ($event instanceof ConsoleExceptionEvent) { - // ... - } - } -} -``` - -In the example above, if you subscribe to the `PRE_CAPTURE` event you may -get an event object that caters more toward a response to a web request (e.g. -`GetResponseForExceptionEvent`) or one for actions taken at the command line -(e.g. `ConsoleExceptionEvent`). Depending on what and how the code was -invoked, and whether or not you need to distinguish between these events -during pre-capture, it might be best to test for the type of the event (as is -demonstrated above) before you do any relevant processing of the object. - -To configure the above add the following configuration to your services -definitions: - -```yaml -app.my_sentry_event_subscriber: - class: AppBundle\EventSubscriber\MySentryEventSubscriber - arguments: - - '@sentry.client' - tags: - - { name: kernel.event_subscriber } -``` +Those listeners are `final` so not extendable, but you can look at those to know how to add more information to the +current `Scope` and enrich you Sentry events. [Last stable image]: https://poser.pugx.org/sentry/sentry-symfony/version.svg [Last unstable image]: https://poser.pugx.org/sentry/sentry-symfony/v/unstable.svg From 2c40ae8dd301e6ec24c60752f5695f95c1df88d5 Mon Sep 17 00:00:00 2001 From: Alessandro Lai Date: Thu, 24 Jan 2019 12:29:44 +0100 Subject: [PATCH 33/49] Fix installation instructions --- README.md | 27 +++++++++++++++------------ 1 file changed, 15 insertions(+), 12 deletions(-) diff --git a/README.md b/README.md index 7e03135f..3bec6acf 100644 --- a/README.md +++ b/README.md @@ -35,17 +35,22 @@ Use sentry-symfony for: ## Installation ### Step 1: Download the Bundle +You can install this bundle using Composer. Since the Sentry SDK uses HTTPlug to remain transport-agnostic, you need to +manually require two additional packages that provides [`php-http/async-client-implementation`](https://packagist.org/providers/php-http/async-client-implementation) +and [`http-message-implementation`](https://packagist.org/providers/psr/http-message-implementation). -Open a command console, enter your project directory and execute the -following command to download the latest stable version of this bundle: +For example, if you want to install/upgrade using Curl as transport and the PSR-7 implementation by Guzzle, you can use: ```bash -$ composer require sentry/sentry-symfony +composer require sentry/sentry-symfony:^3.0 php-http/curl-client guzzlehttp/psr7 ``` -This command requires you to have Composer installed globally, as explained -in the [installation chapter](https://getcomposer.org/doc/00-intro.md) -of the Composer documentation. +Or, if you want to use only Guzzle 6, you can use: +```bash +composer require sentry/sentry-symfony:^3.0 php-http/guzzle6-adapter guzzlehttp/psr7 +``` + +> TODO: Flex recipe ### Step 2: Enable the Bundle @@ -61,20 +66,18 @@ class AppKernel extends Kernel { public function registerBundles() { - $bundles = array( + $bundles = [ // ... - ); + new Sentry\SentryBundle\SentryBundle(), + ]; - if (in_array($this->getEnvironment(), ['staging', 'prod'], true)) { - $bundles[] = new Sentry\SentryBundle\SentryBundle(); - } // ... } // ... } ``` -Note that, with this snippet of code, the bundle will be enabled only for the `staging` and `prod` environment; adjust it to your needs. It's discouraged to enable this bundle in the `test` environment, because the Sentry client will change the error handler, which is already used by other packages like Symfony's deprecation handler (see [#46](https://github.com/getsentry/sentry-symfony/issues/46) and [#95](https://github.com/getsentry/sentry-symfony/issues/95)). +Note that, unlike before version 3 of this bundle, the bundle will be enabled in all environments. ### Step 3: Configure the SDK From c1b60a132d6b8cadb00ef448f411a2e36e461b33 Mon Sep 17 00:00:00 2001 From: Alessandro Lai Date: Thu, 24 Jan 2019 12:34:37 +0100 Subject: [PATCH 34/49] Describe better the CI job --- .travis.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.travis.yml b/.travis.yml index 0c91b705..376b17c9 100644 --- a/.travis.yml +++ b/.travis.yml @@ -26,6 +26,8 @@ jobs: include: - stage: Test php: 7.1 + env: + - DEPS=--prefer-lowest install: - travis_retry travis_wait composer install --no-interaction --prefer-dist - travis_retry travis_wait composer update --no-interaction --prefer-dist --prefer-stable --prefer-lowest From 5b993cd9376b6024e1d77cbc81dc048df065ea27 Mon Sep 17 00:00:00 2001 From: Alessandro Lai Date: Thu, 24 Jan 2019 12:35:30 +0100 Subject: [PATCH 35/49] Fix typos --- UPGRADE-3.0.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/UPGRADE-3.0.md b/UPGRADE-3.0.md index d2569587..6db7058b 100644 --- a/UPGRADE-3.0.md +++ b/UPGRADE-3.0.md @@ -38,11 +38,11 @@ This is a brief description of the services registered by this bundle: * `Sentry\State\HubInterface`: this is the central root of the SDK; it's the `Hub` that the bundle will instantiate at startup, and the current one too if you do not change it * `Sentry\ClientInterface`: this is the proper client; compared to the 1.x SDK version it's a lot more slimmed down, - since a lot of the stuff has been splitted in separated component, so you probably will not interact with it as much as + since a lot of the stuff has been splitted in separated components, so you probably will not interact with it as much as in the past. You also have to remind you that the client is bound to the `Hub`, and has to be changed there if you want to use it automatically in error reporting * `Sentry\ClientBuilderInterface`: this is the factory that builds the client; you can call its methods to change all - the settings and dependency that will be injected in the latter created client. You can use this service to obtain more + the settings and dependencies that will be injected in the latter created client. You can use this service to obtain more customized clients for your needs * `Sentry\Options`: this class holds all the configuration used in the client and other SDK components; it's populated starting from the bundle configuration From 5d0ff2ece8e8836173f994fc177558e6eac6d4ce Mon Sep 17 00:00:00 2001 From: Alessandro Lai Date: Fri, 25 Jan 2019 12:25:07 +0100 Subject: [PATCH 36/49] Update PHPStan --- composer.json | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/composer.json b/composer.json index a786b245..f314a7ca 100644 --- a/composer.json +++ b/composer.json @@ -33,8 +33,8 @@ "friendsofphp/php-cs-fixer": "^2.8", "jangregor/phpstan-prophecy": "^0.3.0", "php-http/mock-client": "^1.0", - "phpstan/phpstan": "^0.10", - "phpstan/phpstan-phpunit": "^0.10", + "phpstan/phpstan": "^0.11", + "phpstan/phpstan-phpunit": "^0.11", "phpunit/phpunit": "^7", "scrutinizer/ocular": "^1.4", "symfony/expression-language": "^3.0||^4.0" From 52591323d8195b567a223f56a09dd7db0b59168e Mon Sep 17 00:00:00 2001 From: Alessandro Lai Date: Mon, 28 Jan 2019 16:36:41 +0100 Subject: [PATCH 37/49] Map the remaining scalar options --- src/DependencyInjection/Configuration.php | 32 ++++++++++ src/DependencyInjection/SentryExtension.php | 20 +++++++ src/ErrorTypesParser.php | 2 +- .../DependencyInjection/ConfigurationTest.php | 60 +++++++++++++++++++ .../SentryExtensionTest.php | 40 ++++++++++++- 5 files changed, 152 insertions(+), 2 deletions(-) diff --git a/src/DependencyInjection/Configuration.php b/src/DependencyInjection/Configuration.php index 975f6886..dd6b3201 100644 --- a/src/DependencyInjection/Configuration.php +++ b/src/DependencyInjection/Configuration.php @@ -45,12 +45,35 @@ public function getConfigTreeBuilder() $optionsNode ->children() + ->booleanNode('attach_stacktrace')->end() + // TODO -- before_breadcrumb + // TODO -- before_send ->booleanNode('default_integrations')->end() + ->integerNode('context_lines') + ->min(0) + ->max(99) + ->end() + ->booleanNode('enable_compression')->end() + ->scalarNode('environment') + ->defaultValue('%kernel.environment%') + ->cannotBeEmpty() + ->end() + ->scalarNode('error_types') + ->end() + ->arrayNode('excluded_app_path') + ->defaultValue($defaultValues->getExcludedProjectPaths()) + ->scalarPrototype()->end() + ->end() ->arrayNode('excluded_exceptions') ->defaultValue($defaultValues->getExcludedExceptions()) ->scalarPrototype()->end() ->end() // TODO -- integrations + ->scalarNode('logger') + ->end() + ->integerNode('max_breadcrumbs') + ->min(1) + ->end() ->arrayNode('prefixes') ->defaultValue($defaultValues->getPrefixes()) ->scalarPrototype()->end() @@ -58,6 +81,8 @@ public function getConfigTreeBuilder() ->scalarNode('project_root') ->defaultValue($this->getProjectRoot()) ->end() + ->scalarNode('release') + ->end() ->floatNode('sample_rate') ->min(0.0) ->max(1.0) @@ -65,6 +90,13 @@ public function getConfigTreeBuilder() ->integerNode('send_attempts') ->min(1) ->end() + ->booleanNode('send_default_pii')->end() + ->scalarNode('server_name') + ->end() + ->arrayNode('tags') + ->normalizeKeys(false) + ->scalarPrototype() + ->end() ; // Bundle-specific configuration diff --git a/src/DependencyInjection/SentryExtension.php b/src/DependencyInjection/SentryExtension.php index 42d94d63..bff96147 100644 --- a/src/DependencyInjection/SentryExtension.php +++ b/src/DependencyInjection/SentryExtension.php @@ -4,6 +4,7 @@ use Sentry\ClientBuilderInterface; use Sentry\Options; +use Sentry\SentryBundle\ErrorTypesParser; use Sentry\SentryBundle\SentryBundle; use Symfony\Component\Config\Definition\Exception\InvalidConfigurationException; use Symfony\Component\Config\FileLocator; @@ -48,12 +49,22 @@ private function passConfigurationToOptions(ContainerBuilder $container, array $ $processedOptions = $processedConfiguration['options']; $mappableOptions = [ + 'attach_stacktrace', + 'context_lines', 'default_integrations', + 'enable_compression', + 'environment', 'excluded_exceptions', + 'logger', + 'max_breadcrumbs', 'prefixes', 'project_root', + 'release', 'sample_rate', 'send_attempts', + 'send_default_pii', + 'server_name', + 'tags', ]; foreach ($mappableOptions as $optionName) { @@ -62,5 +73,14 @@ private function passConfigurationToOptions(ContainerBuilder $container, array $ $options->addMethodCall($setterMethod, [$processedOptions[$optionName]]); } } + + if (\array_key_exists('excluded_app_path', $processedOptions)) { + $options->addMethodCall('setExcludedProjectPaths', [$processedOptions['excluded_app_path']]); + } + + if (\array_key_exists('error_types', $processedOptions)) { + $parsedValue = (new ErrorTypesParser($processedOptions['error_types']))->parse(); + $options->addMethodCall('setErrorTypes', [$parsedValue]); + } } } diff --git a/src/ErrorTypesParser.php b/src/ErrorTypesParser.php index c31d2afc..c54861ed 100644 --- a/src/ErrorTypesParser.php +++ b/src/ErrorTypesParser.php @@ -73,7 +73,7 @@ private function compute(string $expression): int { // catch anything which could be a security issue if (0 !== preg_match("/[^\d.+*%^|&~<>\/()-]/", $this->expression)) { - throw new \InvalidArgumentException('Wrong value in error types config value'); + throw new \InvalidArgumentException('Wrong value in error types config value:' . $this->expression); } return 0 + (int)eval('return ' . $expression . ';'); diff --git a/test/DependencyInjection/ConfigurationTest.php b/test/DependencyInjection/ConfigurationTest.php index 74c6a013..f4cb06d5 100644 --- a/test/DependencyInjection/ConfigurationTest.php +++ b/test/DependencyInjection/ConfigurationTest.php @@ -11,6 +11,32 @@ class ConfigurationTest extends TestCase { + public const SUPPORTED_SENTRY_OPTIONS_COUNT = 18; + + public function testDataProviderIsMappingTheRightNumberOfOptions(): void + { + $providerData = $this->optionValuesProvider(); + $supportedOptions = \array_unique(\array_column($providerData, 0)); + + $this->assertCount( + self::SUPPORTED_SENTRY_OPTIONS_COUNT, + $supportedOptions, + 'Provider for configuration options mismatch: ' . PHP_EOL . print_r($supportedOptions, true) + ); + } + + public function testInvalidDataProviderIsMappingTheRightNumberOfOptions(): void + { + $providerData = $this->invalidValuesProvider(); + $supportedOptions = \array_unique(\array_column($providerData, 0)); + + $this->assertCount( + self::SUPPORTED_SENTRY_OPTIONS_COUNT, + $supportedOptions, + 'Provider for invalid configuration options mismatch: ' . PHP_EOL . print_r($supportedOptions, true) + ); + } + public function testConfigurationDefaults(): void { $defaultSdkValues = new Options(); @@ -22,9 +48,12 @@ public function testConfigurationDefaults(): void 'console' => 1, ], 'options' => [ + 'environment' => '%kernel.environment%', + 'excluded_app_path' => $defaultSdkValues->getExcludedProjectPaths(), 'excluded_exceptions' => $defaultSdkValues->getExcludedExceptions(), 'prefixes' => $defaultSdkValues->getPrefixes(), 'project_root' => '%kernel.root_dir%/..', + 'tags' => [], ], ]; @@ -33,6 +62,7 @@ public function testConfigurationDefaults(): void } $this->assertEquals($expectedDefaults, $processed); + $this->assertArrayNotHasKey('server_name', $processed['options'], 'server_name has to be fetched at runtime, not before (see #181)'); } /** @@ -49,14 +79,28 @@ public function testOptionValuesProcessing(string $option, $value): void public function optionValuesProvider(): array { return [ + ['attach_stacktrace', true], + ['context_lines', 4], + ['context_lines', 99], ['default_integrations', true], ['default_integrations', false], + ['enable_compression', false], + ['environment', 'staging'], + ['error_types', E_ALL], + ['excluded_app_path', ['some/path']], + ['excluded_exceptions', [\Throwable::class]], + ['logger', 'some-logger'], + ['max_breadcrumbs', 15], ['prefixes', ['some-string']], ['project_root', '/some/dir'], + ['release', 'abc0123'], ['sample_rate', 0], ['sample_rate', 1], ['send_attempts', 1], ['send_attempts', 999], + ['send_default_pii', true], + ['server_name', 'server001.example.com'], + ['tags', ['tag-name' => 'value']], ]; } @@ -75,15 +119,31 @@ public function testInvalidValues(string $option, $value): void public function invalidValuesProvider(): array { return [ + ['attach_stacktrace', 'string'], + ['context_lines', -1], + ['context_lines', 99999], + ['context_lines', 'string'], ['default_integrations', 'true'], ['default_integrations', 1], + ['enable_compression', 'string'], + ['environment', ''], + ['error_types', []], + ['excluded_app_path', 'some/single/path'], + ['excluded_exceptions', 'some-string'], + ['logger', []], + ['max_breadcrumbs', -1], + ['max_breadcrumbs', 'string'], ['prefixes', 'string'], ['project_root', []], + ['release', []], ['sample_rate', 1.1], ['sample_rate', -1], ['send_attempts', 1.5], ['send_attempts', 0], ['send_attempts', -1], + ['send_default_pii', 'false'], + ['server_name', []], + ['tags', 'invalid-unmapped-tag'], ]; } diff --git a/test/DependencyInjection/SentryExtensionTest.php b/test/DependencyInjection/SentryExtensionTest.php index 31c28071..fa5bef7b 100644 --- a/test/DependencyInjection/SentryExtensionTest.php +++ b/test/DependencyInjection/SentryExtensionTest.php @@ -16,11 +16,22 @@ class SentryExtensionTest extends TestCase { - private const SUPPORTED_SENTRY_OPTIONS_COUNT = 35; private const REQUEST_LISTENER_TEST_PUBLIC_ALIAS = 'sentry.request_listener.public_alias'; private const CONSOLE_LISTENER_TEST_PUBLIC_ALIAS = 'sentry.console_listener.public_alias'; private const OPTIONS_TEST_PUBLIC_ALIAS = 'sentry.options.public_alias'; + public function testDataProviderIsMappingTheRightNumberOfOptions(): void + { + $providerData = $this->optionsValueProvider(); + $supportedOptions = \array_unique(\array_column($providerData, 0)); + + $this->assertCount( + ConfigurationTest::SUPPORTED_SENTRY_OPTIONS_COUNT, + $supportedOptions, + 'Provider for configuration options mismatch: ' . PHP_EOL . print_r($supportedOptions, true) + ); + } + public function testOptionsDefaultValues(): void { $container = $this->getContainer(); @@ -32,6 +43,7 @@ public function testOptionsDefaultValues(): void $this->assertSame('kernel/root/..', $options->getProjectRoot()); } $this->assertNull($options->getDsn()); + $this->assertSame('test', $options->getEnvironment()); $this->assertSame(1, $container->getParameter('sentry.listener_priorities.request')); $this->assertSame(1, $container->getParameter('sentry.listener_priorities.console')); @@ -70,15 +82,41 @@ public function testValuesArePassedToOptions(string $name, $value, string $gette public function optionsValueProvider(): array { return [ + ['attach_stacktrace', true, 'shouldAttachStacktrace'], + ['context_lines', 1], ['default_integrations', false, 'hasDefaultIntegrations'], + ['enable_compression', false, 'isCompressionEnabled'], + ['environment', 'staging'], + ['error_types', E_ALL & ! E_NOTICE], + ['excluded_app_path', ['some/path'], 'getExcludedProjectPaths'], ['excluded_exceptions', [\Throwable::class]], + ['logger', 'sentry-logger'], + ['max_breadcrumbs', 15], ['prefixes', ['/some/path/prefix/']], ['project_root', '/some/project/'], + ['release', 'abc0123'], ['sample_rate', 0.5], ['send_attempts', 2], + ['send_default_pii', true, 'shouldSendDefaultPii'], + ['server_name', 'server.example.com'], + ['tags', ['tag-name' => 'tag-value']], ]; } + public function testErrorTypesAreParsed(): void + { + $container = $this->getContainer(['options' => ['error_types' => 'E_ALL & ~E_NOTICE']]); + + $this->assertSame(E_ALL & ~E_NOTICE, $this->getOptionsFrom($container)->getErrorTypes()); + + $defaultContainer = $this->getContainer(); + $this->assertNotEquals( + $this->getOptionsFrom($defaultContainer)->getErrorTypes(), + $this->getOptionsFrom($container)->getErrorTypes(), + 'Bad data: value is same as default' + ); + } + /** * @dataProvider emptyDsnValueProvider */ From 632bdb09f2765f64ec944dffddf4d1e35a9343b0 Mon Sep 17 00:00:00 2001 From: Alessandro Lai Date: Mon, 28 Jan 2019 16:42:09 +0100 Subject: [PATCH 38/49] Fix PHPStan in CI --- test/DependencyInjection/SentryExtensionTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/DependencyInjection/SentryExtensionTest.php b/test/DependencyInjection/SentryExtensionTest.php index fa5bef7b..60c1d1b2 100644 --- a/test/DependencyInjection/SentryExtensionTest.php +++ b/test/DependencyInjection/SentryExtensionTest.php @@ -87,7 +87,7 @@ public function optionsValueProvider(): array ['default_integrations', false, 'hasDefaultIntegrations'], ['enable_compression', false, 'isCompressionEnabled'], ['environment', 'staging'], - ['error_types', E_ALL & ! E_NOTICE], + ['error_types', E_ALL & ~E_NOTICE], ['excluded_app_path', ['some/path'], 'getExcludedProjectPaths'], ['excluded_exceptions', [\Throwable::class]], ['logger', 'sentry-logger'], From e4e333f7de3d135ec79b1d54c3b253a52e3080b4 Mon Sep 17 00:00:00 2001 From: Alessandro Lai Date: Fri, 15 Feb 2019 16:10:52 +0100 Subject: [PATCH 39/49] Require SDK beta 2 --- composer.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/composer.json b/composer.json index f314a7ca..7fe67799 100644 --- a/composer.json +++ b/composer.json @@ -21,7 +21,7 @@ "require": { "php": "^7.1", "jean85/pretty-package-versions": "^1.0", - "sentry/sentry": "^2.0-beta", + "sentry/sentry": "^2.0-beta2", "symfony/config": "^3.0||^4.0", "symfony/console": "^3.3||^4.0", "symfony/dependency-injection": "^3.0||^4.0", From ce42bb8a2067e4a381fc5cdf9eed59dcf2cfc0d3 Mon Sep 17 00:00:00 2001 From: Alessandro Lai Date: Fri, 15 Feb 2019 16:21:56 +0100 Subject: [PATCH 40/49] Fix tests after updating to beta 2 --- src/DependencyInjection/Configuration.php | 4 ++-- src/DependencyInjection/SentryExtension.php | 4 ++-- test/DependencyInjection/ConfigurationTest.php | 6 +++--- test/DependencyInjection/SentryExtensionTest.php | 4 ++-- 4 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/DependencyInjection/Configuration.php b/src/DependencyInjection/Configuration.php index dd6b3201..53055367 100644 --- a/src/DependencyInjection/Configuration.php +++ b/src/DependencyInjection/Configuration.php @@ -60,8 +60,8 @@ public function getConfigTreeBuilder() ->end() ->scalarNode('error_types') ->end() - ->arrayNode('excluded_app_path') - ->defaultValue($defaultValues->getExcludedProjectPaths()) + ->arrayNode('in_app_exclude') + ->defaultValue($defaultValues->getInAppExcludedPaths()) ->scalarPrototype()->end() ->end() ->arrayNode('excluded_exceptions') diff --git a/src/DependencyInjection/SentryExtension.php b/src/DependencyInjection/SentryExtension.php index bff96147..b2ccc4f3 100644 --- a/src/DependencyInjection/SentryExtension.php +++ b/src/DependencyInjection/SentryExtension.php @@ -74,8 +74,8 @@ private function passConfigurationToOptions(ContainerBuilder $container, array $ } } - if (\array_key_exists('excluded_app_path', $processedOptions)) { - $options->addMethodCall('setExcludedProjectPaths', [$processedOptions['excluded_app_path']]); + if (\array_key_exists('in_app_exclude', $processedOptions)) { + $options->addMethodCall('setInAppExcludedPaths', [$processedOptions['in_app_exclude']]); } if (\array_key_exists('error_types', $processedOptions)) { diff --git a/test/DependencyInjection/ConfigurationTest.php b/test/DependencyInjection/ConfigurationTest.php index f4cb06d5..c1c12ce6 100644 --- a/test/DependencyInjection/ConfigurationTest.php +++ b/test/DependencyInjection/ConfigurationTest.php @@ -49,7 +49,7 @@ public function testConfigurationDefaults(): void ], 'options' => [ 'environment' => '%kernel.environment%', - 'excluded_app_path' => $defaultSdkValues->getExcludedProjectPaths(), + 'in_app_exclude' => $defaultSdkValues->getInAppExcludedPaths(), 'excluded_exceptions' => $defaultSdkValues->getExcludedExceptions(), 'prefixes' => $defaultSdkValues->getPrefixes(), 'project_root' => '%kernel.root_dir%/..', @@ -87,7 +87,7 @@ public function optionValuesProvider(): array ['enable_compression', false], ['environment', 'staging'], ['error_types', E_ALL], - ['excluded_app_path', ['some/path']], + ['in_app_exclude', ['some/path']], ['excluded_exceptions', [\Throwable::class]], ['logger', 'some-logger'], ['max_breadcrumbs', 15], @@ -128,7 +128,7 @@ public function invalidValuesProvider(): array ['enable_compression', 'string'], ['environment', ''], ['error_types', []], - ['excluded_app_path', 'some/single/path'], + ['in_app_exclude', 'some/single/path'], ['excluded_exceptions', 'some-string'], ['logger', []], ['max_breadcrumbs', -1], diff --git a/test/DependencyInjection/SentryExtensionTest.php b/test/DependencyInjection/SentryExtensionTest.php index 60c1d1b2..9b910452 100644 --- a/test/DependencyInjection/SentryExtensionTest.php +++ b/test/DependencyInjection/SentryExtensionTest.php @@ -88,7 +88,7 @@ public function optionsValueProvider(): array ['enable_compression', false, 'isCompressionEnabled'], ['environment', 'staging'], ['error_types', E_ALL & ~E_NOTICE], - ['excluded_app_path', ['some/path'], 'getExcludedProjectPaths'], + ['in_app_exclude', ['/some/path'], 'getInAppExcludedPaths'], ['excluded_exceptions', [\Throwable::class]], ['logger', 'sentry-logger'], ['max_breadcrumbs', 15], @@ -180,7 +180,7 @@ private function getContainer(array $configuration = []): Container $containerBuilder = new ContainerBuilder(); $containerBuilder->setParameter('kernel.root_dir', 'kernel/root'); if (method_exists(Kernel::class, 'getProjectDir')) { - $containerBuilder->setParameter('kernel.project_dir', '/dir/project/root'); + $containerBuilder->setParameter('kernel.project_dir', '/dir/project/root/'); } $containerBuilder->setParameter('kernel.environment', 'test'); From 19c34100b1ee89f133b06266436fe20f1be7a41e Mon Sep 17 00:00:00 2001 From: Alessandro Lai Date: Thu, 21 Feb 2019 16:25:10 +0100 Subject: [PATCH 41/49] Fix test; remove duplicated test --- .../SentryExtensionTest.php | 40 ++----------------- 1 file changed, 3 insertions(+), 37 deletions(-) diff --git a/test/DependencyInjection/SentryExtensionTest.php b/test/DependencyInjection/SentryExtensionTest.php index 9b910452..a346f93d 100644 --- a/test/DependencyInjection/SentryExtensionTest.php +++ b/test/DependencyInjection/SentryExtensionTest.php @@ -120,19 +120,18 @@ public function testErrorTypesAreParsed(): void /** * @dataProvider emptyDsnValueProvider */ - public function test_that_it_ignores_empty_dsn_value($emptyDsn) + public function test_that_it_ignores_empty_dsn_value($emptyDsn): void { - $this->markTestIncomplete(); $container = $this->getContainer( [ 'dsn' => $emptyDsn, ] ); - $this->assertNull($container->getParameter('sentry.dsn')); + $this->assertNull($this->getOptionsFrom($container)->getDsn()); } - public function emptyDsnValueProvider() + public function emptyDsnValueProvider(): array { return [ [null], @@ -142,39 +141,6 @@ public function emptyDsnValueProvider() ]; } - public function test_that_it_has_proper_event_listener_tags_for_exception_listener() - { - $this->markTestIncomplete(); - $containerBuilder = new ContainerBuilder(); - $extension = new SentryExtension(); - $extension->load([], $containerBuilder); - - $definition = $containerBuilder->getDefinition('sentry.exception_listener'); - $tags = $definition->getTag('kernel.event_listener'); - - $this->assertSame( - [ - [ - 'event' => 'kernel.request', - 'method' => 'onKernelRequest', - 'priority' => '%sentry.listener_priorities.request%', - ], - [ - 'event' => 'kernel.exception', - 'method' => 'onKernelException', - 'priority' => '%sentry.listener_priorities.kernel_exception%', - ], - ['event' => 'console.command', 'method' => 'onConsoleCommand'], - [ - 'event' => 'console.error', - 'method' => 'onConsoleError', - 'priority' => '%sentry.listener_priorities.console_exception%', - ], - ], - $tags - ); - } - private function getContainer(array $configuration = []): Container { $containerBuilder = new ContainerBuilder(); From a593ac82156072c29d671eadc6f1c52bb6492aba Mon Sep 17 00:00:00 2001 From: Alessandro Lai Date: Thu, 21 Feb 2019 16:39:40 +0100 Subject: [PATCH 42/49] Refactor and move some listener tests --- src/EventListener/RequestListener.php | 9 +- test/EventListener/ExceptionListenerTest.php | 391 ------------------- test/EventListener/RequestListenerTest.php | 204 ++++++++++ 3 files changed, 209 insertions(+), 395 deletions(-) diff --git a/src/EventListener/RequestListener.php b/src/EventListener/RequestListener.php index cc1a4937..2609dbc8 100644 --- a/src/EventListener/RequestListener.php +++ b/src/EventListener/RequestListener.php @@ -53,14 +53,15 @@ public function onKernelRequest(GetResponseEvent $event): void return; } - if (null === $this->tokenStorage || null === $this->authorizationChecker) { - return; - } + $token = null; - $token = $this->tokenStorage->getToken(); + if ($this->tokenStorage instanceof TokenStorageInterface) { + $token = $this->tokenStorage->getToken(); + } if ( null !== $token + && null !== $this->authorizationChecker && $token->isAuthenticated() && $this->authorizationChecker->isGranted(AuthenticatedVoter::IS_AUTHENTICATED_REMEMBERED) ) { diff --git a/test/EventListener/ExceptionListenerTest.php b/test/EventListener/ExceptionListenerTest.php index 35106fcc..98e920ba 100644 --- a/test/EventListener/ExceptionListenerTest.php +++ b/test/EventListener/ExceptionListenerTest.php @@ -14,15 +14,10 @@ use Symfony\Component\EventDispatcher\EventDispatcherInterface; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpFoundation\RequestStack; -use Symfony\Component\HttpKernel\Event\GetResponseEvent; use Symfony\Component\HttpKernel\Event\GetResponseForExceptionEvent; use Symfony\Component\HttpKernel\Exception\HttpException; -use Symfony\Component\HttpKernel\HttpKernelInterface; use Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorageInterface; -use Symfony\Component\Security\Core\Authentication\Token\TokenInterface; use Symfony\Component\Security\Core\Authorization\AuthorizationCheckerInterface; -use Symfony\Component\Security\Core\Authorization\Voter\AuthenticatedVoter; -use Symfony\Component\Security\Core\User\UserInterface; class ExceptionListenerTest extends TestCase { @@ -65,392 +60,6 @@ public function setUp() $this->containerBuilder = $containerBuilder; } - public function test_that_it_is_an_instance_of_sentry_exception_listener() - { - $this->containerBuilder->compile(); - $listener = $this->getListener(); - - $this->assertInstanceOf(RequestListener::class, $listener); - } - - public function test_that_user_data_is_not_set_on_subrequest() - { - $mockEvent = $this->createMock(GetResponseEvent::class); - - $mockEvent - ->expects($this->once()) - ->method('getRequestType') - ->willReturn(HttpKernelInterface::SUB_REQUEST); - - $this->mockSentryClient - ->expects($this->never()) - ->method('set_user_data') - ->withAnyParameters(); - - $this->mockEventDispatcher - ->expects($this->never()) - ->method('dispatch') - ->withAnyParameters(); - - $this->containerBuilder->compile(); - $listener = $this->getListener(); - $listener->onKernelRequest($mockEvent); - } - - public function test_that_user_data_is_not_set_if_token_storage_not_present() - { - $this->containerBuilder->set('security.token_storage', null); - - $mockEvent = $this->createMock(GetResponseEvent::class); - - $mockEvent - ->expects($this->once()) - ->method('getRequestType') - ->willReturn(HttpKernelInterface::MASTER_REQUEST); - - $this->mockSentryClient - ->expects($this->never()) - ->method('set_user_data') - ->withAnyParameters(); - - $this->mockEventDispatcher - ->expects($this->never()) - ->method('dispatch') - ->withAnyParameters(); - - $this->assertFalse($this->containerBuilder->has('security.token_storage')); - - $this->containerBuilder->compile(); - - $listener = $this->getListener(); - $listener->onKernelRequest($mockEvent); - } - - public function test_that_user_data_is_not_set_if_authorization_checker_not_present() - { - $this->containerBuilder->set('security.authorization_checker', null); - - $mockEvent = $this->createMock(GetResponseEvent::class); - - $mockEvent - ->expects($this->once()) - ->method('getRequestType') - ->willReturn(HttpKernelInterface::MASTER_REQUEST); - - $this->mockSentryClient - ->expects($this->never()) - ->method('set_user_data') - ->withAnyParameters(); - - $this->mockEventDispatcher - ->expects($this->never()) - ->method('dispatch') - ->withAnyParameters(); - - $this->containerBuilder->compile(); - - $this->assertFalse($this->containerBuilder->has('security.authorization_checker')); - - $listener = $this->getListener(); - $listener->onKernelRequest($mockEvent); - } - - public function test_that_user_data_is_not_set_if_token_not_present() - { - $user = $this->createMock(UserInterface::class); - $user->method('getUsername') - ->willReturn('username'); - - $mockEvent = $this->createMock(GetResponseEvent::class); - - $mockEvent - ->expects($this->once()) - ->method('getRequestType') - ->willReturn(HttpKernelInterface::MASTER_REQUEST); - - $this->mockAuthorizationChecker - ->method('isGranted') - ->with($this->identicalTo(AuthenticatedVoter::IS_AUTHENTICATED_REMEMBERED)) - ->willReturn(true); - - $this->mockTokenStorage - ->method('getToken') - ->willReturn(null); - - $this->mockSentryClient - ->expects($this->never()) - ->method('set_user_data') - ->withAnyParameters(); - - $this->mockEventDispatcher - ->expects($this->never()) - ->method('dispatch') - ->withAnyParameters(); - - $this->containerBuilder->compile(); - $listener = $this->getListener(); - $listener->onKernelRequest($mockEvent); - } - - public function test_that_user_data_is_not_set_if_not_authorized() - { - $user = $this->createMock(UserInterface::class); - $user->method('getUsername')->willReturn('username'); - - $mockToken = $this->createMock(TokenInterface::class); - - $mockToken - ->method('getUser') - ->willReturn($user); - - $mockEvent = $this->createMock(GetResponseEvent::class); - - $mockEvent->expects($this->once()) - ->method('getRequestType') - ->willReturn(HttpKernelInterface::MASTER_REQUEST); - - $this->mockAuthorizationChecker - ->method('isGranted') - ->with($this->identicalTo(AuthenticatedVoter::IS_AUTHENTICATED_REMEMBERED)) - ->willReturn(false); - - $this->mockTokenStorage - ->method('getToken') - ->willReturn($mockToken); - - $this->mockSentryClient - ->expects($this->never()) - ->method('set_user_data') - ->withAnyParameters(); - - $this->mockEventDispatcher - ->expects($this->never()) - ->method('dispatch') - ->withAnyParameters(); - - $this->containerBuilder->compile(); - $listener = $this->getListener(); - $listener->onKernelRequest($mockEvent); - } - - public function test_that_username_is_set_from_user_interface_if_token_present_and_user_set_as_user_interface() - { - $user = $this->createMock(UserInterface::class); - $user->method('getUsername')->willReturn('username'); - - $mockToken = $this->createMock(TokenInterface::class); - - $mockToken - ->method('getUser') - ->willReturn($user); - - $mockToken - ->method('isAuthenticated') - ->willReturn(true); - - $mockEvent = $this->createMock(GetResponseEvent::class); - - $mockEvent - ->expects($this->once()) - ->method('getRequestType') - ->willReturn(HttpKernelInterface::MASTER_REQUEST); - - $this - ->mockAuthorizationChecker - ->method('isGranted') - ->with($this->identicalTo(AuthenticatedVoter::IS_AUTHENTICATED_REMEMBERED)) - ->willReturn(true); - - $this->mockTokenStorage - ->method('getToken') - ->willReturn($mockToken); - - $this - ->mockSentryClient - ->expects($this->once()) - ->method('set_user_data') - ->with($this->identicalTo('username')); - - $this - ->mockEventDispatcher - ->expects($this->once()) - ->method('dispatch') - ->with( - $this->identicalTo(SentrySymfonyEvents::SET_USER_CONTEXT), - $this->isInstanceOf(SentryUserContextEvent::class) - ); - - $this->containerBuilder->compile(); - $listener = $this->getListener(); - $listener->onKernelRequest($mockEvent); - } - - public function test_that_username_is_set_from_user_interface_if_token_present_and_user_set_as_string() - { - $mockToken = $this->createMock(TokenInterface::class); - - $mockToken - ->method('getUser') - ->willReturn('some_user'); - - $mockToken - ->method('isAuthenticated') - ->willReturn(true); - - $mockEvent = $this->createMock(GetResponseEvent::class); - - $mockEvent - ->expects($this->once()) - ->method('getRequestType') - ->willReturn(HttpKernelInterface::MASTER_REQUEST); - - $this->mockAuthorizationChecker - ->method('isGranted') - ->with($this->identicalTo(AuthenticatedVoter::IS_AUTHENTICATED_REMEMBERED)) - ->willReturn(true); - - $this->mockTokenStorage - ->method('getToken') - ->willReturn($mockToken); - - $this->mockSentryClient - ->expects($this->once()) - ->method('set_user_data') - ->with($this->identicalTo('some_user')); - - $this->mockEventDispatcher - ->expects($this->once()) - ->method('dispatch') - ->with( - $this->identicalTo(SentrySymfonyEvents::SET_USER_CONTEXT), - $this->isInstanceOf(SentryUserContextEvent::class) - ); - - $this->containerBuilder->compile(); - $listener = $this->getListener(); - $listener->onKernelRequest($mockEvent); - } - - public function test_that_username_is_set_from_user_interface_if_token_present_and_user_set_object_with_to_string() - { - $mockUser = $this->getMockBuilder('stdClass') - ->setMethods(['__toString']) - ->getMock(); - - $mockUser - ->expects($this->once()) - ->method('__toString') - ->willReturn('std_user'); - - $mockToken = $this->createMock(TokenInterface::class); - - $mockToken - ->method('getUser') - ->willReturn($mockUser); - - $mockToken - ->method('isAuthenticated') - ->willReturn(true); - - $mockEvent = $this->createMock(GetResponseEvent::class); - - $mockEvent - ->expects($this->once()) - ->method('getRequestType') - ->willReturn(HttpKernelInterface::MASTER_REQUEST); - - $this->mockAuthorizationChecker - ->method('isGranted') - ->with($this->identicalTo(AuthenticatedVoter::IS_AUTHENTICATED_REMEMBERED)) - ->willReturn(true); - - $this->mockTokenStorage - ->method('getToken') - ->willReturn($mockToken); - - $this->mockSentryClient - ->expects($this->once()) - ->method('set_user_data') - ->with($this->identicalTo('std_user')); - - $this->mockEventDispatcher - ->expects($this->once()) - ->method('dispatch') - ->with( - $this->identicalTo(SentrySymfonyEvents::SET_USER_CONTEXT), - $this->isInstanceOf(SentryUserContextEvent::class) - ); - - $this->containerBuilder->compile(); - $listener = $this->getListener(); - $listener->onKernelRequest($mockEvent); - } - - public function test_that_ip_is_set_from_request_stack() - { - $mockToken = $this->createMock(TokenInterface::class); - - $mockToken - ->method('getUser') - ->willReturn('some_user'); - - $mockToken - ->method('isAuthenticated') - ->willReturn(true); - - $mockEvent = $this->createMock(GetResponseEvent::class); - - $this->requestStack->push(new Request([], [], [], [], [], [ - 'REMOTE_ADDR' => '1.2.3.4', - ])); - - $mockEvent - ->expects($this->once()) - ->method('getRequestType') - ->willReturn(HttpKernelInterface::MASTER_REQUEST); - - $this->mockAuthorizationChecker - ->method('isGranted') - ->with($this->identicalTo(AuthenticatedVoter::IS_AUTHENTICATED_REMEMBERED)) - ->willReturn(true); - - $this->mockTokenStorage - ->method('getToken') - ->willReturn($mockToken); - - $this->mockSentryClient - ->expects($this->once()) - ->method('set_user_data') - ->with($this->identicalTo('some_user'), null, ['ip_address' => '1.2.3.4']); - - $this->containerBuilder->compile(); - $listener = $this->getListener(); - $listener->onKernelRequest($mockEvent); - } - - public function test_regression_with_unauthenticated_user_token_PR_78() - { - $mockToken = $this->createMock(TokenInterface::class); - $mockToken - ->method('isAuthenticated') - ->willReturn(false); - - $mockEvent = $this->createMock(GetResponseEvent::class); - - $mockEvent - ->expects($this->once()) - ->method('getRequestType') - ->willReturn(HttpKernelInterface::MASTER_REQUEST); - - $this->mockTokenStorage - ->method('getToken') - ->willReturn($mockToken); - - $this->containerBuilder->compile(); - $listener = $this->getListener(); - $listener->onKernelRequest($mockEvent); - } - public function test_that_it_does_not_report_http_exception_if_included_in_capture_skip() { $mockEvent = $this->createMock(GetResponseForExceptionEvent::class); diff --git a/test/EventListener/RequestListenerTest.php b/test/EventListener/RequestListenerTest.php index aa59c2fd..e59d743f 100644 --- a/test/EventListener/RequestListenerTest.php +++ b/test/EventListener/RequestListenerTest.php @@ -91,6 +91,180 @@ public function userDataProvider(): \Generator yield [new ToStringUser('john-doe')]; } + public function testOnKernelRequestUsernameIsNotSetIfTokenStorageIsAbsent(): void + { + $authorizationChecker = $this->prophesize(AuthorizationCheckerInterface::class); + $event = $this->prophesize(GetResponseEvent::class); + $request = $this->prophesize(Request::class); + + $event->isMasterRequest() + ->willReturn(true); + + $authorizationChecker->isGranted(AuthenticatedVoter::IS_AUTHENTICATED_REMEMBERED) + ->shouldNotBeCalled(); + + $event->getRequest() + ->willReturn($request->reveal()); + $request->getClientIp() + ->willReturn('1.2.3.4'); + + $listener = new RequestListener( + $this->currentHub->reveal(), + null, + $authorizationChecker->reveal() + ); + + $listener->onKernelRequest($event->reveal()); + + $expectedUserData = [ + 'ip_address' => '1.2.3.4', + ]; + $this->assertEquals($expectedUserData, $this->currentScope->getUser()); + } + + public function testOnKernelRequestUsernameIsNotSetIfAuthorizationCheckerIsAbsent(): void + { + $tokenStorage = $this->prophesize(TokenStorageInterface::class); + $event = $this->prophesize(GetResponseEvent::class); + $request = $this->prophesize(Request::class); + + $event->isMasterRequest() + ->willReturn(true); + + $tokenStorage->getToken() + ->willReturn($this->prophesize(TokenInterface::class)->reveal()); + + $event->getRequest() + ->willReturn($request->reveal()); + $request->getClientIp() + ->willReturn('1.2.3.4'); + + $listener = new RequestListener( + $this->currentHub->reveal(), + $tokenStorage->reveal(), + null + ); + + $listener->onKernelRequest($event->reveal()); + + $expectedUserData = [ + 'ip_address' => '1.2.3.4', + ]; + $this->assertEquals($expectedUserData, $this->currentScope->getUser()); + } + + public function testOnKernelRequestUsernameIsNotSetIfTokenIsAbsent(): void + { + $tokenStorage = $this->prophesize(TokenStorageInterface::class); + $authorizationChecker = $this->prophesize(AuthorizationCheckerInterface::class); + $event = $this->prophesize(GetResponseEvent::class); + $request = $this->prophesize(Request::class); + + $event->isMasterRequest() + ->willReturn(true); + + $tokenStorage->getToken() + ->willReturn(null); + + $authorizationChecker->isGranted(AuthenticatedVoter::IS_AUTHENTICATED_REMEMBERED) + ->shouldNotBeCalled(); + + $event->getRequest() + ->willReturn($request->reveal()); + $request->getClientIp() + ->willReturn('1.2.3.4'); + + $listener = new RequestListener( + $this->currentHub->reveal(), + $tokenStorage->reveal(), + $authorizationChecker->reveal() + ); + + $listener->onKernelRequest($event->reveal()); + + $expectedUserData = [ + 'ip_address' => '1.2.3.4', + ]; + $this->assertEquals($expectedUserData, $this->currentScope->getUser()); + } + + /** + * @ticket #78 + */ + public function testOnKernelRequestUsernameIsNotSetIfTokenIsNotAuthenticated(): void + { + $tokenStorage = $this->prophesize(TokenStorageInterface::class); + $authorizationChecker = $this->prophesize(AuthorizationCheckerInterface::class); + $token = $this->prophesize(TokenInterface::class); + $event = $this->prophesize(GetResponseEvent::class); + $request = $this->prophesize(Request::class); + + $event->isMasterRequest() + ->willReturn(true); + + $tokenStorage->getToken() + ->willReturn($token->reveal()); + + $token->isAuthenticated() + ->willReturn(false); + + $authorizationChecker->isGranted(AuthenticatedVoter::IS_AUTHENTICATED_REMEMBERED) + ->shouldNotBeCalled(); + + $event->getRequest() + ->willReturn($request->reveal()); + $request->getClientIp() + ->willReturn('1.2.3.4'); + + $listener = new RequestListener( + $this->currentHub->reveal(), + $tokenStorage->reveal(), + $authorizationChecker->reveal() + ); + + $listener->onKernelRequest($event->reveal()); + + $expectedUserData = [ + 'ip_address' => '1.2.3.4', + ]; + $this->assertEquals($expectedUserData, $this->currentScope->getUser()); + } + + public function testOnKernelRequestUsernameIsNotSetIfUserIsNotRemembered(): void + { + $tokenStorage = $this->prophesize(TokenStorageInterface::class); + $authorizationChecker = $this->prophesize(AuthorizationCheckerInterface::class); + $event = $this->prophesize(GetResponseEvent::class); + $request = $this->prophesize(Request::class); + + $event->isMasterRequest() + ->willReturn(true); + + $tokenStorage->getToken() + ->willReturn(null); + + $authorizationChecker->isGranted(AuthenticatedVoter::IS_AUTHENTICATED_REMEMBERED) + ->willReturn(false); + + $event->getRequest() + ->willReturn($request->reveal()); + $request->getClientIp() + ->willReturn('1.2.3.4'); + + $listener = new RequestListener( + $this->currentHub->reveal(), + $tokenStorage->reveal(), + $authorizationChecker->reveal() + ); + + $listener->onKernelRequest($event->reveal()); + + $expectedUserData = [ + 'ip_address' => '1.2.3.4', + ]; + $this->assertEquals($expectedUserData, $this->currentScope->getUser()); + } + public function testOnKernelControllerAddsRouteTag(): void { $request = new Request(); @@ -112,6 +286,36 @@ public function testOnKernelControllerAddsRouteTag(): void $this->assertSame(['route' => 'sf-route'], $this->currentScope->getTags()); } + + public function testOnKernelRequestUserDataAndTagsAreNotSetInSubRequest(): void + { + $this->currentHub->getScope() + ->shouldNotBeCalled(); + + $tokenStorage = $this->prophesize(TokenStorageInterface::class); + $authorizationChecker = $this->prophesize(AuthorizationCheckerInterface::class); + $event = $this->prophesize(GetResponseEvent::class); + + $event->isMasterRequest() + ->willReturn(false); + + $tokenStorage->getToken() + ->shouldNotBeCalled(); + + $authorizationChecker->isGranted(AuthenticatedVoter::IS_AUTHENTICATED_REMEMBERED) + ->shouldNotBeCalled(); + + $listener = new RequestListener( + $this->currentHub->reveal(), + $tokenStorage->reveal(), + $authorizationChecker->reveal() + ); + + $listener->onKernelRequest($event->reveal()); + + $this->assertEmpty($this->currentScope->getUser()); + $this->assertEmpty($this->currentScope->getTags()); + } } class ToStringUser From ae197d00abe25bae62a19a307b41e4332a639541 Mon Sep 17 00:00:00 2001 From: Alessandro Lai Date: Thu, 21 Feb 2019 17:15:12 +0100 Subject: [PATCH 43/49] Remove old test; improve parser test --- test/ErrorTypesParserTest.php | 23 +- test/EventListener/ExceptionListenerTest.php | 284 ------------------- 2 files changed, 18 insertions(+), 289 deletions(-) delete mode 100644 test/EventListener/ExceptionListenerTest.php diff --git a/test/ErrorTypesParserTest.php b/test/ErrorTypesParserTest.php index ec13306d..1a049bbd 100644 --- a/test/ErrorTypesParserTest.php +++ b/test/ErrorTypesParserTest.php @@ -7,13 +7,26 @@ class ErrorTypesParserTest extends TestCase { - public function test_error_types_parser() + /** + * @dataProvider parsableValueProvider + */ + public function testParse(string $value, int $expected): void { - $ex = new ErrorTypesParser('E_ALL & ~E_DEPRECATED & ~E_NOTICE'); - $this->assertEquals($ex->parse(), E_ALL & ~E_DEPRECATED & ~E_NOTICE); + $ex = new ErrorTypesParser($value); + $this->assertEquals($expected, $ex->parse()); } - public function test_error_types_parser_throws_exception_for_unwanted_values() + public function parsableValueProvider(): array + { + return [ + ['E_ALL', E_ALL], + ['E_ALL & ~E_DEPRECATED & ~E_NOTICE', E_ALL & ~E_DEPRECATED & ~E_NOTICE], + ['-1', -1], + [-1, -1], + ]; + } + + public function testParseStopsAtDangerousValues(): void { $ex = new ErrorTypesParser('exec(something-dangerous)'); @@ -21,7 +34,7 @@ public function test_error_types_parser_throws_exception_for_unwanted_values() $ex->parse(); } - public function test_error_types_parser_throws_exception_for_unparsable_values() + public function testErrorTypesParserThrowsExceptionForUnparsableValues(): void { $ex = new ErrorTypesParser('something-wrong'); diff --git a/test/EventListener/ExceptionListenerTest.php b/test/EventListener/ExceptionListenerTest.php deleted file mode 100644 index 98e920ba..00000000 --- a/test/EventListener/ExceptionListenerTest.php +++ /dev/null @@ -1,284 +0,0 @@ -markTestSkipped('To be refactored'); - $this->mockTokenStorage = $this->createMock(TokenStorageInterface::class); - $this->mockAuthorizationChecker = $this->createMock(AuthorizationCheckerInterface::class); - $this->mockEventDispatcher = $this->createMock(EventDispatcherInterface::class); - $this->requestStack = new RequestStack(); - - $containerBuilder = new ContainerBuilder(); - $containerBuilder->setParameter('kernel.root_dir', 'kernel/root'); - $containerBuilder->setParameter('kernel.environment', 'test'); - - $containerBuilder->set('request_stack', $this->requestStack); - $containerBuilder->set('security.token_storage', $this->mockTokenStorage); - $containerBuilder->set('security.authorization_checker', $this->mockAuthorizationChecker); - $containerBuilder->set('event_dispatcher', $this->mockEventDispatcher); - $containerBuilder->setAlias(self::LISTENER_TEST_PUBLIC_ALIAS, new Alias('sentry.exception_listener', true)); - - $extension = new SentryExtension(); - $extension->load([], $containerBuilder); - - $this->containerBuilder = $containerBuilder; - } - - public function test_that_it_does_not_report_http_exception_if_included_in_capture_skip() - { - $mockEvent = $this->createMock(GetResponseForExceptionEvent::class); - - $mockEvent - ->expects($this->once()) - ->method('getException') - ->willReturn(new HttpException(401)); - - $this->mockEventDispatcher - ->expects($this->never()) - ->method('dispatch') - ->withAnyParameters(); - - $this->mockSentryClient - ->expects($this->never()) - ->method('captureException') - ->withAnyParameters(); - - $this->containerBuilder->compile(); - $listener = $this->getListener(); - $listener->onKernelException($mockEvent); - } - - public function test_that_it_captures_exception() - { - $reportableException = new \Exception(); - - $mockEvent = $this->createMock(GetResponseForExceptionEvent::class); - $mockEvent - ->expects($this->once()) - ->method('getException') - ->willReturn($reportableException); - - $this->mockEventDispatcher - ->expects($this->once()) - ->method('dispatch') - ->with($this->identicalTo(SentrySymfonyEvents::PRE_CAPTURE), $this->identicalTo($mockEvent)); - - $this->mockSentryClient - ->expects($this->once()) - ->method('captureException') - ->with($this->identicalTo($reportableException)); - - $this->containerBuilder->compile(); - $listener = $this->getListener(); - $listener->onKernelException($mockEvent); - } - - public function test_that_it_captures_exception_with_route() - { - $reportableException = new \Exception(); - - $mockEvent = $this->createMock(GetResponseForExceptionEvent::class); - $mockEvent - ->expects($this->once()) - ->method('getException') - ->willReturn($reportableException); - - $this->mockEventDispatcher - ->expects($this->once()) - ->method('dispatch') - ->with($this->identicalTo(SentrySymfonyEvents::PRE_CAPTURE), $this->identicalTo($mockEvent)); - - $data = [ - 'tags' => [ - 'route' => 'homepage', - ], - ]; - - $this->requestStack->push(new Request([], [], ['_route' => 'homepage'])); - - $this->mockSentryClient - ->expects($this->once()) - ->method('captureException') - ->with($this->identicalTo($reportableException), $data); - - $this->containerBuilder->compile(); - $listener = $this->getListener(); - $listener->onKernelException($mockEvent); - } - - /** - * @dataProvider mockCommandProvider - */ - public function test_that_it_captures_console_exception(?Command $mockCommand, string $expectedCommandName) - { - if (! class_exists('Symfony\Component\Console\Event\ConsoleExceptionEvent')) { - $this->markTestSkipped('ConsoleExceptionEvent does not exist anymore on Symfony 4'); - } - - 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 */ - $input = $this->createMock(InputInterface::class); - /** @var OutputInterface $output */ - $output = $this->createMock(OutputInterface::class); - - $event = new ConsoleErrorEvent($input, $output, $error, $mockCommand); - $event->setExitCode(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($error), - $this->identicalTo([ - 'tags' => [ - 'command' => $expectedCommandName, - 'status_code' => 10, - ], - ]) - ); - - $this->containerBuilder->compile(); - /** @var SentryExceptionListenerInterface $listener */ - $listener = $this->getListener(); - $listener->onConsoleError($event); - } - - public function mockCommandProvider() - { - $mockCommand = $this->createMock(Command::class); - $mockCommand - ->expects($this->once()) - ->method('getName') - ->willReturn('cmd name'); - - return [ - [$mockCommand, 'cmd name'], - [null, 'N/A'], // the error may have been triggered before the command is loaded - ]; - } - - public function test_that_it_can_replace_client() - { - $replacementClient = $this->createMock(\Raven_Client::class); - - $reportableException = new \Exception(); - - $mockEvent = $this->createMock(GetResponseForExceptionEvent::class); - - $mockEvent - ->expects($this->once()) - ->method('getException') - ->willReturn($reportableException); - - $this->mockEventDispatcher - ->expects($this->once()) - ->method('dispatch') - ->with($this->identicalTo(SentrySymfonyEvents::PRE_CAPTURE), $this->identicalTo($mockEvent)); - - $this->mockSentryClient - ->expects($this->never()) - ->method('captureException') - ->withAnyParameters(); - - $replacementClient - ->expects($this->once()) - ->method('captureException') - ->with($this->identicalTo($reportableException)); - - $this->containerBuilder->compile(); - /** @var RequestListener $listener */ - $listener = $this->getListener(); - $this->assertInstanceOf(RequestListener::class, $listener); - - $listener->setClient($replacementClient); - - $listener->onKernelException($mockEvent); - } - - private function getListener(): SentryExceptionListenerInterface - { - return $this->containerBuilder->get(self::LISTENER_TEST_PUBLIC_ALIAS); - } -} From cabd27bf7d78688fa2ae9de022a54196eab9c9bc Mon Sep 17 00:00:00 2001 From: Alessandro Lai Date: Thu, 21 Feb 2019 18:18:25 +0100 Subject: [PATCH 44/49] Fix CS, remove unneeded ignore in PHPStan config --- phpstan.neon | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/phpstan.neon b/phpstan.neon index e1987177..f96cbc54 100644 --- a/phpstan.neon +++ b/phpstan.neon @@ -7,7 +7,7 @@ parameters: - "/Call to function method_exists.. with 'Symfony.+' and 'getProjectDir' will always evaluate to false./" excludes_analyse: - '%currentWorkingDirectory%/src/DependencyInjection/Configuration.php' - - '%currentWorkingDirectory%/test/EventListener/ExceptionListenerTest.php' + includes: - vendor/jangregor/phpstan-prophecy/src/extension.neon - vendor/phpstan/phpstan-phpunit/extension.neon From 57333d98cd54d2385a11058f7eb1ffb4a61975e4 Mon Sep 17 00:00:00 2001 From: Alessandro Lai Date: Thu, 21 Feb 2019 18:26:39 +0100 Subject: [PATCH 45/49] Add cache and vendor dirs to default in app exclusions --- src/DependencyInjection/Configuration.php | 5 ++++- test/DependencyInjection/ConfigurationTest.php | 6 +++++- test/DependencyInjection/SentryExtensionTest.php | 9 +++++++-- 3 files changed, 16 insertions(+), 4 deletions(-) diff --git a/src/DependencyInjection/Configuration.php b/src/DependencyInjection/Configuration.php index 53055367..b9ab0df6 100644 --- a/src/DependencyInjection/Configuration.php +++ b/src/DependencyInjection/Configuration.php @@ -61,7 +61,10 @@ public function getConfigTreeBuilder() ->scalarNode('error_types') ->end() ->arrayNode('in_app_exclude') - ->defaultValue($defaultValues->getInAppExcludedPaths()) + ->defaultValue([ + '%kernel.cache_dir%', + $this->getProjectRoot() . '/vendor', + ]) ->scalarPrototype()->end() ->end() ->arrayNode('excluded_exceptions') diff --git a/test/DependencyInjection/ConfigurationTest.php b/test/DependencyInjection/ConfigurationTest.php index c1c12ce6..d02f0310 100644 --- a/test/DependencyInjection/ConfigurationTest.php +++ b/test/DependencyInjection/ConfigurationTest.php @@ -49,7 +49,10 @@ public function testConfigurationDefaults(): void ], 'options' => [ 'environment' => '%kernel.environment%', - 'in_app_exclude' => $defaultSdkValues->getInAppExcludedPaths(), + 'in_app_exclude' => [ + '%kernel.cache_dir%', + '%kernel.root_dir%/../vendor', + ], 'excluded_exceptions' => $defaultSdkValues->getExcludedExceptions(), 'prefixes' => $defaultSdkValues->getPrefixes(), 'project_root' => '%kernel.root_dir%/..', @@ -59,6 +62,7 @@ public function testConfigurationDefaults(): void if (method_exists(Kernel::class, 'getProjectDir')) { $expectedDefaults['options']['project_root'] = '%kernel.project_dir%'; + $expectedDefaults['options']['in_app_exclude'][1] = '%kernel.project_dir%/vendor'; } $this->assertEquals($expectedDefaults, $processed); diff --git a/test/DependencyInjection/SentryExtensionTest.php b/test/DependencyInjection/SentryExtensionTest.php index a346f93d..830a4e32 100644 --- a/test/DependencyInjection/SentryExtensionTest.php +++ b/test/DependencyInjection/SentryExtensionTest.php @@ -38,12 +38,16 @@ public function testOptionsDefaultValues(): void $options = $this->getOptionsFrom($container); if (method_exists(Kernel::class, 'getProjectDir')) { - $this->assertSame('/dir/project/root/', $options->getProjectRoot()); + $vendorDir = '/dir/project/root/vendor'; + $this->assertSame('/dir/project/root', $options->getProjectRoot()); } else { + $vendorDir = 'kernel/root/../vendor'; $this->assertSame('kernel/root/..', $options->getProjectRoot()); } + $this->assertNull($options->getDsn()); $this->assertSame('test', $options->getEnvironment()); + $this->assertSame(['var/cache', $vendorDir], $options->getInAppExcludedPaths()); $this->assertSame(1, $container->getParameter('sentry.listener_priorities.request')); $this->assertSame(1, $container->getParameter('sentry.listener_priorities.console')); @@ -144,9 +148,10 @@ public function emptyDsnValueProvider(): array private function getContainer(array $configuration = []): Container { $containerBuilder = new ContainerBuilder(); + $containerBuilder->setParameter('kernel.cache_dir', 'var/cache'); $containerBuilder->setParameter('kernel.root_dir', 'kernel/root'); if (method_exists(Kernel::class, 'getProjectDir')) { - $containerBuilder->setParameter('kernel.project_dir', '/dir/project/root/'); + $containerBuilder->setParameter('kernel.project_dir', '/dir/project/root'); } $containerBuilder->setParameter('kernel.environment', 'test'); From aa39a68201340e390b082a34dd52cb49135f9979 Mon Sep 17 00:00:00 2001 From: Alessandro Lai Date: Fri, 22 Feb 2019 09:44:06 +0100 Subject: [PATCH 46/49] Switch from Hub::getScope to configureScope --- src/EventListener/ConsoleListener.php | 6 ++++-- src/EventListener/RequestListener.php | 11 +++++++---- test/EventListener/ConsoleListenerTest.php | 13 ++++++++++--- test/EventListener/RequestListenerTest.php | 15 +++++++++++---- 4 files changed, 32 insertions(+), 13 deletions(-) diff --git a/src/EventListener/ConsoleListener.php b/src/EventListener/ConsoleListener.php index 621be56d..788074ba 100644 --- a/src/EventListener/ConsoleListener.php +++ b/src/EventListener/ConsoleListener.php @@ -4,6 +4,7 @@ use Sentry\State\Hub; use Sentry\State\HubInterface; +use Sentry\State\Scope; use Symfony\Component\Console\Event\ConsoleCommandEvent; final class ConsoleListener @@ -33,7 +34,8 @@ public function onConsoleCommand(ConsoleCommandEvent $event): void $command = $event->getCommand(); Hub::getCurrent() - ->getScope() - ->setTag('command', $command ? $command->getName() : 'N/A'); + ->configureScope(function (Scope $scope) use ($command): void { + $scope->setTag('command', $command ? $command->getName() : 'N/A'); + }); } } diff --git a/src/EventListener/RequestListener.php b/src/EventListener/RequestListener.php index 2609dbc8..b1cfe2e0 100644 --- a/src/EventListener/RequestListener.php +++ b/src/EventListener/RequestListener.php @@ -4,6 +4,7 @@ use Sentry\State\Hub; use Sentry\State\HubInterface; +use Sentry\State\Scope; use Symfony\Component\HttpKernel\Event\FilterControllerEvent; use Symfony\Component\HttpKernel\Event\GetResponseEvent; use Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorageInterface; @@ -71,8 +72,9 @@ public function onKernelRequest(GetResponseEvent $event): void $userData['ip_address'] = $event->getRequest()->getClientIp(); Hub::getCurrent() - ->getScope() - ->setUser($userData); + ->configureScope(function (Scope $scope) use ($userData): void { + $scope->setUser($userData); + }); } public function onKernelController(FilterControllerEvent $event): void @@ -84,8 +86,9 @@ public function onKernelController(FilterControllerEvent $event): void $matchedRoute = $event->getRequest()->attributes->get('_route'); Hub::getCurrent() - ->getScope() - ->setTag('route', $matchedRoute); + ->configureScope(function (Scope $scope) use ($matchedRoute): void { + $scope->setTag('route', $matchedRoute); + }); } /** diff --git a/test/EventListener/ConsoleListenerTest.php b/test/EventListener/ConsoleListenerTest.php index 71862cb7..e94bda94 100644 --- a/test/EventListener/ConsoleListenerTest.php +++ b/test/EventListener/ConsoleListenerTest.php @@ -2,6 +2,8 @@ namespace Sentry\SentryBundle\Test\EventListener; +use PHPUnit\Framework\Assert; +use Prophecy\Argument; use Sentry\SentryBundle\EventListener\ConsoleListener; use Sentry\State\Hub; use Sentry\State\HubInterface; @@ -18,11 +20,16 @@ protected function setUp() { parent::setUp(); - $this->currentScope = new Scope(); + $this->currentScope = $scope = new Scope(); $this->currentHub = $this->prophesize(HubInterface::class); - $this->currentHub->getScope() + $this->currentHub->configureScope(Argument::type('callable')) ->shouldBeCalled() - ->willReturn($this->currentScope); + ->will(function ($arguments) use ($scope): void { + $callable = $arguments[0]; + Assert::assertIsCallable($callable); + + $callable($scope); + }); Hub::setCurrent($this->currentHub->reveal()); } diff --git a/test/EventListener/RequestListenerTest.php b/test/EventListener/RequestListenerTest.php index e59d743f..cee76ec4 100644 --- a/test/EventListener/RequestListenerTest.php +++ b/test/EventListener/RequestListenerTest.php @@ -2,7 +2,9 @@ namespace Sentry\SentryBundle\Test\EventListener; +use PHPUnit\Framework\Assert; use PHPUnit\Framework\TestCase; +use Prophecy\Argument; use Sentry\SentryBundle\EventListener\RequestListener; use Sentry\State\Hub; use Sentry\State\HubInterface; @@ -25,11 +27,16 @@ protected function setUp() { parent::setUp(); - $this->currentScope = new Scope(); + $this->currentScope = $scope = new Scope(); $this->currentHub = $this->prophesize(HubInterface::class); - $this->currentHub->getScope() + $this->currentHub->configureScope(Argument::type('callable')) ->shouldBeCalled() - ->willReturn($this->currentScope); + ->will(function ($arguments) use ($scope): void { + $callable = $arguments[0]; + Assert::assertIsCallable($callable); + + $callable($scope); + }); Hub::setCurrent($this->currentHub->reveal()); } @@ -289,7 +296,7 @@ public function testOnKernelControllerAddsRouteTag(): void public function testOnKernelRequestUserDataAndTagsAreNotSetInSubRequest(): void { - $this->currentHub->getScope() + $this->currentHub->configureScope(Argument::type('callable')) ->shouldNotBeCalled(); $tokenStorage = $this->prophesize(TokenStorageInterface::class); From aadcb7ef29a1c94df70151aa0efa4f39a6bb76dd Mon Sep 17 00:00:00 2001 From: Daniel Griesser Date: Fri, 22 Feb 2019 09:49:29 +0100 Subject: [PATCH 47/49] Update README.md Co-Authored-By: Jean85 --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 3bec6acf..ba381dde 100644 --- a/README.md +++ b/README.md @@ -77,7 +77,7 @@ class AppKernel extends Kernel // ... } ``` -Note that, unlike before version 3 of this bundle, the bundle will be enabled in all environments. +Note that, unlike before version 3, the bundle will be enabled in all environments. ### Step 3: Configure the SDK From cc2f4805cc33f6e65b1ba68bde37fbdd39e28fdd Mon Sep 17 00:00:00 2001 From: Alessandro Lai Date: Fri, 22 Feb 2019 15:46:50 +0100 Subject: [PATCH 48/49] Fix prefer-lowest build --- test/EventListener/ConsoleListenerTest.php | 1 - test/EventListener/RequestListenerTest.php | 1 - 2 files changed, 2 deletions(-) diff --git a/test/EventListener/ConsoleListenerTest.php b/test/EventListener/ConsoleListenerTest.php index e94bda94..acdc271e 100644 --- a/test/EventListener/ConsoleListenerTest.php +++ b/test/EventListener/ConsoleListenerTest.php @@ -26,7 +26,6 @@ protected function setUp() ->shouldBeCalled() ->will(function ($arguments) use ($scope): void { $callable = $arguments[0]; - Assert::assertIsCallable($callable); $callable($scope); }); diff --git a/test/EventListener/RequestListenerTest.php b/test/EventListener/RequestListenerTest.php index cee76ec4..80226d3b 100644 --- a/test/EventListener/RequestListenerTest.php +++ b/test/EventListener/RequestListenerTest.php @@ -33,7 +33,6 @@ protected function setUp() ->shouldBeCalled() ->will(function ($arguments) use ($scope): void { $callable = $arguments[0]; - Assert::assertIsCallable($callable); $callable($scope); }); From 1712f84c1ca8bc25b06aaf9aebf87197fbef40b2 Mon Sep 17 00:00:00 2001 From: Alessandro Lai Date: Fri, 22 Feb 2019 15:57:51 +0100 Subject: [PATCH 49/49] Fix CS --- test/EventListener/ConsoleListenerTest.php | 1 - test/EventListener/RequestListenerTest.php | 1 - 2 files changed, 2 deletions(-) diff --git a/test/EventListener/ConsoleListenerTest.php b/test/EventListener/ConsoleListenerTest.php index acdc271e..2df663e8 100644 --- a/test/EventListener/ConsoleListenerTest.php +++ b/test/EventListener/ConsoleListenerTest.php @@ -2,7 +2,6 @@ namespace Sentry\SentryBundle\Test\EventListener; -use PHPUnit\Framework\Assert; use Prophecy\Argument; use Sentry\SentryBundle\EventListener\ConsoleListener; use Sentry\State\Hub; diff --git a/test/EventListener/RequestListenerTest.php b/test/EventListener/RequestListenerTest.php index 80226d3b..44006edb 100644 --- a/test/EventListener/RequestListenerTest.php +++ b/test/EventListener/RequestListenerTest.php @@ -2,7 +2,6 @@ namespace Sentry\SentryBundle\Test\EventListener; -use PHPUnit\Framework\Assert; use PHPUnit\Framework\TestCase; use Prophecy\Argument; use Sentry\SentryBundle\EventListener\RequestListener;