Closed abacaphiliac closed 7 years ago
Hello. Thanks for you work. I suggest the following:
CallableTrait
that provide the arguments()
method to pass extra arguments to the handler and executeCallable()
private method to execute the callable safely. You can see handler($request, $response)
, used by routers and other error handlers, using the server attributes to pass other objects. For example, errorHandler saves the exception in the request.setWhatever
methods in other middlewares, so I suggest to rename setErrorHandler
to simply errorHandler
, to keep the api consistency.You can see, for example in the errorHandler middleware:
@oscarotero thank you. will make updates ASAP.
@oscarotero implemented suggestions.
are you ok with the public constant KEY
for the attribute key? i named it for consistency. is there a value i should use instead of "JSON_ERRORS"?
@abacaphiliac JSON_ERRORS is fine but maybe I'd use a more specific key (such JSON_VALIDATION_ERRORS) to say clearly that are validation errors, not parsing errors (or other type of errors). Anyway, I think it's a great work. Thank you.
@oscarotero thank you for the guidance. i'll update to the more specific key value, add some error handler docs per suggestion by @sbol-coolblue , and squash the commits.
@oscarotero squashed. please let me know if there is something else i can do.
@abacaphiliac Just one thing and I'll merge (I swear): A way to get the errors in the same way than, for example ErrorHandler
So, the error handler could be something like this:
$errors = JsonValidator::getErrors($request);
echo json_encode($errors, JSON_UNESCAPED_SLASHES);
Thanks
@oscarotero no worries. we're in this together, however long it takes : )
the static helper simplified my unit tests. thanks for the suggestion. i do have another recommendation now. i'll submit another request.
implemented the suggestion, btw.
@oscarotero checking in. any outstanding TODOs that i'm missing?
Sorry, I did not received the notification. Merged. Thanks for your effort (and patience) 👍
@oscarotero my pleasure. i hope we get the chance to collaborate again : )
based on some feedback here: https://github.com/oscarotero/psr7-middlewares/pull/46
the error handler receives the psr request, response, and an array of validation errors. the default error handler responds with a 422 status code, application/json content-type, and the json-encoded array of errors.
the route-based multi-file wrapper also accepts an error handler override, which is passed through to the underlying validator.
@oscarotero @sbol-coolblue please tell me what you think.