java-spring-web
java-spring-web copied to clipboard
Errors happening in the controller in Spring boot should not create follows-from span
This is related to this comment: https://github.com/opentracing-contrib/java-spring-web/pull/1#issuecomment-280684207 and this spring issue: https://jira.spring.io/browse/SPR-15265
Basically now in Spring Boot with embedded container the request which throws exception from the controller creates two spans, the timeline is following:
- span GET /url is started
- exception thrown inside controller
- span GET /url is finished
- dispatcher servlet forwards to /error, span GET /error is started as follows-from the GET /url span
- span GET /error is finished
I'd expect that there is a span which spans (duh!) the whole processing of the request - one that is started in 1) and finished in 5). Ideally the first span (GET /url) would not finish in 3) but in 5).
I did some debugging and it seems that the forward to /error happens in tomcat code - at the end of org.apache.catalina.core.StandardHostValve.throwable():
// A custom error-page has not been defined for the exception
// that was thrown during request processing. Check if an
// error-page for error code 500 was specified and if so,
// send that page back as the response.
response.setStatus(HttpServletResponse.SC_INTERNAL_SERVER_ERROR);
// The response is an error
response.setError();
status(request, response);
It's similar in Jetty but in this case this behaviour is a part of spring code - the forward happens in org.eclipse.jetty.servlet.ServletHandler.doHandle():
if (!response.isCommitted())
{
baseRequest.getResponse().getHttpFields().put(HttpHeader.CONNECTION,HttpHeaderValue.CLOSE);
if (th instanceof UnavailableException)
{
UnavailableException ue = (UnavailableException)th;
if (ue.isPermanent())
response.sendError(HttpServletResponse.SC_NOT_FOUND);
else
response.sendError(HttpServletResponse.SC_SERVICE_UNAVAILABLE);
}
else
response.sendError(HttpServletResponse.SC_INTERNAL_SERVER_ERROR); // <----- HERE
}
Now - there are several cases where this behaviour is not present - e.g. if the response is committed (above code). But also (see org.springframework.web.servlet.DispatcherServlet.processDispatchResult):
- if exception extends ModelAndViewDefiningException - it defines where to forward
- if it can be resolved by any of registered
HandlerExceptionResolver
The case of HandlerExceptionResolver is interesting because this the way to handle exceptions (http://docs.spring.io/spring/docs/current/spring-framework-reference/htmlsingle/#mvc-ann-exceptionhandler).
Registering such handler completely changes the order of span starts/stops to the expected one (A start, B start, B finish, A finish).
The forward to /error still happens (this time inside org.springframework.web.servlet.mvc.method.annotation.ServletInvocableHandlerMethod.invokeAndHandle) but before the original request is "completed". But only if the exception handler returned response text - if only response code is set then the forward won't happen.
I have hard time believing what's going on in this code...
But there's more. The above behaviour happens when I use @ResponseStatus:
@ResponseStatus(value = HttpStatus.I_AM_A_TEAPOT, reason = "Teapot failure")
@ExceptionHandler(Exception.class)
public void mapit() {}
But if instead I return ResponseEntity the forward is gone and there's only 1 span (see ServletInvocableHandlerMethod.setResponseStatus()):
@ExceptionHandler(Exception.class)
public ResponseEntity<String> mapit() {
return ResponseEntity.status(418).body("rotfl");
}
So I guess this will be my new workaround.
Using @ResponseBody also works this way.
But now the exception is not really captured, it has to be extracted from request.getAttribute(DispatcherServlet.EXCEPTION_ATTRIBUTE).
TL,DR: To avoid redirect to /error:
- define
@ExceptionHandler - don't use
@ResponseStatusor make sure that reason is an empty string
See also: http://stackoverflow.com/questions/28902374/spring-boot-rest-service-exception-handling#answer-30193013 (I haven't checked if those properties work)