Merge pull request #230 from clue-labs/eyeballs-dns-errors

Improve error reporting when DNS lookup fails (happy eyeballs)
reactphp · May 11, 2020 · f2ea6e7 · f2ea6e7
2 parents 9574106 + 4db274b
commit f2ea6e7
Show file tree

Hide file tree

Showing 3 changed files with 45 additions and 10 deletions.
diff --git a/src/HappyEyeBallsConnectionBuilder.php b/src/HappyEyeBallsConnectionBuilder.php
@@ -80,7 +80,7 @@ public function connect()
             };
 
             $that->resolverPromises[Message::TYPE_AAAA] = $that->resolve(Message::TYPE_AAAA, $reject)->then($lookupResolve(Message::TYPE_AAAA));
-            $that->resolverPromises[Message::TYPE_A] = $that->resolve(Message::TYPE_A, $reject)->then(function ($ips) use ($that, &$timer) {
+            $that->resolverPromises[Message::TYPE_A] = $that->resolve(Message::TYPE_A, $reject)->then(function (array $ips) use ($that, &$timer) {
                 // happy path: IPv6 has resolved already, continue with IPv4 addresses
                 if ($that->resolved[Message::TYPE_AAAA] === true) {
                     return $ips;
@@ -112,22 +112,24 @@ public function connect()
 
     /**
      * @internal
+     * @param int      $type   DNS query type
+     * @param callable $reject
+     * @return \React\Promise\PromiseInterface<string[],\Exception> Returns a promise
+     *     that resolves list of IP addresses on success or rejects with an \Exception on error.
      */
     public function resolve($type, $reject)
     {
         $that = $this;
-        return $that->resolver->resolveAll($that->host, $type)->then(null, function () use ($type, $reject, $that) {
+        return $that->resolver->resolveAll($that->host, $type)->then(null, function (\Exception $e) use ($type, $reject, $that) {
             unset($that->resolverPromises[$type]);
             $that->resolved[$type] = true;
 
-            if ($that->hasBeenResolved() === false) {
-                return;
-            }
-
-            if ($that->ipsCount === 0) {
+            if ($that->hasBeenResolved() && $that->ipsCount === 0) {
                 $that->resolverPromises = null;
-                $reject(new \RuntimeException('Connection to ' . $that->uri . ' failed during DNS lookup: DNS error'));
+                $reject(new \RuntimeException('Connection to ' . $that->uri . ' failed during DNS lookup: ' . $e->getMessage()));
             }
+
+            throw $e;
         });
     }
 

diff --git a/tests/HappyEyeBallsConnectionBuilderTest.php b/tests/HappyEyeBallsConnectionBuilderTest.php
@@ -32,6 +32,39 @@ public function testConnectWillResolveTwiceViaResolver()
         $builder->connect();
     }
 
+    public function testConnectWillRejectWhenBothDnsLookupsReject()
+    {
+        $loop = $this->getMockBuilder('React\EventLoop\LoopInterface')->getMock();
+        $loop->expects($this->never())->method('addTimer');
+
+        $connector = $this->getMockBuilder('React\Socket\ConnectorInterface')->getMock();
+        $connector->expects($this->never())->method('connect');
+
+        $resolver = $this->getMockBuilder('React\Dns\Resolver\ResolverInterface')->getMock();
+        $resolver->expects($this->exactly(2))->method('resolveAll')->withConsecutive(
+            array('reactphp.org', Message::TYPE_AAAA),
+            array('reactphp.org', Message::TYPE_A)
+        )->willReturn(new Promise(function () {
+            throw new \RuntimeException('DNS lookup error');
+        }));
+
+        $uri = 'tcp://reactphp.org:80';
+        $host = 'reactphp.org';
+        $parts = parse_url($uri);
+
+        $builder = new HappyEyeBallsConnectionBuilder($loop, $connector, $resolver, $uri, $host, $parts);
+
+        $promise = $builder->connect();
+
+        $exception = null;
+        $promise->then(null, function ($e) use (&$exception) {
+            $exception = $e;
+        });
+
+        $this->assertInstanceOf('RuntimeException', $exception);
+        $this->assertEquals('Connection to tcp://reactphp.org:80 failed during DNS lookup: DNS lookup error', $exception->getMessage());
+    }
+
     public function testConnectWillStartTimerWhenIpv4ResolvesAndIpv6IsPending()
     {
         $loop = $this->getMockBuilder('React\EventLoop\LoopInterface')->getMock();

diff --git a/tests/HappyEyeBallsConnectorTest.php b/tests/HappyEyeBallsConnectorTest.php
@@ -176,7 +176,7 @@ public function testIpv6ResolvesFirstSoIsTheFirstToConnect(array $ipv6, array $i
         $this->connector->connect('scheme://google.com:80/?hostname=google.com');
 
         $this->loop->addTimer(0.07, function () use ($deferred) {
-            $deferred->reject();
+            $deferred->reject(new \RuntimeException());
         });
 
         $this->loop->run();
@@ -196,7 +196,7 @@ public function testIpv6DoesntResolvesWhileIpv4DoesFirstSoIpv4Connects(array $ip
         $this->connector->connect('scheme://google.com:80/?hostname=google.com');
 
         $this->loop->addTimer(0.07, function () use ($deferred) {
-            $deferred->reject();
+            $deferred->reject(new \RuntimeException());
         });
 
         $this->loop->run();