Chained Annotated Exception Handling - @ExceptionHandler [SPR-13726]

[spring-projects-issues]

Timothy Tong opened SPR-13726 and commented

Occasionally in the process of resolving an exception, we may raise another exception. This new exception may be caused by a variety of reasons -- the resolution itself failed, the root cause was something entirely different, etc.

If we were to throw a new exception from an @ExceptionHandler method, the new exception would be swallowed immediately in ExceptionHandlerExceptionResolver and we can no longer service the new exception.

We're able to service newly thrown exceptions by implementing our own DispatcherServlet and surrounding the super.processHandlerException() with a try-catch.

However, it would be a useful if we could chain ExceptionHandlers whenever one decides to throw an exception. Example: The exception handler for ExceptionOne causes an error and throws an ExceptionTwo in the process. When this ExceptionTwo gets thrown, it will be caught by the second exception handler.

Example:

@ExceptionHandler(ExceptionOne.class)
public ModelAndView handleExceptionOne() {
    if(somethingVeryBadHappens) {
       // Throw this exception and let the following handler service it.
        throw new ExceptionTwo(...);
    }
}

@ExceptionHandler(ExceptionTwo.class)
public ModelAndView handleExceptionTwo() {
}

Here's a simple recursive example of chaining it within ExceptionHandlerExceptionResolver. It most likely doesn't cover all cases, but it does allow the handlers to be chained.

protected ModelAndView doResolveHandlerMethodException(...) {
        try {
            if (logger.isDebugEnabled()) {
                logger.debug("Invoking @ExceptionHandler method: " + exceptionHandlerMethod);
            }
            exceptionHandlerMethod.invokeAndHandle(webRequest, mavContainer, exception);
        }
        catch (Exception invocationEx) {
            return doResolveHandlerMethodException(request, response, handlerMethod, invocationEx);
        }
}

---

No further details from SPR-13726

[spring-projects-issues]

Rossen Stoyanchev commented

This could lead to endless recursion, not to mention increased complexity. In general the idea of chained exception handling is fine. I'm just not sure layering it on top of the existing exception handler design through re-cursive invocation is the way to do it.

I'd recommend sticking to a simpler approach, i.e. using the existing exception handler mechanism and also installing a stop-gap error handler at the Servlet container level. See the reference docs section on that.

[spring-projects-issues]

Timothy Tong commented

I think a more precise feature request title would be "Chained Annotated Exception Handling - @ExceptionHandler".

I don't quite see how using the existing annotated exception handler mechanism and having a catch-all at the dispatcher level will allow chained exception handling. ExceptionHandlerExceptionResolver will swallow the exception and return null. The servlet will only see a null return value.

[spring-projects-issues]

Rossen Stoyanchev commented

What I meant by "simpler approach" is thinking within the current design. Keep exception resolution error-free as much as possible and treat failures as low level server-side error. If the exception remains unresolved it should be re-thrown and reach the Servlet container.

For chained annotated exception handling we'd have to have some way to determine whether a resolution exception should be handled further or not (i.e. when to stop recursion), some way to find out if an exception is from a handler or from an exception handler and what other exceptions we tried to resolve before (so we can decide whether or what to do), and at the end of the day there may not be much to do. Imagine a conversion exception while writing to the response body. The response has been committed and we've even written some of it out.

It's worth looking deeper if the feature is really worth it. You mentioned "a variety of reasons – the resolution itself failed, the root cause was something entirely different, etc." I'd argue that if there is a root cause you can delegate within your @ExceptionHandler code to another method. For other general resolution failures without further details I'd say they should be treated as unrecoverable errors.

[spring-projects-issues]

Timothy Tong commented

Thank you for your help. We found a strategy that served our needs.

[spring-projects-issues]

Rossen Stoyanchev commented

Thanks for confirming. If there is any insight you can share it might give ideas to others who come across this ticket.

[spring-projects-issues]

Timothy Tong commented

I apologize for my delayed comment.

So what we know is that ExceptionHandlerExceptionResolver.doResolveHandlerMethod(), wraps exceptionHandlerMethod.invokeAndHandle within a try-catch block and swallows the exception if any of the annotated @ExceptionHandler methods were to throw an exception.

However, right before it invokes the handler method it will call getExceptionHandlerMethod. Our solution involved overriding that method and using a delegator pattern. Our delegator then calls invoke and handle within our own try-catch.

@Override
protected ServletInvocableHandlerMethod(...) {
    ServletInvocableHandlerMethod method = super.getExceptionHandlerMethod(...);
    
    if(method != null) {
        method =  new DelegatorServletInvocableHandlerMethod(method);
    }

    return method;
}

public class DelegatorServletInvocableHandlerMethod {
    private ServletInvocableHandlerMethod delegate;

    @Override
    public void invokeAndHandle(...) {
       try {
           delegate.invokeAndHandle(...);
        } catch(Exception e) {
            // Final handling
        }
    }
}

The reason as to why we need this is because this is more of a middle tier service. During normal processing, our code can throw exceptions. While trying to resolve this exception, we sometimes need to communicate with another service which has the potential of also just failing. This newly thrown exception has a defined exception handler, but it won't be caught because it is thrown from the exception handler.

So what we have in our try-catch is effectively a switch statement for very few well-expected exceptions, otherwise we have a default behavior that we know will not fail. I would still like to propose that the ExceptionHandlerExceptionResolver allows us one additional chance to recover if the ExceptionHandler methods fail.

// Lines 359 - 370 of ExceptionHandlerExceptionResolver (Spring 4.2.2)
try {
    if (logger.isDebugEnabled()) {
        logger.debug("Invoking @ExceptionHandler method: " + exceptionHandlerMethod);
    }
    exceptionHandlerMethod.invokeAndHandle(webRequest, mavContainer, exception, handlerMethod);
}
catch (Exception invocationEx) {
    // webRequest, original exception, new exception, modelAndViewContainer
    lastChanceRecovery.handle(webRequest, ex, invocationEx, mavContainer);
}