Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[doc] Update @ControllerAdvice Javadoc to discuss ordering [SPR-15432] #19993

Closed
spring-issuemaster opened this issue Apr 11, 2017 · 3 comments

Comments

Projects
None yet
2 participants
@spring-issuemaster
Copy link
Collaborator

commented Apr 11, 2017

Andrzej Martynowicz opened SPR-15432 and commented

When there are multiple @ControllerAdvice annotated classes it seems that they are selected basing on alphabetical order rather than trying to do "best match".

Consider following two controllers. First is ordinary spring webmvc controller (annotated with @Controller) , the other is rest controlller (annotated with @RestController):

// this one is annotated with @Controller
@Controller
public class TestController {
  
  @ResponseBody
  @RequestMapping(method = GET, value = "/test")
  public String testMethod() {
    if (true) {
      throw new RuntimeException("asdasdsadsad");
    }
    return "asdsadsad";
  }
}

@RestController
@RequestMapping(value = "/soume-url", produces = APPLICATION_JSON_UTF8_VALUE, consumes = APPLICATION_JSON_UTF8_VALUE)
public class SomeController {
  
  @RequestMapping(method = GET)
  public ResponseEntity<? extends Something> someMethod() {
    if (true) {
        throw new RuntimeException("ewewe");
    }
     return new Something();
    }
  }
}

Now let's create two exception handler classes annotated with @ControllerAdvice. The second one is annotated with @ControllerAdvice(annotations = RestController.class) with the intention to be executed only for @RestController annotated classes.

@ControllerAdvice
public class ControllerExceptionHandler {

  @ExceptionHandler(Exception.class)
  public ResponseEntity<String> handleUnexpectedException(Exception e) {
    return new ResponseEntity<String>("some message", HttpStatus.INTERNAL_SERVER_ERROR);
  }
}

@ControllerAdvice(annotations = RestController.class)
public class RestControllerExceptionHandler {

  @ExceptionHandler(Exception.class)
  @ResponseBody // not needed(?), but addded just in cases
  public ResponseEntity<ErrorResponse> handleUnexpectedException(Exception e) {

    ErrorResponse errorResponse = new ErrorResponse("my error response");
    return new ResponseEntity<ErrorResponse>(errorResponse, HttpStatus.INTERNAL_SERVER_ERROR);
  }
}

My expectation would be that exceptions thrown by @RestController classes are handled by RestControllerExceptionHandler because this one is more specialized (annotated with @ControllerAdvice(annotations = RestController.class)). However this does not work like this - the request is handled always by ControllerExceptionHandler.

Now the interesting thing is when you rename the RestControllerExceptionHandler to ARestControllerExceptionHandler (so that it alphabetically precedes ControllerExceptionHandler) all works as exepected - ARestControllerExceptionHandler handles exceptions from classes annotated with @RestController and @ControllerExceptionHandler handles all the other.

See also: http://stackoverflow.com/questions/43325685/spring-different-exception-handler-for-restcontroller-and-controller where I tried to ask for assistance and finally ended up with this bug,


Affects: 4.3.7

Referenced from: commits fce8ed6, 546d7dd

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Apr 11, 2017

Rossen Stoyanchev commented

The controller advice beans are sorted with AnnotationAwareOrderComparator - i.e. @Order or implements Ordered on the advice bean -- and it is determined at startup and therefore fixed. At runtime we stop at the first matching controller advice class so you need to use ordering. Since RestControllerExceptionHandler is more specific it can be safely ordered ahead.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Apr 12, 2017

Andrzej Martynowicz commented

Ok. The @Order solved the case. I tried this before but it seems that I imported wrong @Order annotation (from log4j2 instead of spring :/). Anyway I would vote for updating the javadoc of @ControllerAdvice to include the information about @Order and implements Ordered.

.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Apr 12, 2017

Rossen Stoyanchev commented

Good idea, I'll add to the Javadoc.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.