Merge pull request #86 from clue-labs/timeout

Support connection timeouts

Author: clue

README.md

@@ -186,6 +186,15 @@ $factory->createClient('redis+unix:///tmp/redis.sock?password=secret&db=2');
 $factory->createClient('redis+unix://:secret@/tmp/redis.sock');
 ```
 
+This method respects PHP's `default_socket_timeout` setting (default 60s)
+as a timeout for establishing the connection and waiting for successful
+authentication. You can explicitly pass a custom timeout value in seconds
+(or use a negative number to not apply a timeout) like this:
+
+```php
+$factory->createClient('localhost?timeout=0.5');
+```
+
 ### Client
 
 The `Client` is responsible for exchanging messages with Redis

composer.json

@@ -16,6 +16,7 @@
         "evenement/evenement": "^3.0 || ^2.0 || ^1.0",
         "react/event-loop": "^1.0 || ^0.5 || ^0.4 || ^0.3",
         "react/promise": "^2.0 || ^1.1",
+        "react/promise-timer": "^1.5",
         "react/socket": "^1.1"
     },
     "autoload": {

src/Factory.php

@@ -4,15 +4,16 @@
 
 use Clue\Redis\Protocol\Factory as ProtocolFactory;
 use React\EventLoop\LoopInterface;
-use React\Promise;
 use React\Promise\Deferred;
+use React\Promise\Timer\TimeoutException;
 use React\Socket\ConnectionInterface;
 use React\Socket\Connector;
 use React\Socket\ConnectorInterface;
 use InvalidArgumentException;
 
 class Factory
 {
+    private $loop;
     private $connector;
     private $protocol;
 
@@ -32,6 +33,7 @@ public function __construct(LoopInterface $loop, ConnectorInterface $connector =
             $protocol = new ProtocolFactory();
         }
 
+        $this->loop = $loop;
         $this->connector = $connector;
         $this->protocol = $protocol;
     }
@@ -47,7 +49,7 @@ public function createClient($target)
         try {
             $parts = $this->parseUrl($target);
         } catch (InvalidArgumentException $e) {
-            return Promise\reject($e);
+            return \React\Promise\reject($e);
         }
 
         $connecting = $this->connector->connect($parts['authority']);
@@ -97,7 +99,20 @@ function ($error) use ($client) {
 
         $promise->then(array($deferred, 'resolve'), array($deferred, 'reject'));
 
-        return $deferred->promise();
+        // use timeout from explicit ?timeout=x parameter or default to PHP's default_socket_timeout (60)
+        $timeout = (float) isset($parts['timeout']) ? $parts['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(
+                    'Connection to database server timed out after ' . $e->getTimeout() . ' seconds'
+                );
+            }
+            throw $e;
+        });
     }
 
     /**
@@ -150,6 +165,10 @@ private function parseUrl($target)
             if (isset($args['db'])) {
                 $ret['db'] = $args['db'];
             }
+
+            if (isset($args['timeout'])) {
+                $ret['timeout'] = $args['timeout'];
+            }
         }
 
         return $ret;

tests/FactoryTest.php

@@ -186,4 +186,53 @@ public function testCancelWillCloseConnectionWhenConnectionWaitsForSelect()
         $promise = $this->factory->createClient('redis://127.0.0.1:2/123');
         $promise->cancel();
     }
+
+    public function testCreateClientWithTimeoutParameterWillStartTimerAndRejectOnExplicitTimeout()
+    {
+        $timeout = null;
+        $this->loop->expects($this->once())->method('addTimer')->with(0, $this->callback(function ($cb) use (&$timeout) {
+            $timeout = $cb;
+            return true;
+        }));
+
+        $deferred = new Deferred();
+        $this->connector->expects($this->once())->method('connect')->with('127.0.0.1:2')->willReturn($deferred->promise());
+
+        $promise = $this->factory->createClient('redis://127.0.0.1:2?timeout=0');
+
+        $this->assertNotNull($timeout);
+        $timeout();
+
+        $promise->then(null, $this->expectCallableOnceWith(
+            $this->logicalAnd(
+                $this->isInstanceOf('Exception'),
+                $this->callback(function (\Exception $e) {
+                    return $e->getMessage() === 'Connection to database server timed out after 0 seconds';
+                })
+            )
+        ));
+    }
+
+    public function testCreateClientWithNegativeTimeoutParameterWillNotStartTimer()
+    {
+        $this->loop->expects($this->never())->method('addTimer');
+
+        $deferred = new Deferred();
+        $this->connector->expects($this->once())->method('connect')->with('127.0.0.1:2')->willReturn($deferred->promise());
+
+        $this->factory->createClient('redis://127.0.0.1:2?timeout=-1');
+    }
+
+    public function testCreateClientWithoutTimeoutParameterWillStartTimerWithDefaultTimeoutFromIni()
+    {
+        $this->loop->expects($this->once())->method('addTimer')->with(1.5, $this->anything());
+
+        $deferred = new Deferred();
+        $this->connector->expects($this->once())->method('connect')->with('127.0.0.1:2')->willReturn($deferred->promise());
+
+        $old = ini_get('default_socket_timeout');
+        ini_set('default_socket_timeout', '1.5');
+        $this->factory->createClient('redis://127.0.0.1:2');
+        ini_set('default_socket_timeout', $old);
+    }
 }