Skip to content

Commit 3778614

Browse files
kelunikkelunik
authored
Throw UnhandledThrowable if event loop stops due to an exception (#32)
1 parent 1120c87 commit 3778614

File tree

7 files changed

+145
-69
lines changed

7 files changed

+145
-69
lines changed

‎src/EventLoop/Driver/StreamSelectDriver.php

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -257,8 +257,7 @@ private function selectStreams(array $read, array $write, float $timeout): void
257257
}
258258

259259
if (!$result) {
260-
$this->error(new \Exception('Unknown error during stream_select'));
261-
return;
260+
throw new \Exception('Unknown error during stream_select');
262261
}
263262

264263
foreach ($read as $stream) {

‎src/EventLoop/Internal/AbstractDriver.php

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
use Revolt\EventLoop\Driver;
66
use Revolt\EventLoop\InvalidCallbackError;
77
use Revolt\EventLoop\Suspension;
8+
use Revolt\EventLoop\UncaughtThrowable;
89

910
/**
1011
* Event loop driver which implements all basic operations to allow interoperability.
@@ -70,7 +71,7 @@ private static function checkFiberSupport(): void
7071

7172
private \stdClass $internalSuspensionMarker;
7273

73-
/** @var \SplQueue<array{callable, array}> */
74+
/** @var \SplQueue<array{\Closure, array}> */
7475
private \SplQueue $microtaskQueue;
7576

7677
/** @var \SplQueue<DriverCallback> */
@@ -398,10 +399,10 @@ final protected function enqueueCallback(DriverCallback $callback): void
398399
*
399400
* @param \Throwable $exception The exception thrown from an event callback.
400401
*/
401-
final protected function error(\Throwable $exception): void
402+
final protected function error(\Closure $closure, \Throwable $exception): void
402403
{
403404
if ($this->errorHandler === null) {
404-
$this->setInterrupt(static fn () => throw $exception);
405+
$this->setInterrupt(static fn () => throw UncaughtThrowable::throwingCallback($closure, $exception));
405406
return;
406407
}
407408

@@ -427,7 +428,7 @@ private function invokeMicrotasks(): void
427428
try {
428429
$callback(...$args);
429430
} catch (\Throwable $exception) {
430-
$this->error($exception);
431+
$this->error($callback, $exception);
431432
}
432433

433434
unset($callback, $args);
@@ -597,7 +598,7 @@ private function createCallbackFiber(): void
597598
throw InvalidCallbackError::nonNullReturn($callback->id, $callback->closure);
598599
}
599600
} catch (\Throwable $exception) {
600-
$this->error($exception);
601+
$this->error($callback->closure, $exception);
601602
}
602603

603604
unset($callback);
@@ -622,7 +623,9 @@ private function createErrorCallback(): void
622623
try {
623624
$errorHandler($exception);
624625
} catch (\Throwable $exception) {
625-
$this->interrupt = static fn () => throw $exception;
626+
$this->setInterrupt(
627+
static fn () => throw UncaughtThrowable::throwingErrorHandler($errorHandler, $exception)
628+
);
626629
}
627630
};
628631
}

‎src/EventLoop/Internal/ClosureHelper.php

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
<?php
2+
3+
namespace Revolt\EventLoop\Internal;
4+
5+
/** @internal */
6+
final class ClosureHelper
7+
{
8+
public static function getDescription(\Closure $closure): string
9+
{
10+
try {
11+
$reflection = new \ReflectionFunction($closure);
12+
13+
$description = $reflection->name;
14+
15+
if ($scopeClass = $reflection->getClosureScopeClass()) {
16+
$description = $scopeClass->name . '::' . $description;
17+
}
18+
19+
if ($reflection->getFileName() && $reflection->getStartLine()) {
20+
$description .= " defined in " . $reflection->getFileName() . ':' . $reflection->getStartLine();
21+
}
22+
23+
return $description;
24+
} catch (\ReflectionException) {
25+
return '???';
26+
}
27+
}
28+
}

‎src/EventLoop/InvalidCallbackError.php

Lines changed: 3 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@
22

33
namespace Revolt\EventLoop;
44

5+
use Revolt\EventLoop\Internal\ClosureHelper;
6+
57
final class InvalidCallbackError extends \Error
68
{
79
public const E_NONNULL_RETURN = 1;
@@ -12,12 +14,10 @@ final class InvalidCallbackError extends \Error
1214
*/
1315
public static function nonNullReturn(string $callbackId, \Closure $closure): self
1416
{
15-
$description = self::getClosureDescription($closure);
16-
1717
return new self(
1818
$callbackId,
1919
self::E_NONNULL_RETURN,
20-
'Non-null return value received from callback ' . $description
20+
'Non-null return value received from callback ' . ClosureHelper::getDescription($closure)
2121
);
2222
}
2323

@@ -31,27 +31,6 @@ public static function invalidIdentifier(string $callbackId): self
3131
return new self($callbackId, self::E_INVALID_IDENTIFIER, 'Invalid callback identifier ' . $callbackId);
3232
}
3333

34-
private static function getClosureDescription(\Closure $closure): string
35-
{
36-
try {
37-
$reflection = new \ReflectionFunction($closure);
38-
39-
$description = $reflection->name;
40-
41-
if ($scopeClass = $reflection->getClosureScopeClass()) {
42-
$description = $scopeClass->name . '::' . $description;
43-
}
44-
45-
if ($reflection->getFileName() && $reflection->getStartLine()) {
46-
$description .= " defined in " . $reflection->getFileName() . ':' . $reflection->getStartLine();
47-
}
48-
49-
return $description;
50-
} catch (\ReflectionException) {
51-
return '???';
52-
}
53-
}
54-
5534
/** @var string */
5635
private string $rawMessage;
5736

‎src/EventLoop/UncaughtThrowable.php

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
<?php
2+
3+
namespace Revolt\EventLoop;
4+
5+
use Revolt\EventLoop\Internal\ClosureHelper;
6+
7+
final class UncaughtThrowable extends \Error
8+
{
9+
public static function throwingCallback(\Closure $closure, \Throwable $previous): self
10+
{
11+
return new self(
12+
"Uncaught %s thrown in event loop callback %s; use Revolt\EventLoop::setErrorHandler() to gracefully handle such exceptions%s",
13+
$closure,
14+
$previous
15+
);
16+
}
17+
18+
public static function throwingErrorHandler(\Closure $closure, \Throwable $previous): self
19+
{
20+
return new self("Uncaught %s thrown in event loop error handler %s%s", $closure, $previous);
21+
}
22+
23+
private function __construct(string $message, \Closure $closure, \Throwable $previous)
24+
{
25+
parent::__construct(\sprintf(
26+
$message,
27+
\str_replace("\0", '@', \get_class($previous)), // replace NUL-byte in anonymous class name
28+
ClosureHelper::getDescription($closure),
29+
$previous->getMessage() !== '' ? ': ' . $previous->getMessage() : ''
30+
), 0, $previous);
31+
}
32+
}

‎test/Driver/DriverTest.php

Lines changed: 70 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
use PHPUnit\Framework\TestCase;
66
use Revolt\EventLoop\Driver;
77
use Revolt\EventLoop\InvalidCallbackError;
8+
use Revolt\EventLoop\UncaughtThrowable;
89
use Revolt\EventLoop\UnsupportedFeatureException;
910

1011
if (!\defined("SIGUSR1")) {
@@ -822,22 +823,30 @@ public function testInvalidityOnDefer(): void
822823
{
823824
$this->expectException(InvalidCallbackError::class);
824825

825-
$this->start(function (Driver $loop): void {
826-
$loop->defer(function ($callbackId) use ($loop): void {
827-
$loop->enable($callbackId);
826+
try {
827+
$this->start(function (Driver $loop): void {
828+
$loop->defer(function ($callbackId) use ($loop): void {
829+
$loop->enable($callbackId);
830+
});
828831
});
829-
});
832+
} catch (UncaughtThrowable $e) {
833+
throw $e->getPrevious();
834+
}
830835
}
831836

832837
public function testInvalidityOnDelay(): void
833838
{
834839
$this->expectException(InvalidCallbackError::class);
835840

836-
$this->start(function (Driver $loop): void {
837-
$loop->delay(0, function ($callbackId) use ($loop): void {
838-
$loop->enable($callbackId);
841+
try {
842+
$this->start(function (Driver $loop): void {
843+
$loop->delay(0, function ($callbackId) use ($loop): void {
844+
$loop->enable($callbackId);
845+
});
839846
});
840-
});
847+
} catch (UncaughtThrowable $e) {
848+
throw $e->getPrevious();
849+
}
841850
}
842851

843852
public function testEventsNotExecutedInSameTickAsEnabled(): void
@@ -1022,23 +1031,31 @@ public function testLoopAllowsExceptionToBubbleUpDuringStart(): void
10221031
$this->expectException(\Exception::class);
10231032
$this->expectExceptionMessage("loop error");
10241033

1025-
$this->start(function (Driver $loop): void {
1026-
$loop->defer(function (): void {
1027-
throw new \Exception("loop error");
1034+
try {
1035+
$this->start(function (Driver $loop): void {
1036+
$loop->defer(function (): void {
1037+
throw new \Exception("loop error");
1038+
});
10281039
});
1029-
});
1040+
} catch (UncaughtThrowable $e) {
1041+
throw $e->getPrevious();
1042+
}
10301043
}
10311044

10321045
public function testLoopAllowsExceptionToBubbleUpFromRepeatingAlarmDuringStart(): void
10331046
{
10341047
$this->expectException(\RuntimeException::class);
10351048
$this->expectExceptionMessage("test");
10361049

1037-
$this->start(function (Driver $loop): void {
1038-
$loop->repeat(0.001, function (): void {
1039-
throw new \RuntimeException("test");
1050+
try {
1051+
$this->start(function (Driver $loop): void {
1052+
$loop->repeat(0.001, function (): void {
1053+
throw new \RuntimeException("test");
1054+
});
10401055
});
1041-
});
1056+
} catch (UncaughtThrowable $e) {
1057+
throw $e->getPrevious();
1058+
}
10421059
}
10431060

10441061
public function testErrorHandlerCapturesUncaughtException(): void
@@ -1066,26 +1083,40 @@ public function testOnErrorFailure(): void
10661083
$this->loop->setErrorHandler(function (): void {
10671084
throw new \Exception("errorception");
10681085
});
1069-
$this->start(function (Driver $loop): void {
1070-
$loop->delay(0.005, function () {
1071-
throw new \Exception("error");
1086+
1087+
try {
1088+
$this->start(function (Driver $loop): void {
1089+
$loop->delay(0.005, function () {
1090+
throw new \Exception("error");
1091+
});
10721092
});
1073-
});
1093+
} catch (UncaughtThrowable $e) {
1094+
throw $e->getPrevious();
1095+
}
10741096
}
10751097

10761098
public function testLoopException(): void
10771099
{
10781100
$this->expectException(\RuntimeException::class);
10791101
$this->expectExceptionMessage("test");
10801102

1081-
$this->start(function (Driver $loop): void {
1082-
$loop->defer(function () use ($loop): void {
1083-
// force next tick, outside of primary startup tick
1084-
$loop->defer(function () {
1085-
throw new \RuntimeException("test");
1103+
try {
1104+
$this->start(function (Driver $loop): void {
1105+
$loop->defer(function () use ($loop): void {
1106+
// force next tick, outside of primary startup tick
1107+
$loop->defer(function () {
1108+
throw new \RuntimeException("test");
1109+
});
10861110
});
10871111
});
1088-
});
1112+
} catch (UncaughtThrowable $e) {
1113+
$this->assertStringStartsWith(
1114+
'Uncaught RuntimeException thrown in event loop callback Revolt\EventLoop',
1115+
$e->getMessage()
1116+
);
1117+
1118+
throw $e->getPrevious();
1119+
}
10891120
}
10901121

10911122
public function testOnSignalCallback(): void
@@ -1197,12 +1228,16 @@ public function testStreamDoesntSwallowExceptions(): void
11971228
{
11981229
$this->expectException(\RuntimeException::class);
11991230

1200-
$this->start(function (Driver $loop): void {
1201-
$loop->onWritable(STDOUT, function () {
1202-
throw new \RuntimeException();
1231+
try {
1232+
$this->start(function (Driver $loop): void {
1233+
$loop->onWritable(STDOUT, function () {
1234+
throw new \RuntimeException();
1235+
});
1236+
$loop->delay(0.005, \Closure::fromCallable([$loop, "stop"]));
12031237
});
1204-
$loop->delay(0.005, \Closure::fromCallable([$loop, "stop"]));
1205-
});
1238+
} catch (UncaughtThrowable $e) {
1239+
throw $e->getPrevious();
1240+
}
12061241
}
12071242

12081243
public function testReactorRunsUntilNoCallbacksRemain(): void
@@ -1366,8 +1401,8 @@ public function testMicrotaskThrowingStillExecutesNextMicrotask(): void
13661401
$invoked = true;
13671402
});
13681403
});
1369-
} catch (\Exception $e) {
1370-
self::assertSame($exception, $e);
1404+
} catch (UncaughtThrowable $e) {
1405+
self::assertSame($exception, $e->getPrevious());
13711406
}
13721407

13731408
$this->start(fn () => null);
@@ -1426,8 +1461,8 @@ public function testRethrowsFromCallbacks(): void
14261461
$this->loop->run();
14271462

14281463
self::fail("Didn't throw expected exception.");
1429-
} catch (\Exception $e) {
1430-
self::assertSame("rethrow test", $e->getMessage());
1464+
} catch (UncaughtThrowable $e) {
1465+
self::assertSame("rethrow test", $e->getPrevious()->getMessage());
14311466
}
14321467
}
14331468
}

‎test/EventLoopTest.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -207,8 +207,8 @@ public function testSuspensionThrowingErrorViaInterrupt(): void
207207
try {
208208
$suspension->suspend();
209209
self::fail("Error was not thrown");
210-
} catch (\Throwable $t) {
211-
self::assertSame($error, $t);
210+
} catch (UncaughtThrowable $t) {
211+
self::assertSame($error, $t->getPrevious());
212212
}
213213
}
214214
}

0 commit comments

Comments
 (0)