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.

Sign up for GitHub

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

Include timeout logic to avoid dependency on reactphp/promise-timer #213

Merged
merged 1 commit into from
Jun 2, 2023
Merged

Include timeout logic to avoid dependency on reactphp/promise-timer #213

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
6 changes: 3 additions & 3 deletions composer.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,12 +29,12 @@
"php": ">=5.3.0",
"react/cache": "^1.0 || ^0.6 || ^0.5",
"react/event-loop": "^1.2",
"react/promise": "^3.0 || ^2.7 || ^1.2.1",
"react/promise-timer": "^1.9"
"react/promise": "^3.0 || ^2.7 || ^1.2.1"
},
"require-dev": {
"phpunit/phpunit": "^9.5 || ^4.8.35",
"react/async": "^4 || ^3 || ^2"
"react/async": "^4 || ^3 || ^2",
"react/promise-timer": "^1.9"
},
"autoload": {
"psr-4": {
Expand Down
48 changes: 43 additions & 5 deletions src/Query/TimeoutExecutor.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

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

final class TimeoutExecutor implements ExecutorInterface
{
Expand All @@ -21,11 +21,49 @@ public function __construct(ExecutorInterface $executor, $timeout, LoopInterface

public function query(Query $query)
{
return Timer\timeout($this->executor->query($query), $this->timeout, $this->loop)->then(null, function ($e) use ($query) {
if ($e instanceof Timer\TimeoutException) {
$e = new TimeoutException(sprintf("DNS query for %s timed out", $query->describe()), 0, $e);
$promise = $this->executor->query($query);

$loop = $this->loop;
$time = $this->timeout;
return new Promise(function ($resolve, $reject) use ($loop, $time, $promise, $query) {
$timer = null;
$promise = $promise->then(function ($v) use (&$timer, $loop, $resolve) {
if ($timer) {
$loop->cancelTimer($timer);
}
$timer = false;
$resolve($v);
}, function ($v) use (&$timer, $loop, $reject) {
if ($timer) {
$loop->cancelTimer($timer);
}
$timer = false;
$reject($v);
});

// promise already resolved => no need to start timer
if ($timer === false) {
return;
}
throw $e;

// start timeout timer which will cancel the pending promise
$timer = $loop->addTimer($time, function () use ($time, &$promise, $reject, $query) {
$reject(new TimeoutException(
'DNS query for ' . $query->describe() . ' timed out'
));

// Cancel pending query to clean up any underlying resources and references.
// Avoid garbage references in call stack by passing pending promise by reference.
assert(\method_exists($promise, 'cancel'));
$promise->cancel();
$promise = null;
});
}, function () use (&$promise) {
// Cancelling this promise will cancel the pending query, thus triggering the rejection logic above.
// Avoid garbage references in call stack by passing pending promise by reference.
assert(\method_exists($promise, 'cancel'));
$promise->cancel();
$promise = null;
});
}
}
87 changes: 75 additions & 12 deletions tests/Query/TimeoutExecutorTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ class TimeoutExecutorTest extends TestCase
{
private $wrapped;
private $executor;
private $loop;

/**
* @before
Expand All @@ -23,7 +24,9 @@ public function setUpExecutor()
{
$this->wrapped = $this->getMockBuilder('React\Dns\Query\ExecutorInterface')->getMock();

$this->executor = new TimeoutExecutor($this->wrapped, 5.0);
$this->loop = $this->getMockBuilder('React\EventLoop\LoopInterface')->getMock();

$this->executor = new TimeoutExecutor($this->wrapped, 5.0, $this->loop);
}

public function testCtorWithoutLoopShouldAssignDefaultLoop()
Expand All @@ -39,6 +42,10 @@ public function testCtorWithoutLoopShouldAssignDefaultLoop()

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

$cancelled = 0;

$this->wrapped
Expand All @@ -63,8 +70,11 @@ public function testCancellingPromiseWillCancelWrapped()
$promise->then($this->expectCallableNever(), $this->expectCallableOnce());
}

public function testResolvesPromiseWhenWrappedResolves()
public function testResolvesPromiseWithoutStartingTimerWhenWrappedReturnsResolvedPromise()
{
$this->loop->expects($this->never())->method('addTimer');
$this->loop->expects($this->never())->method('cancelTimer');

$this->wrapped
->expects($this->once())
->method('query')
Expand All @@ -76,8 +86,31 @@ public function testResolvesPromiseWhenWrappedResolves()
$promise->then($this->expectCallableOnce(), $this->expectCallableNever());
}

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

$deferred = new Deferred();
$this->wrapped
->expects($this->once())
->method('query')
->willReturn($deferred->promise());

$query = new Query('igor.io', Message::TYPE_A, Message::CLASS_IN);
$promise = $this->executor->query($query);

$deferred->resolve('0.0.0.0');

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

public function testRejectsPromiseWithoutStartingTimerWhenWrappedReturnsRejectedPromise()
{
$this->loop->expects($this->never())->method('addTimer');
$this->loop->expects($this->never())->method('cancelTimer');

$this->wrapped
->expects($this->once())
->method('query')
Expand All @@ -89,9 +122,35 @@ public function testRejectsPromiseWhenWrappedRejects()
$promise->then($this->expectCallableNever(), $this->expectCallableOnceWith(new \RuntimeException()));
}

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

$deferred = new Deferred();
$this->wrapped
->expects($this->once())
->method('query')
->willReturn($deferred->promise());

$query = new Query('igor.io', Message::TYPE_A, Message::CLASS_IN);
$promise = $this->executor->query($query);

$deferred->reject(new \RuntimeException());

$promise->then($this->expectCallableNever(), $this->expectCallableOnceWith(new \RuntimeException()));
}

public function testRejectsPromiseAndCancelsPendingQueryWhenTimeoutTriggers()
{
$this->executor = new TimeoutExecutor($this->wrapped, 0);
$timerCallback = null;
$timer = $this->getMockBuilder('React\EventLoop\TimerInterface')->getMock();
$this->loop->expects($this->once())->method('addTimer')->with(5.0, $this->callback(function ($callback) use (&$timerCallback) {
$timerCallback = $callback;
return true;
}))->willReturn($timer);
$this->loop->expects($this->once())->method('cancelTimer')->with($timer);

$cancelled = 0;

Expand All @@ -112,14 +171,18 @@ public function testWrappedWillBeCancelledOnTimeout()

$this->assertEquals(0, $cancelled);

try {
\React\Async\await(\React\Promise\Timer\sleep(0));
\React\Async\await($promise);
$this->fail();
} catch (TimeoutException $exception) {
$this->assertEquals('DNS query for igor.io (A) timed out' , $exception->getMessage());
}
$this->assertNotNull($timerCallback);
$timerCallback();

$this->assertEquals(1, $cancelled);

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

assert($exception instanceof TimeoutException);
$this->assertInstanceOf('React\Dns\Query\TimeoutException', $exception);
$this->assertEquals('DNS query for igor.io (A) timed out' , $exception->getMessage());
}
}