Skip to content
This repository has been archived by the owner on Jan 15, 2024. It is now read-only.

fix validation exception message #46

Closed
wants to merge 5 commits into from
Closed

fix validation exception message #46

wants to merge 5 commits into from

Conversation

puzakov
Copy link

@puzakov puzakov commented Feb 4, 2016

No description provided.

@@ -72,7 +72,7 @@ public function onKernelException(GetResponseForExceptionEvent $event)
} else {
switch (substr($code, 0, 1)) {
case '4':
$message = 'Input Error';
$message = $exception->getMessage();
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not a fan of exposing exception messages by default. An option could be to allow this conditionally, using an injected flag (which could for example be set to the value of kernel.debug).

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're probably looking for this: #27. If you check the exception type (should be InvalidParametersException), I guess getMessage() would be acceptable, even in prod.

@kleijnweb
Copy link
Owner

I think 'instanceof' (and a use statement) would work better. You'll have to fix the failing unit test though, and make sure the exception message actually makes sense as output.

@puzakov
Copy link
Author

puzakov commented Feb 5, 2016

sure. I`ll do it a bit latter

@kleijnweb
Copy link
Owner

Note that the 3.0 is going to have comprehensive validation error feedback, see #51. That'll take some time though, probably a couple of weeks.

If you fix the build checks I will merge this to master and create a new 2.2.x release, if you don't want to wait. Otherwise I will close this.

@kleijnweb
Copy link
Owner

@kleijnweb kleijnweb closed this Feb 14, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants