Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Serializer] the problem normalizer does not include previous exceptions #54428

Open
kennaar opened this issue Mar 28, 2024 · 2 comments
Open

Comments

@kennaar
Copy link

kennaar commented Mar 28, 2024

Description

Symfony's profiler supports showing the stack trace for all exceptions thrown during a request:
image

I was expecting that the ProblemNormalizer would include all exceptions + stack trace in the normalized data as well. This is not the case. I have implemented this in our own code base temporarily but decorating the ProblemNormalizer, but I believe this should be handled by Symfony itself.

Example

Example implementation:

<?php

declare(strict_types=1);

namespace Foo\Bar\Baz;

use Symfony\Component\DependencyInjection\Attribute\AsDecorator;
use Symfony\Component\DependencyInjection\Attribute\AutowireDecorated;
use Symfony\Component\ErrorHandler\Exception\FlattenException;
use Symfony\Component\Serializer\Exception\InvalidArgumentException;
use Symfony\Component\Serializer\Normalizer\NormalizerInterface;
use Symfony\Component\Serializer\Normalizer\ProblemNormalizer as SymfonyProblemNormalizer;

#[AsDecorator(decorates: 'serializer.normalizer.problem')]
final class ProblemNormalizer implements NormalizerInterface
{
    public function __construct(
        #[AutowireDecorated]
        private readonly SymfonyProblemNormalizer $decoratedNormalizer,
    ) {
    }

    public function normalize(mixed $object, string $format = null, array $context = []): array
    {
        if (!$object instanceof FlattenException) {
            throw new InvalidArgumentException(sprintf('The object must implement "%s".', FlattenException::class));
        }

        $normalized = $this->decoratedNormalizer->normalize($object, $format, $context);

        $normalized['previous'] = array_map(
            fn (FlattenException $e): array => $this->decoratedNormalizer->normalize($e, $format, $context),
            $object->getAllPrevious()
        );

        return $normalized;
    }

    public function supportsNormalization(mixed $data, string $format = null): bool
    {
        return $this->decoratedNormalizer->supportsNormalization($data, $format);
    }
}
@smnandre
Copy link
Contributor

As i see it, the ProblemNormalizer tries to stay close to the RFC 7807 it's inspired from.

Your suggestion would go against two particular paragraphs of the RFC.

3.1 Member of a Problem Details Object

The "detail" member, if present, ought to focus on helping the client
correct the problem, rather than giving debugging information.

5. Security considerations

When defining a new problem type, the information included must be
carefully vetted. Likewise, when actually generating a problem --
however it is serialized -- the details given must also be
scrutinized.

@kennaar
Copy link
Author

kennaar commented Apr 2, 2024

The current implementation already includes debugging information when the debug flag is set to true. Either all that debugging information should be removed then, or the current information should be extended some more IMO.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants