From 7058eb2fe61fa251b8c1aa38efb5921ace958d94 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Christian=20L=C3=BCck?= <christian@clue.engineering>
Date: Thu, 2 Jun 2022 15:15:16 +0200
Subject: [PATCH] Consistent cancellation semantics for `coroutine()`

---
 src/functions.php       |  8 +++----
 tests/CoroutineTest.php | 47 +++++++++++++++++++++++++----------------
 2 files changed, 32 insertions(+), 23 deletions(-)

diff --git a/src/functions.php b/src/functions.php
index 95c0bee..122c656 100644
--- a/src/functions.php
+++ b/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 */
diff --git a/tests/CoroutineTest.php b/tests/CoroutineTest.php
index 6e461d5..adc82bc 100644
--- a/tests/CoroutineTest.php
+++ b/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()