Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enforce throwables/exceptions as rejection reasons #93

Merged
merged 12 commits into from
Apr 18, 2019
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
42 changes: 13 additions & 29 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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()

Expand Down Expand Up @@ -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.
Expand All @@ -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`.
Expand Down Expand Up @@ -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 {
Expand All @@ -520,7 +509,7 @@ getAwesomeResultPromise()
function ($value) {
// Deferred resolved, do something with $value
},
function ($reason) {
function (\Exception $reason) {
// Deferred rejected, do something with $reason
}
);
Expand Down Expand Up @@ -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
-------

Expand Down
2 changes: 1 addition & 1 deletion src/Deferred.php
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand Down
30 changes: 30 additions & 0 deletions src/Exception/CompositeException.php
Original file line number Diff line number Diff line change
@@ -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
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about MultiReasonException?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No objections to either class name, but the use of this class appears a bit unclear. Does it make sense to add documentation for this class and/or mark this class a @internal?

Copy link
Member Author

@jsor jsor Sep 21, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added documentation in 07b7afd. I think it makes sense to keep it public API as it might be useful for custom collection functions.

{
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;
}
}
4 changes: 2 additions & 2 deletions src/Promise.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -198,7 +198,7 @@ private function call(callable $callback)
function ($value = null) {
$this->resolve($value);
},
function ($reason = null) {
function ($reason) {
$this->reject($reason);
}
);
Expand Down
20 changes: 11 additions & 9 deletions src/RejectedPromise.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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 {
Expand All @@ -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) {
Expand Down
31 changes: 0 additions & 31 deletions src/UnhandledRejectionException.php

This file was deleted.

20 changes: 11 additions & 9 deletions src/functions.php
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

namespace React\Promise;

use React\Promise\Exception\CompositeException;

/**
* Creates a promise for the supplied `$promiseOrValue`.
*
Expand All @@ -16,6 +18,7 @@
* @param mixed $promiseOrValue
* @return PromiseInterface
*/

function resolve($promiseOrValue = null)
{
if ($promiseOrValue instanceof PromiseInterface) {
Expand Down Expand Up @@ -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);
}

/**
Expand Down Expand Up @@ -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.'
)
);
}
};

Expand Down
7 changes: 5 additions & 2 deletions tests/FunctionAllTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand Down
19 changes: 16 additions & 3 deletions tests/FunctionAnyTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

namespace React\Promise;

use React\Promise\Exception\CompositeException;
use React\Promise\Exception\LengthException;

class FunctionAnyTest extends TestCase
Expand Down Expand Up @@ -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);
}

Expand Down
7 changes: 5 additions & 2 deletions tests/FunctionMapTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
Loading