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.

Sign up for GitHub

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

Trigger an E_USER_ERROR instead of throwing an exception from done() #97

Merged
merged 1 commit into from
Sep 19, 2017
Merged

Trigger an E_USER_ERROR instead of throwing an exception from done() #97

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -201,8 +201,8 @@ $promise->done(callable $onFulfilled = null, callable $onRejected = null);
Consumes the promise's ultimate value if the promise fulfills, or handles the
ultimate error.

It will cause a fatal error if either `$onFulfilled` or `$onRejected` throw or
return a rejected promise.
It will cause a fatal error (`E_USER_ERROR`) if either `$onFulfilled` or
`$onRejected` throw or return a rejected promise.

Since the purpose of `done()` is consumption rather than transformation,
`done()` always returns `null`.
Expand Down Expand Up @@ -651,8 +651,8 @@ by the promise machinery and used to reject the promise returned by `then()`.

Calling `done()` transfers all responsibility for errors to your code. If an
error (either a thrown exception or returned rejection) escapes the
`$onFulfilled` or `$onRejected` callbacks you provide to done, it will be
rethrown in an uncatchable way causing a fatal error.
`$onFulfilled` or `$onRejected` callbacks you provide to `done()`, it will cause
a fatal error.

```php
function getJsonResult()
Expand Down
8 changes: 7 additions & 1 deletion src/FulfilledPromise.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,13 @@ public function done(callable $onFulfilled = null, callable $onRejected = null)
}

enqueue(function () use ($onFulfilled) {
$result = $onFulfilled($this->value);
try {
$result = $onFulfilled($this->value);
} catch (\Throwable $exception) {
return fatalError($exception);
} catch (\Exception $exception) {
return fatalError($exception);
}

if ($result instanceof PromiseInterface) {
$result->done();
Expand Down
16 changes: 13 additions & 3 deletions src/RejectedPromise.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,13 +38,23 @@ public function done(callable $onFulfilled = null, callable $onRejected = null)
{
enqueue(function () use ($onRejected) {
if (null === $onRejected) {
throw UnhandledRejectionException::resolve($this->reason);
return fatalError(
UnhandledRejectionException::resolve($this->reason)
);
}

$result = $onRejected($this->reason);
try {
$result = $onRejected($this->reason);
} catch (\Throwable $exception) {
return fatalError($exception);
} catch (\Exception $exception) {
return fatalError($exception);
}

if ($result instanceof self) {
throw UnhandledRejectionException::resolve($result->reason);
return fatalError(
UnhandledRejectionException::resolve($result->reason)
);
}

if ($result instanceof PromiseInterface) {
Expand Down
16 changes: 16 additions & 0 deletions src/functions.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -203,6 +203,22 @@ function enqueue(callable $task)
$queue->enqueue($task);
}

/**
* @internal
*/
function fatalError($error)
{
try {
trigger_error($error, E_USER_ERROR);
} catch (\Throwable $e) {
set_error_handler(null);
trigger_error($error, E_USER_ERROR);
} catch (\Exception $e) {
set_error_handler(null);
trigger_error($error, E_USER_ERROR);
}
}

/**
* @internal
*/
Expand Down
29 changes: 29 additions & 0 deletions tests/ErrorCollector.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
<?php

namespace React\Promise;

final class ErrorCollector
{
private $errors = [];

public function start()
{
$errors = [];

set_error_handler(function ($errno, $errstr, $errfile, $errline, $errcontext) use (&$errors) {
$errors[] = compact('errno', 'errstr', 'errfile', 'errline', 'errcontext');
});

$this->errors = &$errors;
}

public function stop()
{
$errors = $this->errors;
$this->errors = [];

restore_error_handler();

return $errors;
}
}
30 changes: 23 additions & 7 deletions tests/PromiseTest/PromiseFulfilledTestTrait.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

namespace React\Promise\PromiseTest;

use React\Promise\ErrorCollector;

trait PromiseFulfilledTestTrait
{
/**
Expand Down Expand Up @@ -212,29 +214,43 @@ public function doneShouldInvokeFulfillmentHandlerForFulfilledPromise()
}

/** @test */
public function doneShouldThrowExceptionThrownFulfillmentHandlerForFulfilledPromise()
public function doneShouldTriggerFatalErrorThrownFulfillmentHandlerForFulfilledPromise()
{
$adapter = $this->getPromiseTestAdapter();

$this->setExpectedException('\Exception', 'UnhandledRejectionException');

$adapter->resolve(1);

$errorCollector = new ErrorCollector();
$errorCollector->start();

$this->assertNull($adapter->promise()->done(function () {
throw new \Exception('UnhandledRejectionException');
throw new \Exception('Unhandled Rejection');
}));

$errors = $errorCollector->stop();

$this->assertEquals(E_USER_ERROR, $errors[0]['errno']);
$this->assertContains('Unhandled Rejection', $errors[0]['errstr']);
}

/** @test */
public function doneShouldThrowUnhandledRejectionExceptionWhenFulfillmentHandlerRejectsForFulfilledPromise()
public function doneShouldTriggerFatalErrorUnhandledRejectionExceptionWhenFulfillmentHandlerRejectsForFulfilledPromise()
{
$adapter = $this->getPromiseTestAdapter();

$this->setExpectedException('React\\Promise\\UnhandledRejectionException');

$adapter->resolve(1);

$errorCollector = new ErrorCollector();
$errorCollector->start();

$this->assertNull($adapter->promise()->done(function () {
return \React\Promise\reject();
}));

$errors = $errorCollector->stop();

$this->assertEquals(E_USER_ERROR, $errors[0]['errno']);
$this->assertContains('Unhandled Rejection: null', $errors[0]['errstr']);
}

/** @test */
Expand Down
97 changes: 71 additions & 26 deletions tests/PromiseTest/PromiseRejectedTestTrait.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
namespace React\Promise\PromiseTest;

use React\Promise\Deferred;
use React\Promise\ErrorCollector;
use React\Promise\UnhandledRejectionException;

trait PromiseRejectedTestTrait
Expand Down Expand Up @@ -200,27 +201,40 @@ public function doneShouldInvokeRejectionHandlerForRejectedPromise()
}

/** @test */
public function doneShouldThrowExceptionThrownByRejectionHandlerForRejectedPromise()
public function doneShouldTriggerFatalErrorExceptionThrownByRejectionHandlerForRejectedPromise()
{
$adapter = $this->getPromiseTestAdapter();
$errorCollector = new ErrorCollector();
$errorCollector->start();

$this->setExpectedException('\Exception', 'UnhandledRejectionException');
$adapter = $this->getPromiseTestAdapter();

$adapter->reject(1);
$this->assertNull($adapter->promise()->done(null, function () {
throw new \Exception('UnhandledRejectionException');
throw new \Exception('Unhandled Rejection');
}));

$errors = $errorCollector->stop();

$this->assertEquals(E_USER_ERROR, $errors[0]['errno']);
$this->assertContains('Unhandled Rejection', $errors[0]['errstr']);
}

/** @test */
public function doneShouldThrowUnhandledRejectionExceptionWhenRejectedWithNonExceptionForRejectedPromise()
public function doneShouldTriggerFatalErrorUnhandledRejectionExceptionWhenRejectedWithNonExceptionForRejectedPromise()
{
$adapter = $this->getPromiseTestAdapter();

$this->setExpectedException('React\\Promise\\UnhandledRejectionException');

$adapter->reject(1);

$errorCollector = new ErrorCollector();
$errorCollector->start();

$this->assertNull($adapter->promise()->done());

$errors = $errorCollector->stop();

$this->assertEquals(E_USER_ERROR, $errors[0]['errno']);
$this->assertContains('Unhandled Rejection: 1', $errors[0]['errstr']);
}

/** @test */
Expand All @@ -232,57 +246,83 @@ public function unhandledRejectionExceptionThrownByDoneHoldsRejectionValue()

$adapter->reject($expected);

try {
$adapter->promise()->done();
} catch (UnhandledRejectionException $e) {
$this->assertSame($expected, $e->getReason());
return;
}
$errorCollector = new ErrorCollector();
$errorCollector->start();

$adapter->promise()->done();

$errors = $errorCollector->stop();

$this->fail();
$this->assertEquals(E_USER_ERROR, $errors[0]['errno']);
$this->assertContains('Unhandled Rejection: {}', $errors[0]['errstr']);

$this->assertArrayHasKey('error', $errors[0]['errcontext']);
$this->assertInstanceOf('React\Promise\UnhandledRejectionException', $errors[0]['errcontext']['error']);
$this->assertSame($expected, $errors[0]['errcontext']['error']->getReason());
}

/** @test */
public function doneShouldThrowUnhandledRejectionExceptionWhenRejectionHandlerRejectsForRejectedPromise()
public function doneShouldTriggerFatalErrorUnhandledRejectionExceptionWhenRejectionHandlerRejectsForRejectedPromise()
{
$adapter = $this->getPromiseTestAdapter();
$errorCollector = new ErrorCollector();
$errorCollector->start();

$this->setExpectedException('React\\Promise\\UnhandledRejectionException');
$adapter = $this->getPromiseTestAdapter();

$adapter->reject(1);
$this->assertNull($adapter->promise()->done(null, function () {
return \React\Promise\reject();
}));

$errors = $errorCollector->stop();

$this->assertEquals(E_USER_ERROR, $errors[0]['errno']);
$this->assertContains('Unhandled Rejection: null', $errors[0]['errstr']);
}

/** @test */
public function doneShouldThrowRejectionExceptionWhenRejectionHandlerRejectsWithExceptionForRejectedPromise()
public function doneShouldTriggerFatalErrorRejectionExceptionWhenRejectionHandlerRejectsWithExceptionForRejectedPromise()
{
$adapter = $this->getPromiseTestAdapter();
$errorCollector = new ErrorCollector();
$errorCollector->start();

$this->setExpectedException('\Exception', 'UnhandledRejectionException');
$adapter = $this->getPromiseTestAdapter();

$adapter->reject(1);
$this->assertNull($adapter->promise()->done(null, function () {
return \React\Promise\reject(new \Exception('UnhandledRejectionException'));
return \React\Promise\reject(new \Exception('Unhandled Rejection'));
}));

$errors = $errorCollector->stop();

$this->assertEquals(E_USER_ERROR, $errors[0]['errno']);
$this->assertContains('Unhandled Rejection', $errors[0]['errstr']);
}

/** @test */
public function doneShouldThrowExceptionProvidedAsRejectionValueForRejectedPromise()
public function doneShouldTriggerFatalErrorExceptionProvidedAsRejectionValueForRejectedPromise()
{
$errorCollector = new ErrorCollector();
$errorCollector->start();

$adapter = $this->getPromiseTestAdapter();

$this->setExpectedException('\Exception', 'UnhandledRejectionException');
$exception = new \Exception('Unhandled Rejection');

$adapter->reject(new \Exception('UnhandledRejectionException'));
$adapter->reject($exception);
$this->assertNull($adapter->promise()->done());

$errors = $errorCollector->stop();

$this->assertEquals(E_USER_ERROR, $errors[0]['errno']);
$this->assertEquals((string) $exception, $errors[0]['errstr']);
}

/** @test */
public function doneShouldThrowWithDeepNestingPromiseChainsForRejectedPromise()
public function doneShouldTriggerFatalErrorWithDeepNestingPromiseChainsForRejectedPromise()
{
$this->setExpectedException('\Exception', 'UnhandledRejectionException');
$errorCollector = new ErrorCollector();
$errorCollector->start();

$exception = new \Exception('UnhandledRejectionException');

Expand All @@ -301,6 +341,11 @@ function () use ($exception) {
})));

$result->done();

$errors = $errorCollector->stop();

$this->assertEquals(E_USER_ERROR, $errors[0]['errno']);
$this->assertEquals((string) $exception, $errors[0]['errstr']);
}

/** @test */
Expand Down
Loading