-
Notifications
You must be signed in to change notification settings - Fork 41.6k
Description
By default BasicErrorController handles uncaught exceptions. It has one method to handle browser based clients, and another to handle machine based clients.
Currently the 'machine' (e.g. not html) error handle returns a 200 OK status code for errors. It would be better for it to return a 500 error (as shown below), since this is an unhandled error and returning a 200 OK is misleading to an API client.
@RequestMapping(value = "${error.path:/error}")
@ResponseBody
@ResponseStatus(value = HttpStatus.INTERNAL_SERVER_ERROR)
public Map<String, Object> error(HttpServletRequest request) {
ServletRequestAttributes attributes = new ServletRequestAttributes(request);
String trace = request.getParameter("trace");
return extract(attributes, trace != null && !"false".equals(trace.toLowerCase()),
true);
}A possibly better solution is to inspect the status attribute from the extracted map and use that as the status code. That would require changing the return type of the method
@RequestMapping(value = "${error.path:/error}")
public ResponseEntity<Map<String, Object>> error(HttpServletRequest request) {
ServletRequestAttributes attributes = new ServletRequestAttributes(request);
String trace = request.getParameter("trace");
Map<String, Object> extractedError = extract(attributes, trace != null && !"false".equals(trace.toLowerCase()),
true);
HttpStatus statusCode;
try {
statusCode = HttpStatus.valueOf((Integer) extractedError.get("status"));
}
catch (Exception e){
statusCode = HttpStatus.INTERNAL_SERVER_ERROR;
}
return new ResponseEntity<Map<String,Object>>(extractedError, statusCode);
}I can submit a patch for either of the above fixes (if you let me know which one is appropriate), or since the code changes are quite minor feel free to copy from the ticket. I have signed the agreement (76520140326040007)