Merge pull request #114 from clue-labs/timeouts

Add HTTP timeout option

Author: clue

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,15 +494,16 @@ can be controlled via the following API (and their defaults):
 
 ```php
 $newBrowser = $browser->withOptions(array(
+    'timeout' => null,
     'followRedirects' => true,
     'maxRedirects' => 10,
     'obeySuccessCode' => true,
     'streaming' => false,
 ));
 ```
 
-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

composer.json

@@ -20,6 +20,7 @@
         "react/http-client": "^0.5.8",
         "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"

src/Browser.php

@@ -19,6 +19,9 @@ class Browser
     private $messageFactory;
     private $baseUri = null;
 
+    /** @var LoopInterface $loop */
+    private $loop;
+
     /**
      * The `Browser` is responsible for sending HTTP requests to your HTTP server
      * and keeps track of pending incoming HTTP responses.
@@ -58,7 +61,8 @@ public function __construct(LoopInterface $loop, ConnectorInterface $connector =
         $this->messageFactory = new MessageFactory();
         $this->transaction = new Transaction(
             Sender::createFromLoop($loop, $connector, $this->messageFactory),
-            $this->messageFactory
+            $this->messageFactory,
+            $loop
         );
     }
 
@@ -249,15 +253,16 @@ public function withoutBase()
      *
      * ```php
      * $newBrowser = $browser->withOptions(array(
+     *     'timeout' => null,
      *     'followRedirects' => true,
      *     'maxRedirects' => 10,
      *     'obeySuccessCode' => true,
      *     'streaming' => false,
      * ));
      * ```
      *
-     * 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

src/Io/Transaction.php

@@ -7,8 +7,10 @@
 use Psr\Http\Message\RequestInterface;
 use Psr\Http\Message\ResponseInterface;
 use Psr\Http\Message\UriInterface;
+use React\EventLoop\LoopInterface;
 use React\Promise\Deferred;
 use React\Promise\PromiseInterface;
+use React\Promise\Timer\TimeoutException;
 use React\Stream\ReadableStreamInterface;
 
 /**
@@ -18,22 +20,27 @@ class Transaction
 {
     private $sender;
     private $messageFactory;
+    private $loop;
 
-    // context: http.follow_location
+    // context: http.timeout (ini_get('default_socket_timeout'): 60)
+    private $timeout;
+
+    // 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;
 
-    public function __construct(Sender $sender, MessageFactory $messageFactory)
+    public function __construct(Sender $sender, MessageFactory $messageFactory, LoopInterface $loop)
     {
         $this->sender = $sender;
         $this->messageFactory = $messageFactory;
+        $this->loop = $loop;
     }
 
     /**
@@ -47,7 +54,7 @@ public function withOptions(array $options)
             if (property_exists($transaction, $name)) {
                 // restore default value if null is given
                 if ($value === null) {
-                    $default = new self($this->sender, $this->messageFactory);
+                    $default = new self($this->sender, $this->messageFactory, $this->loop);
                     $value = $default->$name;
                 }
 
@@ -74,7 +81,20 @@ public function send(RequestInterface $request)
             array($deferred, 'reject')
         );
 
-        return $deferred->promise();
+        // 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)

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