Merge pull request #93 from reactphp/enforce-exception-reasons

Enforce throwables/exceptions as rejection reasons

Author: WyriHaximus

CHANGELOG.md

@@ -44,3 +44,7 @@ CHANGELOG for 3.x
           \React\Promise\resolve($promise)->cancel();
       }
       ```
+    * BC break: When rejecting a promise, a `\Throwable` or `\Exception`
+      instance is enforced as the rejection reason (#93).
+      This means, it is not longer possible to reject a promise without a reason
+      or with another promise.

README.md

@@ -93,7 +93,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.
@@ -128,17 +128,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
@@ -359,8 +356,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`.
 
 ### Functions
 
@@ -390,20 +386,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()
 
@@ -439,7 +425,9 @@ Returns a promise that will resolve when any one of the items in
 will be the resolution value of the triggering item.
 
 The returned promise will only reject if *all* items in `$promisesOrValues` are
-rejected. The rejection value will be an array of all rejection reasons.
+rejected. The rejection value will be a `React\Promise\Exception\CompositeException`
+which holds all rejection reasons. The rejection reasons can be obtained with
+`CompositeException::getExceptions()`.
 
 The returned promise will also reject with a `React\Promise\Exception\LengthException`
 if `$promisesOrValues` contains 0 items.
@@ -457,8 +445,9 @@ triggering items.
 
 The returned promise will reject if it becomes impossible for `$howMany` items
 to resolve (that is, when `(count($promisesOrValues) - $howMany) + 1` items
-reject). The rejection value will be an array of
-`(count($promisesOrValues) - $howMany) + 1` rejection reasons.
+reject). The rejection value will be a `React\Promise\Exception\CompositeException`
+which holds `(count($promisesOrValues) - $howMany) + 1` rejection reasons.
+The rejection reasons can be obtained with `CompositeException::getExceptions()`.
 
 The returned promise will also reject with a `React\Promise\Exception\LengthException`
 if `$promisesOrValues` contains less items than `$howMany`.
@@ -503,7 +492,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 {
@@ -520,7 +509,7 @@ getAwesomeResultPromise()
         function ($value) {
             // Deferred resolved, do something with $value
         },
-        function ($reason) {
+        function (\Exception $reason) {
             // Deferred rejected, do something with $reason
         }
     );
@@ -701,11 +690,6 @@ getJsonResult()
     );
 ```
 
-Note that if a rejection value is not an instance of `\Exception`, it will be
-wrapped in an exception of the type `React\Promise\UnhandledRejectionException`.
-
-You can get the original rejection reason by calling `$exception->getReason()`.
-
 Credits
 -------
 

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();
 

src/Exception/CompositeException.php

@@ -0,0 +1,30 @@
+<?php
+
+namespace React\Promise\Exception;
+
+/**
+ * Represents an exception that is a composite of one or more other exceptions.
+ *
+ * This exception is useful in situations where a promise must be rejected
+ * with multiple exceptions. It is used for example to reject the returned
+ * promise from `some()` and `any()` when too many input promises reject.
+ */
+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;
+    }
+}

src/Promise.php

@@ -129,7 +129,7 @@ private function resolve($value = null)
         $this->settle(resolve($value));
     }
 
-    private function reject($reason = null)
+    private function reject($reason)
     {
         if (null !== $this->result) {
             return;
@@ -198,7 +198,7 @@ private function call(callable $callback)
                     function ($value = null) {
                         $this->resolve($value);
                     },
-                    function ($reason = null) {
+                    function ($reason) {
                         $this->reject($reason);
                     }
                 );

src/RejectedPromise.php

@@ -6,10 +6,16 @@ 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 new \InvalidArgumentException(
+                sprintf(
+                    'A Promise must be rejected with a \Throwable or \Exception instance, got "%s" instead.',
+                    is_object($reason) ? get_class($reason) : gettype($reason)
+                )
+
+            );
         }
 
         $this->reason = $reason;
@@ -38,9 +44,7 @@ public function done(callable $onFulfilled = null, callable $onRejected = null)
     {
         enqueue(function () use ($onRejected) {
             if (null === $onRejected) {
-                return fatalError(
-                    UnhandledRejectionException::resolve($this->reason)
-                );
+                return fatalError($this->reason);
             }
 
             try {
@@ -52,9 +56,7 @@ public function done(callable $onFulfilled = null, callable $onRejected = null)
             }
 
             if ($result instanceof self) {
-                return fatalError(
-                    UnhandledRejectionException::resolve($result->reason)
-                );
+                return fatalError($result->reason);
             }
 
             if ($result instanceof PromiseInterface) {

src/UnhandledRejectionException.php

@@ -2,6 +2,8 @@
 
 namespace React\Promise;
 
+use React\Promise\Exception\CompositeException;
+
 /**
  * Creates a promise for the supplied `$promiseOrValue`.
  *
@@ -16,6 +18,7 @@
  * @param mixed $promiseOrValue
  * @return PromiseInterface
  */
+
 function resolve($promiseOrValue = null)
 {
     if ($promiseOrValue instanceof PromiseInterface) {
@@ -53,15 +56,9 @@ function resolve($promiseOrValue = null)
  * @param mixed $promiseOrValue
  * @return PromiseInterface
  */
-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);
 }
 
 /**
@@ -199,7 +196,12 @@ function some(array $promisesOrValues, $howMany)
                 $reasons[$i] = $reason;
 
                 if (0 === --$toReject) {
-                    $reject($reasons);
+                    $reject(
+                        new CompositeException(
+                            $reasons,
+                            'Too many promises rejected.'
+                        )
+                    );
                 }
             };
 

src/functions.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);
     }
 

tests/FunctionAllTest.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,38 @@ public function shouldResolveWithAPromisedInputValue()
     /** @test */
     public function shouldRejectWithAllRejectedInputValuesIfAllInputsAreRejected()
     {
+        $exception1 = new \Exception();
+        $exception2 = new \Exception();
+        $exception3 = new \Exception();
+
+        $compositeException = new CompositeException(
+            [0 => $exception1, 1 => $exception2, 2 => $exception3],
+            'Too many promises rejected.'
+        );
+
         $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);
     }