[ErrorHandler] Serialize FlattenException #38800
Conversation
| { | ||
| $this->trace = []; | ||
| if ($file) |
There was a problem hiding this comment.
I had to leave it after 2 hours that late last night.
I would like to get some feedback from @yceruto and/or someone that knows about property access.
Also, is it a good idea to modify a class "just because" serialization?
| @@ -275,19 +275,22 @@ public function setTraceFromThrowable(\Throwable $throwable): self | |||
| /** | |||
| * @return $this | |||
| */ | |||
| public function setTrace($trace, $file, $line): self | |||
| public function setTrace($trace, $file = null, $line = null): self | |||
There was a problem hiding this comment.
Isn't this a BC break for inheriting classes? Any other idea?
| /** @var \ReflectionParameter $parameter */ | ||
| $parameter = $method->getParameters()[0]; | ||
| if (\array_key_exists('value', $context) && null === $context['value'] && !$parameter->allowsNull()) { | ||
| $errors[] = sprintf('The method "%s" in class "%s" was found but does not allow null.', $methodName, $class); |
There was a problem hiding this comment.
This breaks PropertyAccess test suite (see Travis's failure)
7e170c5 to
9686df0
Compare
There was a problem hiding this comment.
Hey! As far as I understand, this FlattenException should be handled by Symfony\Component\Messenger\Transport\Serialization\Normalizer\FlattenExceptionNormalizer and Symfony\Component\Messenger\Transport\Serialization\Serializer when the Messenger component is installed.
I did some tests using this code and it works without any other change:
use Symfony\Component\Messenger\Transport\Serialization\Normalizer\FlattenExceptionNormalizer;
use Symfony\Component\Messenger\Transport\Serialization\Serializer as MessengerSerializer;
// ...
public function testSerialize()
{
$serializer = new Serializer([new FlattenExceptionNormalizer()], [new JsonEncoder()]);
$context = [MessengerSerializer::MESSENGER_SERIALIZATION_CONTEXT => true];
$flattened = FlattenException::createFromThrowable(new \LogicException('Bad things happened'));
$trace = $flattened->getTraceAsString();
$data = $serializer->serialize($flattened, 'json', $context);
$restored = $serializer->deserialize($data, FlattenException::class, 'json', $context);
$restoredTrace = $restored->getTraceAsString();
$this->assertSame(substr($trace, 0, 100), substr($restoredTrace, 0, 100));
$this->assertSame(\count(explode(\PHP_EOL, $trace)), \count(explode(\PHP_EOL, $restoredTrace)));
}The matter is that this FlattenExceptionNormalizer doesn't exist in 5.1, but since 5.2 #37087, and the related issue is talking about this error on 5.1, but also it happens in 4.3 #32719
By the way, I can't reproduce the issue on 5.2:
Exception:
==========
(no data)
Maybe I'm missing something, but it seems AddErrorDetailsStampListener is not registered by default, hence the ErrorDetailsStamp is not added.
|
Thank you for the explanation. So we are closing this as "wontfix" for 4.4?
That is true. I though that was intentional... but Im not sure. |
Well, it's possible to fix it if we use
Ok it's being fixed here #38941. |
|
Im happy with closing this issue then. |
The
FlatternExceptionis used with messenger. When it is serialized with the Serializer component we fail to restore all its properties.When
FlatternException::$traceAsStringis null and we callFlatternException::getTraceAsString()we get an exception thrown in our face. That is what #38792 is about.This PR does two things (sorry about that).
It modifies
FlatternException::setTrace()so the serializer can populate the trace property when unserializing. I also try to rebuild the traceAsString from the$traceIt modifies the PropertyAccess and PropertyInfo component to be aware of nullable setters. Ie, It finds
FlatternException::setPrevious(), but that function does not allow nullable parameters, but it still try to set the valuenulland we get another exception thrown in our face.