The failing test on 7.3 is due to the fact that Travis loads the deps from4.4.x-dev and can't find the new class there, so I guess that's to be expected.

What withtraceAsString field? Is it properly serialized/deserialized?

@mleczakm That's fixed. Nice catch.

This fix would also help in#33049

Wouldn't it better to serialize the error following/extending the formats defined by RFC 7807 or Hydra (as we do for validation errors:#22150)?
I feel like we're duplicating logic introduced in the new ErrorRenderer component.

Maybe that a better approach would be to:

• convert the error renderers as simple normalizers, in the serializer component (as we've done for validation errors)
• use the Serializer component as a dependency of the new ErrorHandler component
• remove the ErrorRenderer component (it's not too late, it hasn't been tagged yet)

There are several benefits to this approach:

• this use case with Messenger is fixed
• API Platform will automatically support the new ErrorHandler component (API Platform already has normalizers for\Exception and\FlattenException for most modern JSON error formats including RFC 7807, Hydra and JSON:API)
• it removes yet another component dealing with serialization

At least, the error renderers should implement Serializer's interfaces (and these interfaces should probably moved tosymfony/contracts).

/cc@yceruto

@dunglas I'm not sure I follow how you'd expect the same set of classes to satisfy the needs of both Messenger and ErrorRenderer as the use cases are totally different.

Also, this PR is about fixing bugs in 4.3, so anything that involves components that haven't been released yet probably won't apply.

• In Messenger, objects have to be serialized then deserialized with no loss of information (this PR is actually more about fixing deserialization than serialization).
• For error reporting, serialization is a one-way street, potentially destructive, and context dependent (i.e: AFAIK the behaviour of both the api-platform normalizers and ErrorRenderer depend on the value ofdebug).

As for following RFC 7807 more closely, I'm all for it. However as some fields in the RFC only make sense in an HTTP context, there's not much we could do I guess. Would you have anything else than the following in mind ?

• type,title andinstance will have to be omitted (that's ok if I understand the RFC)
• Renamestatus_code tostatus
• Renamemessage todetail

@dunglas I failed to see as well the approach described and its relationship with this PR + ErrorRenderer component in 4.4.

The ErrorRenderer component is not (and should not be) concerned with the full normalization of the FlattenException object or its denormalization at all, but standardized error responses in different formats, included the HTML error/exception page. It's more like (https://github.com/filp/whoops)

I don't know how all those features would fit into the Serializer component instead.

@skalpa

the needs of both Messenger and ErrorRenderer as the use cases are totally different

They don't look that different to me. Also, the benefit of using JSON with Messenger (instead of the native PHPserialize() format) is to be able to consume the messages with other programming languages. It's exactly there are standards for error representations. And converting an internal data structure (such as\Exception) to a public (or even better, standard) representation that can be consumed by another system is exactly the responsibility of the Serialize component.

In Messenger, objects have to be serialized then deserialized with no loss of information

It can be added to extra keys, as done by API Platform (such information could also be useful for developers trying to debug). Seehttps://github.com/api-platform/core/blob/master/src/Hydra/Serializer/ErrorNormalizer.php#L58-L60 for instance.

For error reporting, serialization is a one-way street

Normalizers in API Platform currently only support normalization, but there are no technical reasons preventing to support deserialization.

potentially destructive, and context dependent (i.e: AFAIK the behaviour of both the api-platform normalizers and ErrorRenderer depend on the value of debug).

We have all the infrastructure we need in the Serializer to deal with this. An option could be passed to the context (and we can now set default context options easily) to choose if debug informations for instance should be serialized or not.

As for following RFC 7807 more closely, I'm all for it. However as some fields in the RFC only make sense in an HTTP context, there's not much we could do I guess. Would you have anything else than the following in mind ?

Great. I indeed had something like that in mind.

@yceruto

but standardized error responses in different formats

For instance, in API Platform we already support generating standard error response for RFC 7807, JSON:API and Hydra using normalizers.

• https://github.com/api-platform/core/blob/master/src/Problem/Serializer/ErrorNormalizer.php
• https://github.com/api-platform/core/blob/master/src/Hydra/Serializer/ErrorNormalizer.php
• https://github.com/api-platform/core/blob/master/src/JsonApi/Serializer/ErrorNormalizer.php

If the ErrorHandler component could use any class implementing theSerializerInterface or (theNormalizerInterface), it would allow to reuse those normalizers very easily. Also, FOSRest comes with similar normalizers, and I'm such we can find others. To me it's the responsibility of the Serializer to transform a PHP object (\Exception here) in a standard open format (RFC 7807...).

included the HTML error/exception page. It's more like (https://github.com/filp/whoops)

For (advanced) HTML like this agree. Creating something like whoops is totally out of scope of the Symfony Serializer and should stay in ErrorRenderer. However, it's not the same case for non-HTML renderers. They should be moved to the Serializer component.
Even if it doesn't belong to the Serializer component, the HTML component could also easily implement theSerializerInterface (it still converts an \Exception to a string).

@dunglas I renamedstatus_code andmessage as discussed. As the RFC states thatstatus should be a number if present, it is not in the normalized array if the exception has no status code set.

Regarding your other points, I now understand your point of view better, thank you. However, I believe this is a different discussion and don't think this should keep us from merging this PR, which is only for theFlattenException class of the Debug component at this point.

We'll still be able to add support for the ErrorRenderer class and/or use the Serializer in the new components on 4.4 once this is done.

ping@dunglas

We can merge this one, but can we mark it as@experimental in case we have to modify it for ErrorHandler/ErrorRenderer?

@dunglas Depending on what you decide to do, it might be best to create another normalizer for ErrorRenderer, and just retire this one with the Debug component in 5.0.

Anyway, I added the@experimental tag as it can't hurt.

Does#34312 change the approach proposed here? Can you please have a look for 4.4?

I suppose the only thing needed to merge this PR is to change this PR to use the new error handler component (and remove symfony/debug old component) right and put this agaisn't 4.4?

@weaverryan This fix will close#32719 . I tested this locally (to use the new error-handler component), but if I execute the command bin/console messenger:failed:retry -vv I get:

In FlattenException.php line 360:                                                                                                                                     [TypeError]                                                                                                                        Return value of Symfony\Component\ErrorHandler\Exception\FlattenException::getTraceAsString() must be of the type string, null     returned

BTW, this exception does not discard the event from the queue, but without applying this PR fix, it discarded.

What I'm using to test:

• Transport: AMQP (RabbitMQ)
• Serializer: My custom serializer (I'm using a lot of code from the Symfony messenger serializer, specially the encoding/decoding stamps).
• symfony version: 5.0.8

I found the problem with the error. For some reason the Flatten Normalizer in this PR was not being called when normalizing theFlattenException object.

You see there is a missing trace_as_string parameter, that's why the error.
If I omit the priority on the Flatten Normalizer (in this PR) everything works as expected and the commandbin/console messenger:failed:retry -vv does not fail.

Continued in#37087, thank you@skalpa