Skip to content

[Serializer] Upgrade to 7.1 - NotNormalizableValueException with $useMessageForUser=true - message lost #57427

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

Closed
yblatti opened this issue Jun 17, 2024 · 4 comments

Comments

@yblatti
Copy link

yblatti commented Jun 17, 2024

Symfony version(s) affected

7.1.1

Description

This concerns the denormalisation of a nested object using a custom (de)normalizer and COLLECT_DENORMALIZATION_ERRORS=true.

In Symfony 6.4 & 7.0, when a custom (de)normalizer throws a NotNormalizableValueException with $useMessageForUser = true in the denormalize() function, if Symfony Serializer’s context is set with DenormalizerInterface::COLLECT_DENORMALIZATION_ERRORS => true, it throws a PartialDenormalizationException and uses message, path, and expectedTypes defined in NotNormalizableValueException::createForUnexpectedDataType() to inform the user.

In Symfony 7.1, a broader message is generated using the class of the nested object, and loses the valuable information given by the custom normalizer.


Code in a minimalistic Normalizer

    public function denormalize(mixed $data, string $type, string $format = null, array $context = []): MySpecialDTO
    {
        $mySpecialDTO = new MySpecialDTO();

        $valueThatNeedsProcessing = $data['key'];
        if ('goodValue' !== $valueThatNeedsProcessing) {
            // ****************************************************************
            // The values & message set here used to be forwarded to client.
            // Not anymore on 7.1.
            // ****************************************************************
            throw NotNormalizableValueException::createForUnexpectedDataType('A clear message for the end user', 'Sample', ['ValidTypeIExpect'], $context['deserialization_path'].'.key', true);
        }

        $mySpecialDTO->key = $valueThatNeedsProcessing;

        return $mySpecialDTO;
    }

Symfony 6.4 & 7.0 output, with desired message :

{
    "type": "https:\/\/symfony.com\/errors\/validation",
    "title": "Validation Failed",
    "status": 422,
    "detail": "mySpecialField.key: This value should be of type ValidTypeIExpect.",
    "violations": [
        {
            "propertyPath": "mySpecialField.key",
            "title": "This value should be of type ValidTypeIExpect.",
            "template": "This value should be of type {{ type }}.",
            "parameters": {
                "{{ type }}": "ValidTypeIExpect"
            },
            "hint": "A clear message for the end user"
        }
    ]
}

Symfony 7.1 output, with a broader message:

{
    "type": "https:\/\/symfony.com\/errors\/validation",
    "title": "Validation Failed",
    "status": 422,
    "detail": "mySpecialField: This value should be of type App\\DTO\\MySpecialDTO|null.",
    "violations": [
        {
            "propertyPath": "mySpecialField",
            "title": "This value should be of type App\\DTO\\MySpecialDTO|null.",
            "template": "This value should be of type {{ type }}.",
            "parameters": {
                "{{ type }}": "App\\DTO\\MySpecialDTO|null"
            }
        }
    ]
}

How to reproduce

I wrote a simple reproducer, here : https://github.com/yblatti/repro-denorm-error

You'll find relevant code in src/Controller, src/DTO and src/Serialization.

The same code is used, only the Symfony version changes accros 3 branches : sf_6.4, sf_7.0, sf_7.1,

To test on different versions :

git clone https://github.com/yblatti/repro-denorm-error.git
cd repro-denorm-error

# Symfony 6.4
git checkout origin/sf_6.4
composer install
bin/console cache:clear --env=test
bin/phpunit --testdox
# all green results

# Symfony 7.0
git checkout origin/sf_7.0
composer install
bin/console cache:clear --env=test
bin/phpunit --testdox
# all green results

# Symfony 7.1
git checkout origin/sf_7.1
composer install
bin/console cache:clear --env=test
bin/phpunit --testdox
# the second test fails, as the message set in NotNormalizableValueException::createForUnexpectedDataType() is not used

Possible Solution

No response

Additional Context

No response

@yblatti
Copy link
Author

yblatti commented Jun 17, 2024

git bisect seems to indicate d32e81c#diff-29383d2d4706675943994bda3c2e1be15e8008fa4928d4272cf3980454471115 as the first commit to fail

@xabbuh
Copy link
Member

xabbuh commented Jun 17, 2024

Can you confirm that #57433 fixes your issue?

@yblatti
Copy link
Author

yblatti commented Jun 17, 2024

Hi and thank you for your ultra quick fix !
Yes ! It works like a charm, both in the reproducer and I my production app I'm upgrading to 7.1 !

@xabbuh
Copy link
Member

xabbuh commented Jun 17, 2024

Thank YOU very much for providing such a great issue with a reproducer that after all made it possible to fix it that quickly! 👍

nicolas-grekas added a commit that referenced this issue Jun 28, 2024
…ct (HypeMC)

This PR was merged into the 5.4 branch.

Discussion
----------

[Serializer] Check if exception message in test is correct

| Q             | A
| ------------- | ---
| Branch?       | 5.4
| Bug fix?      | no
| New feature?  | no
| Deprecations? | no
| Issues        | Fix #57427 (just test)
| License       | MIT

This is a test for #57433

Commits
-------

2fc8789 [Serializer] Check if exception message in test is correct
nicolas-grekas added a commit that referenced this issue Jun 28, 2024
…ectNormalizer` (HypeMC, xabbuh)

This PR was merged into the 7.1 branch.

Discussion
----------

[Serializer] forward exceptions caught in the `AbstractObjectNormalizer`

| Q             | A
| ------------- | ---
| Branch?       | 7.1
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Issues        | Fix #57427
| License       | MIT

Commits
-------

6a16b59 forward exceptions caught in the AbstractObjectNormalizer
71d67d4 [Serializer] Check if exception message in test is correct
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

4 participants