diff --git a/src/HappyEyeBallsConnectionBuilder.php b/src/HappyEyeBallsConnectionBuilder.php index 1d122363..0f610cd0 100644 --- a/src/HappyEyeBallsConnectionBuilder.php +++ b/src/HappyEyeBallsConnectionBuilder.php @@ -145,18 +145,33 @@ public function check($resolve, $reject) { $ip = \array_shift($this->connectQueue); + // start connection attempt and remember array position to later unset again + $this->connectionPromises[] = $this->attemptConnection($ip); + \end($this->connectionPromises); + $index = \key($this->connectionPromises); + $that = $this; - $that->connectionPromises[$ip] = $this->attemptConnection($ip)->then(function ($connection) use ($that, $ip, $resolve) { - unset($that->connectionPromises[$ip]); + $that->connectionPromises[$index]->then(function ($connection) use ($that, $index, $resolve) { + unset($that->connectionPromises[$index]); $that->cleanUp(); $resolve($connection); - }, function (\Exception $e) use ($that, $ip, $reject) { - unset($that->connectionPromises[$ip]); + }, function (\Exception $e) use ($that, $index, $resolve, $reject) { + unset($that->connectionPromises[$index]); $that->failureCount++; + // start next connection attempt immediately on error + if ($that->connectQueue) { + if ($that->nextAttemptTimer !== null) { + $that->loop->cancelTimer($that->nextAttemptTimer); + $that->nextAttemptTimer = null; + } + + $that->check($resolve, $reject); + } + if ($that->hasBeenResolved() === false) { return; } @@ -170,7 +185,7 @@ public function check($resolve, $reject) // Allow next connection attempt in 100ms: https://tools.ietf.org/html/rfc8305#section-5 // Only start timer when more IPs are queued or when DNS query is still pending (might add more IPs) - if (\count($this->connectQueue) > 0 || $this->resolved[Message::TYPE_A] === false || $this->resolved[Message::TYPE_AAAA] === false) { + if ($this->nextAttemptTimer === null && (\count($this->connectQueue) > 0 || $this->resolved[Message::TYPE_A] === false || $this->resolved[Message::TYPE_AAAA] === false)) { $this->nextAttemptTimer = $this->loop->addTimer(self::CONNECTION_ATTEMPT_DELAY, function () use ($that, $resolve, $reject) { $that->nextAttemptTimer = null; @@ -236,6 +251,9 @@ public function attemptConnection($ip) */ public function cleanUp() { + // clear list of outstanding IPs to avoid creating new connections + $this->connectQueue = array(); + foreach ($this->connectionPromises as $connectionPromise) { if ($connectionPromise instanceof CancellablePromiseInterface) { $connectionPromise->cancel(); diff --git a/tests/HappyEyeBallsConnectionBuilderTest.php b/tests/HappyEyeBallsConnectionBuilderTest.php index da521f44..4521ab09 100644 --- a/tests/HappyEyeBallsConnectionBuilderTest.php +++ b/tests/HappyEyeBallsConnectionBuilderTest.php @@ -217,6 +217,77 @@ public function testConnectWillStartConnectingWithAttemptTimerButWithoutResoluti $deferred->reject(new \RuntimeException()); } + public function testConnectWillStartConnectingWithAttemptTimerWhenIpv6AndIpv4ResolvesAndWillStartNextConnectionAttemptWithoutAttemptTimerImmediatelyWhenFirstConnectionAttemptFails() + { + $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')->withConsecutive( + array('tcp://[::1]:80?hostname=reactphp.org'), + array('tcp://127.0.0.1:80?hostname=reactphp.org') + )->willReturnOnConsecutiveCalls( + $deferred->promise(), + new Promise(function () { }) + ); + + $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'; + $host = 'reactphp.org'; + $parts = parse_url($uri); + + $builder = new HappyEyeBallsConnectionBuilder($loop, $connector, $resolver, $uri, $host, $parts); + + $builder->connect(); + + $deferred->reject(new \RuntimeException()); + } + + public function testConnectWillStartConnectingWithAttemptTimerWhenOnlyIpv6ResolvesAndWillStartNextConnectionAttemptWithoutAttemptTimerImmediatelyWhenFirstConnectionAttemptFails() + { + $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); + + $connector = $this->getMockBuilder('React\Socket\ConnectorInterface')->getMock(); + $connector->expects($this->exactly(2))->method('connect')->withConsecutive( + array('tcp://[::1]:80?hostname=reactphp.org'), + array('tcp://[::2]:80?hostname=reactphp.org') + )->willReturnOnConsecutiveCalls( + \React\Promise\reject(new \RuntimeException()), + new Promise(function () { }) + ); + + $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', '::2')), + \React\Promise\reject(new \RuntimeException()) + ); + + $uri = 'tcp://reactphp.org:80'; + $host = 'reactphp.org'; + $parts = parse_url($uri); + + $builder = new HappyEyeBallsConnectionBuilder($loop, $connector, $resolver, $uri, $host, $parts); + + $builder->connect(); + } + public function testConnectWillStartConnectingAndWillStartNextConnectionWithoutNewAttemptTimerWhenNextAttemptTimerFiresAfterIpv4Rejected() { $timer = null; @@ -492,4 +563,89 @@ public function testAttemptConnectionWillConnectViaConnectorToGivenIpv6WithAllUr $builder->attemptConnection('::1'); } + + public function testCheckCallsRejectFunctionImmediateWithoutLeavingDanglingPromiseWhenConnectorRejectsImmediately() + { + $loop = $this->getMockBuilder('React\EventLoop\LoopInterface')->getMock(); + + $connector = $this->getMockBuilder('React\Socket\ConnectorInterface')->getMock(); + $connector->expects($this->once())->method('connect')->with('tcp://[::1]:80/path?test=yes&hostname=reactphp.org#start')->willReturn(\React\Promise\reject(new \RuntimeException())); + + $resolver = $this->getMockBuilder('React\Dns\Resolver\ResolverInterface')->getMock(); + $resolver->expects($this->never())->method('resolveAll'); + + $uri = 'tcp://reactphp.org:80/path?test=yes#start'; + $host = 'reactphp.org'; + $parts = parse_url($uri); + + $builder = new HappyEyeBallsConnectionBuilder($loop, $connector, $resolver, $uri, $host, $parts); + + $ref = new \ReflectionProperty($builder, 'connectQueue'); + $ref->setAccessible(true); + $ref->setValue($builder, array('::1')); + + $builder->check($this->expectCallableNever(), function () { }); + + $ref = new \ReflectionProperty($builder, 'connectionPromises'); + $ref->setAccessible(true); + $promises = $ref->getValue($builder); + + $this->assertEquals(array(), $promises); + } + + public function testCleanUpCancelsAllPendingConnectionAttempts() + { + $loop = $this->getMockBuilder('React\EventLoop\LoopInterface')->getMock(); + + $connector = $this->getMockBuilder('React\Socket\ConnectorInterface')->getMock(); + $connector->expects($this->exactly(2))->method('connect')->with('tcp://[::1]:80/path?test=yes&hostname=reactphp.org#start')->willReturnOnConsecutiveCalls( + new Promise(function () { }, $this->expectCallableOnce()), + new Promise(function () { }, $this->expectCallableOnce()) + ); + + $resolver = $this->getMockBuilder('React\Dns\Resolver\ResolverInterface')->getMock(); + $resolver->expects($this->never())->method('resolveAll'); + + $uri = 'tcp://reactphp.org:80/path?test=yes#start'; + $host = 'reactphp.org'; + $parts = parse_url($uri); + + $builder = new HappyEyeBallsConnectionBuilder($loop, $connector, $resolver, $uri, $host, $parts); + + $ref = new \ReflectionProperty($builder, 'connectQueue'); + $ref->setAccessible(true); + $ref->setValue($builder, array('::1', '::1')); + + $builder->check($this->expectCallableNever(), function () { }); + $builder->check($this->expectCallableNever(), function () { }); + + $builder->cleanUp(); + } + + public function testCleanUpCancelsAllPendingConnectionAttemptsWithoutStartingNewAttemptsDueToCancellationRejection() + { + $loop = $this->getMockBuilder('React\EventLoop\LoopInterface')->getMock(); + + $connector = $this->getMockBuilder('React\Socket\ConnectorInterface')->getMock(); + $connector->expects($this->once())->method('connect')->with('tcp://[::1]:80/path?test=yes&hostname=reactphp.org#start')->willReturn(new Promise(function () { }, function () { + throw new \RuntimeException(); + })); + + $resolver = $this->getMockBuilder('React\Dns\Resolver\ResolverInterface')->getMock(); + $resolver->expects($this->never())->method('resolveAll'); + + $uri = 'tcp://reactphp.org:80/path?test=yes#start'; + $host = 'reactphp.org'; + $parts = parse_url($uri); + + $builder = new HappyEyeBallsConnectionBuilder($loop, $connector, $resolver, $uri, $host, $parts); + + $ref = new \ReflectionProperty($builder, 'connectQueue'); + $ref->setAccessible(true); + $ref->setValue($builder, array('::1', '::1')); + + $builder->check($this->expectCallableNever(), function () { }); + + $builder->cleanUp(); + } } diff --git a/tests/HappyEyeBallsConnectorTest.php b/tests/HappyEyeBallsConnectorTest.php index eb8f468c..6c1c24a6 100644 --- a/tests/HappyEyeBallsConnectorTest.php +++ b/tests/HappyEyeBallsConnectorTest.php @@ -138,30 +138,6 @@ public function testPassThroughResolverIfGivenExplicitHost() $this->loop->run(); } - /** - * @dataProvider provideIpvAddresses - */ - public function testIpv4ResolvesFirstSoButIPv6IsTheFirstToConnect(array $ipv6, array $ipv4) - { - $this->resolver->expects($this->at(0))->method('resolveAll')->with('google.com', Message::TYPE_AAAA)->will($this->returnValue(Promise\Timer\resolve(0.001, $this->loop)->then(function () use ($ipv6) { - return Promise\resolve($ipv6); - }))); - $this->resolver->expects($this->at(1))->method('resolveAll')->with('google.com', Message::TYPE_A)->will($this->returnValue(Promise\resolve($ipv4))); - $i = 0; - while (count($ipv6) > 0 || count($ipv4) > 0) { - if (count($ipv6) > 0) { - $this->tcp->expects($this->at($i++))->method('connect')->with($this->equalTo('scheme://[' . array_shift($ipv6) . ']:80/?hostname=google.com'))->will($this->returnValue(Promise\resolve())); - } - if (count($ipv4) > 0) { - $this->tcp->expects($this->at($i++))->method('connect')->with($this->equalTo('scheme://' . array_shift($ipv4) . ':80/?hostname=google.com'))->will($this->returnValue(Promise\resolve())); - } - } - - $this->connector->connect('scheme://google.com:80/?hostname=google.com'); - - $this->loop->run(); - } - /** * @dataProvider provideIpvAddresses */ @@ -202,29 +178,6 @@ public function testIpv6DoesntResolvesWhileIpv4DoesFirstSoIpv4Connects(array $ip $this->loop->run(); } - /** - * @dataProvider provideIpvAddresses - */ - public function testAttemptsToConnectBothIpv6AndIpv4Addresses(array $ipv6, array $ipv4) - { - $this->resolver->expects($this->at(0))->method('resolveAll')->with('google.com', Message::TYPE_AAAA)->will($this->returnValue(Promise\resolve($ipv6))); - $this->resolver->expects($this->at(1))->method('resolveAll')->with('google.com', Message::TYPE_A)->will($this->returnValue(Promise\resolve($ipv4))); - - $i = 0; - while (count($ipv6) > 0 || count($ipv4) > 0) { - if (count($ipv6) > 0) { - $this->tcp->expects($this->at($i++))->method('connect')->with($this->equalTo('scheme://[' . array_shift($ipv6) . ']:80/?hostname=google.com'))->will($this->returnValue(Promise\resolve())); - } - if (count($ipv4) > 0) { - $this->tcp->expects($this->at($i++))->method('connect')->with($this->equalTo('scheme://' . array_shift($ipv4) . ':80/?hostname=google.com'))->will($this->returnValue(Promise\resolve())); - } - } - - $this->connector->connect('scheme://google.com:80/?hostname=google.com'); - - $this->loop->run(); - } - public function testThatTheIpv4ConnectionWillWait100MilisecondsWhenIpv6AndIpv4ResolveSimultaniously() { $timings = array();