From 53a97cbd92d6805c10c2ee285633b82fff8c3fc2 Mon Sep 17 00:00:00 2001 From: Aaron Piotrowski Date: Tue, 11 Feb 2020 11:06:30 -0600 Subject: [PATCH] Expand context & task exceptions --- lib/Sync/ContextPanicError.php | 83 +++++++++++++++++++++++ lib/Sync/ExitFailure.php | 16 +---- lib/Sync/PanicError.php | 12 +++- lib/Sync/functions.php | 31 ++++++--- lib/Worker/Internal/TaskFailure.php | 21 ++---- lib/Worker/TaskError.php | 11 ++- lib/Worker/TaskException.php | 11 ++- lib/Worker/TaskFailureError.php | 86 ++++++++++++++++++++++++ lib/Worker/TaskFailureException.php | 86 ++++++++++++++++++++++++ lib/Worker/TaskFailureThrowable.php | 38 +++++++++++ test/Context/AbstractContextTest.php | 16 ++--- test/Worker/AbstractWorkerTest.php | 30 ++++----- test/Worker/TaskFailureErrorTest.php | 28 ++++++++ test/Worker/TaskFailureExceptionTest.php | 28 ++++++++ 14 files changed, 428 insertions(+), 69 deletions(-) create mode 100644 lib/Sync/ContextPanicError.php create mode 100644 lib/Worker/TaskFailureError.php create mode 100644 lib/Worker/TaskFailureException.php create mode 100644 lib/Worker/TaskFailureThrowable.php create mode 100644 test/Worker/TaskFailureErrorTest.php create mode 100644 test/Worker/TaskFailureExceptionTest.php diff --git a/lib/Sync/ContextPanicError.php b/lib/Sync/ContextPanicError.php new file mode 100644 index 0000000..0438d00 --- /dev/null +++ b/lib/Sync/ContextPanicError.php @@ -0,0 +1,83 @@ +originalMessage = $message; + $this->originalCode = $code; + $this->originalTrace = $trace; + } + + /** + * @return string Original exception class name. + */ + public function getOriginalClassName(): string + { + return $this->getName(); + } + + /** + * @return string Original exception message. + */ + public function getOriginalMessage(): string + { + return $this->originalMessage; + } + + /** + * @return int|string Original exception code. + */ + public function getOriginalCode() + { + return $this->originalCode; + } + + /** + * Original exception stack trace. + * + * @return array Same as {@see Throwable::getTrace()}, except all function arguments are formatted as strings. + */ + public function getOriginalTrace(): array + { + return $this->originalTrace; + } + + /** + * Original backtrace flattened to a human-readable string. + * + * @return string + */ + public function getOriginalTraceAsString(): string + { + return $this->getPanicTrace(); + } +} diff --git a/lib/Sync/ExitFailure.php b/lib/Sync/ExitFailure.php index eb78b19..ed1c421 100644 --- a/lib/Sync/ExitFailure.php +++ b/lib/Sync/ExitFailure.php @@ -39,22 +39,10 @@ final class ExitFailure implements ExitResult throw $this->createException(); } - private function createException(): PanicError + private function createException(): ContextPanicError { $previous = $this->previous ? $this->previous->createException() : null; - return new PanicError( - $this->type, - \sprintf( - 'Uncaught %s in worker with message "%s" and code "%s"; use %s::getPanicTrace() ' - . 'for the stack trace in the context', - $this->type, - $this->message, - $this->code, - PanicError::class - ), - \implode("\n", $this->trace), - $previous - ); + return new ContextPanicError($this->type, $this->message, $this->code, $this->trace, $previous); } } diff --git a/lib/Sync/PanicError.php b/lib/Sync/PanicError.php index f3bbc17..8122232 100644 --- a/lib/Sync/PanicError.php +++ b/lib/Sync/PanicError.php @@ -2,7 +2,11 @@ namespace Amp\Parallel\Sync; -final class PanicError extends \Error +/** + * @deprecated ContextPanicError will be thrown from uncaught exceptions in child processes and threads instead of + * this class. + */ +class PanicError extends \Error { /** @var string Class name of uncaught exception. */ private $name; @@ -18,7 +22,7 @@ final class PanicError extends \Error * @param string $trace The panic stack trace. * @param \Throwable|null $previous Previous exception. */ - public function __construct(string $name, string $message = '', string $trace = '', \Throwable $previous = null) + public function __construct(string $name, string $message = '', string $trace = '', ?\Throwable $previous = null) { parent::__construct($message, 0, $previous); @@ -27,6 +31,8 @@ final class PanicError extends \Error } /** + * @deprecated Use ContextPanicError::getOriginalClassName() instead. + * * Returns the class name of the uncaught exception. * * @return string @@ -37,6 +43,8 @@ final class PanicError extends \Error } /** + * @deprecated Use ContextPanicError::getOriginalTraceAsString() instead. + * * Gets the stack trace at the point the panic occurred. * * @return string diff --git a/lib/Sync/functions.php b/lib/Sync/functions.php index 875b16d..805a966 100644 --- a/lib/Sync/functions.php +++ b/lib/Sync/functions.php @@ -5,34 +5,47 @@ namespace Amp\Parallel\Sync; /** * @param \Throwable $exception * - * @return string[] Serializable array of strings representing the exception backtrace including function arguments. + * @return array Serializable exception backtrace, with all function arguments flattened to strings. */ function flattenThrowableBacktrace(\Throwable $exception): array { - $output = []; - $counter = 0; $trace = $exception->getTrace(); - foreach ($trace as $call) { + foreach ($trace as &$call) { + unset($call['object']); + $call['args'] = \array_map(__NAMESPACE__ . '\\flattenArgument', $call['args']); + } + + return $trace; +} + +/** + * @param array $trace Backtrace produced by {@see formatFlattenedBacktrace()}. + * + * @return string + */ +function formatFlattenedBacktrace(array $trace): string +{ + $output = []; + + foreach ($trace as $index => $call) { if (isset($call['class'])) { $name = $call['class'] . $call['type'] . $call['function']; } else { $name = $call['function']; } - $args = \implode(', ', \array_map(__NAMESPACE__ . '\\flattenArgument', $call['args'])); - $output[] = \sprintf( '#%d %s(%d): %s(%s)', - $counter++, + $index, $call['file'] ?? '[internal function]', $call['line'] ?? 0, $name, - $args + \implode(', ', $call['args']) ); } - return $output; + return \implode("\n", $output); } /** diff --git a/lib/Worker/Internal/TaskFailure.php b/lib/Worker/Internal/TaskFailure.php index 32e2fbf..5dcfdb7 100644 --- a/lib/Worker/Internal/TaskFailure.php +++ b/lib/Worker/Internal/TaskFailure.php @@ -4,8 +4,8 @@ namespace Amp\Parallel\Worker\Internal; use Amp\Failure; use Amp\Parallel\Sync; -use Amp\Parallel\Worker\TaskError; -use Amp\Parallel\Worker\TaskException; +use Amp\Parallel\Worker\TaskFailureError; +use Amp\Parallel\Worker\TaskFailureException; use Amp\Promise; /** @internal */ @@ -55,23 +55,10 @@ final class TaskFailure extends TaskResult { $previous = $this->previous ? $this->previous->createException() : null; - $format = 'Uncaught %s in worker with message "%s" and code "%s"; use %s::getWorkerTrace() ' - . 'for the stack trace in the worker'; - if ($this->parent === self::PARENT_ERROR) { - return new TaskError( - $this->type, - \sprintf($format, $this->type, $this->message, $this->code, TaskError::class), - \implode("\n", $this->trace), - $previous - ); + return new TaskFailureError($this->type, $this->message, $this->code, $this->trace, $previous); } - return new TaskException( - $this->type, - \sprintf($format, $this->type, $this->message, $this->code, TaskException::class), - \implode("\n", $this->trace), - $previous - ); + return new TaskFailureException($this->type, $this->message, $this->code, $this->trace, $previous); } } diff --git a/lib/Worker/TaskError.php b/lib/Worker/TaskError.php index 3ec9f98..d5925f3 100644 --- a/lib/Worker/TaskError.php +++ b/lib/Worker/TaskError.php @@ -2,7 +2,10 @@ namespace Amp\Parallel\Worker; -final class TaskError extends \Error +/** + * @deprecated TaskFailureError will be thrown from failed Tasks instead of this class. + */ +class TaskError extends \Error { /** @var string Class name of error thrown from task. */ private $name; @@ -16,7 +19,7 @@ final class TaskError extends \Error * @param string $trace The panic stack trace. * @param \Throwable|null $previous Previous exception. */ - public function __construct(string $name, string $message = '', string $trace = '', \Throwable $previous = null) + public function __construct(string $name, string $message = '', string $trace = '', ?\Throwable $previous = null) { parent::__construct($message, 0, $previous); @@ -25,6 +28,8 @@ final class TaskError extends \Error } /** + * @deprecated Use TaskFailureThrowable::getOriginalClassName() instead. + * * Returns the class name of the error thrown from the task. * * @return string @@ -35,6 +40,8 @@ final class TaskError extends \Error } /** + * @deprecated Use TaskFailureThrowable::getOriginalTraceAsString() instead. + * * Gets the stack trace at the point the error was thrown in the task. * * @return string diff --git a/lib/Worker/TaskException.php b/lib/Worker/TaskException.php index 96f969b..7d7d3bb 100644 --- a/lib/Worker/TaskException.php +++ b/lib/Worker/TaskException.php @@ -2,7 +2,10 @@ namespace Amp\Parallel\Worker; -final class TaskException extends \Exception +/** + * @deprecated TaskFailureException will be thrown from failed Tasks instead of this class. + */ +class TaskException extends \Exception { /** @var string Class name of exception thrown from task. */ private $name; @@ -16,7 +19,7 @@ final class TaskException extends \Exception * @param string $trace The panic stack trace. * @param \Throwable|null $previous Previous exception. */ - public function __construct(string $name, string $message = '', string $trace = '', \Throwable $previous = null) + public function __construct(string $name, string $message = '', string $trace = '', ?\Throwable $previous = null) { parent::__construct($message, 0, $previous); @@ -25,6 +28,8 @@ final class TaskException extends \Exception } /** + * @deprecated Use TaskFailureThrowable::getOriginalClassName() instead. + * * Returns the class name of the exception thrown from the task. * * @return string @@ -35,6 +40,8 @@ final class TaskException extends \Exception } /** + * @deprecated Use TaskFailureThrowable::getOriginalTraceAsString() instead. + * * Gets the stack trace at the point the exception was thrown in the task. * * @return string diff --git a/lib/Worker/TaskFailureError.php b/lib/Worker/TaskFailureError.php new file mode 100644 index 0000000..2c0e3e3 --- /dev/null +++ b/lib/Worker/TaskFailureError.php @@ -0,0 +1,86 @@ +originalMessage = $message; + $this->originalCode = $code; + $this->originalTrace = $trace; + } + + /** + * @return string Original exception class name. + */ + public function getOriginalClassName(): string + { + return $this->getName(); + } + + /** + * @return string Original exception message. + */ + public function getOriginalMessage(): string + { + return $this->originalMessage; + } + + /** + * @return int|string Original exception code. + */ + public function getOriginalCode() + { + return $this->originalCode; + } + + /** + * Returns the original exception stack trace. + * + * @return array Same as {@see Throwable::getTrace()}, except all function arguments are formatted as strings. + */ + public function getOriginalTrace(): array + { + return $this->originalTrace; + } + + /** + * Original backtrace flattened to a human-readable string. + * + * @return string + */ + public function getOriginalTraceAsString(): string + { + return $this->getWorkerTrace(); + } +} diff --git a/lib/Worker/TaskFailureException.php b/lib/Worker/TaskFailureException.php new file mode 100644 index 0000000..1e32d51 --- /dev/null +++ b/lib/Worker/TaskFailureException.php @@ -0,0 +1,86 @@ +originalMessage = $message; + $this->originalCode = $code; + $this->originalTrace = $trace; + } + + /** + * @return string Original exception class name. + */ + public function getOriginalClassName(): string + { + return $this->getName(); + } + + /** + * @return string Original exception message. + */ + public function getOriginalMessage(): string + { + return $this->originalMessage; + } + + /** + * @return int|string Original exception code. + */ + public function getOriginalCode() + { + return $this->originalCode; + } + + /** + * Returns the original exception stack trace. + * + * @return array Same as {@see Throwable::getTrace()}, except all function arguments are formatted as strings. + */ + public function getOriginalTrace(): array + { + return $this->originalTrace; + } + + /** + * Original backtrace flattened to a human-readable string. + * + * @return string + */ + public function getOriginalTraceAsString(): string + { + return $this->getWorkerTrace(); + } +} diff --git a/lib/Worker/TaskFailureThrowable.php b/lib/Worker/TaskFailureThrowable.php new file mode 100644 index 0000000..e5adb70 --- /dev/null +++ b/lib/Worker/TaskFailureThrowable.php @@ -0,0 +1,38 @@ +expectException(PanicError::class); + $this->expectException(ContextPanicError::class); $this->expectExceptionMessage('No string provided'); $context = $this->createContext(__DIR__ . "/Fixtures/test-process.php"); @@ -34,7 +34,7 @@ abstract class AbstractContextTest extends AsyncTestCase public function testThrowingProcessOnReceive() { - $this->expectException(PanicError::class); + $this->expectException(ContextPanicError::class); $this->expectExceptionMessage('Test message'); $context = $this->createContext(__DIR__ . "/Fixtures/throwing-process.php"); @@ -44,7 +44,7 @@ abstract class AbstractContextTest extends AsyncTestCase public function testThrowingProcessOnSend() { - $this->expectException(PanicError::class); + $this->expectException(ContextPanicError::class); $this->expectExceptionMessage('Test message'); $context = $this->createContext(__DIR__ . "/Fixtures/throwing-process.php"); @@ -55,7 +55,7 @@ abstract class AbstractContextTest extends AsyncTestCase public function testInvalidScriptPath() { - $this->expectException(PanicError::class); + $this->expectException(ContextPanicError::class); $this->expectExceptionMessage("No script found at '../test-process.php'"); $context = $this->createContext("../test-process.php"); @@ -65,7 +65,7 @@ abstract class AbstractContextTest extends AsyncTestCase public function testInvalidResult() { - $this->expectException(PanicError::class); + $this->expectException(ContextPanicError::class); $this->expectExceptionMessage('The given data cannot be sent because it is not serializable'); $context = $this->createContext(__DIR__ . "/Fixtures/invalid-result-process.php"); @@ -75,7 +75,7 @@ abstract class AbstractContextTest extends AsyncTestCase public function testNoCallbackReturned() { - $this->expectException(PanicError::class); + $this->expectException(ContextPanicError::class); $this->expectExceptionMessage('did not return a callable function'); $context = $this->createContext(__DIR__ . "/Fixtures/no-callback-process.php"); @@ -85,7 +85,7 @@ abstract class AbstractContextTest extends AsyncTestCase public function testParseError() { - $this->expectException(PanicError::class); + $this->expectException(ContextPanicError::class); $this->expectExceptionMessage('contains a parse error'); $context = $this->createContext(__DIR__ . "/Fixtures/parse-error-process.inc"); diff --git a/test/Worker/AbstractWorkerTest.php b/test/Worker/AbstractWorkerTest.php index 8246090..6c33f26 100644 --- a/test/Worker/AbstractWorkerTest.php +++ b/test/Worker/AbstractWorkerTest.php @@ -3,13 +3,13 @@ namespace Amp\Parallel\Test\Worker; use Amp\Parallel\Context\StatusError; -use Amp\Parallel\Sync\PanicError; +use Amp\Parallel\Sync\ContextPanicError; use Amp\Parallel\Sync\SerializationException; use Amp\Parallel\Worker\BasicEnvironment; use Amp\Parallel\Worker\Environment; use Amp\Parallel\Worker\Task; -use Amp\Parallel\Worker\TaskError; -use Amp\Parallel\Worker\TaskException; +use Amp\Parallel\Worker\TaskFailureError; +use Amp\Parallel\Worker\TaskFailureException; use Amp\Parallel\Worker\WorkerException; use Amp\PHPUnit\AsyncTestCase; @@ -172,8 +172,8 @@ abstract class AbstractWorkerTest extends AsyncTestCase try { yield $worker->enqueue(new Fixtures\FailingTask(\Exception::class)); - } catch (TaskException $exception) { - $this->assertSame(\Exception::class, $exception->getName()); + } catch (TaskFailureException $exception) { + $this->assertSame(\Exception::class, $exception->getOriginalClassName()); } yield $worker->shutdown(); @@ -185,8 +185,8 @@ abstract class AbstractWorkerTest extends AsyncTestCase try { yield $worker->enqueue(new Fixtures\FailingTask(\Error::class)); - } catch (TaskError $exception) { - $this->assertSame(\Error::class, $exception->getName()); + } catch (TaskFailureError $exception) { + $this->assertSame(\Error::class, $exception->getOriginalClassName()); } yield $worker->shutdown(); @@ -198,11 +198,11 @@ abstract class AbstractWorkerTest extends AsyncTestCase try { yield $worker->enqueue(new Fixtures\FailingTask(\Error::class, \Exception::class)); - } catch (TaskError $exception) { - $this->assertSame(\Error::class, $exception->getName()); + } catch (TaskFailureError $exception) { + $this->assertSame(\Error::class, $exception->getOriginalClassName()); $previous = $exception->getPrevious(); - $this->assertInstanceOf(TaskException::class, $previous); - $this->assertSame(\Exception::class, $previous->getName()); + $this->assertInstanceOf(TaskFailureException::class, $previous); + $this->assertSame(\Exception::class, $previous->getOriginalClassName()); } yield $worker->shutdown(); @@ -215,8 +215,8 @@ abstract class AbstractWorkerTest extends AsyncTestCase try { yield $worker->enqueue(new NonAutoloadableTask); $this->fail("Tasks that cannot be autoloaded should throw an exception"); - } catch (TaskError $exception) { - $this->assertSame("Error", $exception->getName()); + } catch (TaskFailureError $exception) { + $this->assertSame("Error", $exception->getOriginalClassName()); $this->assertGreaterThan(0, \strpos($exception->getMessage(), \sprintf("Classes implementing %s", Task::class))); } @@ -248,7 +248,7 @@ abstract class AbstractWorkerTest extends AsyncTestCase try { yield $worker->enqueue(new Fixtures\UnserializableResultTask); $this->fail("Tasks results that cannot be serialized should throw an exception"); - } catch (TaskException $exception) { + } catch (TaskFailureException $exception) { $this->assertSame(0, \strpos($exception->getMessage(), "Uncaught Amp\Parallel\Sync\SerializationException in worker")); } @@ -296,7 +296,7 @@ abstract class AbstractWorkerTest extends AsyncTestCase public function testInvalidCustomAutoloader() { - $this->expectException(PanicError::class); + $this->expectException(ContextPanicError::class); $this->expectExceptionMessage('No file found at bootstrap file path given'); $worker = $this->createWorker(BasicEnvironment::class, __DIR__ . '/Fixtures/not-found.php'); diff --git a/test/Worker/TaskFailureErrorTest.php b/test/Worker/TaskFailureErrorTest.php new file mode 100644 index 0000000..646dd66 --- /dev/null +++ b/test/Worker/TaskFailureErrorTest.php @@ -0,0 +1,28 @@ + 'error_message_trace', + 'file' => 'file-name.php', + 'line' => 1, + 'args' => [], + ] + ]; + + $exception = new TaskFailureError('name', 'error_message', 0, $trace); + + $this->assertSame('name', $exception->getOriginalClassName()); + $this->assertSame('error_message', $exception->getOriginalMessage()); + $this->assertSame(0, $exception->getOriginalCode()); + $this->assertSame($trace, $exception->getOriginalTrace()); + } +} diff --git a/test/Worker/TaskFailureExceptionTest.php b/test/Worker/TaskFailureExceptionTest.php new file mode 100644 index 0000000..0e490bf --- /dev/null +++ b/test/Worker/TaskFailureExceptionTest.php @@ -0,0 +1,28 @@ + 'error_message_trace', + 'file' => 'file-name.php', + 'line' => 1, + 'args' => [], + ] + ]; + + $exception = new TaskFailureException('name', 'error_message', 0, $trace); + + $this->assertSame('name', $exception->getOriginalClassName()); + $this->assertSame('error_message', $exception->getOriginalMessage()); + $this->assertSame(0, $exception->getOriginalCode()); + $this->assertSame($trace, $exception->getOriginalTrace()); + } +}