Merge pull request #25 from clue-labs/pending

Only start timers if input Promise is still pending

Author: WyriHaximus

README.md

@@ -50,6 +50,14 @@ It returns a new `Promise` with the following resolution behavior:
 * If the input `$promise` rejects before `$time` seconds, reject the resulting promise with its rejection value.
 * If the input `$promise` does not settle before `$time` seconds, *cancel* the operation and reject the resulting promise with a [`TimeoutException`](#timeoutexception).
 
+Internally, the given `$time` value will be used to start a timer that will
+*cancel* the pending operation once it triggers.
+This implies that if you pass a really small (or negative) value, it will still
+start a timer and will thus trigger at the earliest possible time in the future.
+
+If the input `$promise` is already settled, then the resulting promise will
+resolve or reject immediately without starting a timer at all.
+
 A common use case for handling only resolved values looks like this:
 
 ```php
@@ -269,6 +277,11 @@ Timer\resolve(1.5, $loop)->then(function ($time) {
 });
 ```
 
+Internally, the given `$time` value will be used to start a timer that will
+resolve the promise once it triggers.
+This implies that if you pass a really small (or negative) value, it will still
+start a timer and will thus trigger at the earliest possible time in the future.
+
 #### Resolve cancellation
 
 You can explicitly `cancel()` the resulting timer promise at any time:
@@ -292,6 +305,11 @@ Timer\reject(2.0, $loop)->then(null, function (TimeoutException $e) {
 });
 ```
 
+Internally, the given `$time` value will be used to start a timer that will
+reject the promise once it triggers.
+This implies that if you pass a really small (or negative) value, it will still
+start a timer and will thus trigger at the earliest possible time in the future.
+
 This function complements the [`resolve()`](#resolve) function
 and can be used as a basic building block for higher-level promise consumers.
 

src/functions.php

@@ -17,21 +17,34 @@ function timeout(PromiseInterface $promise, $time, LoopInterface $loop)
     }
 
     return new Promise(function ($resolve, $reject) use ($loop, $time, $promise) {
+        $timer = null;
+        $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;
+        }
+
+        // start timeout timer which will cancel the input promise
         $timer = $loop->addTimer($time, function () use ($time, $promise, $reject) {
             $reject(new TimeoutException($time, 'Timed out after ' . $time . ' seconds'));
 
             if ($promise instanceof CancellablePromiseInterface) {
                 $promise->cancel();
             }
         });
-
-        $promise->then(function ($v) use ($timer, $loop, $resolve) {
-            $loop->cancelTimer($timer);
-            $resolve($v);
-        }, function ($v) use ($timer, $loop, $reject) {
-            $loop->cancelTimer($timer);
-            $reject($v);
-        });
     }, $canceller);
 }
 

tests/FunctionTimeoutTest.php

@@ -98,6 +98,30 @@ public function testCancelTimeoutWithoutCancellationhandlerWillNotCancelTimerAnd
         $this->expectPromisePending($timeout);
     }
 
+    public function testResolvedPromiseWillNotStartTimer()
+    {
+        $promise = new \React\Promise\Promise(function ($resolve) { $resolve(true); });
+
+        $loop = $this->getMockBuilder('React\EventLoop\LoopInterface')->getMock();
+        $loop->expects($this->never())->method('addTimer');
+
+        $timeout = Timer\timeout($promise, 0.01, $loop);
+
+        $this->expectPromiseResolved($timeout);
+    }
+
+    public function testRejectedPromiseWillNotStartTimer()
+    {
+        $promise = Promise\reject(new \RuntimeException());
+
+        $loop = $this->getMockBuilder('React\EventLoop\LoopInterface')->getMock();
+        $loop->expects($this->never())->method('addTimer');
+
+        $timeout = Timer\timeout($promise, 0.01, $loop);
+
+        $this->expectPromiseRejected($timeout);
+    }
+
     public function testCancelTimeoutWillCancelGivenPromise()
     {
         $promise = new \React\Promise\Promise(function () { }, $this->expectCallableOnce());