Skip to content

Commit

Permalink
Merge pull request #176 from clue-labs/timeout-errors
Browse files Browse the repository at this point in the history
Improve timeout error messages
  • Loading branch information
jsor authored Sep 25, 2018
2 parents 1ab77ec + d9b136b commit a966cc0
Show file tree
Hide file tree
Showing 4 changed files with 88 additions and 37 deletions.
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());
}
}

0 comments on commit a966cc0

Please sign in to comment.