ExceptionHandlerExceptionResolver should support exceptions from any handler

[vghero]

Affects: Spring 5.0.9

I have a Spring Boot service where custom JSON error responses are returned for cases like 400, 404, 415, 500 etc. This works great by using @ExceptionHandler.

Recently we added static resources to the mix. But when accessing one of them that does not exist, the default Spring 404 JSON body is returned instead of ours. As it turns out, ExceptionHandlerExceptionResolver and superclasses are refusing to resolve due to the fact that the handler method is not instance of HandlerMethod but ResourceHttpRequestHandler and error handling ends up somewhere in dispatcher servlet with the "default" 404 response.

I've managed to re-use ExceptionHandlerExceptionResolver also for static resources not found, but it is quite hackish and does not feel right - see below. Are there any plans to add support for this to ExceptionHandlerExceptionResolver? Or did I overlook something?

@EnableWebMvc
@Configuration
public class WebConfiguration implements WebMvcConfigurer {

    @Override
    public void addResourceHandlers(ResourceHandlerRegistry registry) {
        registry
                .addResourceHandler("/apidocs/**")
                .addResourceLocations("classpath:/apidocs/")
                .resourceChain(true)
                .addResolver(new ThrowingPathResourceResolver());
    }

    @Override
    public void extendHandlerExceptionResolvers(List<HandlerExceptionResolver> resolvers) {

        ExceptionHandlerExceptionResolver defaultResolver = (ExceptionHandlerExceptionResolver) resolvers.stream()
                .filter(resolver -> resolver instanceof ExceptionHandlerExceptionResolver).findAny()
                .orElseThrow(() -> new IllegalStateException(
                        "No registered " + ExceptionHandlerExceptionResolver.class.getSimpleName() + " found."));

        ExceptionHandlerExceptionResolver resolver = new ResourceExceptionHandlerExceptionResolver();
        resolver.setApplicationContext(defaultResolver.getApplicationContext());
        resolver.setContentNegotiationManager(defaultResolver.getContentNegotiationManager());
        resolver.setCustomArgumentResolvers(defaultResolver.getCustomArgumentResolvers());
        resolver.setCustomReturnValueHandlers(defaultResolver.getCustomReturnValueHandlers());

        resolver.afterPropertiesSet();

        resolver.setReturnValueHandlers(defaultResolver.getReturnValueHandlers() == null ? null
                : defaultResolver.getReturnValueHandlers().getHandlers());
        resolver.setArgumentResolvers(defaultResolver.getArgumentResolvers() == null ? null
                : defaultResolver.getArgumentResolvers().getResolvers());

        resolvers.add(resolver);
    }

    private class ResourceExceptionHandlerExceptionResolver extends ExceptionHandlerExceptionResolver {

        @Override
        public ModelAndView resolveException(HttpServletRequest request, HttpServletResponse response, Object handler,
                Exception ex) {

            if (handler instanceof ResourceHttpRequestHandler) {
                return doResolveException(request, response, (HandlerMethod) null, ex);
            }
            return null;
        }
    }

    public class ResourceNotFoundException extends RuntimeException {
        private static final long serialVersionUID = 1L;

        public ResourceNotFoundException(String message) {
            super(message);
        }
    }

    private class ThrowingPathResourceResolver extends PathResourceResolver {

        @Override
        public Resource resolveResource(HttpServletRequest request, String requestPath,
                List<? extends Resource> locations, ResourceResolverChain chain) {

            Resource resource = super.resolveResource(request, requestPath, locations, chain);
            if (resource == null) {
                throw new ResourceNotFoundException(
                        "The resource " + request.getRequestURL() + " could not be found.");
            }

            return resource;
        }
    }

[rstoyanchev]

Originally @ExceptionHandler was meant to handle exceptions from handler methods in the same controller class or pass on to other resolvers to try. Later on @ControllerAdvice made it possible to handle exceptions from any handler.

I think we could use the mappedHandlers and mappedHandlerClasses from the base class to determine if the request should be handled in case the handler is not a HandlerMethod. The doResolveHandlerMethodException method already accepts null as the HandlerMethod so if not a HandlerMethod just pass null.

[joshlong]

It would be nice to be able to centralize error handing and also to be able to contribute error handling logic, eg, in a Spring Boot autoconfig. As it is I can’t reuse @ControllerAdvice for exceptions thrown from HandlerFunction's. I want a way for spring Boot autoconfig to be able to contribute advice exception handling logic without requiring the client to install it into the filter chain. As it is I can use @ExceptionHandler / controller advice w Spring MVC-style @RestController's, but I can’t share that same with HandlerFunction's.

[joshlong]

As we are now supporting the functional reactive webflux dsl in spring MVC it would also be nice to share @ExceptionHandler logic across webflux and MVC, across .fn DSL and @Controller's

[rstoyanchev]

@joshlong indeed this is another use case for this. I've broadened the title accordingly.

[adaykin]

I think one thing that would be nice would be an easy way to configure on setup a method that gets called after an exception gets handled. Currently I'm using controlleradvice to handle exceptions, but this gets missed when an exception occurs from a filter. I want a way to just easily log all exceptions that are uncaught.

[exceptionplayer]

I have also encountered this problem. I can handle all exceptions in a custom ExceptionFilter just like Spring Security did with ExceptionTranslationFilter.
For now, all the exception handlers are only suitable for exceptions occurred after filters, i agree with @adaykin that it would be better for spring to provide a centralized exception handler also suitable for exceptions in filters.

[grimsa]

Ran into the same issue.

We use Atlassian's swagger-request-validator for validating requests against OpenAPI spec. It's implemented as a servlet filter that throws an exception in case the request does not match OpenAPI spec.

A centralized @ControllerAdvice class with @ExceptionHandler method then translates this validation exception into a nicely formatted HTTP response.

This works perfectly when request URL is handled by a Controller (i.e. HandlerMethod), but if a client requests the incorrect URL (i.e. no matching HandlerMethod, and handler is ResourceHttpRequestHandler), then our exception translating @ExceptionHandler method no longer works.

[rstoyanchev]

@adaykin, @medusar, @grimsa, the original request is for @ExceptionHandler methods to handle exceptions from Spring MVC handlers other than controllers with @RequestMapping methods. That's still within Spring MVC though. The handling of exceptions from filters is a separate discussion and not in scope for this issue.

[rstoyanchev]

This is now supported through the mappedHandlers or mappedHandlerClasses properties which can be set to indicate to which handlers exceptions should apply.

[haupv1m]

@rstoyanchev Sorry for requesting reopen. I encountered problem when setting mappedHandlerClasses and mappedHandlers for ExceptionHandlerExceptionResolver.

• First, I setup mapped for handle Functional Controller (HandlerFunction class)
• This leads to ExceptionHandlerExceptionResolver cannot resolve null handler case (ex: NotFoundHandlerException). Also, to handle exception in Filter, I implement a highest order Filter (called ExceptionRecoveryFilter) to capture all inner Filter's exceptions and resolve it in the same way as Security Spring did with ExceptionTranslationFilter. Because of setting mappedHandlerClasses, resolver cannot resolve exception from ExceptionRecoveryFilter filter.

Is there any clever way to resolve this issue? Currently, I have to work around in the same way @vghero do.

[rstoyanchev]

@haupv1m can you create a separate issue?

[haupv1m]

@rstoyanchev I already create new issue #26772