NotificationsYou must be signed in to change notification settings
Fork9.7k
Star30.8k

Catch JsonException and rethrow in JsonEncode#32206

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Jump to bottom

Merged

nicolas-grekas merged 1 commit intosymfony:3.4fromphil-davis:catch-JsonException-in-JsonEncode-3.4

Jun 28, 2019

Merged

Catch JsonException and rethrow in JsonEncode#32206

nicolas-grekas merged 1 commit intosymfony:3.4fromphil-davis:catch-JsonException-in-JsonEncode-3.4

Jun 28, 2019

Conversation

Copy link

Contributor

phil-davis commentedJun 27, 2019•
edited
Loading

Q	A
Branch?	3.4
Bug fix?	yes
New feature?	no
BC breaks?	no
Deprecations?	no
Tests pass?	yes
Fixed tickets	adjustment to implementation of previous PRs for issue#31447
License	MIT
Doc PR	not applicable

PR#31860 provided handling of PHP 7.3JSON_THROW_ON_ERROR behavior in the variousJsonEncode and related classes/methods.

PR#31869 adjusted that. In particular, it adjusted src/Symfony/Component/Serializer/Encoder/JsonDecode.php so that it catches anyJsonException and re-throws it asNotEncodableValueException. That preserves the previous behavior ofJsonDecode:decode - it always throwsNotEncodableValueException when something goes wrong.

IMOJsonEncode:encode needs the same logic. At the moment, if a caller specifiesJSON_THROW_ON_ERROR then the method can throwJsonException, but actually the "standard" forJsonEncode:encode is that it throwsNotEncodableValueException

AdjustJsonEncode:encode to catchJsonException and rethrow it asNotEncodableValueException

carsonbot added Status: Needs Review Bug labels

Jun 27, 2019

Catch JsonException and rethrow in JsonEncode

9c76790

nicolas-grekas added this to the3.4 milestone

Jun 28, 2019

carsonbot removed the Status: Needs Review label

Jun 28, 2019

nicolas-grekas approved these changes

Jun 28, 2019

View reviewed changes

carsonbot added the Status: Reviewed label

Jun 28, 2019

Copy link

Member

nicolas-grekas commentedJun 28, 2019

Thank you@phil-davis.

nicolas-grekas merged commit9c76790 intosymfony:3.4

Jun 28, 2019

nicolas-grekas added a commit that referenced this pull request

Jun 28, 2019

bug#32206Catch JsonException and rethrow in JsonEncode (phil-davis)

2bab37d

This PR was merged into the 3.4 branch.Discussion----------Catch JsonException and rethrow in JsonEncode| Q             | A| ------------- | ---| Branch?       | 3.4| Bug fix?      | yes| New feature?  | no| BC breaks?    | no| Deprecations? | no| Tests pass?   | yes| Fixed tickets | adjustment to implementation of previous PRs for issue#31447| License       | MIT| Doc PR        | not applicablePR#31860 provided handling of PHP  7.3 `JSON_THROW_ON_ERROR` behavior in the various `JsonEncode` and related classes/methods.PR#31869 adjusted that. In particular, it adjusted ` src/Symfony/Component/Serializer/Encoder/JsonDecode.php` so that it catches any `JsonException` and re-throws it as `NotEncodableValueException`. That preserves the previous behavior of `JsonDecode:decode` - it always throws `NotEncodableValueException` when something goes wrong.IMO `JsonEncode:encode` needs the same logic. At the moment, if a caller specifies `JSON_THROW_ON_ERROR` then the method can throw `JsonException`, but actually the "standard" for `JsonEncode:encode` is that it throws `NotEncodableValueException`Adjust `JsonEncode:encode` to catch `JsonException` and rethrow it as `NotEncodableValueException`Commits-------9c76790 Catch JsonException and rethrow in JsonEncode