From e707f7ee2f4a468df0f1a1fa2794dda07c66307b Mon Sep 17 00:00:00 2001 From: Cees-Jan Kiewiet Date: Sun, 28 Jun 2020 20:03:31 +0200 Subject: [PATCH] Trigger E_USER_ERROR on unhandled exceptions --- src/Internal/RejectedPromise.php | 39 +++++++++++++++++++ tests/DeferredTest.php | 4 +- tests/FunctionRaceTest.php | 2 +- tests/FunctionSomeTest.php | 4 +- tests/Internal/RejectedPromiseTest.php | 16 ++++++++ tests/PromiseTest.php | 12 +++++- tests/PromiseTest/CancelTestTrait.php | 14 +++---- .../PromiseTest/PromiseRejectedTestTrait.php | 9 ++++- tests/PromiseTest/PromiseSettledTestTrait.php | 16 +++++++- 9 files changed, 101 insertions(+), 15 deletions(-) diff --git a/src/Internal/RejectedPromise.php b/src/Internal/RejectedPromise.php index da50cde4..57d4e9e9 100644 --- a/src/Internal/RejectedPromise.php +++ b/src/Internal/RejectedPromise.php @@ -4,6 +4,7 @@ use React\Promise\Promise; use React\Promise\PromiseInterface; +use Throwable; use function React\Promise\_checkTypehint; use function React\Promise\enqueue; use function React\Promise\fatalError; @@ -15,18 +16,54 @@ final class RejectedPromise implements PromiseInterface { private $reason; + private $handled = false; public function __construct(\Throwable $reason) { $this->reason = $reason; } + public function __destruct() + { + if ($this->handled) { + return; + } + + $message = 'Unhandled promise rejection with '; + + if ($this->reason instanceof Throwable) { + $message .= get_class($this->reason) . ': ' . $this->reason->getMessage(); + $message .= ' raised in ' . $this->reason->getFile() . ' on line ' . $this->reason->getLine(); + $message .= PHP_EOL . $this->reason->getTraceAsString(); + } else { + if ($this->reason === null) { + $message .= 'null'; + } else { + $message .= (is_object($this->reason) ? get_class($this->reason) : gettype($this->reason)); + } + + $trace = debug_backtrace(DEBUG_BACKTRACE_IGNORE_ARGS); + if (isset($trace[0]['file'], $trace[0]['line'])) { + $message .= ' detected in ' . $trace[0]['file'] . ' on line ' . $trace[0]['line']; + } + + ob_start(); + debug_print_backtrace(DEBUG_BACKTRACE_IGNORE_ARGS); + $message .= PHP_EOL . ob_get_clean(); + } + + $message .= PHP_EOL; + fatalError($message); + } + public function then(callable $onFulfilled = null, callable $onRejected = null): PromiseInterface { if (null === $onRejected) { return $this; } + $this->handled = true; + return new Promise(function (callable $resolve, callable $reject) use ($onRejected): void { enqueue(function () use ($resolve, $reject, $onRejected): void { try { @@ -41,6 +78,8 @@ public function then(callable $onFulfilled = null, callable $onRejected = null): public function done(callable $onFulfilled = null, callable $onRejected = null): void { enqueue(function () use ($onRejected) { + $this->handled = true; + if (null === $onRejected) { return fatalError($this->reason); } diff --git a/tests/DeferredTest.php b/tests/DeferredTest.php index 423625c3..2ec12cda 100644 --- a/tests/DeferredTest.php +++ b/tests/DeferredTest.php @@ -27,6 +27,7 @@ public function shouldRejectWithoutCreatingGarbageCyclesIfCancellerRejectsWithEx $deferred = new Deferred(function ($resolve, $reject) { $reject(new \Exception('foo')); }); + $deferred->promise()->then(null, function () { }); $deferred->promise()->cancel(); unset($deferred); @@ -42,7 +43,7 @@ public function shouldRejectWithoutCreatingGarbageCyclesIfParentCancellerRejects $deferred = new Deferred(function ($resolve, $reject) { $reject(new \Exception('foo')); }); - $deferred->promise()->then()->cancel(); + $deferred->promise()->then(null, function () { })->cancel(); unset($deferred); $this->assertSame(0, gc_collect_cycles()); @@ -56,6 +57,7 @@ public function shouldRejectWithoutCreatingGarbageCyclesIfCancellerHoldsReferenc $deferred = new Deferred(function () use (&$deferred) { }); $deferred->reject(new \Exception('foo')); + $deferred->promise()->then(null, function () { }); unset($deferred); $this->assertSame(0, gc_collect_cycles()); diff --git a/tests/FunctionRaceTest.php b/tests/FunctionRaceTest.php index 83649173..63891262 100644 --- a/tests/FunctionRaceTest.php +++ b/tests/FunctionRaceTest.php @@ -118,6 +118,6 @@ public function shouldNotCancelOtherPendingInputArrayPromisesIfOnePromiseRejects $promise2 = new Promise(function () {}, $this->expectCallableNever()); - race([$deferred->promise(), $promise2])->cancel(); + race([$deferred->promise(), $promise2])->then(null, function () { })->cancel(); } } diff --git a/tests/FunctionSomeTest.php b/tests/FunctionSomeTest.php index 9af1c1e0..91699ed8 100644 --- a/tests/FunctionSomeTest.php +++ b/tests/FunctionSomeTest.php @@ -163,6 +163,8 @@ public function shouldNotCancelOtherPendingInputArrayPromisesIfEnoughPromisesRej $promise2 = new Promise(function () {}, $this->expectCallableNever()); - some([$deferred->promise(), $promise2], 2); + $ret = some([$deferred->promise(), $promise2], 2); + + $ret->then(null, function () { }); } } diff --git a/tests/Internal/RejectedPromiseTest.php b/tests/Internal/RejectedPromiseTest.php index 38eb3b40..568e2e1d 100644 --- a/tests/Internal/RejectedPromiseTest.php +++ b/tests/Internal/RejectedPromiseTest.php @@ -4,6 +4,7 @@ use Exception; use LogicException; +use React\Promise\ErrorCollector; use React\Promise\PromiseAdapter\CallbackPromiseAdapter; use React\Promise\PromiseTest\PromiseRejectedTestTrait; use React\Promise\PromiseTest\PromiseSettledTestTrait; @@ -45,4 +46,19 @@ public function getPromiseTestAdapter(callable $canceller = null) }, ]); } + + /** @test */ + public function unhandledRejectionShouldTriggerFatalError() + { + $errorCollector = new ErrorCollector(); + $errorCollector->start(); + + $promise = new RejectedPromise(new Exception('foo')); + unset($promise); + + $errors = $errorCollector->stop(); + + self::assertEquals(E_USER_ERROR, $errors[0]['errno']); + self::assertContains('Unhandled promise rejection with Exception: foo raised in ', $errors[0]['errstr']); + } } diff --git a/tests/PromiseTest.php b/tests/PromiseTest.php index 6af65ce5..56e95582 100644 --- a/tests/PromiseTest.php +++ b/tests/PromiseTest.php @@ -66,6 +66,7 @@ public function shouldRejectWithoutCreatingGarbageCyclesIfResolverThrowsExceptio $promise = new Promise(function () { throw new \Exception('foo'); }); + $promise->then(null, function () { }); unset($promise); $this->assertSame(0, gc_collect_cycles()); @@ -78,6 +79,7 @@ public function shouldRejectWithoutCreatingGarbageCyclesIfResolverRejectsWithExc $promise = new Promise(function ($resolve, $reject) { $reject(new \Exception('foo')); }); + $promise->then(null, function () { }); unset($promise); $this->assertSame(0, gc_collect_cycles()); @@ -91,6 +93,7 @@ public function shouldRejectWithoutCreatingGarbageCyclesIfCancellerRejectsWithEx $reject(new \Exception('foo')); }); $promise->cancel(); + $promise->then(null, function () { }); unset($promise); $this->assertSame(0, gc_collect_cycles()); @@ -103,7 +106,7 @@ public function shouldRejectWithoutCreatingGarbageCyclesIfParentCancellerRejects $promise = new Promise(function ($resolve, $reject) { }, function ($resolve, $reject) { $reject(new \Exception('foo')); }); - $promise->then()->then()->then()->cancel(); + $promise->then()->then()->then(null, function () { })->cancel(); unset($promise); $this->assertSame(0, gc_collect_cycles()); @@ -116,6 +119,7 @@ public function shouldRejectWithoutCreatingGarbageCyclesIfResolverThrowsExceptio $promise = new Promise(function ($resolve, $reject) { throw new \Exception('foo'); }); + $promise->then(null, function () { }); unset($promise); $this->assertSame(0, gc_collect_cycles()); @@ -141,6 +145,7 @@ public function shouldRejectWithoutCreatingGarbageCyclesIfCancellerWithReference throw new \Exception('foo'); }); $promise->cancel(); + $promise->then(null, function () { }); unset($promise); $this->assertSame(0, gc_collect_cycles()); @@ -157,6 +162,7 @@ public function shouldRejectWithoutCreatingGarbageCyclesIfResolverWithReferenceT $promise = new Promise(function () use (&$promise) { throw new \Exception('foo'); }); + $promise->then(null, function () { }); unset($promise); $this->assertSame(0, gc_collect_cycles()); @@ -173,6 +179,7 @@ public function shouldRejectWithoutCreatingGarbageCyclesIfCancellerHoldsReferenc $promise = new Promise(function () { throw new \Exception('foo'); }, function () use (&$promise) { }); + $promise->then(null, function () { }); unset($promise); $this->assertSame(0, gc_collect_cycles()); @@ -186,7 +193,7 @@ public function shouldIgnoreNotifyAfterReject() $notify(42); }); - $promise->then(null, null, $this->expectCallableNever()); + $promise->then(null, function () { }, $this->expectCallableNever()); $promise->cancel(); } @@ -263,6 +270,7 @@ public function shouldFulfillIfFullfilledWithSimplePromise() $promise = new Promise(function () { throw new Exception('foo'); }); + $promise->then(null, function () { }); unset($promise); self::assertSame(0, gc_collect_cycles()); diff --git a/tests/PromiseTest/CancelTestTrait.php b/tests/PromiseTest/CancelTestTrait.php index d6626655..a353fce5 100644 --- a/tests/PromiseTest/CancelTestTrait.php +++ b/tests/PromiseTest/CancelTestTrait.php @@ -105,15 +105,13 @@ public function cancelShouldRejectPromiseWithExceptionIfCancellerThrows() /** @test */ public function cancelShouldCallCancellerOnlyOnceIfCancellerResolves() { - $mock = $this->createCallableMock(); - $mock - ->expects($this->once()) - ->method('__invoke') - ->will($this->returnCallback(function ($resolve) { - $resolve(); - })); + $once = $this->expectCallableOnce(); + $canceller = function ($resolve) use ($once) { + $resolve(); + $once(); + }; - $adapter = $this->getPromiseTestAdapter($mock); + $adapter = $this->getPromiseTestAdapter($canceller); $adapter->promise()->cancel(); $adapter->promise()->cancel(); diff --git a/tests/PromiseTest/PromiseRejectedTestTrait.php b/tests/PromiseTest/PromiseRejectedTestTrait.php index d2fb8ac0..1fb38a0c 100644 --- a/tests/PromiseTest/PromiseRejectedTestTrait.php +++ b/tests/PromiseTest/PromiseRejectedTestTrait.php @@ -378,10 +378,13 @@ public function otherwiseShouldNotInvokeRejectionHandlerIfReaonsDoesNotMatchType $mock = $this->expectCallableNever(); $adapter->reject($exception); - $adapter->promise() + $ret = $adapter->promise() ->otherwise(function (InvalidArgumentException $reason) use ($mock) { $mock($reason); }); + + $ret->then(null, function () { }); + $adapter->promise()->then(null, function () { }); } /** @test */ @@ -497,6 +500,8 @@ public function cancelShouldReturnNullForRejectedPromise() $adapter->reject(new Exception()); self::assertNull($adapter->promise()->cancel()); + + $adapter->promise()->then(null, function () { }); } /** @test */ @@ -507,5 +512,7 @@ public function cancelShouldHaveNoEffectForRejectedPromise() $adapter->reject(new Exception()); $adapter->promise()->cancel(); + + $adapter->promise()->then(null, function () { }); } } diff --git a/tests/PromiseTest/PromiseSettledTestTrait.php b/tests/PromiseTest/PromiseSettledTestTrait.php index 626011e1..5a4e2c3e 100644 --- a/tests/PromiseTest/PromiseSettledTestTrait.php +++ b/tests/PromiseTest/PromiseSettledTestTrait.php @@ -19,6 +19,8 @@ public function thenShouldReturnAPromiseForSettledPromise() $adapter->settle(); self::assertInstanceOf(PromiseInterface::class, $adapter->promise()->then()); + + $adapter->promise()->then(null, function () { }); } /** @test */ @@ -28,6 +30,8 @@ public function thenShouldReturnAllowNullForSettledPromise() $adapter->settle(); self::assertInstanceOf(PromiseInterface::class, $adapter->promise()->then(null, null)); + + $adapter->promise()->then(null, function () { }); } /** @test */ @@ -38,6 +42,8 @@ public function cancelShouldReturnNullForSettledPromise() $adapter->settle(); self::assertNull($adapter->promise()->cancel()); + + $adapter->promise()->then(null, function () { }); } /** @test */ @@ -48,6 +54,8 @@ public function cancelShouldHaveNoEffectForSettledPromise() $adapter->settle(); $adapter->promise()->cancel(); + + $adapter->promise()->then(null, function () { }); } /** @test */ @@ -57,6 +65,8 @@ public function doneShouldReturnNullForSettledPromise() $adapter->settle(); self::assertNull($adapter->promise()->done(null, function () {})); + + $adapter->promise()->then(null, function () { }); } /** @test */ @@ -66,6 +76,8 @@ public function doneShouldReturnAllowNullForSettledPromise() $adapter->settle(); self::assertNull($adapter->promise()->done(null, function () {}, null)); + + $adapter->promise()->then(null, function () { }); } /** @test */ @@ -74,6 +86,8 @@ public function alwaysShouldReturnAPromiseForSettledPromise() $adapter = $this->getPromiseTestAdapter(); $adapter->settle(); - self::assertInstanceOf(PromiseInterface::class, $adapter->promise()->always(function () {})); + self::assertInstanceOf(PromiseInterface::class, $ret = $adapter->promise()->always(function () {})); + + $ret->then(null, function () { }); } }