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
28 changes: 7 additions & 21 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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

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

Expand Down Expand Up @@ -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 {
Expand All @@ -540,7 +526,7 @@ getAwesomeResultPromise()
function ($value) {
// Deferred resolved, do something with $value
},
function ($reason) {
function (\Exception $reason) {
// Deferred rejected, do something with $reason
}
);
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
31 changes: 31 additions & 0 deletions src/Exception/CompositeException.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
<?php

namespace React\Promise\Exception;

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;
}

public static function tooManyPromisesRejected(array $reasons)
jsor marked this conversation as resolved.
Show resolved Hide resolved
{
return new self(
$reasons,
'Too many promises rejected.'
);
}
}
16 changes: 16 additions & 0 deletions src/Exception/InvalidArgumentException.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
<?php

namespace React\Promise\Exception;

class InvalidArgumentException extends \InvalidArgumentException
jsor marked this conversation as resolved.
Show resolved Hide resolved
{
public static function invalidRejectionReason($reason)
jsor marked this conversation as resolved.
Show resolved Hide resolved
{
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)
)
);
}
}
4 changes: 2 additions & 2 deletions src/Promise.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -154,7 +154,7 @@ private function call(callable $callback)
function ($value = null) {
$this->resolve($value);
},
function ($reason = null) {
function ($reason) {
$this->reject($reason);
}
);
Expand Down
12 changes: 7 additions & 5 deletions src/RejectedPromise.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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) {
Expand Down
31 changes: 0 additions & 31 deletions src/UnhandledRejectionException.php

This file was deleted.

20 changes: 7 additions & 13 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;

function resolve($promiseOrValue = null)
{
if ($promiseOrValue instanceof PromiseInterface) {
Expand All @@ -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)
Expand Down Expand Up @@ -118,7 +114,9 @@ function some(array $promisesOrValues, $howMany)
$reasons[$i] = $reason;

if (0 === --$toReject) {
$reject($reasons);
$reject(
CompositeException::tooManyPromisesRejected($reasons)
);
}
};

Expand Down Expand Up @@ -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) {
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
18 changes: 15 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,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);
}

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
8 changes: 5 additions & 3 deletions tests/FunctionRaceTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -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);
Expand Down Expand Up @@ -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')
Expand Down
Loading