[Debug][ErrorHandler] Preserve next error handler

fancyweb · cuchac · fancyweb · commit 6621e8e53682 · 2019-02-15T18:32:14.000+01:00
Co-authored-by: Joe &lt;cuchac@email.cz&gt;
diff --git a/src/Symfony/Component/Debug/ErrorHandler.php b/src/Symfony/Component/Debug/ErrorHandler.php
@@ -517,15 +517,20 @@ public function handleError($type, $message, $file, $line)
                 $errorAsException ? ['exception' => $errorAsException] : [],
             ];
         } else {
+            if (!$hhvm = \defined('HHVM_VERSION')) {
+                $currentErrorHandler = set_error_handler('var_dump');
+                restore_error_handler();
+            }
+
             try {
                 $this->isRecursive = true;
                 $level = ($type & $level) ? $this->loggers[$type][1] : LogLevel::DEBUG;
                 $this->loggers[$type][0]->log($level, $logMessage, $errorAsException ? ['exception' => $errorAsException] : []);
             } finally {
                 $this->isRecursive = false;
 
-                if (!\defined('HHVM_VERSION')) {
-                    set_error_handler([$this, __FUNCTION__]);
+                if (!$hhvm) {
+                    set_error_handler($currentErrorHandler);
                 }
             }
         }
diff --git a/src/Symfony/Component/Debug/Tests/ErrorHandlerTest.php b/src/Symfony/Component/Debug/Tests/ErrorHandlerTest.php
@@ -18,6 +18,7 @@
 use Symfony\Component\Debug\BufferingLogger;
 use Symfony\Component\Debug\ErrorHandler;
 use Symfony\Component\Debug\Exception\SilencedErrorContext;
+use Symfony\Component\Debug\Tests\Fixtures\ErrorHandlerThatUsesThePreviousOne;
 use Symfony\Component\Debug\Tests\Fixtures\LoggerThatSetAnErrorHandler;
 
 /**
@@ -590,26 +591,40 @@ public function testCustomExceptionHandler()
     }
 
     /**
-     * @dataProvider errorHandlerIsNotLostWhenLoggingProvider
+     * @dataProvider errorHandlerWhenLoggingProvider
      */
-    public function testErrorHandlerIsNotLostWhenLogging($customErrorHandlerHasBeenPreviouslyDefined, LoggerInterface $logger)
+    public function testErrorHandlerWhenLogging($previousHandlerWasDefined, $loggerSetsAnotherHandler, $nextHandlerIsDefined)
     {
         try {
-            if ($customErrorHandlerHasBeenPreviouslyDefined) {
+            if ($previousHandlerWasDefined) {
                 set_error_handler('count');
             }
 
+            $logger = $loggerSetsAnotherHandler ? new LoggerThatSetAnErrorHandler() : new NullLogger();
+
             $handler = ErrorHandler::register();
             $handler->setDefaultLogger($logger);
 
+            if ($nextHandlerIsDefined) {
+                $handler = ErrorHandlerThatUsesThePreviousOne::register();
+            }
+
             @trigger_error('foo', E_USER_DEPRECATED);
             @trigger_error('bar', E_USER_DEPRECATED);
 
             $this->assertSame([$handler, 'handleError'], set_error_handler('var_dump'));
 
+            if ($logger instanceof LoggerThatSetAnErrorHandler) {
+                $this->assertCount(2, $logger->cleanLogs());
+            }
+
             restore_error_handler();
 
-            if ($customErrorHandlerHasBeenPreviouslyDefined) {
+            if ($previousHandlerWasDefined) {
+                restore_error_handler();
+            }
+
+            if ($nextHandlerIsDefined) {
                 restore_error_handler();
             }
         } finally {
@@ -618,13 +633,14 @@ public function testErrorHandlerIsNotLostWhenLogging($customErrorHandlerHasBeenP
         }
     }
 
-    public function errorHandlerIsNotLostWhenLoggingProvider()
+    public function errorHandlerWhenLoggingProvider()
     {
-        return [
-            [false, new NullLogger()],
-            [true, new NullLogger()],
-            [false, new LoggerThatSetAnErrorHandler()],
-            [true, new LoggerThatSetAnErrorHandler()],
-        ];
+        foreach ([false, true] as $previousHandlerWasDefined) {
+            foreach ([false, true] as $loggerSetsAnotherHandler) {
+                foreach ([false, true] as $nextHandlerIsDefined) {
+                    yield [$previousHandlerWasDefined, $loggerSetsAnotherHandler, $nextHandlerIsDefined];
+                }
+            }
+        }
     }
 }
diff --git a/src/Symfony/Component/Debug/Tests/Fixtures/ErrorHandlerThatUsesThePreviousOne.php b/src/Symfony/Component/Debug/Tests/Fixtures/ErrorHandlerThatUsesThePreviousOne.php
@@ -0,0 +1,29 @@
+<?php
+
+namespace Symfony\Component\Debug\Tests\Fixtures;
+
+class ErrorHandlerThatUsesThePreviousOne
+{
+    private static $previous;
+
+    public static function register()
+    {
+        $handler = new static();
+
+        self::$previous = set_error_handler([$handler, 'handleError']);
+
+        return $handler;
+    }
+
+    public function handleError($type, $message, $file, $line, $context)
+    {
+        return call_user_func(
+            self::$previous,
+            $type,
+            $message,
+            $file,
+            $line,
+            $context
+        );
+    }
+}
diff --git a/src/Symfony/Component/Debug/Tests/Fixtures/LoggerThatSetAnErrorHandler.php b/src/Symfony/Component/Debug/Tests/Fixtures/LoggerThatSetAnErrorHandler.php
@@ -2,13 +2,14 @@
 
 namespace Symfony\Component\Debug\Tests\Fixtures;
 
-use Psr\Log\AbstractLogger;
+use Symfony\Component\Debug\BufferingLogger;
 
-class LoggerThatSetAnErrorHandler extends AbstractLogger
+class LoggerThatSetAnErrorHandler extends BufferingLogger
 {
     public function log($level, $message, array $context = [])
     {
         set_error_handler('is_string');
+        parent::log($level, $message, $context);
         restore_error_handler();
     }
 }

Original file line number	Diff line number	Diff line change
`@@ -2,13 +2,14 @@`
`2`	`2`
`3`	`3`	`namespace Symfony\Component\Debug\Tests\Fixtures;`
`4`	`4`
`5`		`-use Psr\Log\AbstractLogger;`
	`5`	`+use Symfony\Component\Debug\BufferingLogger;`
`6`	`6`
`7`		`-class LoggerThatSetAnErrorHandler extends AbstractLogger`
	`7`	`+class LoggerThatSetAnErrorHandler extends BufferingLogger`
`8`	`8`	`{`
`9`	`9`	`public function log($level, $message, array $context = [])`
`10`	`10`	`{`
`11`	`11`	`set_error_handler('is_string');`
	`12`	`+ parent::log($level, $message, $context);`
`12`	`13`	`restore_error_handler();`
`13`	`14`	`}`
`14`	`15`	`}`