Skip to content

The error handling is inconsistent in the serializer decoders #9393

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
stof opened this issue Oct 28, 2013 · 2 comments
Closed

The error handling is inconsistent in the serializer decoders #9393

stof opened this issue Oct 28, 2013 · 2 comments
Labels
Enhancement Good first issue Ideal for your first contribution! (some Symfony experience may be required) Serializer

Comments

@stof
Copy link
Member

stof commented Oct 28, 2013

Here is what happens currently:

  • the ChainDecoder throws a Symfony\Component\Serializer\Exception\RuntimeException when it cannot find a decoder for the format (and then an exception thown by the decoder being used)
  • the XmlEncoder throws a Symfony\Component\Serializer\Exception\UnexpectedValueException on failure
  • the JsonDecode and JsonEncoder fail silently (you get null as value) and then provide a method to get the json decoding error code (not the same name in them btw), which is not part of the interface (making the method hard to use as we don't have any way to access it when it is used in the serializer)
  • the Serializer just proxies the call to its ChainDecoder so the behavior is the same.

Both exceptions used here implement the exception interface of the component.

My suggestion is to refactor the json decoder classes to make them throw the UnexpectedValueException on failure like the xml format instead of using a separate method which cannot be accessed.
The phpdoc of the interface should also be updated to document the expected behavior on failure, which is currently not specified

@rodrigodiez
Copy link

@stof I done some work at https://github.com/rodrigodiez/symfony/compare/ticket_9393 Do I need to provide tests?

@stof
Copy link
Member Author

stof commented Nov 23, 2013

Please open a PR for your changes

fabpot added a commit that referenced this issue Dec 28, 2013
…e serializer decoders (fabpot)

This PR was merged into the 2.5-dev branch.

Discussion
----------

[Serializer] error handling inconsistencies fixed in the serializer decoders

| Q             | A
| ------------- | ---
| Bug fix?      | no
| New feature?  | no
| BC breaks?    | no
| Deprecations? | yes
| Tests pass?   | yes
| Fixed tickets | #9393
| License       | MIT
| Doc PR        | none

see #9586 for the original PR

Commits
-------

a1ab939 [Serializer] fixed CS
6d9f0be Json encoder classes now throws UnexpectedValueException as XML classes
@fabpot fabpot closed this as completed Dec 28, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement Good first issue Ideal for your first contribution! (some Symfony experience may be required) Serializer
Projects
None yet
Development

No branches or pull requests

3 participants