Merge pull request #290 from clue-labs/error-handler

Improve error reporting when custom error handler is used
reactphp · Apr 19, 2022 · ccca668 · ccca668
2 parents aa95c15 + 4227053
commit ccca668
Show file tree

Hide file tree

Showing 8 changed files with 132 additions and 42 deletions.
diff --git a/src/FdServer.php b/src/FdServer.php
@@ -89,15 +89,21 @@ public function __construct($fd, LoopInterface $loop = null)
 
         $this->loop = $loop ?: Loop::get();
 
-        $this->master = @\fopen('php://fd/' . $fd, 'r+');
-        if (false === $this->master) {
+        $errno = 0;
+        $errstr = '';
+        \set_error_handler(function ($_, $error) use (&$errno, &$errstr) {
             // Match errstr from PHP's warning message.
             // fopen(php://fd/3): Failed to open stream: Error duping file descriptor 3; possibly it doesn't exist: [9]: Bad file descriptor
-            $error = \error_get_last();
-            \preg_match('/\[(\d+)\]: (.*)/', $error['message'], $m);
+            \preg_match('/\[(\d+)\]: (.*)/', $error, $m);
             $errno = isset($m[1]) ? (int) $m[1] : 0;
-            $errstr = isset($m[2]) ? $m[2] : $error['message'];
+            $errstr = isset($m[2]) ? $m[2] : $error;
+        });
+
+        $this->master = \fopen('php://fd/' . $fd, 'r+');
 
+        \restore_error_handler();
+
+        if (false === $this->master) {
             throw new \RuntimeException(
                 'Failed to listen on FD ' . $fd . ': ' . $errstr . SocketServer::errconst($errno),
                 $errno

diff --git a/src/SocketServer.php b/src/SocketServer.php
@@ -106,15 +106,20 @@ public function close()
      */
     public static function accept($socket)
     {
-        $newSocket = @\stream_socket_accept($socket, 0);
-
-        if (false === $newSocket) {
+        $errno = 0;
+        $errstr = '';
+        \set_error_handler(function ($_, $error) use (&$errno, &$errstr) {
             // Match errstr from PHP's warning message.
             // stream_socket_accept(): accept failed: Connection timed out
-            $error = \error_get_last();
-            $errstr = \preg_replace('#.*: #', '', $error['message']);
-            $errno = self::errno($errstr);
+            $errstr = \preg_replace('#.*: #', '', $error);
+            $errno = SocketServer::errno($errstr);
+        });
 
+        $newSocket = \stream_socket_accept($socket, 0);
+
+        \restore_error_handler();
+
+        if (false === $newSocket) {
             throw new \RuntimeException(
                 'Unable to accept new connection: ' . $errstr . self::errconst($errno),
                 $errno

diff --git a/src/TcpConnector.php b/src/TcpConnector.php
@@ -116,13 +116,19 @@ public function connect($uri)
                         // Linux reports socket errno and errstr again when trying to write to the dead socket.
                         // Suppress error reporting to get error message below and close dead socket before rejecting.
                         // This is only known to work on Linux, Mac and Windows are known to not support this.
-                        @\fwrite($stream, \PHP_EOL);
-                        $error = \error_get_last();
-
-                        // fwrite(): send of 2 bytes failed with errno=111 Connection refused
-                        \preg_match('/errno=(\d+) (.+)/', $error['message'], $m);
-                        $errno = isset($m[1]) ? (int) $m[1] : 0;
-                        $errstr = isset($m[2]) ? $m[2] : $error['message'];
+                        $errno = 0;
+                        $errstr = '';
+                        \set_error_handler(function ($_, $error) use (&$errno, &$errstr) {
+                            // Match errstr from PHP's warning message.
+                            // fwrite(): send of 1 bytes failed with errno=111 Connection refused
+                            \preg_match('/errno=(\d+) (.+)/', $error, $m);
+                            $errno = isset($m[1]) ? (int) $m[1] : 0;
+                            $errstr = isset($m[2]) ? $m[2] : $error;
+                        });
+
+                        \fwrite($stream, \PHP_EOL);
+
+                        \restore_error_handler();
                     } else {
                         // Not on Linux and ext-sockets not available? Too bad.
                         $errno = \defined('SOCKET_ECONNREFUSED') ? \SOCKET_ECONNREFUSED : 111;

diff --git a/src/UnixServer.php b/src/UnixServer.php
@@ -63,25 +63,29 @@ public function __construct($path, LoopInterface $loop = null, array $context =
             );
         }
 
-        $this->master = @\stream_socket_server(
+        $errno = 0;
+        $errstr = '';
+        \set_error_handler(function ($_, $error) use (&$errno, &$errstr) {
+            // PHP does not seem to report errno/errstr for Unix domain sockets (UDS) right now.
+            // This only applies to UDS server sockets, see also https://3v4l.org/NAhpr.
+            // Parse PHP warning message containing unknown error, HHVM reports proper info at least.
+            if (\preg_match('/\(([^\)]+)\)|\[(\d+)\]: (.*)/', $error, $match)) {
+                $errstr = isset($match[3]) ? $match['3'] : $match[1];
+                $errno = isset($match[2]) ? (int)$match[2] : 0;
+            }
+        });
+
+        $this->master = \stream_socket_server(
             $path,
             $errno,
             $errstr,
             \STREAM_SERVER_BIND | \STREAM_SERVER_LISTEN,
             \stream_context_create(array('socket' => $context))
         );
-        if (false === $this->master) {
-            // PHP does not seem to report errno/errstr for Unix domain sockets (UDS) right now.
-            // This only applies to UDS server sockets, see also https://3v4l.org/NAhpr.
-            // Parse PHP warning message containing unknown error, HHVM reports proper info at least.
-            if ($errno === 0 && $errstr === '') {
-                $error = \error_get_last();
-                if (\preg_match('/\(([^\)]+)\)|\[(\d+)\]: (.*)/', $error['message'], $match)) {
-                    $errstr = isset($match[3]) ? $match['3'] : $match[1];
-                    $errno = isset($match[2]) ? (int)$match[2] : 0;
-                }
-            }
 
+        \restore_error_handler();
+
+        if (false === $this->master) {
             throw new \RuntimeException(
                 'Failed to listen on Unix domain socket "' . $path . '": ' . $errstr . SocketServer::errconst($errno),
                 $errno

diff --git a/tests/FdServerTest.php b/tests/FdServerTest.php
@@ -51,7 +51,7 @@ public function testCtorThrowsForInvalidUrl()
         new FdServer('tcp://127.0.0.1:8080', $loop);
     }
 
-    public function testCtorThrowsForUnknownFd()
+    public function testCtorThrowsForUnknownFdWithoutCallingCustomErrorHandler()
     {
         if (!is_dir('/dev/fd') || defined('HHVM_VERSION')) {
             $this->markTestSkipped('Not supported on your platform');
@@ -62,12 +62,27 @@ public function testCtorThrowsForUnknownFd()
         $loop = $this->getMockBuilder('React\EventLoop\LoopInterface')->getMock();
         $loop->expects($this->never())->method('addReadStream');
 
+        $error = null;
+        set_error_handler(function ($_, $errstr) use (&$error) {
+            $error = $errstr;
+        });
+
         $this->setExpectedException(
             'RuntimeException',
             'Failed to listen on FD ' . $fd . ': ' . (function_exists('socket_strerror') ? socket_strerror(SOCKET_EBADF) . ' (EBADF)' : 'Bad file descriptor'),
             defined('SOCKET_EBADF') ? SOCKET_EBADF : 9
         );
-        new FdServer($fd, $loop);
+
+        try {
+            new FdServer($fd, $loop);
+
+            restore_error_handler();
+        } catch (\Exception $e) {
+            restore_error_handler();
+            $this->assertNull($error);
+
+            throw $e;
+        }
     }
 
     public function testCtorThrowsIfFdIsAFileAndNotASocket()
@@ -319,7 +334,7 @@ public function testServerEmitsConnectionEventForNewConnection()
         $server->close();
     }
 
-    public function testEmitsErrorWhenAcceptListenerFails()
+    public function testEmitsErrorWhenAcceptListenerFailsWithoutCallingCustomErrorHandler()
     {
         if (!is_dir('/dev/fd') || defined('HHVM_VERSION')) {
             $this->markTestSkipped('Not supported on your platform');
@@ -346,10 +361,18 @@ public function testEmitsErrorWhenAcceptListenerFails()
         $this->assertNotNull($listener);
         $socket = stream_socket_server('tcp://127.0.0.1:0');
 
+        $error = null;
+        set_error_handler(function ($_, $errstr) use (&$error) {
+            $error = $errstr;
+        });
+
         $time = microtime(true);
         $listener($socket);
         $time = microtime(true) - $time;
 
+        restore_error_handler();
+        $this->assertNull($error);
+
         $this->assertLessThan(1, $time);
 
         $this->assertInstanceOf('RuntimeException', $exception);
@@ -362,7 +385,7 @@ public function testEmitsErrorWhenAcceptListenerFails()
     /**
      * @param \RuntimeException $e
      * @requires extension sockets
-     * @depends testEmitsErrorWhenAcceptListenerFails
+     * @depends testEmitsErrorWhenAcceptListenerFailsWithoutCallingCustomErrorHandler
      */
     public function testEmitsTimeoutErrorWhenAcceptListenerFails(\RuntimeException $exception)
     {

diff --git a/tests/TcpConnectorTest.php b/tests/TcpConnectorTest.php
@@ -25,17 +25,32 @@ public function testConstructWithoutLoopAssignsLoopAutomatically()
     }
 
     /** @test */
-    public function connectionToEmptyPortShouldFail()
+    public function connectionToEmptyPortShouldFailWithoutCallingCustomErrorHandler()
     {
         $connector = new TcpConnector();
         $promise = $connector->connect('127.0.0.1:9999');
 
+        $error = null;
+        set_error_handler(function ($_, $errstr) use (&$error) {
+            $error = $errstr;
+        });
+
         $this->setExpectedException(
             'RuntimeException',
             'Connection to tcp://127.0.0.1:9999 failed: Connection refused' . (function_exists('socket_import_stream') ? ' (ECONNREFUSED)' : ''),
             defined('SOCKET_ECONNREFUSED') ? SOCKET_ECONNREFUSED : 111
         );
-        Block\await($promise, null, self::TIMEOUT);
+
+        try {
+            Block\await($promise, null, self::TIMEOUT);
+
+            restore_error_handler();
+        } catch (\Exception $e) {
+            restore_error_handler();
+            $this->assertNull($error);
+
+            throw $e;
+        }
     }
 
     /** @test */

diff --git a/tests/TcpServerTest.php b/tests/TcpServerTest.php
@@ -276,7 +276,7 @@ public function testCloseRemovesResourceFromLoop()
         $server->close();
     }
 
-    public function testEmitsErrorWhenAcceptListenerFails()
+    public function testEmitsErrorWhenAcceptListenerFailsWithoutCallingCustomErrorHandler()
     {
         $listener = null;
         $loop = $this->getMockBuilder('React\EventLoop\LoopInterface')->getMock();
@@ -295,10 +295,18 @@ public function testEmitsErrorWhenAcceptListenerFails()
         $this->assertNotNull($listener);
         $socket = stream_socket_server('tcp://127.0.0.1:0');
 
+        $error = null;
+        set_error_handler(function ($_, $errstr) use (&$error) {
+            $error = $errstr;
+        });
+
         $time = microtime(true);
         $listener($socket);
         $time = microtime(true) - $time;
 
+        restore_error_handler();
+        $this->assertNull($error);
+
         $this->assertLessThan(1, $time);
 
         $this->assertInstanceOf('RuntimeException', $exception);
@@ -311,7 +319,7 @@ public function testEmitsErrorWhenAcceptListenerFails()
     /**
      * @param \RuntimeException $e
      * @requires extension sockets
-     * @depends testEmitsErrorWhenAcceptListenerFails
+     * @depends testEmitsErrorWhenAcceptListenerFailsWithoutCallingCustomErrorHandler
      */
     public function testEmitsTimeoutErrorWhenAcceptListenerFails(\RuntimeException $exception)
     {

diff --git a/tests/UnixServerTest.php b/tests/UnixServerTest.php
@@ -240,12 +240,27 @@ public function testCtorThrowsForInvalidAddressScheme()
         new UnixServer('tcp://localhost:0', $loop);
     }
 
-    public function testCtorThrowsWhenPathIsNotWritable()
+    public function testCtorThrowsWhenPathIsNotWritableWithoutCallingCustomErrorHandler()
     {
         $loop = $this->getMockBuilder('React\EventLoop\LoopInterface')->getMock();
 
+        $error = null;
+        set_error_handler(function ($_, $errstr) use (&$error) {
+            $error = $errstr;
+        });
+
         $this->setExpectedException('RuntimeException');
-        $server = new UnixServer('/dev/null', $loop);
+
+        try {
+            new UnixServer('/dev/null', $loop);
+
+            restore_error_handler();
+        } catch (\Exception $e) {
+            restore_error_handler();
+            $this->assertNull($error);
+
+            throw $e;
+        }
     }
 
     public function testResumeWithoutPauseIsNoOp()
@@ -285,7 +300,7 @@ public function testCloseRemovesResourceFromLoop()
         $server->close();
     }
 
-    public function testEmitsErrorWhenAcceptListenerFails()
+    public function testEmitsErrorWhenAcceptListenerFailsWithoutCallingCustomErrorHandler()
     {
         $listener = null;
         $loop = $this->getMockBuilder('React\EventLoop\LoopInterface')->getMock();
@@ -304,10 +319,18 @@ public function testEmitsErrorWhenAcceptListenerFails()
         $this->assertNotNull($listener);
         $socket = stream_socket_server('tcp://127.0.0.1:0');
 
+        $error = null;
+        set_error_handler(function ($_, $errstr) use (&$error) {
+            $error = $errstr;
+        });
+
         $time = microtime(true);
         $listener($socket);
         $time = microtime(true) - $time;
 
+        restore_error_handler();
+        $this->assertNull($error);
+
         $this->assertLessThan(1, $time);
 
         $this->assertInstanceOf('RuntimeException', $exception);
@@ -320,7 +343,7 @@ public function testEmitsErrorWhenAcceptListenerFails()
     /**
      * @param \RuntimeException $e
      * @requires extension sockets
-     * @depends testEmitsErrorWhenAcceptListenerFails
+     * @depends testEmitsErrorWhenAcceptListenerFailsWithoutCallingCustomErrorHandler
      */
     public function testEmitsTimeoutErrorWhenAcceptListenerFails(\RuntimeException $exception)
     {