Catch Exceptions From Inputs #33
Conversation
- When InputFilter is validating, catch Exceptions thrown by Inputs - Return ApiProblemResponse (422) with message - Previous behavior was to return Fatal with Stack Trace and (200) - Test to confirm fix
throw exceptions the validator will now return the proper response.
|
First of all I tested your changes on default PHP configuration (5.4 and 5.5) and your test does not cover your changes in the code. Please generate coverage in PHPUnit to see it. I had a look on |
| if ($inputFilter->isValid()) { | ||
| return; | ||
| } | ||
| } catch (FilterInvalidArgumentException $ex) { |
There was a problem hiding this comment.
As noted by another reviewer, this is not a valid exception type.
In fact, looking through the various exceptions thrown by Zend\Filter, the only time I see InvalidArgumentException thrown is typically for setters; DateTimeFormatter and the various File filters are anomalies here.
Additionally, also noted by another reviewer, validators may also throw exceptions, so type-hinting against filter exceptions only will mean those bubble outwards.
As such, this should likely be hinting against Exception, which also means that the message must change. I'll note that below.
|
While I do think catching exceptions makes sense here, I also think the approach needs more thinking. In almost all situations, we cannot know if the error is a bad value, or rather due to bad configuration or environmental concerns; in most cases that an exception is thrown from a filter or validator, it's one of these latter two at fault. As such, I feel a 500 status code with a standard error response such as we use in the |
|
Closing due to inactivity. |
Fixes issue #32