Merge pull request #54 from clue-labs/cancellation-v3

[3.x] Consistent cancellation semantics for `coroutine()`

Author: WyriHaximus

src/functions.php

@@ -233,12 +233,10 @@ function coroutine(callable $function, ...$args): PromiseInterface
 
     $promise = null;
     $deferred = new Deferred(function () use (&$promise) {
-        // cancel pending promise(s) as long as generator function keeps yielding
-        while ($promise instanceof PromiseInterface && \method_exists($promise, 'cancel')) {
-            $temp = $promise;
-            $promise = null;
-            $temp->cancel();
+        if ($promise instanceof PromiseInterface && \method_exists($promise, 'cancel')) {
+            $promise->cancel();
         }
+        $promise = null;
     });
 
     /** @var callable $next */

tests/CoroutineTest.php

@@ -106,42 +106,53 @@ public function testCoroutineReturnsRejectedPromiseIfFunctionYieldsInvalidValue(
         $promise->then(null, $this->expectCallableOnceWith(new \UnexpectedValueException('Expected coroutine to yield React\Promise\PromiseInterface, but got integer')));
     }
 
-
-    public function testCoroutineWillCancelPendingPromiseWhenCallingCancelOnResultingPromise()
+    public function testCancelCoroutineWillReturnRejectedPromiseWhenCancellingPendingPromiseRejects()
     {
-        $cancelled = 0;
-        $promise = coroutine(function () use (&$cancelled) {
-            yield new Promise(function () use (&$cancelled) {
-                ++$cancelled;
+        $promise = coroutine(function () {
+            yield new Promise(function () { }, function () {
+                throw new \RuntimeException('Operation cancelled');
             });
         });
 
         $promise->cancel();
 
-        $this->assertEquals(1, $cancelled);
+        $promise->then(null, $this->expectCallableOnceWith(new \RuntimeException('Operation cancelled')));
     }
 
-    public function testCoroutineWillCancelAllPendingPromisesWhenFunctionContinuesToYieldWhenCallingCancelOnResultingPromise()
+    public function testCancelCoroutineWillReturnFulfilledPromiseWhenCancellingPendingPromiseRejectsInsideCatchThatReturnsValue()
     {
         $promise = coroutine(function () {
-            $promise = new Promise(function () { }, function () {
-                throw new \RuntimeException('Frist operation cancelled', 21);
-            });
-
             try {
-                yield $promise;
+                yield new Promise(function () { }, function () {
+                    throw new \RuntimeException('Operation cancelled');
+                });
             } catch (\RuntimeException $e) {
-                // ignore exception and continue
+                return 42;
             }
+        });
 
-            yield new Promise(function () { }, function () {
-                throw new \RuntimeException('Second operation cancelled', 42);
-            });
+        $promise->cancel();
+
+        $promise->then($this->expectCallableOnceWith(42));
+    }
+
+    public function testCancelCoroutineWillReturnPendigPromiseWhenCancellingFirstPromiseRejectsInsideCatchThatYieldsSecondPromise()
+    {
+        $promise = coroutine(function () {
+            try {
+                yield new Promise(function () { }, function () {
+                    throw new \RuntimeException('First operation cancelled');
+                });
+            } catch (\RuntimeException $e) {
+                yield new Promise(function () { }, function () {
+                    throw new \RuntimeException('Second operation never cancelled');
+                });
+            }
         });
 
         $promise->cancel();
 
-        $promise->then(null, $this->expectCallableOnceWith(new \RuntimeException('Second operation cancelled', 42)));
+        $promise->then($this->expectCallableNever(), $this->expectCallableNever());
     }
 
     public function testCoroutineShouldNotCreateAnyGarbageReferencesWhenGeneratorReturns()