Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
[FrameworkBundle] Added friendly exception when constraint validator class does not exist#19601
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
Uh oh!
There was an error while loading.Please reload this page.
Conversation
234116b to58a2f49Compare| if (!isset($this->validators[$name])) { | ||
| if (!class_exists($name)) { | ||
| thrownewInvalidArgumentException(sprintf('Constraint validator class "%s" does not exist.',$name)); |
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.
It's the correct exception class in this case? you could improve the text message? see the PR description.
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.
Maybe a simpleValidatorException as used in theValidatorBuilder ?
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.
Thanks, agree with you.
Now, what do you think about the message ? Should be a little bit friendly ?
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.
I think it's friendly enough, but perhaps you can mention thevalidatedBy method.
d923926 tof070965Compareyceruto commentedAug 16, 2016
Updated! both exception class and text message have been improved.@ogizanagi thanks one more time. |
d444bed to80dda7cCompare| if (!isset($this->validators[$name])) { | ||
| if (!class_exists($name)) { | ||
| thrownewValidatorException(sprintf( |
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.
on one line please
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.
fixed 👍
80dda7c to0142a83Compare| if (!isset($this->validators[$name])) { | ||
| if (!class_exists($name)) { | ||
| thrownewValidatorException(sprintf('Constraint validator "%s" does not exist or it is not enabled. Check the "validatedBy" method in your constraint class "%s"',$name,get_class($constraint))); |
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.
Missing dot at the end of the exception message.
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.
Fixed!
fabpot commentedAug 23, 2016
As it already throws an exception, I think this should be merged in 2.7. ping @symfony/deciders |
0142a83 to18cd50cCompareyceruto commentedAug 23, 2016 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
@fabpot I should create a separate PR for 2.7 branch? |
HeahDude commentedAug 23, 2016
👍 as a bug fix |
1 similar comment
dunglas commentedAug 23, 2016
👍 as a bug fix |
fabpot commentedAug 24, 2016
Thank you@yceruto. |
… validator class does not exist (yceruto)This PR was submitted for the master branch but it was merged into the 2.7 branch instead (closes#19601).Discussion----------[FrameworkBundle] Added friendly exception when constraint validator class does not exist| Q | A| ------------- | ---| Branch? | master| Bug fix? | no| New feature? | yes| BC breaks? | no| Deprecations? | no| Tests pass? | yes| Fixed tickets | -| License | MIT| Doc PR | -Currently when mistakenly we type a [Custom Constraint Validator class](http://symfony.com/doc/current/validation/custom_constraint.html#creating-the-validator-itself) or the "alias name" from [validator service](http://symfony.com/doc/current/validation/custom_constraint.html#constraint-validators-with-dependencies) (which would occurs frequently for newcomers) is shown `ClassNotFoundException`:> Attempted to load class "alias_name" from namespace "Symfony\Component\Validator".Did you forget a "use" statement for another namespace?500 Internal Server Error - ClassNotFoundException**This PR tries to improve the error message when this happen.**But I'm not sure about the exception class used ([`InvalidArgumentException`](https://github.com/yceruto/symfony/blob/master/src/Symfony/Component/Validator/Exception/InvalidArgumentException.php)) : ?* [`ConstraintDefinitionException`](https://github.com/yceruto/symfony/blob/master/src/Symfony/Component/Validator/Exception/ConstraintDefinitionException.php) would be another option, because the source of the error comes from the custom [`Constraint`](https://github.com/yceruto/symfony/blob/master/src/Symfony/Bundle/FrameworkBundle/Validator/ConstraintValidatorFactory.php#L68) definition.The text message more convenient from this little mistake: ?* This mistake happen for two reason: FQCN or alias name supplied by constraint not found.* The constraint validator service was declared incorrectly (missing alias)* Perhaps some hint how the developer should resolve the mistake.Maybe some documentation core member would help me ?Commits-------b66ea5e added friendly exception when constraint validator does not exist or it is not enabled
fabpot commentedAug 24, 2016
@yceruto FYI, we are able to merge PR on a different branch without you having to create a new PR :) |
Uh oh!
There was an error while loading.Please reload this page.
Currently when mistakenly we type aCustom Constraint Validator class or the "alias name" fromvalidator service (which would occurs frequently for newcomers) is shown
ClassNotFoundException:This PR tries to improve the error message when this happen.
But I'm not sure about the exception class used (
InvalidArgumentException) : ?ConstraintDefinitionExceptionwould be another option, because the source of the error comes from the customConstraintdefinition.The text message more convenient from this little mistake: ?
Maybe some documentation core member would help me ?