Skip to content

Commit 17306ce

Browse files
SimonFringsSimonFrings
authored
Merge pull request #305 from clue-labs/no-timeout
Include timeout logic to avoid dependency on reactphp/promise-timer
2 parents bf11696 + 78dc111 commit 17306ce

File tree

3 files changed

+179
-75
lines changed

3 files changed

+179
-75
lines changed

‎composer.json‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -28,16 +28,16 @@
2828
"require": {
2929
"php": ">=5.3.0",
3030
"evenement/evenement": "^3.0 || ^2.0 || ^1.0",
31-
"react/dns": "^1.8",
31+
"react/dns": "^1.11",
3232
"react/event-loop": "^1.2",
3333
"react/promise": "^3 || ^2.6 || ^1.2.1",
34-
"react/promise-timer": "^1.9",
3534
"react/stream": "^1.2"
3635
},
3736
"require-dev": {
3837
"phpunit/phpunit": "^9.5 || ^5.7 || ^4.8.35",
3938
"react/async": "^4 || ^3 || ^2",
40-
"react/promise-stream": "^1.4"
39+
"react/promise-stream": "^1.4",
40+
"react/promise-timer": "^1.9"
4141
},
4242
"autoload": {
4343
"psr-4": {

‎src/TimeoutConnector.php‎

Lines changed: 43 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,7 @@
44

55
use React\EventLoop\Loop;
66
use React\EventLoop\LoopInterface;
7-
use React\Promise\Timer;
8-
use React\Promise\Timer\TimeoutException;
7+
use React\Promise\Promise;
98

109
final class TimeoutConnector implements ConnectorInterface
1110
{
@@ -22,30 +21,50 @@ public function __construct(ConnectorInterface $connector, $timeout, LoopInterfa
2221

2322
public function connect($uri)
2423
{
25-
return Timer\timeout($this->connector->connect($uri), $this->timeout, $this->loop)->then(null, self::handler($uri));
26-
}
24+
$promise = $this->connector->connect($uri);
2725

28-
/**
29-
* Creates a static rejection handler that reports a proper error message in case of a timeout.
30-
*
31-
* This uses a private static helper method to ensure this closure is not
32-
* bound to this instance and the exception trace does not include a
33-
* reference to this instance and its connector stack as a result.
34-
*
35-
* @param string $uri
36-
* @return callable
37-
*/
38-
private static function handler($uri)
39-
{
40-
return function (\Exception $e) use ($uri) {
41-
if ($e instanceof TimeoutException) {
42-
throw new \RuntimeException(
43-
'Connection to ' . $uri . ' timed out after ' . $e->getTimeout() . ' seconds (ETIMEDOUT)',
44-
\defined('SOCKET_ETIMEDOUT') ? \SOCKET_ETIMEDOUT : 110
45-
);
26+
$loop = $this->loop;
27+
$time = $this->timeout;
28+
return new Promise(function ($resolve, $reject) use ($loop, $time, $promise, $uri) {
29+
$timer = null;
30+
$promise = $promise->then(function ($v) use (&$timer, $loop, $resolve) {
31+
if ($timer) {
32+
$loop->cancelTimer($timer);
33+
}
34+
$timer = false;
35+
$resolve($v);
36+
}, function ($v) use (&$timer, $loop, $reject) {
37+
if ($timer) {
38+
$loop->cancelTimer($timer);
39+
}
40+
$timer = false;
41+
$reject($v);
42+
});
43+
44+
// promise already resolved => no need to start timer
45+
if ($timer === false) {
46+
return;
4647
}
4748

48-
throw $e;
49-
};
49+
// start timeout timer which will cancel the pending promise
50+
$timer = $loop->addTimer($time, function () use ($time, &$promise, $reject, $uri) {
51+
$reject(new \RuntimeException(
52+
'Connection to ' . $uri . ' timed out after ' . $time . ' seconds (ETIMEDOUT)',
53+
\defined('SOCKET_ETIMEDOUT') ? \SOCKET_ETIMEDOUT : 110
54+
));
55+
56+
// Cancel pending connection to clean up any underlying resources and references.
57+
// Avoid garbage references in call stack by passing pending promise by reference.
58+
assert(\method_exists($promise, 'cancel'));
59+
$promise->cancel();
60+
$promise = null;
61+
});
62+
}, function () use (&$promise) {
63+
// Cancelling this promise will cancel the pending connection, thus triggering the rejection logic above.
64+
// Avoid garbage references in call stack by passing pending promise by reference.
65+
assert(\method_exists($promise, 'cancel'));
66+
$promise->cancel();
67+
$promise = null;
68+
});
5069
}
5170
}

‎tests/TimeoutConnectorTest.php‎

Lines changed: 133 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,8 @@
33
namespace React\Tests\Socket;
44

55
use React\EventLoop\Loop;
6-
use React\Promise;
76
use React\Promise\Deferred;
7+
use React\Promise\Promise;
88
use React\Socket\TimeoutConnector;
99

1010
class TimeoutConnectorTest extends TestCase
@@ -22,90 +22,175 @@ public function testConstructWithoutLoopAssignsLoopAutomatically()
2222
$this->assertInstanceOf('React\EventLoop\LoopInterface', $loop);
2323
}
2424

25-
public function testRejectsWithTimeoutReasonOnTimeout()
25+
public function testRejectsPromiseWithoutStartingTimerWhenWrappedConnectorReturnsRejectedPromise()
2626
{
27-
$promise = new Promise\Promise(function () { });
27+
$loop = $this->getMockBuilder('React\EventLoop\LoopInterface')->getMock();
28+
$loop->expects($this->never())->method('addTimer');
29+
$loop->expects($this->never())->method('cancelTimer');
2830

2931
$connector = $this->getMockBuilder('React\Socket\ConnectorInterface')->getMock();
30-
$connector->expects($this->once())->method('connect')->with('google.com:80')->will($this->returnValue($promise));
32+
$connector->expects($this->once())->method('connect')->with('example.com:80')->willReturn(\React\Promise\reject(new \RuntimeException('Failed', 42)));
3133

32-
$timeout = new TimeoutConnector($connector, 0.01);
34+
$timeout = new TimeoutConnector($connector, 5.0, $loop);
3335

34-
$promise = $timeout->connect('google.com:80');
35-
Loop::run();
36+
$promise = $timeout->connect('example.com:80');
37+
38+
$exception = null;
39+
$promise->then(null, function ($reason) use (&$exception) {
40+
$exception = $reason;
41+
});
3642

37-
$this->setExpectedException(
38-
'RuntimeException',
39-
'Connection to google.com:80 timed out after 0.01 seconds (ETIMEDOUT)',
40-
\defined('SOCKET_ETIMEDOUT') ? \SOCKET_ETIMEDOUT : 110
41-
);
42-
\React\Async\await($promise);
43+
assert($exception instanceof \RuntimeException);
44+
$this->assertEquals('Failed', $exception->getMessage());
45+
$this->assertEquals(42, $exception->getCode());
4346
}
4447

45-
public function testRejectsWithOriginalReasonWhenConnectorRejects()
48+
public function testRejectsPromiseAfterCancellingTimerWhenWrappedConnectorReturnsPendingPromiseThatRejects()
4649
{
47-
$promise = Promise\reject(new \RuntimeException('Failed', 42));
50+
$timer = $this->getMockBuilder('React\EventLoop\TimerInterface')->getMock();
51+
$loop = $this->getMockBuilder('React\EventLoop\LoopInterface')->getMock();
52+
$loop->expects($this->once())->method('addTimer')->with(5.0, $this->anything())->willReturn($timer);
53+
$loop->expects($this->once())->method('cancelTimer')->with($timer);
4854

55+
$deferred = new Deferred();
4956
$connector = $this->getMockBuilder('React\Socket\ConnectorInterface')->getMock();
50-
$connector->expects($this->once())->method('connect')->with('google.com:80')->will($this->returnValue($promise));
57+
$connector->expects($this->once())->method('connect')->with('example.com:80')->willReturn($deferred->promise());
5158

52-
$timeout = new TimeoutConnector($connector, 5.0);
59+
$timeout = new TimeoutConnector($connector, 5.0, $loop);
60+
61+
$promise = $timeout->connect('example.com:80');
62+
63+
$deferred->reject(new \RuntimeException('Failed', 42));
64+
65+
$exception = null;
66+
$promise->then(null, function ($reason) use (&$exception) {
67+
$exception = $reason;
68+
});
5369

54-
$this->setExpectedException(
55-
'RuntimeException',
56-
'Failed',
57-
42
58-
);
59-
\React\Async\await($timeout->connect('google.com:80'));
70+
assert($exception instanceof \RuntimeException);
71+
$this->assertEquals('Failed', $exception->getMessage());
72+
$this->assertEquals(42, $exception->getCode());
6073
}
6174

62-
public function testResolvesWhenConnectorResolves()
75+
public function testResolvesPromiseWithoutStartingTimerWhenWrappedConnectorReturnsResolvedPromise()
6376
{
64-
$promise = Promise\resolve(null);
77+
$loop = $this->getMockBuilder('React\EventLoop\LoopInterface')->getMock();
78+
$loop->expects($this->never())->method('addTimer');
79+
$loop->expects($this->never())->method('cancelTimer');
6580

81+
$connection = $this->getMockBuilder('React\Socket\ConnectionInterface')->getMock();
6682
$connector = $this->getMockBuilder('React\Socket\ConnectorInterface')->getMock();
67-
$connector->expects($this->once())->method('connect')->with('google.com:80')->will($this->returnValue($promise));
83+
$connector->expects($this->once())->method('connect')->with('example.com:80')->willReturn(\React\Promise\resolve($connection));
6884

69-
$timeout = new TimeoutConnector($connector, 5.0);
85+
$timeout = new TimeoutConnector($connector, 5.0, $loop);
7086

71-
$timeout->connect('google.com:80')->then(
72-
$this->expectCallableOnce(),
73-
$this->expectCallableNever()
74-
);
87+
$promise = $timeout->connect('example.com:80');
7588

76-
Loop::run();
89+
$resolved = null;
90+
$promise->then(function ($value) use (&$resolved) {
91+
$resolved = $value;
92+
});
93+
94+
$this->assertSame($connection, $resolved);
7795
}
7896

79-
public function testRejectsAndCancelsPendingPromiseOnTimeout()
97+
public function testResolvesPromiseAfterCancellingTimerWhenWrappedConnectorReturnsPendingPromiseThatResolves()
8098
{
81-
$promise = new Promise\Promise(function () { }, $this->expectCallableOnce());
99+
$timer = $this->getMockBuilder('React\EventLoop\TimerInterface')->getMock();
100+
$loop = $this->getMockBuilder('React\EventLoop\LoopInterface')->getMock();
101+
$loop->expects($this->once())->method('addTimer')->with(5.0, $this->anything())->willReturn($timer);
102+
$loop->expects($this->once())->method('cancelTimer')->with($timer);
82103

104+
$deferred = new Deferred();
83105
$connector = $this->getMockBuilder('React\Socket\ConnectorInterface')->getMock();
84-
$connector->expects($this->once())->method('connect')->with('google.com:80')->will($this->returnValue($promise));
106+
$connector->expects($this->once())->method('connect')->with('example.com:80')->willReturn($deferred->promise());
85107

86-
$timeout = new TimeoutConnector($connector, 0.01);
108+
$timeout = new TimeoutConnector($connector, 5.0, $loop);
87109

88-
$timeout->connect('google.com:80')->then(
89-
$this->expectCallableNever(),
90-
$this->expectCallableOnce()
91-
);
110+
$promise = $timeout->connect('example.com:80');
92111

93-
Loop::run();
112+
$connection = $this->getMockBuilder('React\Socket\ConnectionInterface')->getMock();
113+
$deferred->resolve($connection);
114+
115+
$resolved = null;
116+
$promise->then(function ($value) use (&$resolved) {
117+
$resolved = $value;
118+
});
119+
120+
$this->assertSame($connection, $resolved);
94121
}
95122

96-
public function testCancelsPendingPromiseOnCancel()
123+
public function testRejectsPromiseAndCancelsPendingConnectionWhenTimeoutTriggers()
97124
{
98-
$promise = new Promise\Promise(function () { }, function () { throw new \Exception(); });
125+
$timerCallback = null;
126+
$timer = $this->getMockBuilder('React\EventLoop\TimerInterface')->getMock();
127+
$loop = $this->getMockBuilder('React\EventLoop\LoopInterface')->getMock();
128+
$loop->expects($this->once())->method('addTimer')->with(0.01, $this->callback(function ($callback) use (&$timerCallback) {
129+
$timerCallback = $callback;
130+
return true;
131+
}))->willReturn($timer);
132+
$loop->expects($this->once())->method('cancelTimer')->with($timer);
133+
134+
$cancelled = 0;
135+
$connector = $this->getMockBuilder('React\Socket\ConnectorInterface')->getMock();
136+
$connector->expects($this->once())->method('connect')->with('example.com:80')->willReturn(new Promise(function () { }, function () use (&$cancelled) {
137+
++$cancelled;
138+
throw new \RuntimeException();
139+
}));
140+
141+
$timeout = new TimeoutConnector($connector, 0.01, $loop);
142+
143+
$promise = $timeout->connect('example.com:80');
144+
145+
$this->assertEquals(0, $cancelled);
146+
147+
$this->assertNotNull($timerCallback);
148+
$timerCallback();
149+
150+
$this->assertEquals(1, $cancelled);
151+
152+
$exception = null;
153+
$promise->then(null, function ($reason) use (&$exception) {
154+
$exception = $reason;
155+
});
156+
157+
assert($exception instanceof \RuntimeException);
158+
$this->assertEquals('Connection to example.com:80 timed out after 0.01 seconds (ETIMEDOUT)' , $exception->getMessage());
159+
$this->assertEquals(\defined('SOCKET_ETIMEDOUT') ? \SOCKET_ETIMEDOUT : 110, $exception->getCode());
160+
}
161+
162+
public function testCancellingPromiseWillCancelPendingConnectionAndRejectPromise()
163+
{
164+
$timer = $this->getMockBuilder('React\EventLoop\TimerInterface')->getMock();
165+
$loop = $this->getMockBuilder('React\EventLoop\LoopInterface')->getMock();
166+
$loop->expects($this->once())->method('addTimer')->with(0.01, $this->anything())->willReturn($timer);
167+
$loop->expects($this->once())->method('cancelTimer')->with($timer);
99168

169+
$cancelled = 0;
100170
$connector = $this->getMockBuilder('React\Socket\ConnectorInterface')->getMock();
101-
$connector->expects($this->once())->method('connect')->with('google.com:80')->will($this->returnValue($promise));
171+
$connector->expects($this->once())->method('connect')->with('example.com:80')->willReturn(new Promise(function () { }, function () use (&$cancelled) {
172+
++$cancelled;
173+
throw new \RuntimeException('Cancelled');
174+
}));
102175

103-
$timeout = new TimeoutConnector($connector, 0.01);
176+
$timeout = new TimeoutConnector($connector, 0.01, $loop);
177+
178+
$promise = $timeout->connect('example.com:80');
179+
180+
$this->assertEquals(0, $cancelled);
181+
182+
assert(method_exists($promise, 'cancel'));
183+
$promise->cancel();
104184

105-
$out = $timeout->connect('google.com:80');
106-
$out->cancel();
185+
$this->assertEquals(1, $cancelled);
186+
187+
$exception = null;
188+
$promise->then(null, function ($reason) use (&$exception) {
189+
$exception = $reason;
190+
});
107191

108-
$out->then($this->expectCallableNever(), $this->expectCallableOnce());
192+
assert($exception instanceof \RuntimeException);
193+
$this->assertEquals('Cancelled', $exception->getMessage());
109194
}
110195

111196
public function testRejectionDuringConnectionShouldNotCreateAnyGarbageReferences()

0 commit comments

Comments
 (0)