From 559cb094172e42204b0059fa662660fc9aec15c3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20L=C3=BCck?= Date: Fri, 14 Feb 2020 12:01:10 +0100 Subject: [PATCH 1/2] Refactor request timeout handling to prepare for stream handling --- composer.json | 3 +- src/Io/Transaction.php | 23 ++-- tests/Io/TransactionTest.php | 233 ++++++++++++++++++++++------------- 3 files changed, 164 insertions(+), 95 deletions(-) diff --git a/composer.json b/composer.json index ca9ef16..313bd03 100644 --- a/composer.json +++ b/composer.json @@ -19,11 +19,10 @@ "require": { "php": ">=5.3", "psr/http-message": "^1.0", - "react/event-loop": "^1.0 || ^0.5 || ^0.4 || ^0.3", + "react/event-loop": "^1.0 || ^0.5", "react/http-client": "^0.5.10", "react/promise": "^2.2.1 || ^1.2.1", "react/promise-stream": "^1.0 || ^0.1.1", - "react/promise-timer": "^1.2", "react/socket": "^1.1", "react/stream": "^1.0 || ^0.7", "ringcentral/psr7": "^1.2" diff --git a/src/Io/Transaction.php b/src/Io/Transaction.php index e6d9a3c..36d3d06 100644 --- a/src/Io/Transaction.php +++ b/src/Io/Transaction.php @@ -10,7 +10,6 @@ use React\EventLoop\LoopInterface; use React\Promise\Deferred; use React\Promise\PromiseInterface; -use React\Promise\Timer\TimeoutException; use React\Stream\ReadableStreamInterface; /** @@ -87,13 +86,23 @@ public function send(RequestInterface $request) return $deferred->promise(); } - return \React\Promise\Timer\timeout($deferred->promise(), $timeout, $this->loop)->then(null, function ($e) { - if ($e instanceof TimeoutException) { - throw new \RuntimeException( - 'Request timed out after ' . $e->getTimeout() . ' seconds' - ); + $timer = $this->loop->addTimer($timeout, function () use ($timeout, $deferred) { + $deferred->reject(new \RuntimeException( + 'Request timed out after ' . $timeout . ' seconds' + )); + if (isset($deferred->pending)) { + $deferred->pending->cancel(); + unset($deferred->pending); } - throw $e; + }); + + $loop = $this->loop; + return $deferred->promise()->then(function ($result) use ($loop, $timer){ + $loop->cancelTimer($timer); + return $result; + }, function ($reason) use ($loop, $timer) { + $loop->cancelTimer($timer); + throw $reason; }); } diff --git a/tests/Io/TransactionTest.php b/tests/Io/TransactionTest.php index 284e2df..43876ca 100644 --- a/tests/Io/TransactionTest.php +++ b/tests/Io/TransactionTest.php @@ -63,6 +63,150 @@ public function testWithOptionsNullValueReturnsNewInstanceWithDefaultOption() $this->assertTrue($ref->getValue($transaction)); } + public function testTimeoutExplicitOptionWillStartTimeoutTimer() + { + $messageFactory = new MessageFactory(); + + $timer = $this->getMockBuilder('React\EventLoop\TimerInterface')->getMock(); + $loop = $this->getMockBuilder('React\EventLoop\LoopInterface')->getMock(); + $loop->expects($this->once())->method('addTimer')->with(2, $this->anything())->willReturn($timer); + $loop->expects($this->never())->method('cancelTimer'); + + $request = $this->getMockBuilder('Psr\Http\Message\RequestInterface')->getMock(); + + $sender = $this->getMockBuilder('Clue\React\Buzz\Io\Sender')->disableOriginalConstructor()->getMock(); + $sender->expects($this->once())->method('send')->with($this->equalTo($request))->willReturn(new \React\Promise\Promise(function () { })); + + $transaction = new Transaction($sender, $messageFactory, $loop); + $transaction = $transaction->withOptions(array('timeout' => 2)); + $promise = $transaction->send($request); + + $this->assertInstanceOf('React\Promise\PromiseInterface', $promise); + } + + public function testTimeoutImplicitFromIniWillStartTimeoutTimer() + { + $messageFactory = new MessageFactory(); + + $timer = $this->getMockBuilder('React\EventLoop\TimerInterface')->getMock(); + $loop = $this->getMockBuilder('React\EventLoop\LoopInterface')->getMock(); + $loop->expects($this->once())->method('addTimer')->with(2, $this->anything())->willReturn($timer); + $loop->expects($this->never())->method('cancelTimer'); + + $request = $this->getMockBuilder('Psr\Http\Message\RequestInterface')->getMock(); + + $sender = $this->getMockBuilder('Clue\React\Buzz\Io\Sender')->disableOriginalConstructor()->getMock(); + $sender->expects($this->once())->method('send')->with($this->equalTo($request))->willReturn(new \React\Promise\Promise(function () { })); + + $transaction = new Transaction($sender, $messageFactory, $loop); + + $old = ini_get('default_socket_timeout'); + ini_set('default_socket_timeout', '2'); + $promise = $transaction->send($request); + ini_set('default_socket_timeout', $old); + + $this->assertInstanceOf('React\Promise\PromiseInterface', $promise); + } + + public function testTimeoutExplicitOptionWillRejectWhenTimerFires() + { + $messageFactory = new MessageFactory(); + + $timeout = null; + $timer = $this->getMockBuilder('React\EventLoop\TimerInterface')->getMock(); + $loop = $this->getMockBuilder('React\EventLoop\LoopInterface')->getMock(); + $loop->expects($this->once())->method('addTimer')->with(2, $this->callback(function ($cb) use (&$timeout) { + $timeout = $cb; + return true; + }))->willReturn($timer); + $loop->expects($this->once())->method('cancelTimer')->with($timer); + + $request = $this->getMockBuilder('Psr\Http\Message\RequestInterface')->getMock(); + + $sender = $this->getMockBuilder('Clue\React\Buzz\Io\Sender')->disableOriginalConstructor()->getMock(); + $sender->expects($this->once())->method('send')->with($this->equalTo($request))->willReturn(new \React\Promise\Promise(function () { })); + + $transaction = new Transaction($sender, $messageFactory, $loop); + $transaction = $transaction->withOptions(array('timeout' => 2)); + $promise = $transaction->send($request); + + $this->assertNotNull($timeout); + $timeout(); + + $exception = null; + $promise->then(null, function ($e) use (&$exception) { + $exception = $e; + }); + + $this->assertInstanceOf('RuntimeException', $exception); + $this->assertEquals('Request timed out after 2 seconds', $exception->getMessage()); + } + + public function testTimeoutExplicitOptionWillCancelTimeoutTimerWhenSenderResolves() + { + $messageFactory = new MessageFactory(); + + $timer = $this->getMockBuilder('React\EventLoop\TimerInterface')->getMock(); + $loop = $this->getMockBuilder('React\EventLoop\LoopInterface')->getMock(); + $loop->expects($this->once())->method('addTimer')->willReturn($timer); + $loop->expects($this->once())->method('cancelTimer')->with($timer); + + $request = $this->getMockBuilder('Psr\Http\Message\RequestInterface')->getMock(); + $response = $messageFactory->response(1.0, 200, 'OK', array(), ''); + + $sender = $this->getMockBuilder('Clue\React\Buzz\Io\Sender')->disableOriginalConstructor()->getMock(); + $sender->expects($this->once())->method('send')->with($this->equalTo($request))->willReturn(Promise\resolve($response)); + + $transaction = new Transaction($sender, $messageFactory, $loop); + $transaction = $transaction->withOptions(array('timeout' => 0.001)); + $promise = $transaction->send($request); + + $this->assertInstanceOf('React\Promise\PromiseInterface', $promise); + $promise->then($this->expectCallableOnceWith($response)); + } + + public function testTimeoutExplicitOptionWillCancelTimeoutTimerWhenSenderRejects() + { + $messageFactory = new MessageFactory(); + + $timer = $this->getMockBuilder('React\EventLoop\TimerInterface')->getMock(); + $loop = $this->getMockBuilder('React\EventLoop\LoopInterface')->getMock(); + $loop->expects($this->once())->method('addTimer')->willReturn($timer); + $loop->expects($this->once())->method('cancelTimer')->with($timer); + + $request = $this->getMockBuilder('Psr\Http\Message\RequestInterface')->getMock(); + $exception = new \RuntimeException(); + + $sender = $this->getMockBuilder('Clue\React\Buzz\Io\Sender')->disableOriginalConstructor()->getMock(); + $sender->expects($this->once())->method('send')->with($this->equalTo($request))->willReturn(Promise\reject($exception)); + + $transaction = new Transaction($sender, $messageFactory, $loop); + $transaction = $transaction->withOptions(array('timeout' => 0.001)); + $promise = $transaction->send($request); + + $this->assertInstanceOf('React\Promise\PromiseInterface', $promise); + $promise->then(null, $this->expectCallableOnceWith($exception)); + } + + public function testTimeoutExplicitNegativeWillNotStartTimeoutTimer() + { + $messageFactory = new MessageFactory(); + + $loop = $this->getMockBuilder('React\EventLoop\LoopInterface')->getMock(); + $loop->expects($this->never())->method('addTimer'); + + $request = $this->getMockBuilder('Psr\Http\Message\RequestInterface')->getMock(); + + $sender = $this->getMockBuilder('Clue\React\Buzz\Io\Sender')->disableOriginalConstructor()->getMock(); + $sender->expects($this->once())->method('send')->with($this->equalTo($request))->willReturn(new \React\Promise\Promise(function () { })); + + $transaction = new Transaction($sender, $messageFactory, $loop); + $transaction = $transaction->withOptions(array('timeout' => -1)); + $promise = $transaction->send($request); + + $this->assertInstanceOf('React\Promise\PromiseInterface', $promise); + } + public function testReceivingErrorResponseWillRejectWithResponseException() { $request = $this->getMockBuilder('Psr\Http\Message\RequestInterface')->getMock(); @@ -74,6 +218,7 @@ public function testReceivingErrorResponseWillRejectWithResponseException() $sender->expects($this->once())->method('send')->with($this->equalTo($request))->willReturn(Promise\resolve($response)); $transaction = new Transaction($sender, new MessageFactory(), $loop); + $transaction = $transaction->withOptions(array('timeout' => -1)); $promise = $transaction->send($request); try { @@ -151,7 +296,7 @@ public function testReceivingStreamingBodyWillResolveWithStreamingResponseIfStre $sender->expects($this->once())->method('send')->with($this->equalTo($request))->willReturn(Promise\resolve($response)); $transaction = new Transaction($sender, $messageFactory, $loop); - $transaction = $transaction->withOptions(array('streaming' => true)); + $transaction = $transaction->withOptions(array('streaming' => true, 'timeout' => -1)); $promise = $transaction->send($request); $response = Block\await($promise, $loop); @@ -172,6 +317,7 @@ public function testResponseCode304WithoutLocationWillResolveWithResponseAsIs() $sender->expects($this->once())->method('send')->with($request)->willReturn(Promise\resolve($response)); $transaction = new Transaction($sender, $messageFactory, $loop); + $transaction = $transaction->withOptions(array('timeout' => -1)); $promise = $transaction->send($request); $promise->then($this->expectCallableOnceWith($response)); @@ -484,91 +630,6 @@ public function testCancelTransactionShouldCancelSendingPromise() $promise->cancel(); } - /** - * @expectedException RuntimeException - * @expectedExceptionMessage Request timed out after 0.001 seconds - */ - public function testTimeoutExplicitOptionWillThrowException() - { - $messageFactory = new MessageFactory(); - $loop = Factory::create(); - - $stream = new ThroughStream(); - $loop->addTimer(0.01, function () use ($stream) { - $stream->close(); - }); - - $request = $this->getMockBuilder('Psr\Http\Message\RequestInterface')->getMock(); - $response = $messageFactory->response(1.0, 200, 'OK', array(), $stream); - - // mock sender to resolve promise with the given $response in response to the given $request - $sender = $this->getMockBuilder('Clue\React\Buzz\Io\Sender')->disableOriginalConstructor()->getMock(); - $sender->expects($this->once())->method('send')->with($this->equalTo($request))->willReturn(Promise\resolve($response)); - - $transaction = new Transaction($sender, $messageFactory, $loop); - $transaction = $transaction->withOptions(array('timeout' => 0.001)); - $promise = $transaction->send($request); - - Block\await($promise, $loop); - } - - /** - * @expectedException RuntimeException - * @expectedExceptionMessage Request timed out after 0.001 seconds - */ - public function testTimeoutImplicitFromIniWillThrowException() - { - $messageFactory = new MessageFactory(); - $loop = Factory::create(); - - $stream = new ThroughStream(); - $loop->addTimer(0.01, function () use ($stream) { - $stream->close(); - }); - - $request = $this->getMockBuilder('Psr\Http\Message\RequestInterface')->getMock(); - $response = $messageFactory->response(1.0, 200, 'OK', array(), $stream); - - // mock sender to resolve promise with the given $response in response to the given $request - $sender = $this->getMockBuilder('Clue\React\Buzz\Io\Sender')->disableOriginalConstructor()->getMock(); - $sender->expects($this->once())->method('send')->with($this->equalTo($request))->willReturn(Promise\resolve($response)); - - $transaction = new Transaction($sender, $messageFactory, $loop); - - $old = ini_get('default_socket_timeout'); - ini_set('default_socket_timeout', '0.001'); - $promise = $transaction->send($request); - ini_set('default_socket_timeout', $old); - - Block\await($promise, $loop); - } - - public function testTimeoutExplicitNegativeWillNotTimeOut() - { - $messageFactory = new MessageFactory(); - $loop = Factory::create(); - - $stream = new ThroughStream(); - $loop->addTimer(0.001, function () use ($stream) { - $stream->close(); - }); - - $request = $this->getMockBuilder('Psr\Http\Message\RequestInterface')->getMock(); - $response = $messageFactory->response(1.0, 200, 'OK', array(), $stream); - - // mock sender to resolve promise with the given $response in response to the given $request - $sender = $this->getMockBuilder('Clue\React\Buzz\Io\Sender')->disableOriginalConstructor()->getMock(); - $sender->expects($this->once())->method('send')->with($this->equalTo($request))->willReturn(Promise\resolve($response)); - - $transaction = new Transaction($sender, $messageFactory, $loop); - $transaction = $transaction->withOptions(array('timeout' => -1)); - $promise = $transaction->send($request); - - $response = Block\await($promise, $loop); - - $this->assertInstanceOf('Psr\Http\Message\ResponseInterface', $response); - } - /** * @return MockObject */ From 830687d3c60f6833ebc62ad7f7b359eb9492ff5a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20L=C3=BCck?= Date: Tue, 25 Feb 2020 16:42:43 +0100 Subject: [PATCH 2/2] Only start request timeout timer after request body has been sent --- src/Io/Transaction.php | 27 +++++++-- tests/FunctionalBrowserTest.php | 14 +++++ tests/Io/TransactionTest.php | 104 ++++++++++++++++++++++++++++++++ 3 files changed, 141 insertions(+), 4 deletions(-) diff --git a/src/Io/Transaction.php b/src/Io/Transaction.php index 36d3d06..25d6ddd 100644 --- a/src/Io/Transaction.php +++ b/src/Io/Transaction.php @@ -86,6 +86,27 @@ public function send(RequestInterface $request) return $deferred->promise(); } + $body = $request->getBody(); + if ($body instanceof ReadableStreamInterface && $body->isReadable()) { + $that = $this; + $body->on('close', function () use ($that, $deferred, $timeout) { + $that->applyTimeout($deferred, $timeout); + }); + } else { + $this->applyTimeout($deferred, $timeout); + } + + return $deferred->promise(); + } + + /** + * @internal + * @param Deferred $deferred + * @param number $timeout + * @return void + */ + public function applyTimeout(Deferred $deferred, $timeout) + { $timer = $this->loop->addTimer($timeout, function () use ($timeout, $deferred) { $deferred->reject(new \RuntimeException( 'Request timed out after ' . $timeout . ' seconds' @@ -97,12 +118,10 @@ public function send(RequestInterface $request) }); $loop = $this->loop; - return $deferred->promise()->then(function ($result) use ($loop, $timer){ + $deferred->promise()->then(function () use ($loop, $timer){ $loop->cancelTimer($timer); - return $result; - }, function ($reason) use ($loop, $timer) { + }, function () use ($loop, $timer) { $loop->cancelTimer($timer); - throw $reason; }); } diff --git a/tests/FunctionalBrowserTest.php b/tests/FunctionalBrowserTest.php index 62d5c81..d128653 100644 --- a/tests/FunctionalBrowserTest.php +++ b/tests/FunctionalBrowserTest.php @@ -129,6 +129,20 @@ public function testTimeoutDelayedResponseShouldReject() Block\await($promise, $this->loop); } + /** + * @expectedException RuntimeException + * @expectedExceptionMessage Request timed out after 0.1 seconds + * @group online + */ + public function testTimeoutDelayedResponseAfterStreamingRequestShouldReject() + { + $stream = new ThroughStream(); + $promise = $this->browser->withOptions(array('timeout' => 0.1))->post($this->base . 'delay/10', array(), $stream); + $stream->close(); + + Block\await($promise, $this->loop); + } + /** * @group online * @doesNotPerformAssertions diff --git a/tests/Io/TransactionTest.php b/tests/Io/TransactionTest.php index 43876ca..c3f6885 100644 --- a/tests/Io/TransactionTest.php +++ b/tests/Io/TransactionTest.php @@ -207,6 +207,110 @@ public function testTimeoutExplicitNegativeWillNotStartTimeoutTimer() $this->assertInstanceOf('React\Promise\PromiseInterface', $promise); } + public function testTimeoutExplicitOptionWillNotStartTimeoutTimerWhenRequestBodyIsStreaming() + { + $messageFactory = new MessageFactory(); + + $loop = $this->getMockBuilder('React\EventLoop\LoopInterface')->getMock(); + $loop->expects($this->never())->method('addTimer'); + + $stream = new ThroughStream(); + $request = $messageFactory->request('POST', 'http://example.com', array(), $stream); + + $sender = $this->getMockBuilder('Clue\React\Buzz\Io\Sender')->disableOriginalConstructor()->getMock(); + $sender->expects($this->once())->method('send')->with($this->equalTo($request))->willReturn(new \React\Promise\Promise(function () { })); + + $transaction = new Transaction($sender, $messageFactory, $loop); + $transaction = $transaction->withOptions(array('timeout' => 2)); + $promise = $transaction->send($request); + + $this->assertInstanceOf('React\Promise\PromiseInterface', $promise); + } + + public function testTimeoutExplicitOptionWillStartTimeoutTimerWhenStreamingRequestBodyIsAlreadyClosed() + { + $messageFactory = new MessageFactory(); + + $timer = $this->getMockBuilder('React\EventLoop\TimerInterface')->getMock(); + $loop = $this->getMockBuilder('React\EventLoop\LoopInterface')->getMock(); + $loop->expects($this->once())->method('addTimer')->with(2, $this->anything())->willReturn($timer); + $loop->expects($this->never())->method('cancelTimer'); + + $stream = new ThroughStream(); + $stream->close(); + $request = $messageFactory->request('POST', 'http://example.com', array(), $stream); + + $sender = $this->getMockBuilder('Clue\React\Buzz\Io\Sender')->disableOriginalConstructor()->getMock(); + $sender->expects($this->once())->method('send')->with($this->equalTo($request))->willReturn(new \React\Promise\Promise(function () { })); + + $transaction = new Transaction($sender, $messageFactory, $loop); + $transaction = $transaction->withOptions(array('timeout' => 2)); + $promise = $transaction->send($request); + + $this->assertInstanceOf('React\Promise\PromiseInterface', $promise); + } + + public function testTimeoutExplicitOptionWillStartTimeoutTimerWhenStreamingRequestBodyCloses() + { + $messageFactory = new MessageFactory(); + + $timer = $this->getMockBuilder('React\EventLoop\TimerInterface')->getMock(); + $loop = $this->getMockBuilder('React\EventLoop\LoopInterface')->getMock(); + $loop->expects($this->once())->method('addTimer')->with(2, $this->anything())->willReturn($timer); + $loop->expects($this->never())->method('cancelTimer'); + + $stream = new ThroughStream(); + $request = $messageFactory->request('POST', 'http://example.com', array(), $stream); + + $sender = $this->getMockBuilder('Clue\React\Buzz\Io\Sender')->disableOriginalConstructor()->getMock(); + $sender->expects($this->once())->method('send')->with($this->equalTo($request))->willReturn(new \React\Promise\Promise(function () { })); + + $transaction = new Transaction($sender, $messageFactory, $loop); + $transaction = $transaction->withOptions(array('timeout' => 2)); + $promise = $transaction->send($request); + + $stream->close(); + + $this->assertInstanceOf('React\Promise\PromiseInterface', $promise); + } + + public function testTimeoutExplicitOptionWillRejectWhenTimerFiresAfterStreamingRequestBodyCloses() + { + $messageFactory = new MessageFactory(); + + $timeout = null; + $timer = $this->getMockBuilder('React\EventLoop\TimerInterface')->getMock(); + $loop = $this->getMockBuilder('React\EventLoop\LoopInterface')->getMock(); + $loop->expects($this->once())->method('addTimer')->with(2, $this->callback(function ($cb) use (&$timeout) { + $timeout = $cb; + return true; + }))->willReturn($timer); + $loop->expects($this->once())->method('cancelTimer')->with($timer); + + $stream = new ThroughStream(); + $request = $messageFactory->request('POST', 'http://example.com', array(), $stream); + + $sender = $this->getMockBuilder('Clue\React\Buzz\Io\Sender')->disableOriginalConstructor()->getMock(); + $sender->expects($this->once())->method('send')->with($this->equalTo($request))->willReturn(new \React\Promise\Promise(function () { })); + + $transaction = new Transaction($sender, $messageFactory, $loop); + $transaction = $transaction->withOptions(array('timeout' => 2)); + $promise = $transaction->send($request); + + $stream->close(); + + $this->assertNotNull($timeout); + $timeout(); + + $exception = null; + $promise->then(null, function ($e) use (&$exception) { + $exception = $e; + }); + + $this->assertInstanceOf('RuntimeException', $exception); + $this->assertEquals('Request timed out after 2 seconds', $exception->getMessage()); + } + public function testReceivingErrorResponseWillRejectWithResponseException() { $request = $this->getMockBuilder('Psr\Http\Message\RequestInterface')->getMock();