From ae14385879e5b14d5dae94d5b30502a617ef6b06 Mon Sep 17 00:00:00 2001 From: Alessandro Lai Date: Thu, 5 Nov 2020 17:09:43 +0100 Subject: [PATCH] [Messenger] Fix JSON deserialization of ErrorDetailsStamp and normalization of FlattenException::$statusText --- .../AddErrorDetailsStampListener.php | 2 +- Stamp/ErrorDetailsStamp.php | 22 ++++++++++---- .../Command/FailedMessagesShowCommandTest.php | 10 +++---- .../AddErrorDetailsStampListenerTest.php | 6 ++-- Tests/Stamp/ErrorDetailsStampTest.php | 29 +++++++++++++++++-- .../FlattenExceptionNormalizerTest.php | 5 ++++ .../Normalizer/FlattenExceptionNormalizer.php | 2 ++ 7 files changed, 59 insertions(+), 17 deletions(-) diff --git a/EventListener/AddErrorDetailsStampListener.php b/EventListener/AddErrorDetailsStampListener.php index 4a9aaa1d..fd5c9d6f 100644 --- a/EventListener/AddErrorDetailsStampListener.php +++ b/EventListener/AddErrorDetailsStampListener.php @@ -19,7 +19,7 @@ final class AddErrorDetailsStampListener implements EventSubscriberInterface { public function onMessageFailed(WorkerMessageFailedEvent $event): void { - $stamp = new ErrorDetailsStamp($event->getThrowable()); + $stamp = ErrorDetailsStamp::create($event->getThrowable()); $previousStamp = $event->getEnvelope()->last(ErrorDetailsStamp::class); // Do not append duplicate information diff --git a/Stamp/ErrorDetailsStamp.php b/Stamp/ErrorDetailsStamp.php index e06c3adf..ae03de5a 100644 --- a/Stamp/ErrorDetailsStamp.php +++ b/Stamp/ErrorDetailsStamp.php @@ -32,19 +32,29 @@ final class ErrorDetailsStamp implements StampInterface /** @var FlattenException|null */ private $flattenException; - public function __construct(Throwable $throwable) + /** + * @param int|mixed $exceptionCode + */ + public function __construct(string $exceptionClass, $exceptionCode, string $exceptionMessage, FlattenException $flattenException = null) + { + $this->exceptionClass = $exceptionClass; + $this->exceptionCode = $exceptionCode; + $this->exceptionMessage = $exceptionMessage; + $this->flattenException = $flattenException; + } + + public static function create(Throwable $throwable): self { if ($throwable instanceof HandlerFailedException) { $throwable = $throwable->getPrevious(); } - $this->exceptionClass = \get_class($throwable); - $this->exceptionCode = $throwable->getCode(); - $this->exceptionMessage = $throwable->getMessage(); - + $flattenException = null; if (class_exists(FlattenException::class)) { - $this->flattenException = FlattenException::createFromThrowable($throwable); + $flattenException = FlattenException::createFromThrowable($throwable); } + + return new self(\get_class($throwable), $throwable->getCode(), $throwable->getMessage(), $flattenException); } public function getExceptionClass(): string diff --git a/Tests/Command/FailedMessagesShowCommandTest.php b/Tests/Command/FailedMessagesShowCommandTest.php index 0cbae9d5..6c6f110f 100644 --- a/Tests/Command/FailedMessagesShowCommandTest.php +++ b/Tests/Command/FailedMessagesShowCommandTest.php @@ -32,7 +32,7 @@ public function testBasicRun() { $sentToFailureStamp = new SentToFailureTransportStamp('async'); $redeliveryStamp = new RedeliveryStamp(0); - $errorStamp = new ErrorDetailsStamp(new \Exception('Things are bad!', 123)); + $errorStamp = ErrorDetailsStamp::create(new \Exception('Things are bad!', 123)); $envelope = new Envelope(new \stdClass(), [ new TransportMessageIdStamp(15), $sentToFailureStamp, @@ -69,7 +69,7 @@ public function testMultipleRedeliveryFails() { $sentToFailureStamp = new SentToFailureTransportStamp('async'); $redeliveryStamp1 = new RedeliveryStamp(0); - $errorStamp = new ErrorDetailsStamp(new \Exception('Things are bad!', 123)); + $errorStamp = ErrorDetailsStamp::create(new \Exception('Things are bad!', 123)); $redeliveryStamp2 = new RedeliveryStamp(0); $envelope = new Envelope(new \stdClass(), [ new TransportMessageIdStamp(15), @@ -154,7 +154,7 @@ public function testListMessages() { $sentToFailureStamp = new SentToFailureTransportStamp('async'); $redeliveryStamp = new RedeliveryStamp(0); - $errorStamp = new ErrorDetailsStamp(new \RuntimeException('Things are bad!')); + $errorStamp = ErrorDetailsStamp::create(new \RuntimeException('Things are bad!')); $envelope = new Envelope(new \stdClass(), [ new TransportMessageIdStamp(15), $sentToFailureStamp, @@ -201,7 +201,7 @@ public function testListMessagesReturnsPaginatedMessages() new TransportMessageIdStamp(15), $sentToFailureStamp, new RedeliveryStamp(0), - new ErrorDetailsStamp(new \RuntimeException('Things are bad!')), + ErrorDetailsStamp::create(new \RuntimeException('Things are bad!')), ]); $receiver = $this->createMock(ListableReceiverInterface::class); $receiver->expects($this->once())->method('all')->with()->willReturn([$envelope]); @@ -244,7 +244,7 @@ public function testVeryVerboseOutputForSingleMessageContainsExceptionWithTrace( new TransportMessageIdStamp(15), new SentToFailureTransportStamp('async'), new RedeliveryStamp(0), - new ErrorDetailsStamp($exception), + ErrorDetailsStamp::create($exception), ]); $receiver = $this->createMock(ListableReceiverInterface::class); $receiver->expects($this->once())->method('find')->with(42)->willReturn($envelope); diff --git a/Tests/EventListener/AddErrorDetailsStampListenerTest.php b/Tests/EventListener/AddErrorDetailsStampListenerTest.php index 0fb58973..cc01567a 100644 --- a/Tests/EventListener/AddErrorDetailsStampListenerTest.php +++ b/Tests/EventListener/AddErrorDetailsStampListenerTest.php @@ -17,7 +17,7 @@ public function testExceptionDetailsAreAdded(): void $envelope = new Envelope(new \stdClass()); $exception = new \Exception('It failed!'); $event = new WorkerMessageFailedEvent($envelope, 'my_receiver', $exception); - $expectedStamp = new ErrorDetailsStamp($exception); + $expectedStamp = ErrorDetailsStamp::create($exception); $listener->onMessageFailed($event); @@ -29,12 +29,12 @@ public function testWorkerAddsNewErrorDetailsStampOnFailure() $listener = new AddErrorDetailsStampListener(); $envelope = new Envelope(new \stdClass(), [ - new ErrorDetailsStamp(new \InvalidArgumentException('First error!')), + ErrorDetailsStamp::create(new \InvalidArgumentException('First error!')), ]); $exception = new \Exception('Second error!'); $event = new WorkerMessageFailedEvent($envelope, 'my_receiver', $exception); - $expectedStamp = new ErrorDetailsStamp($exception); + $expectedStamp = ErrorDetailsStamp::create($exception); $listener->onMessageFailed($event); diff --git a/Tests/Stamp/ErrorDetailsStampTest.php b/Tests/Stamp/ErrorDetailsStampTest.php index c6db1de1..98277931 100644 --- a/Tests/Stamp/ErrorDetailsStampTest.php +++ b/Tests/Stamp/ErrorDetailsStampTest.php @@ -16,6 +16,12 @@ use Symfony\Component\Messenger\Envelope; use Symfony\Component\Messenger\Exception\HandlerFailedException; use Symfony\Component\Messenger\Stamp\ErrorDetailsStamp; +use Symfony\Component\Messenger\Transport\Serialization\Normalizer\FlattenExceptionNormalizer; +use Symfony\Component\Messenger\Transport\Serialization\Serializer; +use Symfony\Component\Serializer\Encoder\JsonEncoder; +use Symfony\Component\Serializer\Normalizer\ArrayDenormalizer; +use Symfony\Component\Serializer\Normalizer\ObjectNormalizer; +use Symfony\Component\Serializer\Serializer as SymfonySerializer; class ErrorDetailsStampTest extends TestCase { @@ -24,7 +30,7 @@ public function testGetters(): void $exception = new \Exception('exception message'); $flattenException = FlattenException::createFromThrowable($exception); - $stamp = new ErrorDetailsStamp($exception); + $stamp = ErrorDetailsStamp::create($exception); $this->assertSame(\Exception::class, $stamp->getExceptionClass()); $this->assertSame('exception message', $stamp->getExceptionMessage()); @@ -38,11 +44,30 @@ public function testUnwrappingHandlerFailedException(): void $exception = new HandlerFailedException($envelope, [$wrappedException]); $flattenException = FlattenException::createFromThrowable($wrappedException); - $stamp = new ErrorDetailsStamp($exception); + $stamp = ErrorDetailsStamp::create($exception); $this->assertSame(\Exception::class, $stamp->getExceptionClass()); $this->assertSame('I am inside', $stamp->getExceptionMessage()); $this->assertSame(123, $stamp->getExceptionCode()); $this->assertEquals($flattenException, $stamp->getFlattenException()); } + + public function testDeserialization(): void + { + $exception = new \Exception('exception message'); + $stamp = ErrorDetailsStamp::create($exception); + $serializer = new Serializer( + new SymfonySerializer([ + new ArrayDenormalizer(), + new FlattenExceptionNormalizer(), + new ObjectNormalizer(), + ], [new JsonEncoder()]) + ); + + $deserializedEnvelope = $serializer->decode($serializer->encode(new Envelope(new \stdClass(), [$stamp]))); + + $deserializedStamp = $deserializedEnvelope->last(ErrorDetailsStamp::class); + $this->assertInstanceOf(ErrorDetailsStamp::class, $deserializedStamp); + $this->assertEquals($stamp, $deserializedStamp); + } } diff --git a/Tests/Transport/Serialization/Normalizer/FlattenExceptionNormalizerTest.php b/Tests/Transport/Serialization/Normalizer/FlattenExceptionNormalizerTest.php index e73d7b26..971b0c48 100644 --- a/Tests/Transport/Serialization/Normalizer/FlattenExceptionNormalizerTest.php +++ b/Tests/Transport/Serialization/Normalizer/FlattenExceptionNormalizerTest.php @@ -60,6 +60,7 @@ public function testNormalize(FlattenException $exception) $this->assertSame($previous, $normalized['previous']); $this->assertSame($exception->getTrace(), $normalized['trace']); $this->assertSame($exception->getTraceAsString(), $normalized['trace_as_string']); + $this->assertSame($exception->getStatusText(), $normalized['status_text']); } public function provideFlattenException(): array @@ -95,6 +96,7 @@ public function testDenormalizeValidData() 'file' => 'foo.php', 'line' => 123, 'headers' => ['Content-Type' => 'application/json'], + 'status_text' => 'Whoops, looks like something went wrong.', 'trace' => [ [ 'namespace' => '', 'short_class' => '', 'class' => '', 'type' => '', 'function' => '', 'file' => 'foo.php', 'line' => 123, 'args' => [], @@ -108,6 +110,7 @@ public function testDenormalizeValidData() ], ], 'trace_as_string' => '#0 foo.php(123): foo()'.\PHP_EOL.'#1 bar.php(456): bar()', + 'status_text' => 'Whoops, looks like something went wrong.', ]; $exception = $this->normalizer->denormalize($normalized, FlattenException::class); @@ -121,10 +124,12 @@ public function testDenormalizeValidData() $this->assertSame($normalized['line'], $exception->getLine()); $this->assertSame($normalized['trace'], $exception->getTrace()); $this->assertSame($normalized['trace_as_string'], $exception->getTraceAsString()); + $this->assertSame($normalized['status_text'], $exception->getStatusText()); $this->assertInstanceOf(FlattenException::class, $previous = $exception->getPrevious()); $this->assertSame($normalized['previous']['message'], $previous->getMessage()); $this->assertSame($normalized['previous']['code'], $previous->getCode()); + $this->assertSame($normalized['previous']['status_text'], $previous->getStatusText()); } private function getMessengerContext(): array diff --git a/Transport/Serialization/Normalizer/FlattenExceptionNormalizer.php b/Transport/Serialization/Normalizer/FlattenExceptionNormalizer.php index 6ee46c05..344eea7c 100644 --- a/Transport/Serialization/Normalizer/FlattenExceptionNormalizer.php +++ b/Transport/Serialization/Normalizer/FlattenExceptionNormalizer.php @@ -42,6 +42,7 @@ public function normalize($object, $format = null, array $context = []) 'file' => $object->getFile(), 'line' => $object->getLine(), 'previous' => null === $object->getPrevious() ? null : $this->normalize($object->getPrevious(), $format, $context), + 'status_text' => $object->getStatusText(), 'trace' => $object->getTrace(), 'trace_as_string' => $object->getTraceAsString(), ]; @@ -73,6 +74,7 @@ public function denormalize($data, $type, $format = null, array $context = []) $object->setClass($data['class']); $object->setFile($data['file']); $object->setLine($data['line']); + $object->setStatusText($data['status_text']); $object->setHeaders((array) $data['headers']); if (isset($data['previous'])) {