Improve error handling when sending TCP/IP data to DNS server fails (macOS)

src/Query/TcpTransportExecutor.php

@@ -184,6 +184,9 @@ public function query(Query $query)
 
             // set socket to non-blocking and wait for it to become writable (connection success/rejected)
             \stream_set_blocking($socket, false);
+            if (\function_exists('stream_set_chunk_size')) {
+                \stream_set_chunk_size($socket, (1 << 31) - 1); // @codeCoverageIgnore
+            }
             $this->socket = $socket;
         }
 
@@ -234,7 +237,12 @@ public function handleWritable()
 
         $written = @\fwrite($this->socket, $this->writeBuffer);
         if ($written === false || $written === 0) {
-            $this->closeError('Unable to write to closed socket');
+            $error = \error_get_last();
+            \preg_match('/errno=(\d+) (.+)/', $error['message'], $m);
+            $this->closeError(
+                'Unable to send query to DNS server (' . (isset($m[2]) ? $m[2] : $error['message']) . ')',
+                isset($m[1]) ? (int) $m[1] : 0
+            );
             return;
         }
 
@@ -300,8 +308,9 @@ public function handleRead()
     /**
      * @internal
      * @param string $reason
+     * @param int    $code
      */
-    public function closeError($reason)
+    public function closeError($reason, $code = 0)
     {
         $this->readBuffer = '';
         if ($this->readPending) {
@@ -325,7 +334,8 @@ public function closeError($reason)
 
         foreach ($this->names as $id => $name) {
             $this->pending[$id]->reject(new \RuntimeException(
-                'DNS query for ' . $name . ' failed: ' . $reason
+                'DNS query for ' . $name . ' failed: ' . $reason,
+                $code
             ));
         }
         $this->pending = $this->names = array();

tests/Query/TcpTransportExecutorTest.php

@@ -245,14 +245,53 @@ function ($e) use (&$wait) {
         $this->assertFalse($wait);
     }
 
+    public function testQueryStaysPendingWhenClientCanNotSendExcessiveMessageInOneChunk()
+    {
+        $loop = $this->getMockBuilder('React\EventLoop\LoopInterface')->getMock();
+        $loop->expects($this->once())->method('addWriteStream');
+        $loop->expects($this->once())->method('addReadStream');
+        $loop->expects($this->never())->method('removeWriteStream');
+        $loop->expects($this->never())->method('removeReadStream');
+
+        $server = stream_socket_server('tcp://127.0.0.1:0');
+
+        $address = stream_socket_get_name($server, false);
+        $executor = new TcpTransportExecutor($address, $loop);
+
+        $query = new Query('google' . str_repeat('.com', 100), Message::TYPE_A, Message::CLASS_IN);
+
+        // send a bunch of queries and keep reference to last promise
+        for ($i = 0; $i < 8000; ++$i) {
+            $promise = $executor->query($query);
+        }
+
+        $client = stream_socket_accept($server);
+        assert(is_resource($client));
+
+        $executor->handleWritable();
+
+        $promise->then(null, 'printf');
+        $promise->then($this->expectCallableNever(), $this->expectCallableNever());
+
+        $ref = new \ReflectionProperty($executor, 'writePending');
+        $ref->setAccessible(true);
+        $writePending = $ref->getValue($executor);
+
+        $this->assertTrue($writePending);
+    }
+
     public function testQueryStaysPendingWhenClientCanNotSendExcessiveMessageInOneChunkWhenServerClosesSocket()
     {
-        $writableCallback = null;
+        if (PHP_OS === 'Darwin') {
+            // Skip on macOS because it exhibits what looks like a kernal race condition when sending excessive data to a socket that is about to shut down (EPROTOTYPE)
+            // Due to this race condition, this is somewhat flaky. Happens around 75% of the time, use `--repeat=100` to reproduce.
+            // fwrite(): Send of 4260000 bytes failed with errno=41 Protocol wrong type for socket
+            // @link http://erickt.github.io/blog/2014/11/19/adventures-in-debugging-a-potential-osx-kernel-bug/
+            $this->markTestSkipped('Skipped on macOS due to possible race condition');
+        }
+
         $loop = $this->getMockBuilder('React\EventLoop\LoopInterface')->getMock();
-        $loop->expects($this->once())->method('addWriteStream')->with($this->anything(), $this->callback(function ($cb) use (&$writableCallback) {
-            $writableCallback = $cb;
-            return true;
-        }));
+        $loop->expects($this->once())->method('addWriteStream');
         $loop->expects($this->once())->method('addReadStream');
         $loop->expects($this->never())->method('removeWriteStream');
         $loop->expects($this->never())->method('removeReadStream');
@@ -262,10 +301,10 @@ public function testQueryStaysPendingWhenClientCanNotSendExcessiveMessageInOneCh
         $address = stream_socket_get_name($server, false);
         $executor = new TcpTransportExecutor($address, $loop);
 
-        $query = new Query('google' . str_repeat('.com', 10000), Message::TYPE_A, Message::CLASS_IN);
+        $query = new Query('google' . str_repeat('.com', 100), Message::TYPE_A, Message::CLASS_IN);
 
         // send a bunch of queries and keep reference to last promise
-        for ($i = 0; $i < 100; ++$i) {
+        for ($i = 0; $i < 2000; ++$i) {
             $promise = $executor->query($query);
         }
 
@@ -283,6 +322,57 @@ public function testQueryStaysPendingWhenClientCanNotSendExcessiveMessageInOneCh
         $this->assertTrue($writePending);
     }
 
+    public function testQueryRejectsWhenClientKeepsSendingWhenServerClosesSocket()
+    {
+        $loop = $this->getMockBuilder('React\EventLoop\LoopInterface')->getMock();
+        $loop->expects($this->once())->method('addWriteStream');
+        $loop->expects($this->once())->method('addReadStream');
+        $loop->expects($this->once())->method('removeWriteStream');
+        $loop->expects($this->once())->method('removeReadStream');
+
+        $server = stream_socket_server('tcp://127.0.0.1:0');
+
+        $address = stream_socket_get_name($server, false);
+        $executor = new TcpTransportExecutor($address, $loop);
+
+        $query = new Query('google' . str_repeat('.com', 100), Message::TYPE_A, Message::CLASS_IN);
+
+        // send a bunch of queries and keep reference to last promise
+        for ($i = 0; $i < 2000; ++$i) {
+            $promise = $executor->query($query);
+        }
+
+        $client = stream_socket_accept($server);
+        fclose($client);
+
+        $executor->handleWritable();
+
+        $ref = new \ReflectionProperty($executor, 'writePending');
+        $ref->setAccessible(true);
+        $writePending = $ref->getValue($executor);
+
+        // We expect an EPIPE (Broken pipe) on second write.
+        // However, macOS may report EPROTOTYPE (Protocol wrong type for socket) on first write due to kernel race condition.
+        // fwrite(): Send of 4260000 bytes failed with errno=41 Protocol wrong type for socket
+        // @link http://erickt.github.io/blog/2014/11/19/adventures-in-debugging-a-potential-osx-kernel-bug/
+        if ($writePending) {
+            $executor->handleWritable();
+        }
+
+        $exception = null;
+        $promise->then(null, function ($reason) use (&$exception) {
+            $exception = $reason;
+        });
+
+        // expect EPIPE (Broken pipe), except for macOS kernel race condition or legacy HHVM
+        $this->setExpectedException(
+            'RuntimeException',
+            'Unable to send query to DNS server',
+            defined('SOCKET_EPIPE') && !defined('HHVM_VERSION') ? (PHP_OS !== 'Darwin' || $writePending ? SOCKET_EPIPE : SOCKET_EPROTOTYPE) : null
+        );
+        throw $exception;
+    }
+
     public function testQueryRejectsWhenServerClosesConnection()
     {
         $loop = Factory::create();

tests/Query/UdpTransportExecutorTest.php

@@ -153,7 +153,7 @@ public function testQueryRejectsIfSendToServerFailsAfterConnection()
             $exception = $reason;
         });
 
-        // ECONNREFUSED( Connection refused) on Linux, EMSGSIZE (Message too long) on macOS
+        // ECONNREFUSED (Connection refused) on Linux, EMSGSIZE (Message too long) on macOS
         $this->setExpectedException('RuntimeException', 'Unable to send query to DNS server');
         throw $exception;
     }