From 64cc693d1c03e535e7d1c8f439b3608ee9721cc1 Mon Sep 17 00:00:00 2001 From: David Badura Date: Fri, 2 Feb 2024 15:50:01 +0100 Subject: [PATCH 1/3] improve projection errors and traces --- phpstan-baseline.neon | 17 ++++- .../Projection/Store/ErrorContext.php | 34 ++++++++- .../Projection/ErrorContextTest.php | 72 +++++++++++++++++++ 3 files changed, 120 insertions(+), 3 deletions(-) create mode 100644 tests/Unit/Projection/Projection/ErrorContextTest.php diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index bcb0f598f..f20cbde25 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -16,7 +16,7 @@ parameters: path: src/Projection/Projection/ProjectionError.php - - message: "#^Parameter \\#2 \\$errorContext of class Patchlevel\\\\EventSourcing\\\\Projection\\\\Projection\\\\ProjectionError constructor expects array\\\\}\\>\\|null, mixed given\\.$#" + message: "#^Parameter \\#2 \\$errorContext of class Patchlevel\\\\EventSourcing\\\\Projection\\\\Projection\\\\ProjectionError constructor expects array\\\\}\\>\\|null, mixed given\\.$#" count: 1 path: src/Projection/Projection/Store/DoctrineStore.php @@ -30,6 +30,21 @@ parameters: count: 1 path: src/Projection/Projection/Store/ErrorContext.php + - + message: "#^Method Patchlevel\\\\EventSourcing\\\\Projection\\\\Projection\\\\Store\\\\ErrorContext\\:\\:transformTrace\\(\\) has parameter \\$trace with no value type specified in iterable type array\\.$#" + count: 1 + path: src/Projection/Projection/Store/ErrorContext.php + + - + message: "#^Method Patchlevel\\\\EventSourcing\\\\Projection\\\\Projection\\\\Store\\\\ErrorContext\\:\\:transformTrace\\(\\) return type has no value type specified in iterable type array\\.$#" + count: 1 + path: src/Projection/Projection/Store/ErrorContext.php + + - + message: "#^PHPDoc tag @var for variable \\$trace has no value type specified in iterable type array\\.$#" + count: 1 + path: src/Projection/Projection/Store/ErrorContext.php + - message: "#^Method Patchlevel\\\\EventSourcing\\\\Projection\\\\Projector\\\\InMemoryProjectorRepository\\:\\:projectors\\(\\) should return array\\ but returns array\\\\.$#" count: 1 diff --git a/src/Projection/Projection/Store/ErrorContext.php b/src/Projection/Projection/Store/ErrorContext.php index d4bb13962..0ed4cd046 100644 --- a/src/Projection/Projection/Store/ErrorContext.php +++ b/src/Projection/Projection/Store/ErrorContext.php @@ -6,9 +6,15 @@ use Throwable; +use function array_walk_recursive; +use function get_resource_type; +use function is_object; +use function is_resource; +use function sprintf; + /** * @psalm-type Trace = array{file?: string, line?: int, function?: string, class?: string, type?: string, args?: array} - * @psalm-type Context = array{message: string, code: int|string, file: string, line: int, trace: list} + * @psalm-type Context = array{class: class-string, message: string, code: int|string, file: string, line: int, trace: list} */ final class ErrorContext { @@ -29,11 +35,35 @@ public static function fromThrowable(Throwable $error): array private static function transformThrowable(Throwable $error): array { return [ + 'class' => $error::class, 'message' => $error->getMessage(), 'code' => $error->getCode(), 'file' => $error->getFile(), 'line' => $error->getLine(), - 'trace' => $error->getTrace(), + 'trace' => self::transformTrace($error->getTrace()), ]; } + + /** + * @param list $trace + * + * @return list + */ + private static function transformTrace(array $trace): array + { + array_walk_recursive($trace, static function (mixed &$value): void { + if (is_object($value)) { + $value = sprintf('object(%s)', $value::class); + } + + if (!is_resource($value)) { + return; + } + + $value = sprintf('resource(%s)', get_resource_type($value)); + }); + + /** @var list $trace */ + return $trace; + } } diff --git a/tests/Unit/Projection/Projection/ErrorContextTest.php b/tests/Unit/Projection/Projection/ErrorContextTest.php new file mode 100644 index 000000000..5dc596753 --- /dev/null +++ b/tests/Unit/Projection/Projection/ErrorContextTest.php @@ -0,0 +1,72 @@ +createException( + 'test', + new CustomId('test'), + $resource, + ['test' => [1, 2, 3]], + static fn () => null, + ), + ); + fclose($resource); + + $this->assertCount(1, $result); + $error = $result[0]; + + $this->assertSame(RuntimeException::class, $error['class']); + $this->assertSame('test', $error['message']); + $this->assertSame(0, $error['code']); + $this->assertSame(__FILE__, $error['file']); + $this->assertGreaterThan(0, count($error['trace'])); + $this->assertArrayHasKey(0, $error['trace']); + + $firstTrace = $error['trace'][0]; + + $this->assertArrayHasKey('file', $firstTrace); + $this->assertSame(__FILE__, $firstTrace['file'] ?? null); + $this->assertArrayHasKey('line', $firstTrace); + $this->assertSame('createException', $firstTrace['function'] ?? null); + $this->assertArrayHasKey('class', $firstTrace); + $this->assertSame(self::class, $firstTrace['class'] ?? null); + $this->assertArrayHasKey('type', $firstTrace); + $this->assertSame('->', $firstTrace['type'] ?? null); + $this->assertArrayHasKey('args', $firstTrace); + $this->assertSame([ + 'test', + 'object(Patchlevel\EventSourcing\Aggregate\CustomId)', + 'resource(stream)', + ['test' => [1, 2, 3]], + 'object(Closure)', + ], $firstTrace['args'] ?? null); + } + + /** @param resource $resource */ + private function createException( + string $message, + CustomId $id, + $resource, + array $array, + callable $callable, + ): RuntimeException { + return new RuntimeException($message); + } +} From e4c2981ccab942fc53661fc95359a4642a2f51d8 Mon Sep 17 00:00:00 2001 From: David Badura Date: Fri, 2 Feb 2024 16:01:49 +0100 Subject: [PATCH 2/3] fix ini settings for unit tests --- phpunit.xml.dist | 1 + 1 file changed, 1 insertion(+) diff --git a/phpunit.xml.dist b/phpunit.xml.dist index bb45b9abb..000d0cb80 100644 --- a/phpunit.xml.dist +++ b/phpunit.xml.dist @@ -29,6 +29,7 @@ + From 8c8f69161d037a509624b5dc90713ae81dd566ca Mon Sep 17 00:00:00 2001 From: David Badura Date: Fri, 2 Feb 2024 16:22:45 +0100 Subject: [PATCH 3/3] fix transformTrace --- phpstan-baseline.neon | 35 ------------------- .../Projection/Store/ErrorContext.php | 20 +++++++---- .../Projection/ErrorContextTest.php | 5 ++- 3 files changed, 18 insertions(+), 42 deletions(-) diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index f20cbde25..ce6b1d33a 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -1,50 +1,15 @@ parameters: ignoreErrors: - - - message: "#^Method Patchlevel\\\\EventSourcing\\\\Console\\\\Command\\\\ProjectionStatusCommand\\:\\:displayError\\(\\) has parameter \\$context with no value type specified in iterable type array\\.$#" - count: 1 - path: src/Console/Command/ProjectionStatusCommand.php - - message: "#^Method Patchlevel\\\\EventSourcing\\\\EventBus\\\\Message\\:\\:headers\\(\\) should return array\\{aggregateClass\\?\\: class\\-string\\, aggregateId\\?\\: string, playhead\\?\\: int\\<1, max\\>, recordedOn\\?\\: DateTimeImmutable, newStreamStart\\?\\: bool, archived\\?\\: bool\\} but returns non\\-empty\\-array\\\\.$#" count: 1 path: src/EventBus/Message.php - - - message: "#^Method Patchlevel\\\\EventSourcing\\\\Projection\\\\Projection\\\\ProjectionError\\:\\:__construct\\(\\) has parameter \\$errorContext with no value type specified in iterable type array\\.$#" - count: 1 - path: src/Projection/Projection/ProjectionError.php - - message: "#^Parameter \\#2 \\$errorContext of class Patchlevel\\\\EventSourcing\\\\Projection\\\\Projection\\\\ProjectionError constructor expects array\\\\}\\>\\|null, mixed given\\.$#" count: 1 path: src/Projection/Projection/Store/DoctrineStore.php - - - message: "#^Method Patchlevel\\\\EventSourcing\\\\Projection\\\\Projection\\\\Store\\\\ErrorContext\\:\\:fromThrowable\\(\\) return type has no value type specified in iterable type array\\.$#" - count: 1 - path: src/Projection/Projection/Store/ErrorContext.php - - - - message: "#^Method Patchlevel\\\\EventSourcing\\\\Projection\\\\Projection\\\\Store\\\\ErrorContext\\:\\:transformThrowable\\(\\) return type has no value type specified in iterable type array\\.$#" - count: 1 - path: src/Projection/Projection/Store/ErrorContext.php - - - - message: "#^Method Patchlevel\\\\EventSourcing\\\\Projection\\\\Projection\\\\Store\\\\ErrorContext\\:\\:transformTrace\\(\\) has parameter \\$trace with no value type specified in iterable type array\\.$#" - count: 1 - path: src/Projection/Projection/Store/ErrorContext.php - - - - message: "#^Method Patchlevel\\\\EventSourcing\\\\Projection\\\\Projection\\\\Store\\\\ErrorContext\\:\\:transformTrace\\(\\) return type has no value type specified in iterable type array\\.$#" - count: 1 - path: src/Projection/Projection/Store/ErrorContext.php - - - - message: "#^PHPDoc tag @var for variable \\$trace has no value type specified in iterable type array\\.$#" - count: 1 - path: src/Projection/Projection/Store/ErrorContext.php - - message: "#^Method Patchlevel\\\\EventSourcing\\\\Projection\\\\Projector\\\\InMemoryProjectorRepository\\:\\:projectors\\(\\) should return array\\ but returns array\\\\.$#" count: 1 diff --git a/src/Projection/Projection/Store/ErrorContext.php b/src/Projection/Projection/Store/ErrorContext.php index 0ed4cd046..7fa462978 100644 --- a/src/Projection/Projection/Store/ErrorContext.php +++ b/src/Projection/Projection/Store/ErrorContext.php @@ -6,6 +6,8 @@ use Throwable; +use function array_key_exists; +use function array_map; use function array_walk_recursive; use function get_resource_type; use function is_object; @@ -13,7 +15,7 @@ use function sprintf; /** - * @psalm-type Trace = array{file?: string, line?: int, function?: string, class?: string, type?: string, args?: array} + * @psalm-type Trace = array{file?: string, line?: int, function?: string, class?: string, type?: string, args?: array} * @psalm-type Context = array{class: class-string, message: string, code: int|string, file: string, line: int, trace: list} */ final class ErrorContext @@ -34,24 +36,31 @@ public static function fromThrowable(Throwable $error): array /** @return Context */ private static function transformThrowable(Throwable $error): array { + /** @var list $traces */ + $traces = $error->getTrace(); + return [ 'class' => $error::class, 'message' => $error->getMessage(), 'code' => $error->getCode(), 'file' => $error->getFile(), 'line' => $error->getLine(), - 'trace' => self::transformTrace($error->getTrace()), + 'trace' => array_map(self::transformTrace(...), $traces), ]; } /** - * @param list $trace + * @param Trace $trace * - * @return list + * @return Trace */ private static function transformTrace(array $trace): array { - array_walk_recursive($trace, static function (mixed &$value): void { + if (!array_key_exists('args', $trace)) { + return $trace; + } + + array_walk_recursive($trace['args'], static function (mixed &$value): void { if (is_object($value)) { $value = sprintf('object(%s)', $value::class); } @@ -63,7 +72,6 @@ private static function transformTrace(array $trace): array $value = sprintf('resource(%s)', get_resource_type($value)); }); - /** @var list $trace */ return $trace; } } diff --git a/tests/Unit/Projection/Projection/ErrorContextTest.php b/tests/Unit/Projection/Projection/ErrorContextTest.php index 5dc596753..b81d58bec 100644 --- a/tests/Unit/Projection/Projection/ErrorContextTest.php +++ b/tests/Unit/Projection/Projection/ErrorContextTest.php @@ -59,7 +59,10 @@ public function testErrorContext(): void ], $firstTrace['args'] ?? null); } - /** @param resource $resource */ + /** + * @param resource $resource + * @param array $array + */ private function createException( string $message, CustomId $id,