@ExceptionHandler methods should support naked beans as return values [SPR-6693]

[spring-issuemaster]

Scott Frederick opened SPR-6693 and commented

Spring MVC @ExceptionHandler controller methods support a flexible argument list and return value, similiar to @RequestMapping-annotated methods. One option that is supported by @RequestMapping methods but not @ExceptionHandler methods is returning naked bean objects from the exception handler method.

For example, this type of exception handler method would be very convenient, especially when building RESTful services using a MarshallingView (the important detail is the "HelloResponse" return value from the handleBindException method):

@Controller
@RequestMapping("/Hello")
public class HelloController {
  @RequestMapping(value = "/greet", method = RequestMethod.GET)
  public HelloResponse greetGet(@Valid HelloRequest request) {
    String message = "Hello, " + request.getTitle() + " " + request.getName();
    return new HelloResponse(message);
  }

  @ExceptionHandler(BindException.class)
  public HelloResponse handleBindException(BindException be) {
    List<FieldError> errors = be.getFieldErrors();
    HelloResponse response = new HelloResponse();
    for (FieldError error : errors) {
      response.addError(error.toString());
    }
    return response;
  }
}

With a response object that looks like this:

@XmlRootElement(name = "response")
public class HelloResponse {
  private String message;
  private List<String> errors;
  // getters and setters
}

Currently this code will throw an exception when the exception handler method returns to the framework:

java.lang.IllegalArgumentException: Invalid handler method return value: HelloResponse@caf0ed
	at org.springframework.web.servlet.mvc.annotation.AnnotationMethodHandlerExceptionResolver.getModelAndView(AnnotationMethodHandlerExceptionResolver.java:353)
	at org.springframework.web.servlet.mvc.annotation.AnnotationMethodHandlerExceptionResolver.doResolveException(AnnotationMethodHandlerExceptionResolver.java:101)
	at org.springframework.web.servlet.handler.AbstractHandlerExceptionResolver.resolveException(AbstractHandlerExceptionResolver.java:109)
	at org.springframework.web.servlet.DispatcherServlet.processHandlerException(DispatcherServlet.java:1004)
	at org.springframework.web.servlet.DispatcherServlet.doDispatch(DispatcherServlet.java:792)
	at org.springframework.web.servlet.DispatcherServlet.doService(DispatcherServlet.java:716)
	at org.springframework.web.servlet.FrameworkServlet.processRequest(FrameworkServlet.java:647)
	at org.springframework.web.servlet.FrameworkServlet.doGet(FrameworkServlet.java:552)
	at javax.servlet.http.HttpServlet.service(HttpServlet.java:707)
	at javax.servlet.http.HttpServlet.service(HttpServlet.java:820)

---

Affects: 3.0 GA

Attachments:

• ExceptionHandler-return.patch (5.70 kB)

13 votes, 19 watchers

[spring-issuemaster]

Scott Frederick commented

There are two things in the 3.0.0 code that keep this from working:

1. AnnotationMethodHandlerAdapter and AnnotationMethodHandlerExceptionAdapter both have getModelAndView() methods with similar functionality, but the version in AnnotationMethodHandlerAdapter does more than the ExceptionHandler counterpart. Specifically, normal Adapter has a case to handle naked bean returned from a handler method:

else if (!BeanUtils.isSimpleProperty(returnValue.getClass())) {
  // Assume a single model attribute...
  addReturnValueAsModelAttribute(handlerMethod, handlerType, returnValue, implicitModel);
  return new ModelAndView().addAllObjects(implicitModel);
}

This code generates a ModelAndView with the returned bean as the only element in the model and a "null" view name. The ExceptionAdapter.getModelAndView() is missing this "else" case (and a few others that the normal Adapter has), and throws an IllegalArgumentException instead of automatically generating a ModelAndView.

2. In the normal (non-exception) processing path, DispatcherServlet provides a default view name in case the ModelAndView generated by the mapped handler does not provide one (or when the ModelAndView is generated by the HandlerAdapter as in point #1 above). The doDispatch() method of DispatcherServlet has these three lines of code to do this job:

if (mv != null && !mv.hasView()) {
  mv.setViewName(getDefaultViewName(request));
}

This code is in the "try" clause of the main try/catch block, and does not get executed when an exception is caught and the registered exception handlers are called. Without this, the render() method does not default to the correct view after an exception is handled.

If I change the code to address these two problems, returning a naked bean from an exception handler seems to work fine. I will attach a patch with my changes. I'm not sure this is the ideal way to fix this problem, but it seems to work.

Editorial comment: There is a lot of near-duplication in these Adapter classes (and in the HandlerMethodInvoker that AnnotationMethodHandlerAdapter delegates to), but they don't have exactly the same functionality. In particular, the resolveHandlerArguments() and getModelAndView() methods of the classes are close-but-not-quite-the-same. In the long term it seems like it would be beneficial to refactor these classes to share more code and even out the features across all types of handler methods.

[spring-issuemaster]

Scott Frederick commented

Patch file with an example of changes that seem to fix this problem. From 3.0.0-RELEASE branch.

[spring-issuemaster]

vincent lee commented

Will be very helpful!

[spring-issuemaster]

Rossen Stoyanchev commented

I'm resolving this as complete. The new RequestMappingHandlerMapping and ExceptionHandlerExceptionResolver are equal in this regard. They both use DefaultMethodReturnValueHandler, which interprets an Object return as a single model attribute.