Skip to content

Commit 1ae13fa

Browse files
clueclue
authored
Merge pull request #48 from clue-labs/timeout
Add TimeoutExecutor decorator
2 parents e306499 + f6d2af0 commit 1ae13fa

File tree

6 files changed

+213
-9
lines changed

6 files changed

+213
-9
lines changed

‎composer.json‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,8 @@
77
"php": ">=5.3.0",
88
"react/cache": "~0.4.0|~0.3.0",
99
"react/socket": "^0.5 || ^0.4.4",
10-
"react/promise": "~2.1|~1.2"
10+
"react/promise": "~2.1|~1.2",
11+
"react/promise-timer": "~1.1"
1112
},
1213
"autoload": {
1314
"psr-4": { "React\\Dns\\": "src" }

‎src/Query/Executor.php‎

Lines changed: 27 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,17 @@ class Executor implements ExecutorInterface
1717
private $dumper;
1818
private $timeout;
1919

20+
/**
21+
*
22+
* Note that albeit supported, the $timeout parameter is deprecated!
23+
* You should pass a `null` value here instead. If you need timeout handling,
24+
* use the `TimeoutConnector` instead.
25+
*
26+
* @param LoopInterface $loop
27+
* @param Parser $parser
28+
* @param BinaryDumper $dumper
29+
* @param null|float $timeout DEPRECATED: timeout for DNS query or NULL=no timeout
30+
*/
2031
public function __construct(LoopInterface $loop, Parser $parser, BinaryDumper $dumper, $timeout = 5)
2132
{
2233
$this->loop = $loop;
@@ -56,18 +67,23 @@ public function doQuery($nameserver, $transport, $queryData, $name)
5667
$deferred = new Deferred(function ($resolve, $reject) use (&$timer, &$conn, $name) {
5768
$reject(new CancellationException(sprintf('DNS query for %s has been cancelled', $name)));
5869

59-
$timer->cancel();
70+
if ($timer !== null) {
71+
$timer->cancel();
72+
}
6073
$conn->close();
6174
});
6275

6376
$retryWithTcp = function () use ($that, $nameserver, $queryData, $name) {
6477
return $that->doQuery($nameserver, 'tcp', $queryData, $name);
6578
};
6679

67-
$timer = $this->loop->addTimer($this->timeout, function () use (&$conn, $name, $deferred) {
68-
$conn->close();
69-
$deferred->reject(new TimeoutException(sprintf("DNS query for %s timed out", $name)));
70-
});
80+
$timer = null;
81+
if ($this->timeout !== null) {
82+
$timer = $this->loop->addTimer($this->timeout, function () use (&$conn, $name, $deferred) {
83+
$conn->close();
84+
$deferred->reject(new TimeoutException(sprintf("DNS query for %s timed out", $name)));
85+
});
86+
}
7187

7288
try {
7389
try {
@@ -84,7 +100,9 @@ public function doQuery($nameserver, $transport, $queryData, $name)
84100
}
85101
} catch (\Exception $e) {
86102
// both UDP and TCP failed => reject
87-
$timer->cancel();
103+
if ($timer !== null) {
104+
$timer->cancel();
105+
}
88106
$deferred->reject(new \RuntimeException('Unable to connect to DNS server: ' . $e->getMessage(), 0, $e));
89107

90108
return $deferred->promise();
@@ -97,7 +115,9 @@ public function doQuery($nameserver, $transport, $queryData, $name)
97115
return;
98116
}
99117

100-
$timer->cancel();
118+
if ($timer !== null) {
119+
$timer->cancel();
120+
}
101121

102122
if ($response->header->isTruncated()) {
103123
if ('tcp' === $transport) {

‎src/Query/TimeoutExecutor.php‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
<?php
2+
3+
namespace React\Dns\Query;
4+
5+
use React\EventLoop\LoopInterface;
6+
use React\Promise\Deferred;
7+
use React\Promise\CancellablePromiseInterface;
8+
use React\Promise\Timer;
9+
10+
class TimeoutExecutor implements ExecutorInterface
11+
{
12+
private $executor;
13+
private $loop;
14+
private $timeout;
15+
16+
public function __construct(ExecutorInterface $executor, $timeout, LoopInterface $loop)
17+
{
18+
$this->executor = $executor;
19+
$this->loop = $loop;
20+
$this->timeout = $timeout;
21+
}
22+
23+
public function query($nameserver, Query $query)
24+
{
25+
return Timer\timeout($this->executor->query($nameserver, $query), $this->timeout, $this->loop)->then(null, function ($e) use ($query) {
26+
if ($e instanceof Timer\TimeoutException) {
27+
$e = new TimeoutException(sprintf("DNS query for %s timed out", $query->name), 0, $e);
28+
}
29+
throw $e;
30+
});
31+
}
32+
}

‎src/Resolver/Factory.php‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
use React\Dns\Protocol\BinaryDumper;
1212
use React\EventLoop\LoopInterface;
1313
use React\Dns\Query\RetryExecutor;
14+
use React\Dns\Query\TimeoutExecutor;
1415

1516
class Factory
1617
{
@@ -36,7 +37,11 @@ public function createCached($nameserver, LoopInterface $loop, CacheInterface $c
3637

3738
protected function createExecutor(LoopInterface $loop)
3839
{
39-
return new Executor($loop, new Parser(), new BinaryDumper());
40+
return new TimeoutExecutor(
41+
new Executor($loop, new Parser(), new BinaryDumper(), null),
42+
5.0,
43+
$loop
44+
);
4045
}
4146

4247
protected function createRetryExecutor(LoopInterface $loop)

‎tests/Query/ExecutorTest.php‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,11 @@
1010

1111
class ExecutorTest extends \PHPUnit_Framework_TestCase
1212
{
13+
private $loop;
14+
private $parser;
15+
private $dumper;
16+
private $executor;
17+
1318
public function setUp()
1419
{
1520
$this->loop = $this->getMock('React\EventLoop\LoopInterface');
@@ -98,6 +103,32 @@ public function resolveShouldCloseConnectionWhenCancelled()
98103
$promise->then($this->expectCallableNever(), $errorback);
99104
}
100105

106+
/** @test */
107+
public function resolveShouldNotStartOrCancelTimerWhenCancelledWithTimeoutIsNull()
108+
{
109+
$this->loop
110+
->expects($this->never())
111+
->method('addTimer');
112+
113+
$this->executor = new Executor($this->loop, $this->parser, $this->dumper, null);
114+
115+
$query = new Query('igor.io', Message::TYPE_A, Message::CLASS_IN, 1345656451);
116+
$promise = $this->executor->query('8.8.8.8:53', $query);
117+
118+
$promise->cancel();
119+
120+
$errorback = $this->createCallableMock();
121+
$errorback
122+
->expects($this->once())
123+
->method('__invoke')
124+
->with($this->logicalAnd(
125+
$this->isInstanceOf('React\Dns\Query\CancellationException'),
126+
$this->attribute($this->equalTo('DNS query for igor.io has been cancelled'), 'message')
127+
));
128+
129+
$promise->then($this->expectCallableNever(), $errorback);
130+
}
131+
101132
/** @test */
102133
public function resolveShouldRetryWithTcpIfResponseIsTruncated()
103134
{

‎tests/Query/TimeoutExecutorTest.php‎

Lines changed: 115 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,115 @@
1+
<?php
2+
3+
namespace React\Tests\Dns\Query;
4+
5+
use React\Dns\Query\TimeoutExecutor;
6+
use React\Dns\Query\Query;
7+
use React\Dns\Model\Message;
8+
use React\Promise\Deferred;
9+
use React\Dns\Query\CancellationException;
10+
use React\Tests\Dns\TestCase;
11+
use React\EventLoop\Factory;
12+
use React\Promise;
13+
14+
class TimeoutExecutorTest extends TestCase
15+
{
16+
public function setUp()
17+
{
18+
$this->loop = Factory::create();
19+
20+
$this->wrapped = $this->getMock('React\Dns\Query\ExecutorInterface');
21+
22+
$this->executor = new TimeoutExecutor($this->wrapped, 5.0, $this->loop);
23+
}
24+
25+
public function testCancellingPromiseWillCancelWrapped()
26+
{
27+
$cancelled = 0;
28+
29+
$this->wrapped
30+
->expects($this->once())
31+
->method('query')
32+
->will($this->returnCallback(function ($domain, $query) use (&$cancelled) {
33+
$deferred = new Deferred(function ($resolve, $reject) use (&$cancelled) {
34+
++$cancelled;
35+
$reject(new CancellationException('Cancelled'));
36+
});
37+
38+
return $deferred->promise();
39+
}));
40+
41+
$query = new Query('igor.io', Message::TYPE_A, Message::CLASS_IN, 1345656451);
42+
$promise = $this->executor->query('8.8.8.8:53', $query);
43+
44+
$this->assertEquals(0, $cancelled);
45+
$promise->cancel();
46+
$this->assertEquals(1, $cancelled);
47+
48+
$promise->then($this->expectCallableNever(), $this->expectCallableOnce());
49+
}
50+
51+
public function testResolvesPromiseWhenWrappedResolves()
52+
{
53+
$this->wrapped
54+
->expects($this->once())
55+
->method('query')
56+
->willReturn(Promise\resolve('0.0.0.0'));
57+
58+
$query = new Query('igor.io', Message::TYPE_A, Message::CLASS_IN, 1345656451);
59+
$promise = $this->executor->query('8.8.8.8:53', $query);
60+
61+
$promise->then($this->expectCallableOnce(), $this->expectCallableNever());
62+
}
63+
64+
public function testRejectsPromiseWhenWrappedRejects()
65+
{
66+
$this->wrapped
67+
->expects($this->once())
68+
->method('query')
69+
->willReturn(Promise\reject(new \RuntimeException()));
70+
71+
$query = new Query('igor.io', Message::TYPE_A, Message::CLASS_IN, 1345656451);
72+
$promise = $this->executor->query('8.8.8.8:53', $query);
73+
74+
$promise->then($this->expectCallableNever(), $this->expectCallableOnceWith(new \RuntimeException()));
75+
}
76+
77+
public function testWrappedWillBeCancelledOnTimeout()
78+
{
79+
$this->executor = new TimeoutExecutor($this->wrapped, 0.001, $this->loop);
80+
81+
$cancelled = 0;
82+
83+
$this->wrapped
84+
->expects($this->once())
85+
->method('query')
86+
->will($this->returnCallback(function ($domain, $query) use (&$cancelled) {
87+
$deferred = new Deferred(function ($resolve, $reject) use (&$cancelled) {
88+
++$cancelled;
89+
$reject(new CancellationException('Cancelled'));
90+
});
91+
92+
return $deferred->promise();
93+
}));
94+
95+
$callback = $this->expectCallableNever();
96+
97+
$errorback = $this->createCallableMock();
98+
$errorback
99+
->expects($this->once())
100+
->method('__invoke')
101+
->with($this->logicalAnd(
102+
$this->isInstanceOf('React\Dns\Query\TimeoutException'),
103+
$this->attribute($this->equalTo('DNS query for igor.io timed out'), 'message')
104+
));
105+
106+
$query = new Query('igor.io', Message::TYPE_A, Message::CLASS_IN, 1345656451);
107+
$this->executor->query('8.8.8.8:53', $query)->then($callback, $errorback);
108+
109+
$this->assertEquals(0, $cancelled);
110+
111+
$this->loop->run();
112+
113+
$this->assertEquals(1, $cancelled);
114+
}
115+
}

0 commit comments

Comments
 (0)