Merge pull request #174 from clue-labs/errors

Improve error reporting when query fails, include domain and query type and DNS server address where applicable

examples/92-query-any.php

@@ -73,7 +73,7 @@
                 break;
             default:
                 // unknown type uses HEX format
-                $type = 'Type ' . $answer->type;
+                $type = 'TYPE' . $answer->type;
                 $data = wordwrap(strtoupper(bin2hex($data)), 2, ' ', true);
         }
 

src/Query/CachingExecutor.php

@@ -57,7 +57,7 @@ function (Message $message) use ($cache, $id, $that) {
                 $pending = null;
             });
         }, function ($_, $reject) use (&$pending, $query) {
-            $reject(new \RuntimeException('DNS query for ' . $query->name . ' has been cancelled'));
+            $reject(new \RuntimeException('DNS query for ' . $query->describe() . ' has been cancelled'));
             $pending->cancel();
             $pending = null;
         });

src/Query/CoopExecutor.php

@@ -81,7 +81,7 @@ public function query(Query $query)
                 $promise->cancel();
                 $promise = null;
             }
-            throw new \RuntimeException('DNS query for ' . $query->name . ' has been cancelled');
+            throw new \RuntimeException('DNS query for ' . $query->describe() . ' has been cancelled');
         });
     }
 

src/Query/Query.php

@@ -2,6 +2,8 @@
 
 namespace React\Dns\Query;
 
+use React\Dns\Model\Message;
+
 /**
  * This class represents a single question in a query/response message
  *
@@ -39,4 +41,29 @@ public function __construct($name, $type, $class)
         $this->type = $type;
         $this->class = $class;
     }
+
+    /**
+     * Describes the hostname and query type/class for this query
+     *
+     * The output format is supposed to be human readable and is subject to change.
+     * The format is inspired by RFC 3597 when handling unkown types/classes.
+     *
+     * @return string "example.com (A)" or "example.com (CLASS0 TYPE1234)"
+     * @link https://tools.ietf.org/html/rfc3597
+     */
+    public function describe()
+    {
+        $class = $this->class !== Message::CLASS_IN ? 'CLASS' . $this->class . ' ' : '';
+
+        $type = 'TYPE' . $this->type;
+        $ref = new \ReflectionClass('React\Dns\Model\Message');
+        foreach ($ref->getConstants() as $name => $value) {
+            if ($value === $this->type && \strpos($name, 'TYPE_') === 0) {
+                $type = \substr($name, 5);
+                break;
+            }
+        }
+
+        return $this->name . ' (' . $class . $type . ')';
+    }
 }

src/Query/RetryExecutor.php

@@ -43,7 +43,7 @@ public function tryQuery(Query $query, $retries)
             } elseif ($retries <= 0) {
                 $errorback = null;
                 $deferred->reject($e = new \RuntimeException(
-                    'DNS query for ' . $query->name . ' failed: too many retries',
+                    'DNS query for ' . $query->describe() . ' failed: too many retries',
                     0,
                     $e
                 ));

src/Query/TcpTransportExecutor.php

@@ -147,7 +147,7 @@ public function __construct($nameserver, LoopInterface $loop)
             throw new \InvalidArgumentException('Invalid nameserver address given');
         }
 
-        $this->nameserver = $parts['host'] . ':' . (isset($parts['port']) ? $parts['port'] : 53);
+        $this->nameserver = 'tcp://' . $parts['host'] . ':' . (isset($parts['port']) ? $parts['port'] : 53);
         $this->loop = $loop;
         $this->parser = new Parser();
         $this->dumper = new BinaryDumper();
@@ -166,7 +166,7 @@ public function query(Query $query)
         $length = \strlen($queryData);
         if ($length > 0xffff) {
             return \React\Promise\reject(new \RuntimeException(
-                'DNS query for ' . $query->name . ' failed: Query too large for TCP transport'
+                'DNS query for ' . $query->describe() . ' failed: Query too large for TCP transport'
             ));
         }
 
@@ -177,7 +177,7 @@ public function query(Query $query)
             $socket = @\stream_socket_client($this->nameserver, $errno, $errstr, 0, \STREAM_CLIENT_CONNECT | \STREAM_CLIENT_ASYNC_CONNECT);
             if ($socket === false) {
                 return \React\Promise\reject(new \RuntimeException(
-                    'DNS query for ' . $query->name . ' failed: Unable to connect to DNS server ('  . $errstr . ')',
+                    'DNS query for ' . $query->describe() . ' failed: Unable to connect to DNS server ' . $this->nameserver . ' ('  . $errstr . ')',
                     $errno
                 ));
             }
@@ -214,7 +214,7 @@ public function query(Query $query)
         });
 
         $this->pending[$request->id] = $deferred;
-        $this->names[$request->id] = $query->name;
+        $this->names[$request->id] = $query->describe();
 
         return $deferred->promise();
     }
@@ -227,7 +227,19 @@ public function handleWritable()
         if ($this->readPending === false) {
             $name = @\stream_socket_get_name($this->socket, true);
             if ($name === false) {
-                $this->closeError('Connection to DNS server rejected');
+                // Connection failed? Check socket error if available for underlying errno/errstr.
+                // @codeCoverageIgnoreStart
+                if (\function_exists('socket_import_stream')) {
+                    $socket = \socket_import_stream($this->socket);
+                    $errno = \socket_get_option($socket, \SOL_SOCKET, \SO_ERROR);
+                    $errstr = \socket_strerror($errno);
+                } else {
+                    $errno = \defined('SOCKET_ECONNREFUSED') ? \SOCKET_ECONNREFUSED : 111;
+                    $errstr = 'Connection refused';
+                }
+                // @codeCoverageIgnoreEnd
+
+                $this->closeError('Unable to connect to DNS server ' . $this->nameserver . ' (' . $errstr . ')', $errno);
                 return;
             }
 
@@ -240,7 +252,7 @@ public function handleWritable()
             $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']) . ')',
+                'Unable to send query to DNS server ' . $this->nameserver . ' (' . (isset($m[2]) ? $m[2] : $error['message']) . ')',
                 isset($m[1]) ? (int) $m[1] : 0
             );
             return;
@@ -264,7 +276,7 @@ public function handleRead()
         // any error is fatal, this is a stream of TCP/IP data
         $chunk = @\fread($this->socket, 65536);
         if ($chunk === false || $chunk === '') {
-            $this->closeError('Connection to DNS server lost');
+            $this->closeError('Connection to DNS server ' . $this->nameserver . ' lost');
             return;
         }
 
@@ -286,13 +298,13 @@ public function handleRead()
                 $response = $this->parser->parseMessage($data);
             } catch (\Exception $e) {
                 // reject all pending queries if we received an invalid message from remote server
-                $this->closeError('Invalid message received from DNS server');
+                $this->closeError('Invalid message received from DNS server ' . $this->nameserver);
                 return;
             }
 
             // reject all pending queries if we received an unexpected response ID or truncated response
             if (!isset($this->pending[$response->id]) || $response->tc) {
-                $this->closeError('Invalid response message received from DNS server');
+                $this->closeError('Invalid response message received from DNS server ' . $this->nameserver);
                 return;
             }
 

src/Query/TimeoutExecutor.php

@@ -22,7 +22,7 @@ public function query(Query $query)
     {
         return Timer\timeout($this->executor->query($query), $this->timeout, $this->loop)->then(null, function ($e) use ($query) {
             if ($e instanceof Timer\TimeoutException) {
-                $e = new TimeoutException(sprintf("DNS query for %s timed out", $query->name), 0, $e);
+                $e = new TimeoutException(sprintf("DNS query for %s timed out", $query->describe()), 0, $e);
             }
             throw $e;
         });

src/Query/UdpTransportExecutor.php

@@ -128,7 +128,7 @@ public function query(Query $query)
         $queryData = $this->dumper->toBinary($request);
         if (isset($queryData[$this->maxPacketSize])) {
             return \React\Promise\reject(new \RuntimeException(
-                'DNS query for ' . $query->name . ' failed: Query too large for UDP transport',
+                'DNS query for ' . $query->describe() . ' failed: Query too large for UDP transport',
                 \defined('SOCKET_EMSGSIZE') ? \SOCKET_EMSGSIZE : 90
             ));
         }
@@ -137,7 +137,7 @@ public function query(Query $query)
         $socket = @\stream_socket_client($this->nameserver, $errno, $errstr, 0);
         if ($socket === false) {
             return \React\Promise\reject(new \RuntimeException(
-                'DNS query for ' . $query->name . ' failed: Unable to connect to DNS server ('  . $errstr . ')',
+                'DNS query for ' . $query->describe() . ' failed: Unable to connect to DNS server ' . $this->nameserver . ' ('  . $errstr . ')',
                 $errno
             ));
         }
@@ -154,7 +154,7 @@ public function query(Query $query)
             $error = \error_get_last();
             \preg_match('/errno=(\d+) (.+)/', $error['message'], $m);
             return \React\Promise\reject(new \RuntimeException(
-                'DNS query for ' . $query->name . ' failed: Unable to send query to DNS server ('  . (isset($m[2]) ? $m[2] : $error['message']) . ')',
+                '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
             ));
         }
@@ -165,12 +165,13 @@ public function query(Query $query)
             $loop->removeReadStream($socket);
             \fclose($socket);
 
-            throw new CancellationException('DNS query for ' . $query->name . ' has been cancelled');
+            throw new CancellationException('DNS query for ' . $query->describe() . ' has been cancelled');
         });
 
         $max = $this->maxPacketSize;
         $parser = $this->parser;
-        $loop->addReadStream($socket, function ($socket) use ($loop, $deferred, $query, $parser, $request, $max) {
+        $nameserver = $this->nameserver;
+        $loop->addReadStream($socket, function ($socket) use ($loop, $deferred, $query, $parser, $request, $max, $nameserver) {
             // try to read a single data packet from the DNS server
             // ignoring any errors, this is uses UDP packets and not a stream of data
             $data = @\fread($socket, $max);
@@ -198,7 +199,7 @@ public function query(Query $query)
 
             if ($response->tc) {
                 $deferred->reject(new \RuntimeException(
-                    'DNS query for ' . $query->name . ' failed: The server returned a truncated result for a UDP query',
+                    'DNS query for ' . $query->describe() . ' failed: The DNS server ' . $nameserver . ' returned a truncated result for a UDP query',
                     \defined('SOCKET_EMSGSIZE') ? \SOCKET_EMSGSIZE : 90
                 ));
                 return;

src/Resolver/Resolver.php

@@ -72,7 +72,7 @@ public function extractValues(Query $query, Message $response)
                     $message = 'Unknown error response code ' . $code;
             }
             throw new RecordNotFoundException(
-                'DNS query for ' . $query->name . ' returned an error response (' . $message . ')',
+                'DNS query for ' . $query->describe() . ' returned an error response (' . $message . ')',
                 $code
             );
         }
@@ -83,7 +83,7 @@ public function extractValues(Query $query, Message $response)
         // reject if we did not receive a valid answer (domain is valid, but no record for this type could be found)
         if (0 === count($addresses)) {
             throw new RecordNotFoundException(
-                'DNS query for ' . $query->name . ' did not return a valid answer (NOERROR / NODATA)'
+                'DNS query for ' . $query->describe() . ' did not return a valid answer (NOERROR / NODATA)'
             );
         }
 

tests/FunctionalResolverTest.php

@@ -107,34 +107,63 @@ public function testResolveAllGoogleCaaResolvesWithCache()
      */
     public function testResolveInvalidRejects()
     {
-        $ex = $this->callback(function ($param) {
-            return ($param instanceof RecordNotFoundException && $param->getCode() === Message::RCODE_NAME_ERROR);
-        });
-
         $promise = $this->resolver->resolve('example.invalid');
-        $promise->then($this->expectCallableNever(), $this->expectCallableOnceWith($ex));
 
         $this->loop->run();
+
+        $exception = null;
+        $promise->then(null, function ($reason) use (&$exception) {
+            $exception = $reason;
+        });
+
+        /** @var \React\Dns\RecordNotFoundException $exception */
+        $this->assertInstanceOf('React\Dns\RecordNotFoundException', $exception);
+        $this->assertEquals('DNS query for example.invalid (A) returned an error response (Non-Existent Domain / NXDOMAIN)', $exception->getMessage());
+        $this->assertEquals(Message::RCODE_NAME_ERROR, $exception->getCode());
     }
 
     public function testResolveCancelledRejectsImmediately()
     {
         // max_nesting_level was set to 100 for PHP Versions < 5.4 which resulted in failing test for legacy PHP
         ini_set('xdebug.max_nesting_level', 256);
 
-        $ex = $this->callback(function ($param) {
-            return ($param instanceof \RuntimeException && $param->getMessage() === 'DNS query for google.com has been cancelled');
-        });
-
         $promise = $this->resolver->resolve('google.com');
-        $promise->then($this->expectCallableNever(), $this->expectCallableOnceWith($ex));
         $promise->cancel();
 
         $time = microtime(true);
         $this->loop->run();
         $time = microtime(true) - $time;
 
         $this->assertLessThan(0.1, $time);
+
+        $exception = null;
+        $promise->then(null, function ($reason) use (&$exception) {
+            $exception = $reason;
+        });
+
+        /** @var \React\Dns\Query\CancellationException $exception */
+        $this->assertInstanceOf('React\Dns\Query\CancellationException', $exception);
+        $this->assertEquals('DNS query for google.com (A) has been cancelled', $exception->getMessage());
+    }
+
+    /**
+     * @group internet
+     */
+    public function testResolveAllInvalidTypeRejects()
+    {
+        $promise = $this->resolver->resolveAll('google.com', Message::TYPE_PTR);
+
+        $this->loop->run();
+
+        $exception = null;
+        $promise->then(null, function ($reason) use (&$exception) {
+            $exception = $reason;
+        });
+
+        /** @var \React\Dns\RecordNotFoundException $exception */
+        $this->assertInstanceOf('React\Dns\RecordNotFoundException', $exception);
+        $this->assertEquals('DNS query for google.com (PTR) did not return a valid answer (NOERROR / NODATA)', $exception->getMessage());
+        $this->assertEquals(0, $exception->getCode());
     }
 
     public function testInvalidResolverDoesNotResolveGoogle()

tests/Query/CachingExecutorTest.php

@@ -129,7 +129,7 @@ public function testQueryWillReturnRejectedPromiseWhenCacheReturnsMissAndFallbac
         $query = new Query('reactphp.org', Message::TYPE_A, Message::CLASS_IN);
 
         $fallback = $this->getMockBuilder('React\Dns\Query\ExecutorInterface')->getMock();
-        $fallback->expects($this->once())->method('query')->willReturn(\React\Promise\reject(new \RuntimeException()));
+        $fallback->expects($this->once())->method('query')->willReturn(\React\Promise\reject($exception = new \RuntimeException()));
 
         $cache = $this->getMockBuilder('React\Cache\CacheInterface')->getMock();
         $cache->expects($this->once())->method('get')->willReturn(\React\Promise\resolve(null));
@@ -138,7 +138,7 @@ public function testQueryWillReturnRejectedPromiseWhenCacheReturnsMissAndFallbac
 
         $promise = $executor->query($query);
 
-        $promise->then($this->expectCallableNever(), $this->expectCallableOnceWith($this->isInstanceOf('RuntimeException')));
+        $promise->then($this->expectCallableNever(), $this->expectCallableOnceWith($exception));
     }
 
     public function testCancelQueryWillReturnRejectedPromiseAndCancelPendingPromiseFromCache()
@@ -157,7 +157,14 @@ public function testCancelQueryWillReturnRejectedPromiseAndCancelPendingPromiseF
         $promise = $executor->query($query);
         $promise->cancel();
 
-        $promise->then($this->expectCallableNever(), $this->expectCallableOnceWith($this->isInstanceOf('RuntimeException')));
+        $exception = null;
+        $promise->then(null, function ($reason) use (&$exception) {
+            $exception = $reason;
+        });
+
+        /** @var \RuntimeException $exception */
+        $this->assertInstanceOf('RuntimeException', $exception);
+        $this->assertEquals('DNS query for reactphp.org (A) has been cancelled', $exception->getMessage());
     }
 
     public function testCancelQueryWillReturnRejectedPromiseAndCancelPendingPromiseFromFallbackExecutorWhenCacheReturnsMiss()
@@ -178,6 +185,13 @@ public function testCancelQueryWillReturnRejectedPromiseAndCancelPendingPromiseF
         $deferred->resolve(null);
         $promise->cancel();
 
-        $promise->then($this->expectCallableNever(), $this->expectCallableOnceWith($this->isInstanceOf('RuntimeException')));
+        $exception = null;
+        $promise->then(null, function ($reason) use (&$exception) {
+            $exception = $reason;
+        });
+
+        /** @var \RuntimeException $exception */
+        $this->assertInstanceOf('RuntimeException', $exception);
+        $this->assertEquals('DNS query for reactphp.org (A) has been cancelled', $exception->getMessage());
     }
 }

tests/Query/CoopExecutorTest.php

@@ -127,7 +127,14 @@ public function testCancelQueryWillCancelPromiseFromBaseExecutorAndReject()
 
         $promise->cancel();
 
-        $promise->then(null, $this->expectCallableOnce());
+        $exception = null;
+        $promise->then(null, function ($reason) use (&$exception) {
+            $exception = $reason;
+        });
+
+        /** @var \RuntimeException $exception */
+        $this->assertInstanceOf('RuntimeException', $exception);
+        $this->assertEquals('DNS query for reactphp.org (A) has been cancelled', $exception->getMessage());
     }
 
     public function testCancelOneQueryWhenOtherQueryIsStillPendingWillNotCancelPromiseFromBaseExecutorAndRejectCancelled()