From 9236ade19fc004e16a24f13e37e8d53f98f8147b Mon Sep 17 00:00:00 2001 From: Aaron Piotrowski Date: Fri, 25 Jan 2019 14:29:32 -0600 Subject: [PATCH] Capture previous exceptions of exceptions thrown from Task::run() --- lib/Worker/Internal/TaskFailure.php | 59 +++++++------ lib/Worker/TaskError.php | 11 +-- lib/Worker/TaskException.php | 11 +-- test/Worker/AbstractPoolTest.php | 14 ++-- test/Worker/AbstractWorkerTest.php | 82 +++++++++++++++---- test/Worker/Fixtures/FailingTask.php | 35 ++++++++ .../NonAutoloadableResultTask.php | 2 +- test/Worker/{ => Fixtures}/TestTask.php | 2 +- .../UnserializableResultTask.php | 2 +- .../Fixtures/non-autoloadable-class.php | 7 ++ test/Worker/FunctionsTest.php | 2 +- test/Worker/JobTest.php | 4 +- test/Worker/non-autoloadable-class.php | 7 -- 13 files changed, 167 insertions(+), 71 deletions(-) create mode 100644 test/Worker/Fixtures/FailingTask.php rename test/Worker/{ => Fixtures}/NonAutoloadableResultTask.php (86%) rename test/Worker/{ => Fixtures}/TestTask.php (92%) rename test/Worker/{ => Fixtures}/UnserializableResultTask.php (85%) create mode 100644 test/Worker/Fixtures/non-autoloadable-class.php delete mode 100644 test/Worker/non-autoloadable-class.php diff --git a/lib/Worker/Internal/TaskFailure.php b/lib/Worker/Internal/TaskFailure.php index 974d87f..f10b126 100644 --- a/lib/Worker/Internal/TaskFailure.php +++ b/lib/Worker/Internal/TaskFailure.php @@ -28,6 +28,9 @@ final class TaskFailure extends TaskResult /** @var array */ private $trace; + /** @var self|null */ + private $previous; + public function __construct(string $id, \Throwable $exception) { parent::__construct($id); @@ -36,37 +39,45 @@ final class TaskFailure extends TaskResult $this->message = $exception->getMessage(); $this->code = $exception->getCode(); $this->trace = $exception->getTraceAsString(); + + if ($previous = $exception->getPrevious()) { + $this->previous = new self($id, $previous); + } } public function promise(): Promise { - switch ($this->parent) { - case self::PARENT_ERROR: - $exception = new TaskError( - $this->type, - \sprintf( - 'Uncaught %s in worker with message "%s" and code "%s"', - $this->type, - $this->message, - $this->code - ), - $this->trace - ); - break; + return new Failure($this->createException()); + } - default: - $exception = new TaskException( + private function createException(): \Throwable + { + $previous = $this->previous ? $this->previous->createException() : null; + + if ($this->parent === self::PARENT_ERROR) { + return new TaskError( + $this->type, + \sprintf( + 'Uncaught %s in worker with message "%s" and code "%s"', $this->type, - \sprintf( - 'Uncaught %s in worker with message "%s" and code "%s"', - $this->type, - $this->message, - $this->code - ), - $this->trace - ); + $this->message, + $this->code + ), + $this->trace, + $previous + ); } - return new Failure($exception); + return new TaskException( + $this->type, + \sprintf( + 'Uncaught %s in worker with message "%s" and code "%s"', + $this->type, + $this->message, + $this->code + ), + $this->trace, + $previous + ); } } diff --git a/lib/Worker/TaskError.php b/lib/Worker/TaskError.php index b6f49d2..3ec9f98 100644 --- a/lib/Worker/TaskError.php +++ b/lib/Worker/TaskError.php @@ -11,13 +11,14 @@ final class TaskError extends \Error private $trace; /** - * @param string $name The exception class name. - * @param string $message The panic message. - * @param string $trace The panic stack trace. + * @param string $name The exception class name. + * @param string $message The panic message. + * @param string $trace The panic stack trace. + * @param \Throwable|null $previous Previous exception. */ - public function __construct(string $name, string $message = '', string $trace = '') + public function __construct(string $name, string $message = '', string $trace = '', \Throwable $previous = null) { - parent::__construct($message); + parent::__construct($message, 0, $previous); $this->name = $name; $this->trace = $trace; diff --git a/lib/Worker/TaskException.php b/lib/Worker/TaskException.php index f4225c1..96f969b 100644 --- a/lib/Worker/TaskException.php +++ b/lib/Worker/TaskException.php @@ -11,13 +11,14 @@ final class TaskException extends \Exception private $trace; /** - * @param string $name The exception class name. - * @param string $message The panic message. - * @param string $trace The panic stack trace. + * @param string $name The exception class name. + * @param string $message The panic message. + * @param string $trace The panic stack trace. + * @param \Throwable|null $previous Previous exception. */ - public function __construct(string $name, string $message = '', string $trace = '') + public function __construct(string $name, string $message = '', string $trace = '', \Throwable $previous = null) { - parent::__construct($message); + parent::__construct($message, 0, $previous); $this->name = $name; $this->trace = $trace; diff --git a/test/Worker/AbstractPoolTest.php b/test/Worker/AbstractPoolTest.php index f36c9c4..e86e946 100644 --- a/test/Worker/AbstractPoolTest.php +++ b/test/Worker/AbstractPoolTest.php @@ -93,7 +93,7 @@ abstract class AbstractPoolTest extends TestCase Loop::run(function () { $pool = $this->createPool(); - $returnValue = yield $pool->enqueue(new TestTask(42)); + $returnValue = yield $pool->enqueue(new Fixtures\TestTask(42)); $this->assertEquals(42, $returnValue); yield $pool->shutdown(); @@ -106,9 +106,9 @@ abstract class AbstractPoolTest extends TestCase $pool = $this->createPool(); $values = yield \Amp\Promise\all([ - $pool->enqueue(new TestTask(42)), - $pool->enqueue(new TestTask(56)), - $pool->enqueue(new TestTask(72)) + $pool->enqueue(new Fixtures\TestTask(42)), + $pool->enqueue(new Fixtures\TestTask(56)), + $pool->enqueue(new Fixtures\TestTask(72)) ]); $this->assertEquals([42, 56, 72], $values); @@ -136,7 +136,7 @@ abstract class AbstractPoolTest extends TestCase $this->assertTrue($worker->isRunning()); $this->assertTrue($worker->isIdle()); - $this->assertSame(42, yield $worker->enqueue(new TestTask(42))); + $this->assertSame(42, yield $worker->enqueue(new Fixtures\TestTask(42))); yield $worker->shutdown(); @@ -151,7 +151,7 @@ abstract class AbstractPoolTest extends TestCase $values = [42, 56, 72]; $tasks = \array_map(function (int $value): Task { - return new TestTask($value); + return new Fixtures\TestTask($value); }, $values); $promises = \array_map(function (Task $task) use ($pool): Promise { @@ -190,7 +190,7 @@ abstract class AbstractPoolTest extends TestCase $values = \range(1, 50); $tasks = \array_map(function (int $value): Task { - return new TestTask($value); + return new Fixtures\TestTask($value); }, $values); $promises = \array_map(function (Task $task) use ($pool): Promise { diff --git a/test/Worker/AbstractWorkerTest.php b/test/Worker/AbstractWorkerTest.php index ba5f7d3..ab85162 100644 --- a/test/Worker/AbstractWorkerTest.php +++ b/test/Worker/AbstractWorkerTest.php @@ -41,7 +41,7 @@ abstract class AbstractWorkerTest extends TestCase $worker = $this->createWorker(); $this->assertTrue($worker->isRunning()); - $worker->enqueue(new TestTask(42)); // Enqueue a task to start the worker. + $worker->enqueue(new Fixtures\TestTask(42)); // Enqueue a task to start the worker. $this->assertTrue($worker->isRunning()); @@ -73,7 +73,7 @@ abstract class AbstractWorkerTest extends TestCase $this->assertTrue($worker->isIdle()); yield $worker->shutdown(); - yield $worker->enqueue(new TestTask(42)); + yield $worker->enqueue(new Fixtures\TestTask(42)); }); } @@ -82,7 +82,7 @@ abstract class AbstractWorkerTest extends TestCase Loop::run(function () { $worker = $this->createWorker(); - $returnValue = yield $worker->enqueue(new TestTask(42)); + $returnValue = yield $worker->enqueue(new Fixtures\TestTask(42)); $this->assertEquals(42, $returnValue); yield $worker->shutdown(); @@ -95,9 +95,9 @@ abstract class AbstractWorkerTest extends TestCase $worker = $this->createWorker(); $values = yield \Amp\Promise\all([ - $worker->enqueue(new TestTask(42)), - $worker->enqueue(new TestTask(56)), - $worker->enqueue(new TestTask(72)) + $worker->enqueue(new Fixtures\TestTask(42)), + $worker->enqueue(new Fixtures\TestTask(56)), + $worker->enqueue(new Fixtures\TestTask(72)) ]); $this->assertEquals([42, 56, 72], $values); @@ -112,9 +112,9 @@ abstract class AbstractWorkerTest extends TestCase $worker = $this->createWorker(); $promises = [ - $worker->enqueue(new TestTask(42, 200)), - $worker->enqueue(new TestTask(56, 300)), - $worker->enqueue(new TestTask(72, 100)) + $worker->enqueue(new Fixtures\TestTask(42, 200)), + $worker->enqueue(new Fixtures\TestTask(56, 300)), + $worker->enqueue(new Fixtures\TestTask(72, 100)) ]; $expected = [42, 56, 72]; @@ -136,9 +136,9 @@ abstract class AbstractWorkerTest extends TestCase $worker = $this->createWorker(); $promises = [ - $worker->enqueue(new TestTask(42, 200)), - $worker->enqueue(new TestTask(56, 300)), - $worker->enqueue(new TestTask(72, 100)) + $worker->enqueue(new Fixtures\TestTask(42, 200)), + $worker->enqueue(new Fixtures\TestTask(56, 300)), + $worker->enqueue(new Fixtures\TestTask(72, 100)) ]; yield $worker->shutdown(); @@ -158,7 +158,7 @@ abstract class AbstractWorkerTest extends TestCase Loop::run(function () { $worker = $this->createWorker(); - $coroutine = $worker->enqueue(new TestTask(42)); + $coroutine = $worker->enqueue(new Fixtures\TestTask(42)); $this->assertFalse($worker->isIdle()); yield $coroutine; @@ -170,12 +170,60 @@ abstract class AbstractWorkerTest extends TestCase { $worker = $this->createWorker(); - $worker->enqueue(new TestTask(42)); + $worker->enqueue(new Fixtures\TestTask(42)); $this->assertRunTimeLessThan([$worker, 'kill'], 250); $this->assertFalse($worker->isRunning()); } + public function testFailingTaskWithException() + { + Loop::run(function () { + $worker = $this->createWorker(); + + try { + yield $worker->enqueue(new Fixtures\FailingTask(\Exception::class)); + } catch (TaskException $exception) { + $this->assertSame(\Exception::class, $exception->getName()); + } + + yield $worker->shutdown(); + }); + } + + public function testFailingTaskWithError() + { + Loop::run(function () { + $worker = $this->createWorker(); + + try { + yield $worker->enqueue(new Fixtures\FailingTask(\Error::class)); + } catch (TaskError $exception) { + $this->assertSame(\Error::class, $exception->getName()); + } + + yield $worker->shutdown(); + }); + } + + public function testFailingTaskWithPreviousException() + { + Loop::run(function () { + $worker = $this->createWorker(); + + try { + yield $worker->enqueue(new Fixtures\FailingTask(\Error::class, \Exception::class)); + } catch (TaskError $exception) { + $this->assertSame(\Error::class, $exception->getName()); + $previous = $exception->getPrevious(); + $this->assertInstanceOf(TaskException::class, $previous); + $this->assertSame(\Exception::class, $previous->getName()); + } + + yield $worker->shutdown(); + }); + } + public function testNonAutoloadableTask() { Loop::run(function () { @@ -219,7 +267,7 @@ abstract class AbstractWorkerTest extends TestCase $worker = $this->createWorker(); try { - yield $worker->enqueue(new UnserializableResultTask); + yield $worker->enqueue(new Fixtures\UnserializableResultTask); $this->fail("Tasks results that cannot be serialized should throw an exception"); } catch (TaskException $exception) { $this->assertSame(0, \strpos($exception->getMessage(), "Uncaught Amp\Parallel\Sync\SerializationException in worker")); @@ -235,7 +283,7 @@ abstract class AbstractWorkerTest extends TestCase $worker = $this->createWorker(); try { - yield $worker->enqueue(new NonAutoloadableResultTask); + yield $worker->enqueue(new Fixtures\NonAutoloadableResultTask); $this->fail("Tasks results that cannot be autoloaded should throw an exception"); } catch (\Error $exception) { $this->assertSame(0, \strpos($exception->getMessage(), "Class instances returned from Amp\Parallel\Worker\Task::run() must be autoloadable by the Composer autoloader")); @@ -255,7 +303,7 @@ abstract class AbstractWorkerTest extends TestCase { } }); - $promise2 = $worker->enqueue(new TestTask(42)); + $promise2 = $worker->enqueue(new Fixtures\TestTask(42)); $this->assertSame(42, yield $promise2); diff --git a/test/Worker/Fixtures/FailingTask.php b/test/Worker/Fixtures/FailingTask.php new file mode 100644 index 0000000..9ab332e --- /dev/null +++ b/test/Worker/Fixtures/FailingTask.php @@ -0,0 +1,35 @@ +exceptionType = $exceptionType; + $this->previousExceptionType = $previousExceptionType; + } + + /** + * Runs the task inside the caller's context. + * Does not have to be a coroutine, can also be a regular function returning a value. + * + * @param \Amp\Parallel\Worker\Environment + * + * @return mixed|\Amp\Promise|\Generator + */ + public function run(Environment $environment) + { + $previous = $this->previousExceptionType ? new $this->previousExceptionType : null; + throw new $this->exceptionType('Test', 0, $previous); + } +} \ No newline at end of file diff --git a/test/Worker/NonAutoloadableResultTask.php b/test/Worker/Fixtures/NonAutoloadableResultTask.php similarity index 86% rename from test/Worker/NonAutoloadableResultTask.php rename to test/Worker/Fixtures/NonAutoloadableResultTask.php index d32c4e7..b0c18c2 100644 --- a/test/Worker/NonAutoloadableResultTask.php +++ b/test/Worker/Fixtures/NonAutoloadableResultTask.php @@ -1,6 +1,6 @@ assertSame($task, $job->getTask()); } @@ -20,7 +20,7 @@ class JobTest extends TestCase */ public function testUnserialiableClass() { - $task = new TestTask(42); + $task = new Fixtures\TestTask(42); $job = new Job($task); $serialized = \serialize($job); $job = \unserialize($serialized, ['allowed_classes' => [Job::class]]); diff --git a/test/Worker/non-autoloadable-class.php b/test/Worker/non-autoloadable-class.php deleted file mode 100644 index edf4003..0000000 --- a/test/Worker/non-autoloadable-class.php +++ /dev/null @@ -1,7 +0,0 @@ -