Skip to content

Commit

Permalink
Merge pull request #233 from clue-labs/eyeballs-errors
Browse files Browse the repository at this point in the history 
Improve error reporting to include both IPv6 & IPv4 errors (happy eyeballs)
  • Loading branch information
@jsor
jsor authored May 17, 2020
2 parents f710d6e + 1b373da commit f5049f0
Show file tree
Hide file tree
Showing 3 changed files with 163 additions and 30 deletions.
56 changes: 51 additions & 5 deletions src/HappyEyeBallsConnectionBuilder.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,10 @@ final class HappyEyeBallsConnectionBuilder
public $resolve;
public $reject;

public $lastErrorFamily;
public $lastError6;
public $lastError4;

public function __construct(LoopInterface $loop, ConnectorInterface $connector, ResolverInterface $resolver, $uri, $host, $parts)
{
$this->loop = $loop;
Expand Down Expand Up @@ -123,15 +127,22 @@ public function resolve($type, $reject)
unset($that->resolverPromises[$type]);
$that->resolved[$type] = true;

if ($type === Message::TYPE_A) {
$that->lastError4 = $e->getMessage();
$that->lastErrorFamily = 4;
} else {
$that->lastError6 = $e->getMessage();
$that->lastErrorFamily = 6;
}

// cancel next attempt timer when there are no more IPs to connect to anymore
if ($that->nextAttemptTimer !== null && !$that->connectQueue) {
$that->loop->cancelTimer($that->nextAttemptTimer);
$that->nextAttemptTimer = null;
}

if ($that->hasBeenResolved() && $that->ipsCount === 0) {
$that->resolverPromises = null;
$reject(new \RuntimeException('Connection to ' . $that->uri . ' failed during DNS lookup: ' . $e->getMessage()));
$reject(new \RuntimeException($that->error()));
}

throw $e;
Expand All @@ -157,11 +168,19 @@ public function check($resolve, $reject)
$that->cleanUp();

$resolve($connection);
}, function (\Exception $e) use ($that, $index, $resolve, $reject) {
}, function (\Exception $e) use ($that, $index, $ip, $resolve, $reject) {
unset($that->connectionPromises[$index]);

$that->failureCount++;

if (\strpos($ip, ':') === false) {
$that->lastError4 = $e->getMessage();
$that->lastErrorFamily = 4;
} else {
$that->lastError6 = $e->getMessage();
$that->lastErrorFamily = 6;
}

// start next connection attempt immediately on error
if ($that->connectQueue) {
if ($that->nextAttemptTimer !== null) {
Expand All @@ -179,7 +198,7 @@ public function check($resolve, $reject)
if ($that->ipsCount === $that->failureCount) {
$that->cleanUp();

$reject(new \RuntimeException('Connection to ' . $that->uri . ' failed: ' . $e->getMessage()));
$reject(new \RuntimeException($that->error()));
}
});

Expand Down Expand Up @@ -309,4 +328,31 @@ public function mixIpsIntoConnectQueue(array $ips)
}
}
}
}

/**
* @internal
* @return string
*/
public function error()
{
if ($this->lastError4 === $this->lastError6) {
$message = $this->lastError6;
} elseif ($this->lastErrorFamily === 6) {
$message = 'Last error for IPv6: ' . $this->lastError6 . '. Previous error for IPv4: ' . $this->lastError4;
} else {
$message = 'Last error for IPv4: ' . $this->lastError4 . '. Previous error for IPv6: ' . $this->lastError6;
}

if ($this->hasBeenResolved() && $this->ipsCount === 0) {
if ($this->lastError6 === $this->lastError4) {
$message = ' during DNS lookup: ' . $this->lastError6;
} else {
$message = ' during DNS lookup. ' . $message;
}
} else {
$message = ': ' . $message;
}

return 'Connection to ' . $this->uri . ' failed' . $message;
}
}
114 changes: 112 additions & 2 deletions tests/HappyEyeBallsConnectionBuilderTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,42 @@ public function testConnectWillRejectWhenBothDnsLookupsReject()
$this->assertEquals('Connection to tcp://reactphp.org:80 failed during DNS lookup: DNS lookup error', $exception->getMessage());
}

public function testConnectWillRejectWhenBothDnsLookupsRejectWithDifferentMessages()
{
$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');

$deferred = new Deferred();
$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)
)->willReturnOnConsecutiveCalls(
$deferred->promise(),
\React\Promise\reject(new \RuntimeException('DNS4 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();
$deferred->reject(new \RuntimeException('DNS6 error'));

$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. Last error for IPv6: DNS6 error. Previous error for IPv4: DNS4 error', $exception->getMessage());
}

public function testConnectWillStartDelayTimerWhenIpv4ResolvesAndIpv6IsPending()
{
$loop = $this->getMockBuilder('React\EventLoop\LoopInterface')->getMock();
Expand Down Expand Up @@ -364,7 +400,7 @@ public function testConnectWillStartAndCancelResolutionTimerAndStartAttemptTimer
$deferred->resolve(array('::1'));
}

public function testConnectWillRejectWhenOnlyTcpConnectionRejectsAndCancelNextAttemptTimerImmediately()
public function testConnectWillRejectWhenOnlyTcp6ConnectionRejectsAndCancelNextAttemptTimerImmediately()
{
$timer = $this->getMockBuilder('React\EventLoop\TimerInterface')->getMock();
$loop = $this->getMockBuilder('React\EventLoop\LoopInterface')->getMock();
Expand All @@ -381,7 +417,81 @@ public function testConnectWillRejectWhenOnlyTcpConnectionRejectsAndCancelNextAt
array('reactphp.org', Message::TYPE_A)
)->willReturnOnConsecutiveCalls(
\React\Promise\resolve(array('::1')),
\React\Promise\reject(new \RuntimeException('ignored'))
\React\Promise\reject(new \RuntimeException('DNS failed'))
);

$uri = 'tcp://reactphp.org:80';
$host = 'reactphp.org';
$parts = parse_url($uri);

$builder = new HappyEyeBallsConnectionBuilder($loop, $connector, $resolver, $uri, $host, $parts);

$promise = $builder->connect();
$deferred->reject(new \RuntimeException('Connection refused'));

$exception = null;
$promise->then(null, function ($e) use (&$exception) {
$exception = $e;
});

$this->assertInstanceOf('RuntimeException', $exception);
$this->assertEquals('Connection to tcp://reactphp.org:80 failed: Last error for IPv6: Connection refused. Previous error for IPv4: DNS failed', $exception->getMessage());
}

public function testConnectWillRejectWhenOnlyTcp4ConnectionRejectsAndWillNeverStartNextAttemptTimer()
{
$loop = $this->getMockBuilder('React\EventLoop\LoopInterface')->getMock();
$loop->expects($this->never())->method('addTimer');

$deferred = new Deferred();
$connector = $this->getMockBuilder('React\Socket\ConnectorInterface')->getMock();
$connector->expects($this->once())->method('connect')->with('tcp://127.0.0.1:80?hostname=reactphp.org')->willReturn($deferred->promise());

$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)
)->willReturnOnConsecutiveCalls(
\React\Promise\reject(new \RuntimeException('DNS failed')),
\React\Promise\resolve(array('127.0.0.1'))
);

$uri = 'tcp://reactphp.org:80';
$host = 'reactphp.org';
$parts = parse_url($uri);

$builder = new HappyEyeBallsConnectionBuilder($loop, $connector, $resolver, $uri, $host, $parts);

$promise = $builder->connect();
$deferred->reject(new \RuntimeException('Connection refused'));

$exception = null;
$promise->then(null, function ($e) use (&$exception) {
$exception = $e;
});

$this->assertInstanceOf('RuntimeException', $exception);
$this->assertEquals('Connection to tcp://reactphp.org:80 failed: Last error for IPv4: Connection refused. Previous error for IPv6: DNS failed', $exception->getMessage());
}

public function testConnectWillRejectWhenAllConnectionsRejectAndCancelNextAttemptTimerImmediately()
{
$timer = $this->getMockBuilder('React\EventLoop\TimerInterface')->getMock();
$loop = $this->getMockBuilder('React\EventLoop\LoopInterface')->getMock();
$loop->expects($this->once())->method('addTimer')->with(0.1, $this->anything())->willReturn($timer);
$loop->expects($this->once())->method('cancelTimer')->with($timer);

$deferred = new Deferred();
$connector = $this->getMockBuilder('React\Socket\ConnectorInterface')->getMock();
$connector->expects($this->exactly(2))->method('connect')->willReturn($deferred->promise());

$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)
)->willReturnOnConsecutiveCalls(
\React\Promise\resolve(array('::1')),
\React\Promise\resolve(array('127.0.0.1'))
);

$uri = 'tcp://reactphp.org:80';
Expand Down
23 changes: 0 additions & 23 deletions tests/HappyEyeBallsConnectorTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -289,29 +289,6 @@ public function testRejectsWithTcpConnectorRejectionIfGivenIp()
$this->loop->run();
}

/**
* @expectedException RuntimeException
* @expectedExceptionMessage Connection to example.com:80 failed: Connection refused
* @dataProvider provideIpvAddresses
*/
public function testRejectsWithTcpConnectorRejectionAfterDnsIsResolved(array $ipv6, array $ipv4)
{
$that = $this;
$promise = Promise\reject(new \RuntimeException('Connection refused'));
$this->resolver->expects($this->at(0))->method('resolveAll')->with($this->equalTo('example.com'), $this->anything())->willReturn(Promise\resolve($ipv6));
$this->resolver->expects($this->at(1))->method('resolveAll')->with($this->equalTo('example.com'), $this->anything())->willReturn(Promise\resolve($ipv4));
$this->tcp->expects($this->any())->method('connect')->with($this->stringContains(':80?hostname=example.com'))->willReturn($promise);

$promise = $this->connector->connect('example.com:80');
$this->loop->addTimer(0.1 * (count($ipv4) + count($ipv6)), function () use ($that, $promise) {
$promise->cancel();

$that->throwRejection($promise);
});

$this->loop->run();
}

/**
* @expectedException RuntimeException
* @expectedExceptionMessage Connection to example.invalid:80 failed during DNS lookup: DNS error
Expand Down

0 comments on commit f5049f0

Please sign in to comment.