Enforce throwables/exceptions as rejection reasons

reactphp · Mar 7, 2017 · 411a415 · 411a415
1 parent dffdcec
commit 411a415
Show file tree

Hide file tree

Showing 22 changed files with 238 additions and 280 deletions.
diff --git a/README.md b/README.md
@@ -94,7 +94,7 @@ $deferred = new React\Promise\Deferred();
 $promise = $deferred->promise();
 
 $deferred->resolve(mixed $value = null);
-$deferred->reject(mixed $reason = null);
+$deferred->reject(\Throwable|\Exception $reason);
 ```
 
 The `promise` method returns the promise of the deferred.
@@ -129,17 +129,14 @@ this promise once it is resolved.
 #### Deferred::reject()
 
 ```php
-$deferred->reject(mixed $reason = null);
+$deferred->reject(\Throwable|\Exception $reason);
 ```
 
 Rejects the promise returned by `promise()`, signalling that the deferred's
 computation failed.
 All consumers are notified by having `$onRejected` (which they registered via
 `$promise->then()`) called with `$reason`.
 
-If `$reason` itself is a promise, the promise will be rejected with the outcome
-of this promise regardless whether it fulfills or rejects.
-
 ### PromiseInterface
 
 The promise interface provides the common interface for all promise
@@ -358,8 +355,7 @@ Creates a already rejected promise.
 $promise = React\Promise\RejectedPromise($reason);
 ```
 
-Note, that `$reason` **cannot** be a promise. It's recommended to use
-[reject()](#reject) for creating rejected promises.
+Note, that `$reason` **must** be a `\Throwable` or `\Exception`.
 
 ### LazyPromise
 
@@ -410,20 +406,10 @@ If `$promiseOrValue` is a promise, it will be returned as is.
 #### reject()
 
 ```php
-$promise = React\Promise\reject(mixed $promiseOrValue);
+$promise = React\Promise\reject(\Throwable|\Exception $reason);
 ```
 
-Creates a rejected promise for the supplied `$promiseOrValue`.
-
-If `$promiseOrValue` is a value, it will be the rejection value of the
-returned promise.
-
-If `$promiseOrValue` is a promise, its completion value will be the rejected
-value of the returned promise.
-
-This can be useful in situations where you need to reject a promise without
-throwing an exception. For example, it allows you to propagate a rejection with
-the value of another promise.
+Creates a rejected promise for the supplied `$reason`.
 
 #### all()
 
@@ -523,7 +509,7 @@ function getAwesomeResultPromise()
     $deferred = new React\Promise\Deferred();
 
     // Execute a Node.js-style function using the callback pattern
-    computeAwesomeResultAsynchronously(function ($error, $result) use ($deferred) {
+    computeAwesomeResultAsynchronously(function (\Exception $error, $result) use ($deferred) {
         if ($error) {
             $deferred->reject($error);
         } else {
@@ -540,7 +526,7 @@ getAwesomeResultPromise()
         function ($value) {
             // Deferred resolved, do something with $value
         },
-        function ($reason) {
+        function (\Exception $reason) {
             // Deferred rejected, do something with $reason
         }
     );

diff --git a/src/Deferred.php b/src/Deferred.php
@@ -36,7 +36,7 @@ public function resolve($value = null)
         call_user_func($this->resolveCallback, $value);
     }
 
-    public function reject($reason = null)
+    public function reject($reason)
     {
         $this->promise();
 

diff --git a/src/Exception/CompositeException.php b/src/Exception/CompositeException.php
@@ -0,0 +1,31 @@
+<?php
+
+namespace React\Promise\Exception;
+
+class CompositeException extends \Exception
+{
+    private $exceptions;
+
+    public function __construct(array $exceptions, $message = '', $code = 0, $previous = null)
+    {
+        parent::__construct($message, $code, $previous);
+
+        $this->exceptions = $exceptions;
+    }
+
+    /**
+     * @return \Throwable[]|\Exception[]
+     */
+    public function getExceptions()
+    {
+        return $this->exceptions;
+    }
+
+    public static function tooManyPromisesRejected(array $reasons)
+    {
+        return new self(
+            $reasons,
+            'Too many promises rejected.'
+        );
+    }
+}
diff --git a/src/Exception/InvalidArgumentException.php b/src/Exception/InvalidArgumentException.php
@@ -0,0 +1,16 @@
+<?php
+
+namespace React\Promise\Exception;
+
+class InvalidArgumentException extends \InvalidArgumentException
+{
+    public static function invalidRejectionReason($reason)
+    {
+        return new self(
+            sprintf(
+                'A Promise must be rejected with a \Throwable or \Exception instance, got "%s" instead.',
+                is_object($reason) ? get_class($reason) : gettype($reason)
+            )
+        );
+    }
+}
diff --git a/src/Promise.php b/src/Promise.php
@@ -106,7 +106,7 @@ private function resolve($value = null)
         $this->settle(resolve($value));
     }
 
-    private function reject($reason = null)
+    private function reject($reason)
     {
         if (null !== $this->result) {
             return;
@@ -154,7 +154,7 @@ private function call(callable $callback)
                 function ($value = null) {
                     $this->resolve($value);
                 },
-                function ($reason = null) {
+                function ($reason) {
                     $this->reject($reason);
                 }
             );

diff --git a/src/RejectedPromise.php b/src/RejectedPromise.php
@@ -2,14 +2,16 @@
 
 namespace React\Promise;
 
+use React\Promise\Exception\InvalidArgumentException;
+
 final class RejectedPromise implements PromiseInterface
 {
     private $reason;
 
-    public function __construct($reason = null)
+    public function __construct($reason)
     {
-        if ($reason instanceof PromiseInterface) {
-            throw new \InvalidArgumentException('You cannot create React\Promise\RejectedPromise with a promise. Use React\Promise\reject($promiseOrValue) instead.');
+        if (!$reason instanceof \Throwable && !$reason instanceof \Exception) {
+            throw InvalidArgumentException::invalidRejectionReason($reason);
         }
 
         $this->reason = $reason;
@@ -38,13 +40,13 @@ public function done(callable $onFulfilled = null, callable $onRejected = null)
     {
         enqueue(function () use ($onRejected) {
             if (null === $onRejected) {
-                throw UnhandledRejectionException::resolve($this->reason);
+                throw $this->reason;
             }
 
             $result = $onRejected($this->reason);
 
             if ($result instanceof self) {
-                throw UnhandledRejectionException::resolve($result->reason);
+                throw $result->reason;
             }
 
             if ($result instanceof PromiseInterface) {

diff --git a/src/UnhandledRejectionException.php b/src/UnhandledRejectionException.php
diff --git a/src/functions.php b/src/functions.php
@@ -2,6 +2,8 @@
 
 namespace React\Promise;
 
+use React\Promise\Exception\CompositeException;
+
 function resolve($promiseOrValue = null)
 {
     if ($promiseOrValue instanceof PromiseInterface) {
@@ -23,15 +25,9 @@ function resolve($promiseOrValue = null)
     return new FulfilledPromise($promiseOrValue);
 }
 
-function reject($promiseOrValue = null)
+function reject($reason)
 {
-    if ($promiseOrValue instanceof PromiseInterface) {
-        return resolve($promiseOrValue)->then(function ($value) {
-            return new RejectedPromise($value);
-        });
-    }
-
-    return new RejectedPromise($promiseOrValue);
+    return new RejectedPromise($reason);
 }
 
 function all(array $promisesOrValues)
@@ -118,7 +114,9 @@ function some(array $promisesOrValues, $howMany)
                 $reasons[$i] = $reason;
 
                 if (0 === --$toReject) {
-                    $reject($reasons);
+                    $reject(
+                        CompositeException::tooManyPromisesRejected($reasons)
+                    );
                 }
             };
 
@@ -208,10 +206,6 @@ function enqueue(callable $task)
  */
 function _checkTypehint(callable $callback, $object)
 {
-    if (!is_object($object)) {
-        return true;
-    }
-
     if (is_array($callback)) {
         $callbackReflection = new \ReflectionMethod($callback[0], $callback[1]);
     } elseif (is_object($callback) && !$callback instanceof \Closure) {

diff --git a/tests/FunctionAllTest.php b/tests/FunctionAllTest.php
@@ -59,13 +59,16 @@ public function shouldResolveSparseArrayInput()
     /** @test */
     public function shouldRejectIfAnyInputPromiseRejects()
     {
+        $exception2 = new \Exception();
+        $exception3 = new \Exception();
+
         $mock = $this->createCallableMock();
         $mock
             ->expects($this->once())
             ->method('__invoke')
-            ->with($this->identicalTo(2));
+            ->with($this->identicalTo($exception2));
 
-        all([resolve(1), reject(2), resolve(3)])
+        all([resolve(1), reject($exception2), resolve($exception3)])
             ->then($this->expectCallableNever(), $mock);
     }
 

diff --git a/tests/FunctionAnyTest.php b/tests/FunctionAnyTest.php
@@ -2,6 +2,7 @@
 
 namespace React\Promise;
 
+use React\Promise\Exception\CompositeException;
 use React\Promise\Exception\LengthException;
 
 class FunctionAnyTest extends TestCase
@@ -53,26 +54,37 @@ public function shouldResolveWithAPromisedInputValue()
     /** @test */
     public function shouldRejectWithAllRejectedInputValuesIfAllInputsAreRejected()
     {
+        $exception1 = new \Exception();
+        $exception2 = new \Exception();
+        $exception3 = new \Exception();
+
+        $compositeException = CompositeException::tooManyPromisesRejected(
+            [0 => $exception1, 1 => $exception2, 2 => $exception3]
+        );
+
         $mock = $this->createCallableMock();
         $mock
             ->expects($this->once())
             ->method('__invoke')
-            ->with($this->identicalTo([0 => 1, 1 => 2, 2 => 3]));
+            ->with($compositeException);
 
-        any([reject(1), reject(2), reject(3)])
+        any([reject($exception1), reject($exception2), reject($exception3)])
             ->then($this->expectCallableNever(), $mock);
     }
 
     /** @test */
     public function shouldResolveWhenFirstInputPromiseResolves()
     {
+        $exception2 = new \Exception();
+        $exception3 = new \Exception();
+
         $mock = $this->createCallableMock();
         $mock
             ->expects($this->once())
             ->method('__invoke')
             ->with($this->identicalTo(1));
 
-        any([resolve(1), reject(2), reject(3)])
+        any([resolve(1), reject($exception2), reject($exception3)])
             ->then($mock);
     }
 

diff --git a/tests/FunctionMapTest.php b/tests/FunctionMapTest.php
@@ -100,14 +100,17 @@ public function shouldPreserveTheOrderOfArrayWhenResolvingAsyncPromises()
     /** @test */
     public function shouldRejectWhenInputContainsRejection()
     {
+        $exception2 = new \Exception();
+        $exception3 = new \Exception();
+
         $mock = $this->createCallableMock();
         $mock
             ->expects($this->once())
             ->method('__invoke')
-            ->with($this->identicalTo(2));
+            ->with($this->identicalTo($exception2));
 
         map(
-            [resolve(1), reject(2), resolve(3)],
+            [resolve(1), reject($exception2), resolve($exception3)],
             $this->mapper()
         )->then($this->expectCallableNever(), $mock);
     }

diff --git a/tests/FunctionRaceTest.php b/tests/FunctionRaceTest.php
@@ -66,11 +66,13 @@ public function shouldResolveSparseArrayInput()
     /** @test */
     public function shouldRejectIfFirstSettledPromiseRejects()
     {
+        $exception = new \Exception();
+
         $mock = $this->createCallableMock();
         $mock
             ->expects($this->once())
             ->method('__invoke')
-            ->with($this->identicalTo(2));
+            ->with($this->identicalTo($exception));
 
         $d1 = new Deferred();
         $d2 = new Deferred();
@@ -80,7 +82,7 @@ public function shouldRejectIfFirstSettledPromiseRejects()
             [$d1->promise(), $d2->promise(), $d3->promise()]
         )->then($this->expectCallableNever(), $mock);
 
-        $d2->reject(2);
+        $d2->reject($exception);
 
         $d1->resolve(1);
         $d3->resolve(3);
@@ -136,7 +138,7 @@ public function shouldNotCancelOtherPendingInputArrayPromisesIfOnePromiseRejects
             ->method('__invoke');
 
         $deferred = New Deferred($mock);
-        $deferred->reject();
+        $deferred->reject(new \Exception());
 
         $mock2 = $this
             ->getMockBuilder('React\Promise\PromiseInterface')