Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve timeout error messages #176

Merged
merged 1 commit into from
Sep 25, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 26 additions & 1 deletion src/TimeoutConnector.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

use React\EventLoop\LoopInterface;
use React\Promise\Timer;
use React\Promise\Timer\TimeoutException;

final class TimeoutConnector implements ConnectorInterface
{
Expand All @@ -20,6 +21,30 @@ public function __construct(ConnectorInterface $connector, $timeout, LoopInterfa

public function connect($uri)
{
return Timer\timeout($this->connector->connect($uri), $this->timeout, $this->loop);
return Timer\timeout($this->connector->connect($uri), $this->timeout, $this->loop)->then(null, self::handler($uri));
}

/**
* Creates a static rejection handler that reports a proper error message in case of a timeout.
*
* This uses a private static helper method to ensure this closure is not
* bound to this instance and the exception trace does not include a
* reference to this instance and its connector stack as a result.
*
* @param string $uri
* @return callable
*/
private static function handler($uri)
{
return function (\Exception $e) use ($uri) {
if ($e instanceof TimeoutException) {
throw new \RuntimeException(
'Connection to ' . $uri . ' timed out after ' . $e->getTimeout() . ' seconds',
\defined('SOCKET_ETIMEDOUT') ? \SOCKET_ETIMEDOUT : 0
);
}

throw $e;
};
}
}
15 changes: 0 additions & 15 deletions tests/DnsConnectorTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -195,9 +195,6 @@ public function testCancelDuringTcpConnectionCancelsTcpConnectionWithTcpRejectio
$this->throwRejection($promise);
}

/**
* @requires PHP 7
*/
public function testRejectionDuringDnsLookupShouldNotCreateAnyGarbageReferences()
{
if (class_exists('React\Promise\When')) {
Expand All @@ -217,9 +214,6 @@ public function testRejectionDuringDnsLookupShouldNotCreateAnyGarbageReferences(
$this->assertEquals(0, gc_collect_cycles());
}

/**
* @requires PHP 7
*/
public function testRejectionAfterDnsLookupShouldNotCreateAnyGarbageReferences()
{
if (class_exists('React\Promise\When')) {
Expand All @@ -242,9 +236,6 @@ public function testRejectionAfterDnsLookupShouldNotCreateAnyGarbageReferences()
$this->assertEquals(0, gc_collect_cycles());
}

/**
* @requires PHP 7
*/
public function testRejectionAfterDnsLookupShouldNotCreateAnyGarbageReferencesAgain()
{
if (class_exists('React\Promise\When')) {
Expand All @@ -270,9 +261,6 @@ public function testRejectionAfterDnsLookupShouldNotCreateAnyGarbageReferencesAg
$this->assertEquals(0, gc_collect_cycles());
}

/**
* @requires PHP 7
*/
public function testCancelDuringDnsLookupShouldNotCreateAnyGarbageReferences()
{
if (class_exists('React\Promise\When')) {
Expand All @@ -295,9 +283,6 @@ public function testCancelDuringDnsLookupShouldNotCreateAnyGarbageReferences()
$this->assertEquals(0, gc_collect_cycles());
}

/**
* @requires PHP 7
*/
public function testCancelDuringTcpConnectionShouldNotCreateAnyGarbageReferences()
{
if (class_exists('React\Promise\When')) {
Expand Down
6 changes: 0 additions & 6 deletions tests/IntegrationTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -181,9 +181,6 @@ function ($e) use (&$wait) {
$this->assertEquals(0, gc_collect_cycles());
}

/**
* @requires PHP 7
*/
public function testWaitingForConnectionTimeoutDuringDnsLookupShouldNotCreateAnyGarbageReferences()
{
if (class_exists('React\Promise\When')) {
Expand Down Expand Up @@ -217,9 +214,6 @@ function ($e) use (&$wait) {
$this->assertEquals(0, gc_collect_cycles());
}

/**
* @requires PHP 7
*/
public function testWaitingForConnectionTimeoutDuringTcpConnectionShouldNotCreateAnyGarbageReferences()
{
if (class_exists('React\Promise\When')) {
Expand Down
77 changes: 62 additions & 15 deletions tests/TimeoutConnectorTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,19 @@

namespace React\Tests\Socket;

use Clue\React\Block;
use React\Socket\TimeoutConnector;
use React\Promise;
use React\EventLoop\Factory;
use React\Promise\Deferred;

class TimeoutConnectorTest extends TestCase
{
public function testRejectsOnTimeout()
/**
* @expectedException RuntimeException
* @expectedExceptionMessage Connection to google.com:80 timed out after 0.01 seconds
*/
public function testRejectsWithTimeoutReasonOnTimeout()
{
$promise = new Promise\Promise(function () { });

Expand All @@ -19,17 +25,16 @@ public function testRejectsOnTimeout()

$timeout = new TimeoutConnector($connector, 0.01, $loop);

$timeout->connect('google.com:80')->then(
$this->expectCallableNever(),
$this->expectCallableOnce()
);

$loop->run();
Block\await($timeout->connect('google.com:80'), $loop);
}

public function testRejectsWhenConnectorRejects()
/**
* @expectedException RuntimeException
* @expectedExceptionMessage Failed
*/
public function testRejectsWithOriginalReasonWhenConnectorRejects()
{
$promise = Promise\reject(new \RuntimeException());
$promise = Promise\reject(new \RuntimeException('Failed'));

$connector = $this->getMockBuilder('React\Socket\ConnectorInterface')->getMock();
$connector->expects($this->once())->method('connect')->with('google.com:80')->will($this->returnValue($promise));
Expand All @@ -38,12 +43,7 @@ public function testRejectsWhenConnectorRejects()

$timeout = new TimeoutConnector($connector, 5.0, $loop);

$timeout->connect('google.com:80')->then(
$this->expectCallableNever(),
$this->expectCallableOnce()
);

$loop->run();
Block\await($timeout->connect('google.com:80'), $loop);
}

public function testResolvesWhenConnectorResolves()
Expand Down Expand Up @@ -100,4 +100,51 @@ public function testCancelsPendingPromiseOnCancel()

$out->then($this->expectCallableNever(), $this->expectCallableOnce());
}

public function testRejectionDuringConnectionShouldNotCreateAnyGarbageReferences()
{
if (class_exists('React\Promise\When')) {
$this->markTestSkipped('Not supported on legacy Promise v1 API');
}

gc_collect_cycles();

$connection = new Deferred();
$connector = $this->getMockBuilder('React\Socket\ConnectorInterface')->getMock();
$connector->expects($this->once())->method('connect')->with('example.com:80')->willReturn($connection->promise());

$loop = Factory::create();
$timeout = new TimeoutConnector($connector, 0.01, $loop);

$promise = $timeout->connect('example.com:80');
$connection->reject(new \RuntimeException('Connection failed'));
unset($promise, $connection);

$this->assertEquals(0, gc_collect_cycles());
}

public function testRejectionDueToTimeoutShouldNotCreateAnyGarbageReferences()
{
if (class_exists('React\Promise\When')) {
$this->markTestSkipped('Not supported on legacy Promise v1 API');
}

gc_collect_cycles();

$connection = new Deferred(function () {
throw new \RuntimeException('Connection cancelled');
});
$connector = $this->getMockBuilder('React\Socket\ConnectorInterface')->getMock();
$connector->expects($this->once())->method('connect')->with('example.com:80')->willReturn($connection->promise());

$loop = Factory::create();
$timeout = new TimeoutConnector($connector, 0, $loop);

$promise = $timeout->connect('example.com:80');

$loop->run();
unset($promise, $connection);

$this->assertEquals(0, gc_collect_cycles());
}
}