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

Improve error reporting when custom error handler is used

src/Query/TcpTransportExecutor.php

@@ -246,13 +246,24 @@ public function handleWritable()
             $this->loop->addReadStream($this->socket, array($this, 'handleRead'));
         }
 
-        $written = @\fwrite($this->socket, $this->writeBuffer);
+        $errno = 0;
+        $errstr = '';
+        \set_error_handler(function ($_, $error) use (&$errno, &$errstr) {
+            // Match errstr from PHP's warning message.
+            // fwrite(): Send of 327712 bytes failed with errno=32 Broken pipe
+            \preg_match('/errno=(\d+) (.+)/', $error, $m);
+            $errno = isset($m[1]) ? (int) $m[1] : 0;
+            $errstr = isset($m[2]) ? $m[2] : $error;
+        });
+
+        $written = \fwrite($this->socket, $this->writeBuffer);
+
+        \restore_error_handler();
+
         if ($written === false || $written === 0) {
-            $error = \error_get_last();
-            \preg_match('/errno=(\d+) (.+)/', $error['message'], $m);
             $this->closeError(
-                'Unable to send query to DNS server ' . $this->nameserver . ' (' . (isset($m[2]) ? $m[2] : $error['message']) . ')',
-                isset($m[1]) ? (int) $m[1] : 0
+                'Unable to send query to DNS server ' . $this->nameserver . ' (' . $errstr . ')',
+                $errno
             );
             return;
         }

src/Query/UdpTransportExecutor.php

@@ -129,6 +129,8 @@ public function query(Query $query)
         }
 
         // UDP connections are instant, so try connection without a loop or timeout
+        $errno = 0;
+        $errstr = '';
         $socket = @\stream_socket_client($this->nameserver, $errno, $errstr, 0);
         if ($socket === false) {
             return \React\Promise\reject(new \RuntimeException(
@@ -139,18 +141,25 @@ public function query(Query $query)
 
         // set socket to non-blocking and immediately try to send (fill write buffer)
         \stream_set_blocking($socket, false);
-        $written = @\fwrite($socket, $queryData);
 
-        if ($written !== \strlen($queryData)) {
+        \set_error_handler(function ($_, $error) use (&$errno, &$errstr) {
             // Write may potentially fail, but most common errors are already caught by connection check above.
             // Among others, macOS is known to report here when trying to send to broadcast address.
             // This can also be reproduced by writing data exceeding `stream_set_chunk_size()` to a server refusing UDP data.
             // fwrite(): send of 8192 bytes failed with errno=111 Connection refused
-            $error = \error_get_last();
-            \preg_match('/errno=(\d+) (.+)/', $error['message'], $m);
+            \preg_match('/errno=(\d+) (.+)/', $error, $m);
+            $errno = isset($m[1]) ? (int) $m[1] : 0;
+            $errstr = isset($m[2]) ? $m[2] : $error;
+        });
+
+        $written = \fwrite($socket, $queryData);
+
+        \restore_error_handler();
+
+        if ($written !== \strlen($queryData)) {
             return \React\Promise\reject(new \RuntimeException(
-                'DNS query for ' . $query->describe() . ' failed: Unable to send query to DNS server ' . $this->nameserver . ' ('  . (isset($m[2]) ? $m[2] : $error['message']) . ')',
-                isset($m[1]) ? (int) $m[1] : 0
+                'DNS query for ' . $query->describe() . ' failed: Unable to send query to DNS server ' . $this->nameserver . ' ('  . $errstr . ')',
+                $errno
             ));
         }
 

tests/Query/TcpTransportExecutorTest.php

@@ -357,7 +357,7 @@ public function testQueryStaysPendingWhenClientCanNotSendExcessiveMessageInOneCh
         $this->assertTrue($writePending);
     }
 
-    public function testQueryRejectsWhenClientKeepsSendingWhenServerClosesSocket()
+    public function testQueryRejectsWhenClientKeepsSendingWhenServerClosesSocketWithoutCallingCustomErrorHandler()
     {
         $loop = $this->getMockBuilder('React\EventLoop\LoopInterface')->getMock();
         $loop->expects($this->once())->method('addWriteStream');
@@ -380,6 +380,11 @@ public function testQueryRejectsWhenClientKeepsSendingWhenServerClosesSocket()
         $client = stream_socket_accept($server);
         fclose($client);
 
+        $error = null;
+        set_error_handler(function ($_, $errstr) use (&$error) {
+            $error = $errstr;
+        });
+
         $executor->handleWritable();
 
         $ref = new \ReflectionProperty($executor, 'writePending');
@@ -394,6 +399,9 @@ public function testQueryRejectsWhenClientKeepsSendingWhenServerClosesSocket()
             $executor->handleWritable();
         }
 
+        restore_error_handler();
+        $this->assertNull($error);
+
         $exception = null;
         $promise->then(null, function ($reason) use (&$exception) {
             $exception = $reason;

tests/Query/UdpTransportExecutorTest.php

@@ -152,7 +152,7 @@ public function testQueryRejectsIfServerConnectionFails()
         throw $exception;
     }
 
-    public function testQueryRejectsIfSendToServerFailsAfterConnection()
+    public function testQueryRejectsIfSendToServerFailsAfterConnectionWithoutCallingCustomErrorHandler()
     {
         $loop = $this->getMockBuilder('React\EventLoop\LoopInterface')->getMock();
         $loop->expects($this->never())->method('addReadStream');
@@ -164,9 +164,17 @@ public function testQueryRejectsIfSendToServerFailsAfterConnection()
         $ref->setAccessible(true);
         $ref->setValue($executor, PHP_INT_MAX);
 
+        $error = null;
+        set_error_handler(function ($_, $errstr) use (&$error) {
+            $error = $errstr;
+        });
+
         $query = new Query(str_repeat('a.', 100000) . '.example', Message::TYPE_A, Message::CLASS_IN);
         $promise = $executor->query($query);
 
+        restore_error_handler();
+        $this->assertNull($error);
+
         $this->assertInstanceOf('React\Promise\PromiseInterface', $promise);
 
         $exception = null;