From 78dc111bee6c503d4a5a4d94ab286518b5241aa0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20L=C3=BCck?= Date: Fri, 2 Jun 2023 15:20:01 +0200 Subject: [PATCH] Include timeout logic to avoid dependency on reactphp/promise-timer --- composer.json | 6 +- src/TimeoutConnector.php | 67 +++++++----- tests/TimeoutConnectorTest.php | 181 ++++++++++++++++++++++++--------- 3 files changed, 179 insertions(+), 75 deletions(-) diff --git a/composer.json b/composer.json index 8613aa95..5d3240f9 100644 --- a/composer.json +++ b/composer.json @@ -28,16 +28,16 @@ "require": { "php": ">=5.3.0", "evenement/evenement": "^3.0 || ^2.0 || ^1.0", - "react/dns": "^1.8", + "react/dns": "^1.11", "react/event-loop": "^1.2", "react/promise": "^3 || ^2.6 || ^1.2.1", - "react/promise-timer": "^1.9", "react/stream": "^1.2" }, "require-dev": { "phpunit/phpunit": "^9.5 || ^5.7 || ^4.8.35", "react/async": "^4 || ^3 || ^2", - "react/promise-stream": "^1.4" + "react/promise-stream": "^1.4", + "react/promise-timer": "^1.9" }, "autoload": { "psr-4": { diff --git a/src/TimeoutConnector.php b/src/TimeoutConnector.php index 332369f8..a20ea5a6 100644 --- a/src/TimeoutConnector.php +++ b/src/TimeoutConnector.php @@ -4,8 +4,7 @@ use React\EventLoop\Loop; use React\EventLoop\LoopInterface; -use React\Promise\Timer; -use React\Promise\Timer\TimeoutException; +use React\Promise\Promise; final class TimeoutConnector implements ConnectorInterface { @@ -22,30 +21,50 @@ public function __construct(ConnectorInterface $connector, $timeout, LoopInterfa public function connect($uri) { - return Timer\timeout($this->connector->connect($uri), $this->timeout, $this->loop)->then(null, self::handler($uri)); - } + $promise = $this->connector->connect($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 (ETIMEDOUT)', - \defined('SOCKET_ETIMEDOUT') ? \SOCKET_ETIMEDOUT : 110 - ); + $loop = $this->loop; + $time = $this->timeout; + return new Promise(function ($resolve, $reject) use ($loop, $time, $promise, $uri) { + $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, $uri) { + $reject(new \RuntimeException( + 'Connection to ' . $uri . ' timed out after ' . $time . ' seconds (ETIMEDOUT)', + \defined('SOCKET_ETIMEDOUT') ? \SOCKET_ETIMEDOUT : 110 + )); + + // Cancel pending connection 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 connection, 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; + }); } } diff --git a/tests/TimeoutConnectorTest.php b/tests/TimeoutConnectorTest.php index 2ac9690e..71ca5834 100644 --- a/tests/TimeoutConnectorTest.php +++ b/tests/TimeoutConnectorTest.php @@ -3,8 +3,8 @@ namespace React\Tests\Socket; use React\EventLoop\Loop; -use React\Promise; use React\Promise\Deferred; +use React\Promise\Promise; use React\Socket\TimeoutConnector; class TimeoutConnectorTest extends TestCase @@ -22,90 +22,175 @@ public function testConstructWithoutLoopAssignsLoopAutomatically() $this->assertInstanceOf('React\EventLoop\LoopInterface', $loop); } - public function testRejectsWithTimeoutReasonOnTimeout() + public function testRejectsPromiseWithoutStartingTimerWhenWrappedConnectorReturnsRejectedPromise() { - $promise = new Promise\Promise(function () { }); + $loop = $this->getMockBuilder('React\EventLoop\LoopInterface')->getMock(); + $loop->expects($this->never())->method('addTimer'); + $loop->expects($this->never())->method('cancelTimer'); $connector = $this->getMockBuilder('React\Socket\ConnectorInterface')->getMock(); - $connector->expects($this->once())->method('connect')->with('google.com:80')->will($this->returnValue($promise)); + $connector->expects($this->once())->method('connect')->with('example.com:80')->willReturn(\React\Promise\reject(new \RuntimeException('Failed', 42))); - $timeout = new TimeoutConnector($connector, 0.01); + $timeout = new TimeoutConnector($connector, 5.0, $loop); - $promise = $timeout->connect('google.com:80'); - Loop::run(); + $promise = $timeout->connect('example.com:80'); + + $exception = null; + $promise->then(null, function ($reason) use (&$exception) { + $exception = $reason; + }); - $this->setExpectedException( - 'RuntimeException', - 'Connection to google.com:80 timed out after 0.01 seconds (ETIMEDOUT)', - \defined('SOCKET_ETIMEDOUT') ? \SOCKET_ETIMEDOUT : 110 - ); - \React\Async\await($promise); + assert($exception instanceof \RuntimeException); + $this->assertEquals('Failed', $exception->getMessage()); + $this->assertEquals(42, $exception->getCode()); } - public function testRejectsWithOriginalReasonWhenConnectorRejects() + public function testRejectsPromiseAfterCancellingTimerWhenWrappedConnectorReturnsPendingPromiseThatRejects() { - $promise = Promise\reject(new \RuntimeException('Failed', 42)); + $timer = $this->getMockBuilder('React\EventLoop\TimerInterface')->getMock(); + $loop = $this->getMockBuilder('React\EventLoop\LoopInterface')->getMock(); + $loop->expects($this->once())->method('addTimer')->with(5.0, $this->anything())->willReturn($timer); + $loop->expects($this->once())->method('cancelTimer')->with($timer); + $deferred = new Deferred(); $connector = $this->getMockBuilder('React\Socket\ConnectorInterface')->getMock(); - $connector->expects($this->once())->method('connect')->with('google.com:80')->will($this->returnValue($promise)); + $connector->expects($this->once())->method('connect')->with('example.com:80')->willReturn($deferred->promise()); - $timeout = new TimeoutConnector($connector, 5.0); + $timeout = new TimeoutConnector($connector, 5.0, $loop); + + $promise = $timeout->connect('example.com:80'); + + $deferred->reject(new \RuntimeException('Failed', 42)); + + $exception = null; + $promise->then(null, function ($reason) use (&$exception) { + $exception = $reason; + }); - $this->setExpectedException( - 'RuntimeException', - 'Failed', - 42 - ); - \React\Async\await($timeout->connect('google.com:80')); + assert($exception instanceof \RuntimeException); + $this->assertEquals('Failed', $exception->getMessage()); + $this->assertEquals(42, $exception->getCode()); } - public function testResolvesWhenConnectorResolves() + public function testResolvesPromiseWithoutStartingTimerWhenWrappedConnectorReturnsResolvedPromise() { - $promise = Promise\resolve(null); + $loop = $this->getMockBuilder('React\EventLoop\LoopInterface')->getMock(); + $loop->expects($this->never())->method('addTimer'); + $loop->expects($this->never())->method('cancelTimer'); + $connection = $this->getMockBuilder('React\Socket\ConnectionInterface')->getMock(); $connector = $this->getMockBuilder('React\Socket\ConnectorInterface')->getMock(); - $connector->expects($this->once())->method('connect')->with('google.com:80')->will($this->returnValue($promise)); + $connector->expects($this->once())->method('connect')->with('example.com:80')->willReturn(\React\Promise\resolve($connection)); - $timeout = new TimeoutConnector($connector, 5.0); + $timeout = new TimeoutConnector($connector, 5.0, $loop); - $timeout->connect('google.com:80')->then( - $this->expectCallableOnce(), - $this->expectCallableNever() - ); + $promise = $timeout->connect('example.com:80'); - Loop::run(); + $resolved = null; + $promise->then(function ($value) use (&$resolved) { + $resolved = $value; + }); + + $this->assertSame($connection, $resolved); } - public function testRejectsAndCancelsPendingPromiseOnTimeout() + public function testResolvesPromiseAfterCancellingTimerWhenWrappedConnectorReturnsPendingPromiseThatResolves() { - $promise = new Promise\Promise(function () { }, $this->expectCallableOnce()); + $timer = $this->getMockBuilder('React\EventLoop\TimerInterface')->getMock(); + $loop = $this->getMockBuilder('React\EventLoop\LoopInterface')->getMock(); + $loop->expects($this->once())->method('addTimer')->with(5.0, $this->anything())->willReturn($timer); + $loop->expects($this->once())->method('cancelTimer')->with($timer); + $deferred = new Deferred(); $connector = $this->getMockBuilder('React\Socket\ConnectorInterface')->getMock(); - $connector->expects($this->once())->method('connect')->with('google.com:80')->will($this->returnValue($promise)); + $connector->expects($this->once())->method('connect')->with('example.com:80')->willReturn($deferred->promise()); - $timeout = new TimeoutConnector($connector, 0.01); + $timeout = new TimeoutConnector($connector, 5.0, $loop); - $timeout->connect('google.com:80')->then( - $this->expectCallableNever(), - $this->expectCallableOnce() - ); + $promise = $timeout->connect('example.com:80'); - Loop::run(); + $connection = $this->getMockBuilder('React\Socket\ConnectionInterface')->getMock(); + $deferred->resolve($connection); + + $resolved = null; + $promise->then(function ($value) use (&$resolved) { + $resolved = $value; + }); + + $this->assertSame($connection, $resolved); } - public function testCancelsPendingPromiseOnCancel() + public function testRejectsPromiseAndCancelsPendingConnectionWhenTimeoutTriggers() { - $promise = new Promise\Promise(function () { }, function () { throw new \Exception(); }); + $timerCallback = null; + $timer = $this->getMockBuilder('React\EventLoop\TimerInterface')->getMock(); + $loop = $this->getMockBuilder('React\EventLoop\LoopInterface')->getMock(); + $loop->expects($this->once())->method('addTimer')->with(0.01, $this->callback(function ($callback) use (&$timerCallback) { + $timerCallback = $callback; + return true; + }))->willReturn($timer); + $loop->expects($this->once())->method('cancelTimer')->with($timer); + + $cancelled = 0; + $connector = $this->getMockBuilder('React\Socket\ConnectorInterface')->getMock(); + $connector->expects($this->once())->method('connect')->with('example.com:80')->willReturn(new Promise(function () { }, function () use (&$cancelled) { + ++$cancelled; + throw new \RuntimeException(); + })); + + $timeout = new TimeoutConnector($connector, 0.01, $loop); + + $promise = $timeout->connect('example.com:80'); + + $this->assertEquals(0, $cancelled); + + $this->assertNotNull($timerCallback); + $timerCallback(); + + $this->assertEquals(1, $cancelled); + + $exception = null; + $promise->then(null, function ($reason) use (&$exception) { + $exception = $reason; + }); + + assert($exception instanceof \RuntimeException); + $this->assertEquals('Connection to example.com:80 timed out after 0.01 seconds (ETIMEDOUT)' , $exception->getMessage()); + $this->assertEquals(\defined('SOCKET_ETIMEDOUT') ? \SOCKET_ETIMEDOUT : 110, $exception->getCode()); + } + + public function testCancellingPromiseWillCancelPendingConnectionAndRejectPromise() + { + $timer = $this->getMockBuilder('React\EventLoop\TimerInterface')->getMock(); + $loop = $this->getMockBuilder('React\EventLoop\LoopInterface')->getMock(); + $loop->expects($this->once())->method('addTimer')->with(0.01, $this->anything())->willReturn($timer); + $loop->expects($this->once())->method('cancelTimer')->with($timer); + $cancelled = 0; $connector = $this->getMockBuilder('React\Socket\ConnectorInterface')->getMock(); - $connector->expects($this->once())->method('connect')->with('google.com:80')->will($this->returnValue($promise)); + $connector->expects($this->once())->method('connect')->with('example.com:80')->willReturn(new Promise(function () { }, function () use (&$cancelled) { + ++$cancelled; + throw new \RuntimeException('Cancelled'); + })); - $timeout = new TimeoutConnector($connector, 0.01); + $timeout = new TimeoutConnector($connector, 0.01, $loop); + + $promise = $timeout->connect('example.com:80'); + + $this->assertEquals(0, $cancelled); + + assert(method_exists($promise, 'cancel')); + $promise->cancel(); - $out = $timeout->connect('google.com:80'); - $out->cancel(); + $this->assertEquals(1, $cancelled); + + $exception = null; + $promise->then(null, function ($reason) use (&$exception) { + $exception = $reason; + }); - $out->then($this->expectCallableNever(), $this->expectCallableOnce()); + assert($exception instanceof \RuntimeException); + $this->assertEquals('Cancelled', $exception->getMessage()); } public function testRejectionDuringConnectionShouldNotCreateAnyGarbageReferences()