Spring Framework
  1. Spring Framework
  2. SPR-6693

@ExceptionHandler methods should support naked beans as return values

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Major Major
    • Resolution: Complete
    • Affects Version/s: 3.0 GA
    • Fix Version/s: 3.1 M2
    • Component/s: Web
    • Labels:
      None
    • Last commented by a User:
      false

      Description

      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)
      

        Activity

        Hide
        Scott Frederick added a comment -

        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.

        Show
        Scott Frederick added a comment - 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.
        Hide
        Scott Frederick added a comment -

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

        Show
        Scott Frederick added a comment - Patch file with an example of changes that seem to fix this problem. From 3.0.0-RELEASE branch.
        Hide
        vincent lee added a comment -

        Will be very helpful!

        Show
        vincent lee added a comment - Will be very helpful!
        Hide
        Rossen Stoyanchev added a comment -

        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.

        Show
        Rossen Stoyanchev added a comment - 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.

          People

          • Assignee:
            Rossen Stoyanchev
            Reporter:
            Scott Frederick
            Last updater:
            Trevor Marshall
          • Votes:
            13 Vote for this issue
            Watchers:
            19 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:
              Days since last comment:
              2 years, 46 weeks ago