No easy way to share response customisation logic for both sendError and exception handling

[troinine]

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 @ExceptionHandlers 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 #3980 (comment) 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

[wilkinsona]

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.

[wilkinsona]

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.

[wilkinsona]

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.

[SimonKlausLudwig]

Is there any way to handle the problem with static resources and throw-exception-if-no-handler-found ?

[TranNgocKhoa]

Is there any update on this?

[atkawa7]

@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

[wilkinsona]

No, sorry. We work in the open so any updates will appear in this issue when we have them.

[rstoyanchev]

This issue may be resolved by the changes in spring-projects/spring-framework#29491. As of 6.1, the handler for static resources will raise NoResourceFoundException rather than calling sendError directly.

[wilkinsona]

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?