[Debug][ErrorHandler] Preserve next error handler

Co-authored-by: Joe <cuchac@email.cz>
symfony · Feb 15, 2019 · 6621e8e · 6621e8e
1 parent 37b0eeb
commit 6621e8e
Show file tree

Hide file tree

Showing 4 changed files with 66 additions and 15 deletions.
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();
     }
 }