spring-boot
spring-boot copied to clipboard
No easy way to share response customisation logic for both sendError and exception handling
Our need is to map all exceptions to a predefined error response model that we return to clients using our REST services. We are using Spring Boot 1.3.8 but I tried this also with Spring Boot 1.4.2 which comes alongside the Spring IO Platform Athens-SR1 release.
Currently, there is no consistent way to handle Spring MVC specific exceptions. https://docs.spring.io/spring-boot/docs/current/reference/html/boot-features-developing-web-applications.html#boot-features-error-handling discusses about handling exceptions via ErrorController
, ErrorAttributes
and alternatively using Spring MVC's @ExceptionHandler
s and maybe extending ResponseEntityExceptionHandler
which is taking care of the Spring MVC specific exceptions and their mapping.
After many different solutions I ended up extending ResponseEntityExceptionHandler
but the issue is that every other exception except NoHandlerFoundException
can be handled. As suggested, one can define a property for this to change the default behavior of DispatcherServlet
spring.mvc.throw-exception-if-no-handler-found=true
However, this works only if you disable the default resource handling by defining
spring.resources.add-mappings=false
I spotted this from https://github.com/spring-projects/spring-boot/issues/3980#issuecomment-178819364 and it is logic that is not mentioned in the Spring Boot's reference documentation. However, this is not an option if you have static resources. We for example expose our API definition using Swagger and that stops working if you disable the default resource mapping. You might be able to manually map the static resources but I would consider that a workaround.
In my opinion, the most sensible way would be to let spring.mvc.throw-exception-if-no-handler-found=true
to throw NoHandlerFoundException
in all cases if there is handler specified. That's what the property tells anyways. If this is not possible, I hope that there would be some other way than to define your own ErrorController
or ErrorAttributes
beans as they don't get enough information for you to map exceptions because the original request is instead a call to /error
path. And in addition, having multiple components responsible for the error handling logic doesn't sound a great idea.
Here is an application to verify this behavior:
The main application with a REST controller exposing some test endpoints
@SpringBootApplication
public class Main {
public static void main(String[] args) {
SpringApplication.run(Main.class, args);
}
@Bean
public MyExceptionHandler exceptionHandler() {
return new MyExceptionHandler();
}
@RestController
public static class MyController {
@RequestMapping(method = RequestMethod.GET, path = "/fail")
public String get() {
throw new IllegalArgumentException();
}
@RequestMapping(method = RequestMethod.GET, path = "/get-with-param")
public String getWithMandatoryParameter(@RequestParam(required = true) String param) {
throw new IllegalArgumentException();
}
}
}
The custom exception handler:
@ControllerAdvice
public class MyExceptionHandler extends ResponseEntityExceptionHandler {
/**
* For handling Spring MVC exceptions and making sure we are forwards compatible.
*/
@Override
protected ResponseEntity<Object> handleExceptionInternal(
Exception ex,
Object body,
HttpHeaders headers,
HttpStatus status,
WebRequest request) {
return new ResponseEntity<Object>(
new ErrorResponse("my_error_code", "My message"),
headers,
status);
}
@ExceptionHandler(IllegalArgumentException.class)
@ResponseBody
public ErrorResponse handleIllegalArgumentException(
HttpServletRequest request,
HttpServletResponse response,
Exception exception) {
response.setStatus(HttpURLConnection.HTTP_BAD_REQUEST);
return new ErrorResponse("invalid_request", "Failed");
}
public static class ErrorResponse {
private final String error;
private final String message;
public ErrorResponse(String error, String message) {
this.error = error;
this.message = message;
}
public String getError() {
return error;
}
public String getMessage() {
return message;
}
}
}
Tests:
@RunWith(SpringJUnit4ClassRunner.class)
@WebIntegrationTest
@SpringApplicationConfiguration(Main.class)
public class MyExceptionHandlerIT {
@Test
public void testNoHandlerFoundException() {
given()
.accept(ContentType.JSON)
.when()
.log().all()
.get("/not-found")
.then()
.log().all()
.assertThat()
.statusCode(HttpURLConnection.HTTP_NOT_FOUND)
.body("error", Matchers.equalTo("my_error_code"))
.body("message", Matchers.equalTo("My message"));
}
@Test
public void testMissingMandatoryParameter() {
given()
.accept(ContentType.JSON)
.when()
.log().all()
.get("/get-with-param")
.then()
.log().all()
.assertThat()
.statusCode(HttpURLConnection.HTTP_BAD_REQUEST)
.body("error", Matchers.equalTo("my_error_code"))
.body("message", Matchers.equalTo("My message"));
}
@Test
public void testIllegalArgumentException() {
given()
.accept(ContentType.JSON)
.when()
.log().all()
.get("/fail")
.then()
.log().all()
.assertThat()
.statusCode(HttpURLConnection.HTTP_BAD_REQUEST)
.body("error", Matchers.equalTo("invalid_request"))
.body("message", Matchers.equalTo("Failed"));
}
}
testNoHandlerFoundException()
fails but other pass as expected even though I have spring.mvc.throw-exception-if-no-handler-found
set as true
.
As a summary, it would be really great if it would be possible to handle all exceptions in a consistent manner for example by extending the ResponseEntityExceptionHandler
and adding your own exception handlers there to complement the Spring MVC exception handlers coming from the base class.
Edit: Attaching a test project for your convenience: error-handling-test.zip
In my opinion, the most sensible way would be to let spring.mvc.throw-exception-if-no-handler-found=true to throw
NoHandlerFoundException
in all cases if there is handler specified. That's what the property tells anyways.
I believe that the property already behaves as specified. If there is no handler available to handle a particular request, an exception will be thrown in all cases.
The problem is that, by default, ResourceHttpRequestHandler
is mapped to /**
. This means that there always is a handler available to handle a particular request. When ResourceHttpRequestHandler
can't find a resource it calls sendError(404).
it would be really great if it would be possible to handle all exceptions in a consistent manner
It's a subtle detail, but that's already the case. There's no exception involved here so there's nothing for your exception handler to handle.
I think what you're looking for is a static resource handler that will throw an exception if it can't find a resource for the request.
The problem that's described here can be generalised to anywhere that HttpServletResponse.sendError
is called. Even if a static resource handler that throws an exception was used, that would only provide an isolated fix for a specific problem. Anywhere else that calls HttpServletResponse.sendError
would still exhibit the problem.
One problem is that ErrorAttributes
works with RequestAttributes
whereas ResponseEntityExceptionHandler
works with WebRequest
(an extension of RequestAttributes
). #7952 has been opened to explore aligning the two which would make it easier to factor out some common logic.
Is there any way to handle the problem with static resources and throw-exception-if-no-handler-found ?
Is there any update on this?
@wilkinsona Any update on this. It seems this error is causing another issue. #29919 and #2001 when
spring.web.resources.add-mappings=false
spring.mvc.throw-exception-if-no-handler-found=true
No, sorry. We work in the open so any updates will appear in this issue when we have them.
This issue may be resolved by the changes in https://github.com/spring-projects/spring-framework/issues/29491. As of 6.1, the handler for static resources will raise NoResourceFoundException
rather than calling sendError
directly.
Thanks, @rstoyanchev. I've tried updating the original test project to Spring Boot 3.2.0-SNAPSHOT and Spring Framework 6.1.0-SNAPSHOT but I couldn't get exception handling for NoResourceFoundException
to work. The problem appears to be that ExceptionHandlerExceptionResolver
returns false
from shouldApplyTo
:
protected boolean shouldApplyTo(HttpServletRequest request, @Nullable Object handler) {
if (handler == null) {
return super.shouldApplyTo(request, null);
}
else if (handler instanceof HandlerMethod handlerMethod) {
handler = handlerMethod.getBean();
return super.shouldApplyTo(request, handler);
}
else if (hasGlobalExceptionHandlers() && hasHandlerMappings()) {
return super.shouldApplyTo(request, handler);
}
else {
return false;
}
}
The handler
argument is an instance of ResourceHttpRequestHandler
so it's the third if
that's applicable. hasGlobalExceptionHandlers()
returns true
as there are two handlers in the advice cache:
{myExceptionHandler=org.springframework.web.method.annotation.ExceptionHandlerMethodResolver@386651a9, exceptionHandler=org.springframework.web.method.annotation.ExceptionHandlerMethodResolver@ec744e8}
hasHandlerMappings()
returns false
so we drop into the final else
and return false
.
This means that the test project's MyExceptionHandler
is never called to process the NoResourceFoundException
. Could further changes be made to Framework so that this works out of the box or is some more configuration required in the application?