minor #19656 [FrameworkBundle][Debug] Fix default config and cleaning…

… of traces (nicolas-grekas)

This PR was merged into the 3.2-dev branch.

Discussion
----------

[FrameworkBundle][Debug] Fix default config and cleaning of traces

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | yes
| Tests pass?   | yes
| Fixed tickets | Follow up #19568
| License       | MIT
| Doc PR        | -

The default value of `framework.php_errors.log` must be `%kernel.debug%` to have deprecations and silenced errors logged in dev as before.

Cleaning the trace was broken because a closure can't be bound to an internal class.

This PR fixes both issues and enhance trace cleaning a bit by removing arguments from traces so that they take less memory when collected as part of the context of log messages.

Commits
-------

f640870 [FrameworkBundle][Debug] Fix default config and cleaning of traces

UPGRADE-4.0.md

@@ -121,8 +121,6 @@ FrameworkBundle
  * 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
 ----------
 

src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php

@@ -704,8 +704,8 @@ private function addPhpErrorsSection(ArrayNodeDefinition $rootNode)
                     ->children()
                         ->booleanNode('log')
                             ->info('Use the app logger instead of the PHP logger for logging PHP errors.')
-                            ->defaultValue(false)
-                            ->treatNullLike(false)
+                            ->defaultValue($this->debug)
+                            ->treatNullLike($this->debug)
                         ->end()
                         ->booleanNode('throw')
                             ->info('Throw PHP errors as \ErrorException instances.')

src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/ConfigurationTest.php

@@ -275,7 +275,7 @@ protected static function getBundleDefaultConfig()
             ),
             'workflows' => array(),
             'php_errors' => array(
-                'log' => false,
+                'log' => true,
                 'throw' => true,
             ),
         );

src/Symfony/Component/Debug/ErrorHandler.php

@@ -89,6 +89,7 @@ class ErrorHandler
     private $tracedErrors = 0x77FB; // E_ALL - E_STRICT - E_PARSE
     private $screamedErrors = 0x55; // E_ERROR + E_CORE_ERROR + E_COMPILE_ERROR + E_PARSE
     private $loggedErrors = 0;
+    private $traceReflector;
 
     private $isRecursive = 0;
     private $isRoot = false;
@@ -147,6 +148,8 @@ public function __construct(BufferingLogger $bootstrappingLogger = null)
             $this->bootstrappingLogger = $bootstrappingLogger;
             $this->setDefaultLogger($bootstrappingLogger);
         }
+        $this->traceReflector = new \ReflectionProperty('Exception', 'trace');
+        $this->traceReflector->setAccessible(true);
     }
 
     /**
@@ -389,16 +392,37 @@ public function handleError($type, $message, $file, $line, array $context, array
             self::$toStringException = null;
         } elseif (!$throw && !($type & $level)) {
             $errorAsException = new SilencedErrorContext($type, $file, $line);
-        } elseif ($this->scopedErrors & $type) {
-            $errorAsException = new ContextErrorException($logMessage, 0, $type, $file, $line, $context);
         } else {
-            $errorAsException = new \ErrorException($logMessage, 0, $type, $file, $line);
+            if ($this->scopedErrors & $type) {
+                $errorAsException = new ContextErrorException($logMessage, 0, $type, $file, $line, $context);
+            } else {
+                $errorAsException = new \ErrorException($logMessage, 0, $type, $file, $line);
+            }
+
+            // Clean the trace by removing function arguments and the first frames added by the error handler itself.
+            if ($throw || $this->tracedErrors & $type) {
+                $backtrace = $backtrace ?: $errorAsException->getTrace();
+                $lightTrace = $backtrace;
+
+                for ($i = 0; isset($backtrace[$i]); ++$i) {
+                    if (isset($backtrace[$i]['file'], $backtrace[$i]['line']) && $backtrace[$i]['line'] === $line && $backtrace[$i]['file'] === $file) {
+                        $lightTrace = array_slice($lightTrace, 1 + $i);
+                        break;
+                    }
+                }
+                if (!($throw || $this->scopedErrors & $type)) {
+                    for ($i = 0; isset($lightTrace[$i]); ++$i) {
+                        unset($lightTrace[$i]['args']);
+                    }
+                }
+                $this->traceReflector->setValue($errorAsException, $lightTrace);
+            } else {
+                $this->traceReflector->setValue($errorAsException, array());
+            }
         }
 
         if ($throw) {
             if (E_USER_ERROR & $type) {
-                $backtrace = $backtrace ?: $errorAsException->getTrace();
-
                 for ($i = 1; isset($backtrace[$i]); ++$i) {
                     if (isset($backtrace[$i]['function'], $backtrace[$i]['type'], $backtrace[$i - 1]['function'])
                         && '__toString' === $backtrace[$i]['function']
@@ -440,14 +464,6 @@ public function handleError($type, $message, $file, $line, array $context, array
             throw $errorAsException;
         }
 
-        if (!($this->tracedErrors & $type) && $errorAsException instanceof \Exception) {
-            static $freeTrace = null;
-            if (null === $freeTrace) {
-                $freeTrace = \Closure::bind(function ($e) { $e->trace = array(); }, null, \Exception::class);
-            }
-            $freeTrace($errorAsException);
-        }
-
         if ($this->isRecursive) {
             $log = 0;
         } elseif (self::$stackedErrorLevels) {

src/Symfony/Component/Debug/Tests/ErrorHandlerTest.php

@@ -80,20 +80,16 @@ public function testNotice()
             $this->assertArrayHasKey('foobar', $exception->getContext());
 
             $trace = $exception->getTrace();
+
             $this->assertEquals(__FILE__, $trace[0]['file']);
-            $this->assertEquals('Symfony\Component\Debug\ErrorHandler', $trace[0]['class']);
-            $this->assertEquals('handleError', $trace[0]['function']);
-            $this->assertEquals('->', $trace[0]['type']);
+            $this->assertEquals(__CLASS__, $trace[0]['class']);
+            $this->assertEquals('triggerNotice', $trace[0]['function']);
+            $this->assertEquals('::', $trace[0]['type']);
 
-            $this->assertEquals(__FILE__, $trace[1]['file']);
+            $this->assertEquals(__FILE__, $trace[0]['file']);
             $this->assertEquals(__CLASS__, $trace[1]['class']);
-            $this->assertEquals('triggerNotice', $trace[1]['function']);
-            $this->assertEquals('::', $trace[1]['type']);
-
-            $this->assertEquals(__FILE__, $trace[1]['file']);
-            $this->assertEquals(__CLASS__, $trace[2]['class']);
-            $this->assertEquals(__FUNCTION__, $trace[2]['function']);
-            $this->assertEquals('->', $trace[2]['type']);
+            $this->assertEquals(__FUNCTION__, $trace[1]['function']);
+            $this->assertEquals('->', $trace[1]['type']);
         } finally {
             restore_error_handler();
             restore_exception_handler();