fix configuration, extend tests

OxCom · OxCom · commit 936fe1bdee4f · 2018-01-31T23:36:01.000+01:00
diff --git a/DependencyInjection/Configuration.php b/DependencyInjection/Configuration.php
@@ -55,7 +55,11 @@ public function getConfigTreeBuilder()
         $rootNode
             ->children()
                 ->scalarNode('enable')->defaultTrue()->end()
-                ->scalarNode('exclude')->defaultValue(static::$exclude)->end()
+                ->arrayNode('exclude')
+                    ->treatNullLike([])
+                    ->prototype('scalar')->end()
+                    ->defaultValue(static::$exclude)
+                    ->end()
                 ->arrayNode('rollbar')->children()
                     ->scalarNode('access_token')->defaultValue('')->end()
                     ->scalarNode('agent_log_location')->defaultValue('%kernel.logs_dir%/rollbar.log')->end()
@@ -68,9 +72,21 @@ public function getConfigTreeBuilder()
                     ->scalarNode('code_version')->defaultValue('')->end()
                     ->scalarNode('enable_utf8_sanitization')->defaultTrue()->end()
                     ->scalarNode('environment')->defaultValue(static::ENVIRONMENT)->end()
-                    ->scalarNode('custom')->defaultValue([])->end()
-                    ->scalarNode('error_sample_rates')->defaultValue([])->end()
-                    ->scalarNode('exception_sample_rates')->defaultValue([])->end()
+                    ->arrayNode('custom')
+                        ->treatNullLike([])
+                        ->prototype('scalar')->end()
+                        ->defaultValue([])
+                        ->end()
+                    ->arrayNode('error_sample_rates')
+                        ->treatNullLike([])
+                        ->prototype('scalar')->end()
+                        ->defaultValue([])
+                        ->end()
+                    ->arrayNode('exception_sample_rates')
+                        ->treatNullLike([])
+                        ->prototype('scalar')->end()
+                        ->defaultValue([])
+                        ->end()
                     ->scalarNode('fluent_host')->defaultValue(static::FLUENT_HOST)->end()
                     ->scalarNode('fluent_port')->defaultValue(static::FLUENT_PORT)->end()
                     ->scalarNode('fluent_tag')->defaultValue(static::FLUENT_TAG)->end()
@@ -80,10 +96,18 @@ public function getConfigTreeBuilder()
                     ->scalarNode('include_exception_code_context')->defaultFalse()->end()
                     ->scalarNode('included_errno')->defaultValue($defaultErrorMask)->end()
                     ->scalarNode('logger')->defaultNull()->end()
-                    ->scalarNode('person')->defaultValue([])->end()
+                    ->arrayNode('person')
+                        ->treatNullLike([])
+                        ->prototype('scalar')->end()
+                        ->defaultValue([])
+                        ->end()
                     ->scalarNode('person_fn')->defaultNull()->end()
                     ->scalarNode('root')->defaultValue('%kernel.root_dir%')->end()
-                    ->scalarNode('scrub_fields')->defaultValue(static::$scrubFieldsDefault)->end()
+                    ->arrayNode('scrub_fields')
+                        ->treatNullLike([])
+                        ->prototype('scalar')->end()
+                        ->defaultValue(static::$scrubFieldsDefault)
+                        ->end()
                     ->scalarNode('scrub_whitelist')->defaultNull()->end()
                     ->scalarNode('shift_function')->defaultTrue()->end()
                     ->scalarNode('timeout')->defaultValue(static::TIMEOUT)->end()
diff --git a/EventListener/AbstractListener.php b/EventListener/AbstractListener.php
@@ -25,6 +25,11 @@ abstract class AbstractListener implements EventSubscriberInterface
      */
     protected $container;
 
+    /**
+     * @var \SymfonyRollbarBundle\Provider\RollbarHandler
+     */
+    protected $handler;
+
     /**
      * @var array
      */
@@ -47,6 +52,7 @@ public function __construct(ContainerInterface $container)
         $config = $this->getContainer()->getParameter(SymfonyRollbarExtension::ALIAS . '.config');
         $this->exclude = empty($config['exclude']) ? [] : $config['exclude'];
 
+        $this->handler = $rbHandler;
         $this->getLogger()->pushHandler($rbHandler);
     }
 
diff --git a/EventListener/ExceptionListener.php b/EventListener/ExceptionListener.php
@@ -42,16 +42,10 @@ public function onKernelException(GetResponseForExceptionEvent $event)
      */
     public function handleException($exception)
     {
-        if ($exception instanceof \Exception) {
-            // check exception
-            foreach ($this->exclude as $instance) {
-                if (class_exists($instance) && $exception instanceof $instance) {
-                    return;
-                }
-            }
+        if ($exception instanceof \Exception && $this->handler->shouldSkip($exception)) {
+            return;
         }
 
-
         $payload = [];
         // @link http://php.net/manual/en/reserved.constants.php
         // @link http://php.net/manual/en/language.errors.php7.php
diff --git a/Tests/Fixtures/ErrorHandler.php b/Tests/Fixtures/ErrorHandler.php
@@ -1,14 +1,14 @@
 <?php
 
-namespace Tests\Fixtures;
+namespace SymfonyRollbarBundle\Tests\Fixtures;
 
 use Monolog\Handler\AbstractProcessingHandler;
 use Monolog\Logger;
 
 class ErrorHandler extends AbstractProcessingHandler
 {
     /**
-     * @var \Tests\Fixtures\ErrorHandler
+     * @var \SymfonyRollbarBundle\Tests\Fixtures\ErrorHandler
      */
     protected static $instance;
 
@@ -18,7 +18,7 @@ class ErrorHandler extends AbstractProcessingHandler
     protected $assert;
 
     /**
-     * @return \Tests\Fixtures\ErrorHandler
+     * @return \SymfonyRollbarBundle\Tests\Fixtures\ErrorHandler
      */
     public static function getInstance()
     {
diff --git a/Tests/Fixtures/MyAwesomeException.php b/Tests/Fixtures/MyAwesomeException.php
@@ -0,0 +1,12 @@
+<?php
+
+namespace SymfonyRollbarBundle\Tests\Fixtures;
+
+/**
+ * Class MyAwesomeException
+ * @package SymfonyRollbarBundle\Tests\Fixtures
+ */
+class MyAwesomeException extends \Exception
+{
+
+}
diff --git a/Tests/Fixtures/app/config/config_test.yml b/Tests/Fixtures/app/config/config_test.yml
@@ -4,6 +4,9 @@ imports:
 
 symfony_rollbar:
   enable: true
+  exclude:
+      - \Symfony\Component\Debug\Exception\FatalErrorException
+      - \SymfonyRollbarBundle\Tests\Fixtures\MyAwesomeException
   rollbar:
     access_token: '%env(ROLLBAR_ACCESS_TOKEN)%'
     environment: '%kernel.environment%'
diff --git a/Tests/SymfonyRollbarBundle/DependencyInjection/ConfigurationTest.php b/Tests/SymfonyRollbarBundle/DependencyInjection/ConfigurationTest.php
@@ -20,9 +20,12 @@ public function testParameters()
         $config           = $container->getParameter(SymfonyRollbarExtension::ALIAS . '.config');
         $defaultErrorMask = E_ERROR | E_WARNING | E_PARSE | E_CORE_ERROR | E_USER_ERROR | E_RECOVERABLE_ERROR;
 
+        $exclude = Configuration::$exclude;
+        $exclude[] = '\SymfonyRollbarBundle\Tests\Fixtures\MyAwesomeException';
+
         $default = [
             'enable' => true,
-            'exclude' => Configuration::$exclude,
+            'exclude' => $exclude,
             'rollbar' => [
                 'access_token'                   => getenv('ROLLBAR_ACCESS_TOKEN'),
                 'agent_log_location'             => static::$kernel->getLogDir() . '/rollbar.log',
diff --git a/Tests/SymfonyRollbarBundle/DependencyInjection/SymfonyRollbarExtensionTest.php b/Tests/SymfonyRollbarBundle/DependencyInjection/SymfonyRollbarExtensionTest.php
@@ -41,11 +41,13 @@ public function testConfigEnabledVars($var, $value)
 
     public function generatorConfigVars()
     {
+        $exclude = Configuration::$exclude;
+
         return [
             ['symfony_rollbar.event_listener.exception_listener.class', ExceptionListener::class],
             ['symfony_rollbar.event_listener.error_listener.class', ErrorListener::class],
             ['symfony_rollbar.provider.rollbar_handler.class', RollbarHandler::class],
-            ['symfony_rollbar.config', ['enable' => true, 'exclude' => Configuration::$exclude]],
+            ['symfony_rollbar.config', ['enable' => true, 'exclude' => $exclude]],
         ];
     }
 
diff --git a/Tests/SymfonyRollbarBundle/EventListener/ErrorListenerTest.php b/Tests/SymfonyRollbarBundle/EventListener/ErrorListenerTest.php
@@ -8,6 +8,7 @@
 use SymfonyRollbarBundle\EventListener\AbstractListener;
 use SymfonyRollbarBundle\EventListener\ErrorListener;
 use SymfonyRollbarBundle\Provider\RollbarHandler;
+use SymfonyRollbarBundle\Tests\Fixtures\ErrorHandler;
 
 /**
  * Class ErrorListenerTest
@@ -33,7 +34,7 @@ public function testUserError()
          */
         $eventDispatcher = $container->get('event_dispatcher');
         $listeners = $eventDispatcher->getListeners('kernel.exception');
-        $handler = \Tests\Fixtures\ErrorHandler::getInstance();
+        $handler = ErrorHandler::getInstance();
 
         $handler->setAssert(function (array $record) use ($message) {
             $this->assertNotEmpty($record);
@@ -73,7 +74,7 @@ public function testFatalError()
          */
         $eventDispatcher = $container->get('event_dispatcher');
         $listeners       = $eventDispatcher->getListeners('kernel.exception');
-        $handler         = \Tests\Fixtures\ErrorHandler::getInstance();
+        $handler         = ErrorHandler::getInstance();
         $rbHandler       = new RollbarHandler($container);
 
         $handler->setAssert(function (array $record) use ($rbHandler) {
diff --git a/Tests/SymfonyRollbarBundle/EventListener/ExceptionListenerTest.php b/Tests/SymfonyRollbarBundle/EventListener/ExceptionListenerTest.php
@@ -8,8 +8,10 @@
 use \Symfony\Component\HttpKernel\Event\GetResponseForExceptionEvent;
 use Symfony\Component\HttpKernel\HttpKernelInterface;
 use SymfonyRollbarBundle\EventListener\AbstractListener;
+use SymfonyRollbarBundle\EventListener\ExceptionListener;
 use SymfonyRollbarBundle\Provider\RollbarHandler;
-use Tests\Fixtures\ErrorHandler;
+use SymfonyRollbarBundle\Tests\Fixtures\ErrorHandler;
+use SymfonyRollbarBundle\Tests\Fixtures\MyAwesomeException;
 
 /**
  * Class ExceptionListenerTest
@@ -105,7 +107,7 @@ public function testInvalidHandleParams($data)
             $this->assertInstanceOf(\Exception::class, $exception);
         });
 
-        $listener = new \SymfonyRollbarBundle\EventListener\ExceptionListener($container);
+        $listener = new ExceptionListener($container);
         $listener->getLogger()->setHandlers([$handler]);
 
         $listener->handleException($data);
@@ -123,4 +125,40 @@ public function generatorRandomParams()
             [(object)['a' => 'b']],
         ];
     }
+
+    public function testSkipException()
+    {
+        $container = static::$kernel->getContainer();
+
+        /**
+         * @var TraceableEventDispatcher $eventDispatcher
+         */
+        $eventDispatcher = $container->get('event_dispatcher');
+        $listeners       = $eventDispatcher->getListeners('kernel.exception');
+        $event           = new GetResponseForExceptionEvent(
+            static::$kernel,
+            new Request(),
+            HttpKernelInterface::MASTER_REQUEST,
+            new MyAwesomeException("Hello!")
+        );
+
+        $handler = new ErrorHandler();
+        $handler->setAssert(function ($record) {
+            $this->fail("This should be newer called!");
+        });
+
+        foreach ($listeners as $listener) {
+            /**
+             * @var AbstractListener $listener
+             */
+            if (!$listener[0] instanceof AbstractListener) {
+                continue;
+            }
+
+            $listener[0]->getLogger()->setHandlers([$handler]);
+        }
+
+        $eventDispatcher->dispatch('kernel.exception', $event);
+        restore_error_handler();
+    }
 }
diff --git a/Tests/SymfonyRollbarBundle/Provider/RollbarHandlerTest.php b/Tests/SymfonyRollbarBundle/Provider/RollbarHandlerTest.php
@@ -5,6 +5,7 @@
 use Rollbar\Payload\Level;
 use Symfony\Bundle\FrameworkBundle\Test\KernelTestCase;
 use Symfony\Component\Validator\Constraints\DateTime;
+use SymfonyRollbarBundle\Tests\Fixtures\MyAwesomeException;
 
 /**
  * Class RollbarHandlerTest
@@ -81,6 +82,19 @@ public function recordGenerator()
                     'context'    => [],
                 ],
             ],
+            [
+                [
+                    'message'    => 'RecordGenerator :: #3',
+                    'datetime'   => new \DateTime(),
+                    'level'      => \Monolog\Logger::ERROR,
+                    'level_name' => \Monolog\Logger::ERROR,
+                    'channel'    => 'symfony.rollbar',
+                    'extra'      => [],
+                    'context'    => [
+                        'payload' => [1, 2, 3]
+                    ],
+                ],
+            ],
         ];
     }
 
@@ -106,4 +120,62 @@ public function testClose($record)
         $handler->close();
         $this->assertFalse($property->getValue($handler));
     }
+
+    /**
+     * @dataProvider generatorShouldSkip
+     *
+     * @param $e
+     * @param $skip
+     */
+    public function testShouldSkip($e, $skip)
+    {
+        $container = static::$kernel->getContainer();
+        $handler   = new \SymfonyRollbarBundle\Provider\RollbarHandler($container);
+
+        $this->assertEquals($skip, $handler->shouldSkip($e));
+    }
+
+    /**
+     * @return array
+     */
+    public function generatorShouldSkip()
+    {
+        $e = new \ErrorException();
+
+        return [
+            [$e, false],
+            [new \Exception(), false],
+            [new \SymfonyRollbarBundle\Tests\Fixtures\MyAwesomeException(), true],
+            [new \Symfony\Component\Debug\Exception\UndefinedFunctionException("error", $e), true],
+        ];
+    }
+
+    public function testContextPayload()
+    {
+        $record = [
+            'message'    => 'RecordGenerator :: #4',
+            'datetime'   => new \DateTime(),
+            'level'      => \Monolog\Logger::ERROR,
+            'level_name' => \Monolog\Logger::ERROR,
+            'channel'    => 'symfony.rollbar',
+            'extra'      => [],
+            'context'    => [
+                'exception' => new MyAwesomeException('RecordGenerator :: #4'),
+                'payload' => [4, 5, 6]
+            ],
+        ];
+
+        $container = static::$kernel->getContainer();
+        $handler   = new \SymfonyRollbarBundle\Provider\RollbarHandler($container);
+
+        $property = new \ReflectionProperty($handler, 'hasRecords');
+        $property->setAccessible(true);
+
+        $method = new \ReflectionMethod($handler, 'write');
+        $method->setAccessible(true);
+
+        $this->assertFalse($property->getValue($handler));
+        $method->invoke($handler, $record);
+        $this->assertFalse($property->getValue($handler));
+    }
 }

Original file line number	Diff line number	Diff line change
`@@ -41,11 +41,13 @@ public function testConfigEnabledVars($var, $value)`
`41`	`41`
`42`	`42`	`public function generatorConfigVars()`
`43`	`43`	`{`
	`44`	`+ $exclude = Configuration::$exclude;`
	`45`	`+`
`44`	`46`	`return [`
`45`	`47`	`['symfony_rollbar.event_listener.exception_listener.class', ExceptionListener::class],`
`46`	`48`	`['symfony_rollbar.event_listener.error_listener.class', ErrorListener::class],`
`47`	`49`	`['symfony_rollbar.provider.rollbar_handler.class', RollbarHandler::class],`
`48`		`- ['symfony_rollbar.config', ['enable' => true, 'exclude' => Configuration::$exclude]],`
	`50`	`+ ['symfony_rollbar.config', ['enable' => true, 'exclude' => $exclude]],`
`49`	`51`	`];`
`50`	`52`	`}`
`51`	`53`