diff --git a/README.md b/README.md index 4f43163..29846a9 100644 --- a/README.md +++ b/README.md @@ -35,6 +35,7 @@ mess with most of the low-level details. * [Methods](#methods) * [Promises](#promises) * [Cancellation](#cancellation) + * [Timeouts](#timeouts) * [Authentication](#authentication) * [Redirects](#redirects) * [Blocking](#blocking) @@ -172,6 +173,42 @@ $loop->addTimer(2.0, function () use ($promise) { }); ``` +#### Timeouts + +This library uses a very efficient HTTP implementation, so most HTTP requests +should usually be completed in mere milliseconds. However, when sending HTTP +requests over an unreliable network (the internet), there are a number of things +that can go wrong and may cause the request to fail after a time. As such, this +library respects PHP's `default_socket_timeout` setting (default 60s) as a timeout +for sending the outgoing HTTP request and waiting for a successful response and +will otherwise cancel the pending request and reject its value with an Exception. + +Note that this timeout value covers creating the underlying transport connection, +sending the HTTP request, receiving the HTTP response headers and its full +response body and following any eventual [redirects](#redirects). See also +[redirects](#redirects) below to configure the number of redirects to follow (or +disable following redirects altogether) and also [streaming](#streaming) below +to not take receiving large response bodies into account for this timeout. + +You can use the [`timeout` option](#withoptions) to pass a custom timeout value +in seconds like this: + +```php +$browser = $browser->withOptions(array( + 'timeout' => 10.0 +)); + +$browser->get($uri)->then(function (ResponseInterface $response) { + // response received within 10 seconds maximum + var_dump($response->getHeaders()); +}); +``` + +Similarly, you can use a negative timeout value to not apply a timeout at all +or use a `null` value to restore the default handling. Note that the underlying +connection may still impose a different timeout value. See also +[`Browser`](#browser) above and [`withOptions()`](#withoptions) for more details. + #### Authentication This library supports [HTTP Basic Authentication](https://en.wikipedia.org/wiki/Basic_access_authentication) @@ -381,6 +418,12 @@ $body->read(); // throws BadMethodCallException $body->getContents(); // throws BadMethodCallException ``` +Note how [timeouts](#timeouts) apply slightly differently when using streaming. +In streaming mode, the timeout value covers creating the underlying transport +connection, sending the HTTP request, receiving the HTTP response headers and +following any eventual [redirects](#redirects). In particular, the timeout value +does not take receiving (possibly large) response bodies into account. + If you want to integrate the streaming response into a higher level API, then working with Promise objects that resolve with Stream objects is often inconvenient. Consider looking into also using [react/promise-stream](https://github.com/reactphp/promise-stream). @@ -451,6 +494,7 @@ can be controlled via the following API (and their defaults): ```php $newBrowser = $browser->withOptions(array( + 'timeout' => null, 'followRedirects' => true, 'maxRedirects' => 10, 'obeySuccessCode' => true, @@ -458,8 +502,8 @@ $newBrowser = $browser->withOptions(array( )); ``` -See also [redirects](#redirects) and [streaming](#streaming) for more -details. +See also [timeouts](#timeouts), [redirects](#redirects) and +[streaming](#streaming) for more details. Notice that the [`Browser`](#browser) is an immutable object, i.e. this method actually returns a *new* [`Browser`](#browser) instance with the diff --git a/src/Browser.php b/src/Browser.php index e0d3aeb..e4063b8 100644 --- a/src/Browser.php +++ b/src/Browser.php @@ -253,6 +253,7 @@ public function withoutBase() * * ```php * $newBrowser = $browser->withOptions(array( + * 'timeout' => null, * 'followRedirects' => true, * 'maxRedirects' => 10, * 'obeySuccessCode' => true, @@ -260,8 +261,8 @@ public function withoutBase() * )); * ``` * - * See also [redirects](#redirects) and [streaming](#streaming) for more - * details. + * See also [timeouts](#timeouts), [redirects](#redirects) and + * [streaming](#streaming) for more details. * * Notice that the [`Browser`](#browser) is an immutable object, i.e. this * method actually returns a *new* [`Browser`](#browser) instance with the diff --git a/src/Io/Transaction.php b/src/Io/Transaction.php index b200411..75fbd57 100644 --- a/src/Io/Transaction.php +++ b/src/Io/Transaction.php @@ -10,6 +10,7 @@ use React\EventLoop\LoopInterface; use React\Promise\Deferred; use React\Promise\PromiseInterface; +use React\Promise\Timer\TimeoutException; use React\Stream\ReadableStreamInterface; /** @@ -19,29 +20,24 @@ class Transaction { private $sender; private $messageFactory; + private $loop; + + // context: http.timeout (ini_get('default_socket_timeout'): 60) + private $timeout; - // context: http.follow_location + // context: http.follow_location (true) private $followRedirects = true; - // context: http.max_redirects + // context: http.max_redirects (10) private $maxRedirects = 10; - // context: http.ignore_errors + // context: http.ignore_errors (false) private $obeySuccessCode = true; private $streaming = false; - /** @var int $timeout */ - private $timeout; - - /** @var LoopInterface $loop */ - private $loop; - public function __construct(Sender $sender, MessageFactory $messageFactory, LoopInterface $loop) { - // In case the timeout hasn't been set through the options - $this->timeout = ini_get('default_socket_timeout'); - $this->sender = $sender; $this->messageFactory = $messageFactory; $this->loop = $loop; @@ -85,11 +81,20 @@ public function send(RequestInterface $request) array($deferred, 'reject') ); - return \React\Promise\Timer\timeout( - $deferred->promise(), - $this->timeout, - $this->loop - ); + // use timeout from options or default to PHP's default_socket_timeout (60) + $timeout = (float)($this->timeout !== null ? $this->timeout : ini_get("default_socket_timeout")); + if ($timeout < 0) { + 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' + ); + } + throw $e; + }); } private function next(RequestInterface $request, Deferred $deferred) diff --git a/tests/FunctionalBrowserTest.php b/tests/FunctionalBrowserTest.php index 68670f0..d4c027b 100644 --- a/tests/FunctionalBrowserTest.php +++ b/tests/FunctionalBrowserTest.php @@ -114,6 +114,27 @@ public function testCancelRedirectedRequestShouldReject() Block\await($promise, $this->loop); } + /** + * @expectedException RuntimeException + * @expectedExceptionMessage Request timed out after 0.1 seconds + * @group online + */ + public function testTimeoutDelayedResponseShouldReject() + { + $promise = $this->browser->withOptions(array('timeout' => 0.1))->get($this->base . 'delay/10'); + + Block\await($promise, $this->loop); + } + + /** + * @group online + * @doesNotPerformAssertions + */ + public function testTimeoutNegativeShouldResolveSuccessfully() + { + Block\await($this->browser->withOptions(array('timeout' => -1))->get($this->base . 'get'), $this->loop); + } + /** * @group online * @doesNotPerformAssertions diff --git a/tests/Io/TransactionTest.php b/tests/Io/TransactionTest.php index 3b505d9..3cbfa8c 100644 --- a/tests/Io/TransactionTest.php +++ b/tests/Io/TransactionTest.php @@ -441,7 +441,11 @@ public function testCancelTransactionShouldCancelSendingPromise() $promise->cancel(); } - public function testTimeoutPromiseWillThrowTimeoutException() + /** + * @expectedException RuntimeException + * @expectedExceptionMessage Request timed out after 0.001 seconds + */ + public function testTimeoutExplicitOptionWillThrowException() { $messageFactory = new MessageFactory(); $loop = Factory::create(); @@ -462,14 +466,64 @@ public function testTimeoutPromiseWillThrowTimeoutException() $transaction = $transaction->withOptions(array('timeout' => 0.001)); $promise = $transaction->send($request); - $exception = false; - try { - $response = Block\await($promise, $loop); - } catch (Exception $exception) { + 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->assertTrue(is_a($exception, 'React\Promise\Timer\TimeoutException')); + $this->assertInstanceOf('Psr\Http\Message\ResponseInterface', $response); } /**