In RESTServer.inc all "services_errors"/"ServicesExceptions" after the call to the services controller are handled correctly and displayed in the right format as they are going through renderFormatterView():
try {
$result = services_controller_execute($controller, $arguments, $options);
}
catch (ServicesException $e) {
$result = $this->handleException($e);
}
// Set the content type and render output
drupal_add_http_header('Content-type', $mime_type);
return $this->renderFormatterView($controller, $formatter, $result);
However, any errors before this is executed will only return an error message in the header, with an empty body.
In the call to getControllerArguments() one of these errors includes:
services_error(t('Missing required argument @arg', array('@arg' => $info['name'])), 401);
By the time we get to the arguments we already know the controller and the formatter so we might as well display this error message in the right format through renderFormatterView().
I included a patch which does the following things:
- Expand the try/catch so we can catch the "Missing required argument" services_error.
- Move "$arguments = $this->getControllerArguments($controller, $path, $method);" downwards, so that if there is an exception within getControllerArguments the formatters have already been set up and we can display the error correctly through renderFormatterView().
Two lines have been changed and a comment has been added, the rest are indentation changes to allow for the expanded "try" block.
Steps to reproduce:
1. Set up a resource with a required argument.
2. Go to the URL of the resource and notice how the error message is only displayed in the header and the body is empty.
To further clarify, the following error messages are executed before the call to the services controller as well and are not displayed in the body either:
services_error(t('Could not find the controller.'), 404);
services_error(t('Could not find resource @name.', array('@name' => $resource_name)), 404);
services_error(t('Unknown or unsupported response format.'), 406);
We don't know what format to display these in (ie. XML/JSON), so it makes sense to only show the error in the header. Perhaps we could output these in a standard drupal HTML error page in addition to showing the error in the header?
| Comment | File | Size | Author |
|---|---|---|---|
| #8 | services-1937312-format-arguments-errors-reroll-8.patch | 806 bytes | ygerasimov |
| #4 | services-1937312-format-arguments-errors-reroll-4.patch | 1.2 KB | ygerasimov |
| #1 | services-1937312-format-arguments-errors.patch | 1005 bytes | ygerasimov |
| errors.patch | 2.7 KB | stefan.r |
Comments
Comment #0.0
stefan.r commentedtypo
Comment #0.1
stefan.r commentedplaintext won't work
Comment #1
ygerasimov commentedFor displaying arguments handling errors we can have patch attached. For having others I would need to do some refactoring to fix that.
Attaching patch to check tests.
Comment #2
kylebrowning commentedComment #3
kylebrowning commentedThis looks good, open another issue as a feature request for better error handling messages.
Comment #4
ygerasimov commentedHere is reroll of the patch. If it is green, I will commit it.
Comment #6
POVYLAZZZ commented#4: services-1937312-format-arguments-errors-reroll-4.patch queued for re-testing.
Comment #7
kylebrowning commentedneeds another re-roll.
Comment #8
ygerasimov commentedPatch reroll.
Comment #9
ygerasimov commentedCommitted.
Comment #10.0
(not verified) commentedclarification