-
-
Notifications
You must be signed in to change notification settings - Fork 9.7k
[Serializer] throw more specific exceptions #21239
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,19 @@ | ||
<?php | ||
|
||
/* | ||
* This file is part of the Symfony package. | ||
* | ||
* (c) Fabien Potencier <fabien@symfony.com> | ||
* | ||
* For the full copyright and license information, please view the LICENSE | ||
* file that was distributed with this source code. | ||
*/ | ||
|
||
namespace Symfony\Component\Serializer\Exception; | ||
|
||
/** | ||
* @author Christian Flothmann <christian.flothmann@sensiolabs.de> | ||
*/ | ||
class NotEncodableValueException extends UnexpectedValueException | ||
{ | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,19 @@ | ||
<?php | ||
|
||
/* | ||
* This file is part of the Symfony package. | ||
* | ||
* (c) Fabien Potencier <fabien@symfony.com> | ||
* | ||
* For the full copyright and license information, please view the LICENSE | ||
* file that was distributed with this source code. | ||
*/ | ||
|
||
namespace Symfony\Component\Serializer\Exception; | ||
|
||
/** | ||
* @author Christian Flothmann <christian.flothmann@sensiolabs.de> | ||
*/ | ||
class NotNormalizableValueException extends UnexpectedValueException | ||
{ | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -12,7 +12,7 @@ | |
namespace Symfony\Component\Serializer\Normalizer; | ||
|
||
use Symfony\Component\Serializer\Exception\InvalidArgumentException; | ||
use Symfony\Component\Serializer\Exception\UnexpectedValueException; | ||
use Symfony\Component\Serializer\Exception\NotNormalizableValueException; | ||
|
||
/** | ||
* Normalizes an object implementing the {@see \DateTimeInterface} to a date string. | ||
|
@@ -79,7 +79,7 @@ public function supportsNormalization($data, $format = null) | |
/** | ||
* {@inheritdoc} | ||
* | ||
* @throws UnexpectedValueException | ||
* @throws NotNormalizableValueException | ||
*/ | ||
public function denormalize($data, $class, $format = null, array $context = array()) | ||
{ | ||
|
@@ -100,7 +100,7 @@ public function denormalize($data, $class, $format = null, array $context = arra | |
|
||
$dateTimeErrors = \DateTime::class === $class ? \DateTime::getLastErrors() : \DateTimeImmutable::getLastErrors(); | ||
|
||
throw new UnexpectedValueException(sprintf( | ||
throw new NotNormalizableValueException(sprintf( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe can we set a specific exception code depending of the cause to be able to easily detect if it's an invalid data: URI, or an invalid date and so on? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I have thought about that too. The reason I did not add it is consistency. We would only be able to add codes for Symfony's own normalizer. However, it's quite common to add your own normalizers in libraries/projects. As a consequence exception codes would only be useful when they are issued by built-in normalizers. I don't really like that. What do you think? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We can maybe reserve a range for app codes? |
||
'Parsing datetime string "%s" using format "%s" resulted in %d errors:'."\n".'%s', | ||
$data, | ||
$dateTimeFormat, | ||
|
@@ -112,7 +112,7 @@ public function denormalize($data, $class, $format = null, array $context = arra | |
try { | ||
return \DateTime::class === $class ? new \DateTime($data, $timezone) : new \DateTimeImmutable($data, $timezone); | ||
} catch (\Exception $e) { | ||
throw new UnexpectedValueException($e->getMessage(), $e->getCode(), $e); | ||
throw new NotNormalizableValueException($e->getMessage(), $e->getCode(), $e); | ||
} | ||
} | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IMHO this example here should actually continue to throw the old exception. This is decodeable, but just not valid in the domain. I guess this example would be a 422 instead?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But the same is true for the other places where we throw a
NotNormalizableValueException
in this PR, isn't it?