[Debug] Better error handling

1. Send the raw exception in the log context instead of custom formatting 2. Add config option to log in Symfony all PHP errors
symfony · Aug 17, 2016 · 8f24549 · 8f24549
1 parent cf30750
commit 8f24549
Show file tree

Hide file tree

Showing 13 changed files with 373 additions and 215 deletions.
diff --git a/UPGRADE-4.0.md b/UPGRADE-4.0.md
@@ -117,10 +117,12 @@ FrameworkBundle
  * The `framework.serializer.cache` option and the services
    `serializer.mapping.cache.apc` and `serializer.mapping.cache.doctrine.apc`
    have been removed. APCu should now be automatically used when available.
-   
+
  * The `Controller::getUser()` method has been removed in favor of the ability
    to typehint the security user object in the action.
 
+ * The default value of the `framework.php_errors.log` configuration key is set to true.
+
 HttpKernel
 ----------
 

diff --git a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php
@@ -116,6 +116,7 @@ public function getConfigTreeBuilder()
         $this->addPropertyAccessSection($rootNode);
         $this->addPropertyInfoSection($rootNode);
         $this->addCacheSection($rootNode);
+        $this->addPhpErrorsSection($rootNode);
 
         return $treeBuilder;
     }
@@ -692,4 +693,28 @@ private function addCacheSection(ArrayNodeDefinition $rootNode)
             ->end()
         ;
     }
+
+    private function addPhpErrorsSection(ArrayNodeDefinition $rootNode)
+    {
+        $rootNode
+            ->children()
+                ->arrayNode('php_errors')
+                    ->info('PHP errors handling configuration')
+                    ->addDefaultsIfNotSet()
+                    ->children()
+                        ->booleanNode('log')
+                            ->info('Use the app logger instead of the PHP logger for logging PHP errors.')
+                            ->defaultValue(false)
+                            ->treatNullLike(false)
+                        ->end()
+                        ->booleanNode('throw')
+                            ->info('Throw PHP errors as \ErrorException instances.')
+                            ->defaultValue($this->debug)
+                            ->treatNullLike($this->debug)
+                        ->end()
+                    ->end()
+                ->end()
+            ->end()
+        ;
+    }
 }
diff --git a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php
@@ -131,6 +131,7 @@ public function load(array $configs, ContainerBuilder $container)
         $this->registerProfilerConfiguration($config['profiler'], $container, $loader);
         $this->registerCacheConfiguration($config['cache'], $container);
         $this->registerWorkflowConfiguration($config['workflows'], $container, $loader);
+        $this->registerDebugConfiguration($config['php_errors'], $container, $loader);
 
         if ($this->isConfigEnabled($container, $config['router'])) {
             $this->registerRouterConfiguration($config['router'], $container, $loader);
@@ -147,27 +148,6 @@ public function load(array $configs, ContainerBuilder $container)
             $this->registerPropertyInfoConfiguration($config['property_info'], $container, $loader);
         }
 
-        $loader->load('debug_prod.xml');
-        $definition = $container->findDefinition('debug.debug_handlers_listener');
-
-        if ($container->hasParameter('templating.helper.code.file_link_format')) {
-            $definition->replaceArgument(5, '%templating.helper.code.file_link_format%');
-        }
-
-        if ($container->getParameter('kernel.debug')) {
-            $definition->replaceArgument(2, E_ALL & ~(E_COMPILE_ERROR | E_PARSE | E_ERROR | E_CORE_ERROR | E_RECOVERABLE_ERROR));
-
-            $loader->load('debug.xml');
-
-            // replace the regular event_dispatcher service with the debug one
-            $definition = $container->findDefinition('event_dispatcher');
-            $definition->setPublic(false);
-            $container->setDefinition('debug.event_dispatcher.parent', $definition);
-            $container->setAlias('event_dispatcher', 'debug.event_dispatcher');
-        } else {
-            $definition->replaceArgument(1, null);
-        }
-
         $this->addAnnotatedClassesToCompile(array(
             '**Bundle\\Controller\\',
             '**Bundle\\Entity\\',
@@ -417,6 +397,48 @@ private function registerWorkflowConfiguration(array $workflows, ContainerBuilde
         }
     }
 
+    /**
+     * Loads the debug configuration.
+     *
+     * @param array            $config    A php errors configuration array
+     * @param ContainerBuilder $container A ContainerBuilder instance
+     * @param XmlFileLoader    $loader    An XmlFileLoader instance
+     */
+    private function registerDebugConfiguration(array $config, ContainerBuilder $container, XmlFileLoader $loader)
+    {
+        $loader->load('debug_prod.xml');
+
+        $debug = $container->getParameter('kernel.debug');
+
+        if ($debug) {
+            $loader->load('debug.xml');
+
+            // replace the regular event_dispatcher service with the debug one
+            $definition = $container->findDefinition('event_dispatcher');
+            $definition->setPublic(false);
+            $container->setDefinition('debug.event_dispatcher.parent', $definition);
+            $container->setAlias('event_dispatcher', 'debug.event_dispatcher');
+        }
+
+        $definition = $container->findDefinition('debug.debug_handlers_listener');
+
+        if (!$config['log']) {
+            $definition->replaceArgument(1, null);
+        }
+
+        if (!$config['throw']) {
+            $container->setParameter('debug.error_handler.throw_at', 0);
+        }
+
+        $definition->replaceArgument(4, $debug);
+
+        if ($container->hasParameter('templating.helper.code.file_link_format')) {
+            $definition->replaceArgument(5, '%templating.helper.code.file_link_format%');
+        }
+
+        $definition->replaceArgument(6, $debug);
+    }
+
     /**
      * Loads the router configuration.
      *

diff --git a/src/Symfony/Bundle/FrameworkBundle/Resources/config/debug.xml b/src/Symfony/Bundle/FrameworkBundle/Resources/config/debug.xml
@@ -6,7 +6,6 @@
 
     <parameters>
         <parameter key="debug.container.dump">%kernel.cache_dir%/%kernel.container_class%.xml</parameter>
-        <parameter key="debug.error_handler.throw_at">-1</parameter>
     </parameters>
 
     <services>

diff --git a/src/Symfony/Bundle/FrameworkBundle/Resources/config/debug_prod.xml b/src/Symfony/Bundle/FrameworkBundle/Resources/config/debug_prod.xml
@@ -5,7 +5,7 @@
     xsi:schemaLocation="http://symfony.com/schema/dic/services http://symfony.com/schema/dic/services/services-1.0.xsd">
 
     <parameters>
-        <parameter key="debug.error_handler.throw_at">0</parameter>
+        <parameter key="debug.error_handler.throw_at">-1</parameter>
     </parameters>
 
     <services>
@@ -14,10 +14,11 @@
             <tag name="monolog.logger" channel="php" />
             <argument>null</argument><!-- Exception handler -->
             <argument type="service" id="logger" on-invalid="null" />
-            <argument>null</argument><!-- Log levels map for enabled error levels -->
-            <argument>null</argument>
+            <argument>-1</argument><!-- Log levels map for enabled error levels -->
+            <argument>%debug.error_handler.throw_at%</argument>
             <argument>true</argument>
             <argument>null</argument><!-- %templating.helper.code.file_link_format% -->
+            <argument>true</argument>
         </service>
 
         <service id="debug.stopwatch" class="Symfony\Component\Stopwatch\Stopwatch" />

diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/ConfigurationTest.php b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/ConfigurationTest.php
@@ -274,6 +274,10 @@ protected static function getBundleDefaultConfig()
                 'default_redis_provider' => 'redis://localhost',
             ),
             'workflows' => array(),
+            'php_errors' => array(
+                'log' => false,
+                'throw' => true,
+            ),
         );
     }
 }
diff --git a/src/Symfony/Bundle/WebProfilerBundle/Resources/views/Collector/logger.html.twig b/src/Symfony/Bundle/WebProfilerBundle/Resources/views/Collector/logger.html.twig
@@ -6,7 +6,7 @@
     {% if collector.counterrors or collector.countdeprecations or collector.countscreams %}
         {% set icon %}
             {% set status_color = collector.counterrors ? 'red' : collector.countdeprecations ? 'yellow' : '' %}
-            {% set error_count = collector.counterrors + collector.countdeprecations + collector.countscreams %}
+            {% set error_count = collector.counterrors + collector.countdeprecations %}
             {{ include('@WebProfiler/Icon/logger.svg') }}
             <span class="sf-toolbar-value">{{ error_count }}</span>
         {% endset %}
@@ -55,7 +55,7 @@
         {# sort collected logs in groups #}
         {% set deprecation_logs, debug_logs, info_and_error_logs, silenced_logs = [], [], [], [] %}
         {% for log in collector.logs %}
-            {% if log.context.level is defined and log.context.type is defined and log.context.type in [constant('E_DEPRECATED'), constant('E_USER_DEPRECATED')] %}
+            {% if log.context.errorCount is defined and log.context.type is defined and log.context.type in ['E_DEPRECATED', 'E_USER_DEPRECATED'] %}
                 {% set deprecation_logs = deprecation_logs|merge([log]) %}
             {% elseif log.context.scream is defined and log.context.scream == true  %}
                 {% set silenced_logs = silenced_logs|merge([log]) %}
@@ -170,21 +170,22 @@
 {% macro render_log_message(category, log_index, log, is_deprecation = false) %}
     {{ log.message }}
 
+    {% if log.context.errorCount is defined and log.context.errorCount > 1 %}
+        <span class="text-small text-bold">({{ log.context.errorCount }} times)</span>
+    {% endif %}
+
     {% if is_deprecation %}
-        {% set stack = log.context.stack|default([]) %}
-        {% set stack_id = 'sf-call-stack-' ~ category ~ '-' ~ log_index %}
+        {% set trace = log.context.trace|default([]) %}
+        {% set trace_id = 'sf-call-trace-' ~ category ~ '-' ~ log_index %}
 
-        {% if log.context.errorCount is defined %}
-            <span class="text-small text-bold">({{ log.context.errorCount }} times)</span>
-        {% endif %}
 
-        {% if stack %}
-            <button class="btn-link text-small sf-toggle" data-toggle-selector="#{{ stack_id }}" data-toggle-alt-content="Hide stack trace">Show stack trace</button>
+        {% if trace %}
+            <button class="btn-link text-small sf-toggle" data-toggle-selector="#{{ trace_id }}" data-toggle-alt-content="Hide stack trace">Show stack trace</button>
         {% endif %}
 
-        {% for index, call in stack if index > 1 %}
+        {% for index, call in trace if index > 1 %}
             {% if index == 2 %}
-                <ul class="sf-call-stack hidden" id="{{ stack_id }}">
+                <ul class="sf-call-trace hidden" id="{{ trace_id }}">
             {% endif %}
 
             {% if call.class is defined %}
@@ -206,7 +207,7 @@
                 {% endif %}
             </li>
 
-            {% if index == stack|length - 1 %}
+            {% if index == trace|length - 1 %}
                 </ul>
             {% endif %}
         {% endfor %}
@@ -224,7 +225,7 @@
                     <a class="btn-link text-small sf-toggle" data-toggle-selector="#{{ context_id }}" data-toggle-alt-content="Hide full context">Show full context</a>
 
                     <div id="{{ context_id }}" class="context">
-                        <pre>{{ context_dump }}</pre>
+                        {{ dump(log.context) }}
                     </div>
                 {% else %}
                     {{ context_dump }}