bug #39004 [Messenger] Fix JSON deserialization of ErrorDetailsStamp and normalization of FlattenException::$statusText (Jean85)

This PR was squashed before being merged into the 5.2-dev branch.

Discussion
----------

[Messenger] Fix JSON deserialization of ErrorDetailsStamp and normalization of FlattenException::$statusText

| Q             | A
| ------------- | ---
| Branch?       | 5.x (bugfix of a 5.x-only feature)
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Tickets       | Fix #39003
| License       | MIT

~WIP~ This is now complete and, thanks to @yceruto, I've fixed two bugs in this PR:
 * `ErrorDetailsStamp` couldn't be (de)serialized properly with that constructor argument
 * `FlattenException::$statusText` wasn't (de)normalized

Commits
-------

9af554cb41 [Messenger] Fix JSON deserialization of ErrorDetailsStamp and normalization of FlattenException::$statusText
This commit is contained in:
Fabien Potencier 2020-11-10 07:17:29 +01:00
commit c721e025ea
7 changed files with 59 additions and 17 deletions

View File

@ -19,7 +19,7 @@ final class AddErrorDetailsStampListener implements EventSubscriberInterface
{ {
public function onMessageFailed(WorkerMessageFailedEvent $event): void public function onMessageFailed(WorkerMessageFailedEvent $event): void
{ {
$stamp = new ErrorDetailsStamp($event->getThrowable()); $stamp = ErrorDetailsStamp::create($event->getThrowable());
$previousStamp = $event->getEnvelope()->last(ErrorDetailsStamp::class); $previousStamp = $event->getEnvelope()->last(ErrorDetailsStamp::class);
// Do not append duplicate information // Do not append duplicate information

View File

@ -32,19 +32,29 @@ final class ErrorDetailsStamp implements StampInterface
/** @var FlattenException|null */ /** @var FlattenException|null */
private $flattenException; 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) { if ($throwable instanceof HandlerFailedException) {
$throwable = $throwable->getPrevious(); $throwable = $throwable->getPrevious();
} }
$this->exceptionClass = \get_class($throwable); $flattenException = null;
$this->exceptionCode = $throwable->getCode();
$this->exceptionMessage = $throwable->getMessage();
if (class_exists(FlattenException::class)) { 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 public function getExceptionClass(): string

View File

@ -32,7 +32,7 @@ class FailedMessagesShowCommandTest extends TestCase
{ {
$sentToFailureStamp = new SentToFailureTransportStamp('async'); $sentToFailureStamp = new SentToFailureTransportStamp('async');
$redeliveryStamp = new RedeliveryStamp(0); $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(), [ $envelope = new Envelope(new \stdClass(), [
new TransportMessageIdStamp(15), new TransportMessageIdStamp(15),
$sentToFailureStamp, $sentToFailureStamp,
@ -69,7 +69,7 @@ EOF
{ {
$sentToFailureStamp = new SentToFailureTransportStamp('async'); $sentToFailureStamp = new SentToFailureTransportStamp('async');
$redeliveryStamp1 = new RedeliveryStamp(0); $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); $redeliveryStamp2 = new RedeliveryStamp(0);
$envelope = new Envelope(new \stdClass(), [ $envelope = new Envelope(new \stdClass(), [
new TransportMessageIdStamp(15), new TransportMessageIdStamp(15),
@ -154,7 +154,7 @@ EOF
{ {
$sentToFailureStamp = new SentToFailureTransportStamp('async'); $sentToFailureStamp = new SentToFailureTransportStamp('async');
$redeliveryStamp = new RedeliveryStamp(0); $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(), [ $envelope = new Envelope(new \stdClass(), [
new TransportMessageIdStamp(15), new TransportMessageIdStamp(15),
$sentToFailureStamp, $sentToFailureStamp,
@ -201,7 +201,7 @@ EOF
new TransportMessageIdStamp(15), new TransportMessageIdStamp(15),
$sentToFailureStamp, $sentToFailureStamp,
new RedeliveryStamp(0), new RedeliveryStamp(0),
new ErrorDetailsStamp(new \RuntimeException('Things are bad!')), ErrorDetailsStamp::create(new \RuntimeException('Things are bad!')),
]); ]);
$receiver = $this->createMock(ListableReceiverInterface::class); $receiver = $this->createMock(ListableReceiverInterface::class);
$receiver->expects($this->once())->method('all')->with()->willReturn([$envelope]); $receiver->expects($this->once())->method('all')->with()->willReturn([$envelope]);
@ -244,7 +244,7 @@ EOF
new TransportMessageIdStamp(15), new TransportMessageIdStamp(15),
new SentToFailureTransportStamp('async'), new SentToFailureTransportStamp('async'),
new RedeliveryStamp(0), new RedeliveryStamp(0),
new ErrorDetailsStamp($exception), ErrorDetailsStamp::create($exception),
]); ]);
$receiver = $this->createMock(ListableReceiverInterface::class); $receiver = $this->createMock(ListableReceiverInterface::class);
$receiver->expects($this->once())->method('find')->with(42)->willReturn($envelope); $receiver->expects($this->once())->method('find')->with(42)->willReturn($envelope);

View File

@ -17,7 +17,7 @@ final class AddErrorDetailsStampListenerTest extends TestCase
$envelope = new Envelope(new \stdClass()); $envelope = new Envelope(new \stdClass());
$exception = new \Exception('It failed!'); $exception = new \Exception('It failed!');
$event = new WorkerMessageFailedEvent($envelope, 'my_receiver', $exception); $event = new WorkerMessageFailedEvent($envelope, 'my_receiver', $exception);
$expectedStamp = new ErrorDetailsStamp($exception); $expectedStamp = ErrorDetailsStamp::create($exception);
$listener->onMessageFailed($event); $listener->onMessageFailed($event);
@ -29,12 +29,12 @@ final class AddErrorDetailsStampListenerTest extends TestCase
$listener = new AddErrorDetailsStampListener(); $listener = new AddErrorDetailsStampListener();
$envelope = new Envelope(new \stdClass(), [ $envelope = new Envelope(new \stdClass(), [
new ErrorDetailsStamp(new \InvalidArgumentException('First error!')), ErrorDetailsStamp::create(new \InvalidArgumentException('First error!')),
]); ]);
$exception = new \Exception('Second error!'); $exception = new \Exception('Second error!');
$event = new WorkerMessageFailedEvent($envelope, 'my_receiver', $exception); $event = new WorkerMessageFailedEvent($envelope, 'my_receiver', $exception);
$expectedStamp = new ErrorDetailsStamp($exception); $expectedStamp = ErrorDetailsStamp::create($exception);
$listener->onMessageFailed($event); $listener->onMessageFailed($event);

View File

@ -16,6 +16,12 @@ use Symfony\Component\ErrorHandler\Exception\FlattenException;
use Symfony\Component\Messenger\Envelope; use Symfony\Component\Messenger\Envelope;
use Symfony\Component\Messenger\Exception\HandlerFailedException; use Symfony\Component\Messenger\Exception\HandlerFailedException;
use Symfony\Component\Messenger\Stamp\ErrorDetailsStamp; 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 class ErrorDetailsStampTest extends TestCase
{ {
@ -24,7 +30,7 @@ class ErrorDetailsStampTest extends TestCase
$exception = new \Exception('exception message'); $exception = new \Exception('exception message');
$flattenException = FlattenException::createFromThrowable($exception); $flattenException = FlattenException::createFromThrowable($exception);
$stamp = new ErrorDetailsStamp($exception); $stamp = ErrorDetailsStamp::create($exception);
$this->assertSame(\Exception::class, $stamp->getExceptionClass()); $this->assertSame(\Exception::class, $stamp->getExceptionClass());
$this->assertSame('exception message', $stamp->getExceptionMessage()); $this->assertSame('exception message', $stamp->getExceptionMessage());
@ -38,11 +44,30 @@ class ErrorDetailsStampTest extends TestCase
$exception = new HandlerFailedException($envelope, [$wrappedException]); $exception = new HandlerFailedException($envelope, [$wrappedException]);
$flattenException = FlattenException::createFromThrowable($wrappedException); $flattenException = FlattenException::createFromThrowable($wrappedException);
$stamp = new ErrorDetailsStamp($exception); $stamp = ErrorDetailsStamp::create($exception);
$this->assertSame(\Exception::class, $stamp->getExceptionClass()); $this->assertSame(\Exception::class, $stamp->getExceptionClass());
$this->assertSame('I am inside', $stamp->getExceptionMessage()); $this->assertSame('I am inside', $stamp->getExceptionMessage());
$this->assertSame(123, $stamp->getExceptionCode()); $this->assertSame(123, $stamp->getExceptionCode());
$this->assertEquals($flattenException, $stamp->getFlattenException()); $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);
}
} }

View File

@ -60,6 +60,7 @@ class FlattenExceptionNormalizerTest extends TestCase
$this->assertSame($previous, $normalized['previous']); $this->assertSame($previous, $normalized['previous']);
$this->assertSame($exception->getTrace(), $normalized['trace']); $this->assertSame($exception->getTrace(), $normalized['trace']);
$this->assertSame($exception->getTraceAsString(), $normalized['trace_as_string']); $this->assertSame($exception->getTraceAsString(), $normalized['trace_as_string']);
$this->assertSame($exception->getStatusText(), $normalized['status_text']);
} }
public function provideFlattenException(): array public function provideFlattenException(): array
@ -95,6 +96,7 @@ class FlattenExceptionNormalizerTest extends TestCase
'file' => 'foo.php', 'file' => 'foo.php',
'line' => 123, 'line' => 123,
'headers' => ['Content-Type' => 'application/json'], 'headers' => ['Content-Type' => 'application/json'],
'status_text' => 'Whoops, looks like something went wrong.',
'trace' => [ 'trace' => [
[ [
'namespace' => '', 'short_class' => '', 'class' => '', 'type' => '', 'function' => '', 'file' => 'foo.php', 'line' => 123, 'args' => [], 'namespace' => '', 'short_class' => '', 'class' => '', 'type' => '', 'function' => '', 'file' => 'foo.php', 'line' => 123, 'args' => [],
@ -108,6 +110,7 @@ class FlattenExceptionNormalizerTest extends TestCase
], ],
], ],
'trace_as_string' => '#0 foo.php(123): foo()'.\PHP_EOL.'#1 bar.php(456): bar()', '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); $exception = $this->normalizer->denormalize($normalized, FlattenException::class);
@ -121,10 +124,12 @@ class FlattenExceptionNormalizerTest extends TestCase
$this->assertSame($normalized['line'], $exception->getLine()); $this->assertSame($normalized['line'], $exception->getLine());
$this->assertSame($normalized['trace'], $exception->getTrace()); $this->assertSame($normalized['trace'], $exception->getTrace());
$this->assertSame($normalized['trace_as_string'], $exception->getTraceAsString()); $this->assertSame($normalized['trace_as_string'], $exception->getTraceAsString());
$this->assertSame($normalized['status_text'], $exception->getStatusText());
$this->assertInstanceOf(FlattenException::class, $previous = $exception->getPrevious()); $this->assertInstanceOf(FlattenException::class, $previous = $exception->getPrevious());
$this->assertSame($normalized['previous']['message'], $previous->getMessage()); $this->assertSame($normalized['previous']['message'], $previous->getMessage());
$this->assertSame($normalized['previous']['code'], $previous->getCode()); $this->assertSame($normalized['previous']['code'], $previous->getCode());
$this->assertSame($normalized['previous']['status_text'], $previous->getStatusText());
} }
private function getMessengerContext(): array private function getMessengerContext(): array

View File

@ -42,6 +42,7 @@ final class FlattenExceptionNormalizer implements DenormalizerInterface, Context
'file' => $object->getFile(), 'file' => $object->getFile(),
'line' => $object->getLine(), 'line' => $object->getLine(),
'previous' => null === $object->getPrevious() ? null : $this->normalize($object->getPrevious(), $format, $context), 'previous' => null === $object->getPrevious() ? null : $this->normalize($object->getPrevious(), $format, $context),
'status_text' => $object->getStatusText(),
'trace' => $object->getTrace(), 'trace' => $object->getTrace(),
'trace_as_string' => $object->getTraceAsString(), 'trace_as_string' => $object->getTraceAsString(),
]; ];
@ -73,6 +74,7 @@ final class FlattenExceptionNormalizer implements DenormalizerInterface, Context
$object->setClass($data['class']); $object->setClass($data['class']);
$object->setFile($data['file']); $object->setFile($data['file']);
$object->setLine($data['line']); $object->setLine($data['line']);
$object->setStatusText($data['status_text']);
$object->setHeaders((array) $data['headers']); $object->setHeaders((array) $data['headers']);
if (isset($data['previous'])) { if (isset($data['previous'])) {