FriendsOfSymfony / FOSRestBundle

This Bundle provides various tools to rapidly develop RESTful API's with Symfony
http://symfony.com/doc/master/bundles/FOSRestBundle/index.html
MIT License
2.79k stars 704 forks source link

FlattenExceptionNormalizer serializes exceptions for the messenger consumer #2292

Closed goetas closed 2 years ago

goetas commented 3 years ago

\FOS\RestBundle\Serializer\Normalizer\FlattenExceptionNormalizer serializes exceptions for the messenger consumer instead of letting \Symfony\Component\Messenger\Transport\Serialization\Normalizer\FlattenExceptionNormalizer do the serialization and deserialization when is being run by the messenger consumer.

This causes an error when the messenger consumer tries to deserialize the exception... how can i have FOS rest not "overriding" the FlattenExceptionNormalizer provided by the messenger component?

Disabling the exception handling done by FOS solves the problem, but I would like to keep it...

fos_rest:
    exception:
        enabled: false
xabbuh commented 3 years ago

I do not have a good solution for this yet. I guess the underlying issue is that one probably wants to use two different serializer instances for the queue and the HTTP API, right?

goetas commented 3 years ago

As a workaround I've decorated the FOSest normalizer:

use Symfony\Component\Serializer\Normalizer\NormalizerInterface;

use const PHP_SAPI;

final class FlattenExceptionNormalizer implements NormalizerInterface
{
    private NormalizerInterface $normalizer;

    public function __construct(NormalizerInterface $fos, NormalizerInterface $symfony)
    {
        $this->normalizer = PHP_SAPI === 'cli' ? $symfony : $fos ;
    }

    public function normalize($exception, $format = null, array $context = [])
    {
        return $this->normalizer->normalize($exception, $format, $context);
    }

    public function supportsNormalization($data, $format = null)
    {
        return $this->normalizer->supportsNormalization($data, $format);
    }
}

Would it be acceptable if FOSest does something as this? maybe some smarter logic than PHP_SAPI === 'cli'...?

monteiro commented 3 years ago

A potential solution is for https://github.com/FriendsOfSymfony/FOSRestBundle/blob/3.x/Serializer/Normalizer/FlattenExceptionNormalizer.php#L78

do the same as this line.

If the serializer is the Messenger serializer, it sets a context, so the right flatten exception normalizer is used, instead of the FOSRestBundle one.

The other option is just to change the priority to be lower than the Flatten Exception Normalizer from Messenger: If you don't potentially want to do that change in FOSRestBundle, the other option is to change priority, so the FOSRestBundle has lower priority.

goetas commented 3 years ago

I've created https://github.com/FriendsOfSymfony/FOSRestBundle/pull/2294 that uses the suggested approach by @monteiro of adding a context parameter that gives the possibility to selectivly be enabled only where FOS REST is involved in the serialization process.

goetas commented 2 years ago

The fix in https://github.com/FriendsOfSymfony/FOSRestBundle/pull/2294 is green and ready to be reviewed