Skip to content

Commit f5049f0

Browse files
jsorjsor
authored
Merge pull request #233 from clue-labs/eyeballs-errors
Improve error reporting to include both IPv6 & IPv4 errors (happy eyeballs)
2 parents f710d6e + 1b373da commit f5049f0

File tree

3 files changed

+163
-30
lines changed

3 files changed

+163
-30
lines changed

‎src/HappyEyeBallsConnectionBuilder.php‎

Lines changed: 51 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,10 @@ final class HappyEyeBallsConnectionBuilder
4949
public $resolve;
5050
public $reject;
5151

52+
public $lastErrorFamily;
53+
public $lastError6;
54+
public $lastError4;
55+
5256
public function __construct(LoopInterface $loop, ConnectorInterface $connector, ResolverInterface $resolver, $uri, $host, $parts)
5357
{
5458
$this->loop = $loop;
@@ -123,15 +127,22 @@ public function resolve($type, $reject)
123127
unset($that->resolverPromises[$type]);
124128
$that->resolved[$type] = true;
125129

130+
if ($type === Message::TYPE_A) {
131+
$that->lastError4 = $e->getMessage();
132+
$that->lastErrorFamily = 4;
133+
} else {
134+
$that->lastError6 = $e->getMessage();
135+
$that->lastErrorFamily = 6;
136+
}
137+
126138
// cancel next attempt timer when there are no more IPs to connect to anymore
127139
if ($that->nextAttemptTimer !== null && !$that->connectQueue) {
128140
$that->loop->cancelTimer($that->nextAttemptTimer);
129141
$that->nextAttemptTimer = null;
130142
}
131143

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

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

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

163174
$that->failureCount++;
164175

176+
if (\strpos($ip, ':') === false) {
177+
$that->lastError4 = $e->getMessage();
178+
$that->lastErrorFamily = 4;
179+
} else {
180+
$that->lastError6 = $e->getMessage();
181+
$that->lastErrorFamily = 6;
182+
}
183+
165184
// start next connection attempt immediately on error
166185
if ($that->connectQueue) {
167186
if ($that->nextAttemptTimer !== null) {
@@ -179,7 +198,7 @@ public function check($resolve, $reject)
179198
if ($that->ipsCount === $that->failureCount) {
180199
$that->cleanUp();
181200

182-
$reject(new \RuntimeException('Connection to ' . $that->uri . ' failed: ' . $e->getMessage()));
201+
$reject(new \RuntimeException($that->error()));
183202
}
184203
});
185204

@@ -309,4 +328,31 @@ public function mixIpsIntoConnectQueue(array $ips)
309328
}
310329
}
311330
}
312-
}
331+
332+
/**
333+
* @internal
334+
* @return string
335+
*/
336+
public function error()
337+
{
338+
if ($this->lastError4 === $this->lastError6) {
339+
$message = $this->lastError6;
340+
} elseif ($this->lastErrorFamily === 6) {
341+
$message = 'Last error for IPv6: ' . $this->lastError6 . '. Previous error for IPv4: ' . $this->lastError4;
342+
} else {
343+
$message = 'Last error for IPv4: ' . $this->lastError4 . '. Previous error for IPv6: ' . $this->lastError6;
344+
}
345+
346+
if ($this->hasBeenResolved() && $this->ipsCount === 0) {
347+
if ($this->lastError6 === $this->lastError4) {
348+
$message = ' during DNS lookup: ' . $this->lastError6;
349+
} else {
350+
$message = ' during DNS lookup. ' . $message;
351+
}
352+
} else {
353+
$message = ': ' . $message;
354+
}
355+
356+
return 'Connection to ' . $this->uri . ' failed' . $message;
357+
}
358+
}

‎tests/HappyEyeBallsConnectionBuilderTest.php‎

Lines changed: 112 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,42 @@ public function testConnectWillRejectWhenBothDnsLookupsReject()
6565
$this->assertEquals('Connection to tcp://reactphp.org:80 failed during DNS lookup: DNS lookup error', $exception->getMessage());
6666
}
6767

68+
public function testConnectWillRejectWhenBothDnsLookupsRejectWithDifferentMessages()
69+
{
70+
$loop = $this->getMockBuilder('React\EventLoop\LoopInterface')->getMock();
71+
$loop->expects($this->never())->method('addTimer');
72+
73+
$connector = $this->getMockBuilder('React\Socket\ConnectorInterface')->getMock();
74+
$connector->expects($this->never())->method('connect');
75+
76+
$deferred = new Deferred();
77+
$resolver = $this->getMockBuilder('React\Dns\Resolver\ResolverInterface')->getMock();
78+
$resolver->expects($this->exactly(2))->method('resolveAll')->withConsecutive(
79+
array('reactphp.org', Message::TYPE_AAAA),
80+
array('reactphp.org', Message::TYPE_A)
81+
)->willReturnOnConsecutiveCalls(
82+
$deferred->promise(),
83+
\React\Promise\reject(new \RuntimeException('DNS4 error'))
84+
);
85+
86+
$uri = 'tcp://reactphp.org:80';
87+
$host = 'reactphp.org';
88+
$parts = parse_url($uri);
89+
90+
$builder = new HappyEyeBallsConnectionBuilder($loop, $connector, $resolver, $uri, $host, $parts);
91+
92+
$promise = $builder->connect();
93+
$deferred->reject(new \RuntimeException('DNS6 error'));
94+
95+
$exception = null;
96+
$promise->then(null, function ($e) use (&$exception) {
97+
$exception = $e;
98+
});
99+
100+
$this->assertInstanceOf('RuntimeException', $exception);
101+
$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());
102+
}
103+
68104
public function testConnectWillStartDelayTimerWhenIpv4ResolvesAndIpv6IsPending()
69105
{
70106
$loop = $this->getMockBuilder('React\EventLoop\LoopInterface')->getMock();
@@ -364,7 +400,7 @@ public function testConnectWillStartAndCancelResolutionTimerAndStartAttemptTimer
364400
$deferred->resolve(array('::1'));
365401
}
366402

367-
public function testConnectWillRejectWhenOnlyTcpConnectionRejectsAndCancelNextAttemptTimerImmediately()
403+
public function testConnectWillRejectWhenOnlyTcp6ConnectionRejectsAndCancelNextAttemptTimerImmediately()
368404
{
369405
$timer = $this->getMockBuilder('React\EventLoop\TimerInterface')->getMock();
370406
$loop = $this->getMockBuilder('React\EventLoop\LoopInterface')->getMock();
@@ -381,7 +417,81 @@ public function testConnectWillRejectWhenOnlyTcpConnectionRejectsAndCancelNextAt
381417
array('reactphp.org', Message::TYPE_A)
382418
)->willReturnOnConsecutiveCalls(
383419
\React\Promise\resolve(array('::1')),
384-
\React\Promise\reject(new \RuntimeException('ignored'))
420+
\React\Promise\reject(new \RuntimeException('DNS failed'))
421+
);
422+
423+
$uri = 'tcp://reactphp.org:80';
424+
$host = 'reactphp.org';
425+
$parts = parse_url($uri);
426+
427+
$builder = new HappyEyeBallsConnectionBuilder($loop, $connector, $resolver, $uri, $host, $parts);
428+
429+
$promise = $builder->connect();
430+
$deferred->reject(new \RuntimeException('Connection refused'));
431+
432+
$exception = null;
433+
$promise->then(null, function ($e) use (&$exception) {
434+
$exception = $e;
435+
});
436+
437+
$this->assertInstanceOf('RuntimeException', $exception);
438+
$this->assertEquals('Connection to tcp://reactphp.org:80 failed: Last error for IPv6: Connection refused. Previous error for IPv4: DNS failed', $exception->getMessage());
439+
}
440+
441+
public function testConnectWillRejectWhenOnlyTcp4ConnectionRejectsAndWillNeverStartNextAttemptTimer()
442+
{
443+
$loop = $this->getMockBuilder('React\EventLoop\LoopInterface')->getMock();
444+
$loop->expects($this->never())->method('addTimer');
445+
446+
$deferred = new Deferred();
447+
$connector = $this->getMockBuilder('React\Socket\ConnectorInterface')->getMock();
448+
$connector->expects($this->once())->method('connect')->with('tcp://127.0.0.1:80?hostname=reactphp.org')->willReturn($deferred->promise());
449+
450+
$resolver = $this->getMockBuilder('React\Dns\Resolver\ResolverInterface')->getMock();
451+
$resolver->expects($this->exactly(2))->method('resolveAll')->withConsecutive(
452+
array('reactphp.org', Message::TYPE_AAAA),
453+
array('reactphp.org', Message::TYPE_A)
454+
)->willReturnOnConsecutiveCalls(
455+
\React\Promise\reject(new \RuntimeException('DNS failed')),
456+
\React\Promise\resolve(array('127.0.0.1'))
457+
);
458+
459+
$uri = 'tcp://reactphp.org:80';
460+
$host = 'reactphp.org';
461+
$parts = parse_url($uri);
462+
463+
$builder = new HappyEyeBallsConnectionBuilder($loop, $connector, $resolver, $uri, $host, $parts);
464+
465+
$promise = $builder->connect();
466+
$deferred->reject(new \RuntimeException('Connection refused'));
467+
468+
$exception = null;
469+
$promise->then(null, function ($e) use (&$exception) {
470+
$exception = $e;
471+
});
472+
473+
$this->assertInstanceOf('RuntimeException', $exception);
474+
$this->assertEquals('Connection to tcp://reactphp.org:80 failed: Last error for IPv4: Connection refused. Previous error for IPv6: DNS failed', $exception->getMessage());
475+
}
476+
477+
public function testConnectWillRejectWhenAllConnectionsRejectAndCancelNextAttemptTimerImmediately()
478+
{
479+
$timer = $this->getMockBuilder('React\EventLoop\TimerInterface')->getMock();
480+
$loop = $this->getMockBuilder('React\EventLoop\LoopInterface')->getMock();
481+
$loop->expects($this->once())->method('addTimer')->with(0.1, $this->anything())->willReturn($timer);
482+
$loop->expects($this->once())->method('cancelTimer')->with($timer);
483+
484+
$deferred = new Deferred();
485+
$connector = $this->getMockBuilder('React\Socket\ConnectorInterface')->getMock();
486+
$connector->expects($this->exactly(2))->method('connect')->willReturn($deferred->promise());
487+
488+
$resolver = $this->getMockBuilder('React\Dns\Resolver\ResolverInterface')->getMock();
489+
$resolver->expects($this->exactly(2))->method('resolveAll')->withConsecutive(
490+
array('reactphp.org', Message::TYPE_AAAA),
491+
array('reactphp.org', Message::TYPE_A)
492+
)->willReturnOnConsecutiveCalls(
493+
\React\Promise\resolve(array('::1')),
494+
\React\Promise\resolve(array('127.0.0.1'))
385495
);
386496

387497
$uri = 'tcp://reactphp.org:80';

‎tests/HappyEyeBallsConnectorTest.php‎

Lines changed: 0 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -289,29 +289,6 @@ public function testRejectsWithTcpConnectorRejectionIfGivenIp()
289289
$this->loop->run();
290290
}
291291

292-
/**
293-
* @expectedException RuntimeException
294-
* @expectedExceptionMessage Connection to example.com:80 failed: Connection refused
295-
* @dataProvider provideIpvAddresses
296-
*/
297-
public function testRejectsWithTcpConnectorRejectionAfterDnsIsResolved(array $ipv6, array $ipv4)
298-
{
299-
$that = $this;
300-
$promise = Promise\reject(new \RuntimeException('Connection refused'));
301-
$this->resolver->expects($this->at(0))->method('resolveAll')->with($this->equalTo('example.com'), $this->anything())->willReturn(Promise\resolve($ipv6));
302-
$this->resolver->expects($this->at(1))->method('resolveAll')->with($this->equalTo('example.com'), $this->anything())->willReturn(Promise\resolve($ipv4));
303-
$this->tcp->expects($this->any())->method('connect')->with($this->stringContains(':80?hostname=example.com'))->willReturn($promise);
304-
305-
$promise = $this->connector->connect('example.com:80');
306-
$this->loop->addTimer(0.1 * (count($ipv4) + count($ipv6)), function () use ($that, $promise) {
307-
$promise->cancel();
308-
309-
$that->throwRejection($promise);
310-
});
311-
312-
$this->loop->run();
313-
}
314-
315292
/**
316293
* @expectedException RuntimeException
317294
* @expectedExceptionMessage Connection to example.invalid:80 failed during DNS lookup: DNS error

0 commit comments

Comments
 (0)