14

In Spring you can set up "global" exception handler via @ControllerAdvice and @ExceptionHandler annotation. I'm trying to utilize this mechanism to have two global exception handlers:

  • RestControllerExceptionHandler - which should return error responses as json for any controller annotated with @RestController
  • ControllerExceptionHandler - which should print error message to the screen for any other controller (annottated with @Controller)

The problem is that when I declare these two exception handlers spring always uses the ControllerExceptionHandler and never RestControllerExceptionHandler to handle the exception.

How to make this work ? BTW: I tried to use @Order annotation but this does not seem to work.

Here are my exception handlers:

// should handle all exception for classes annotated with         
@ControllerAdvice(annotations = RestController.class)
public class RestControllerExceptionHandler {


  @ExceptionHandler(Exception.class)
  public ResponseEntity<ErrorResponse> handleUnexpectedException(Exception e) {

    // below object should be serialized to json
    ErrorResponse errorResponse = new ErrorResponse("asdasd"); 

    return new ResponseEntity<ErrorResponse>(errorResponse, HttpStatus.INTERNAL_SERVER_ERROR);
  }

// should handle exceptions for all the other controllers
@ControllerAdvice(annotations = Controller.class)
public class ControllerExceptionHandler {


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


}
}

When I remove ControllerExceptionHandler than RestControllerExceptionHandler is correctly called by spring (only for classes annotated with @RestController).... but when I add ControllerExceptionHandler than all goes via ControllerExceptionHandler. Why?

walkeros
  • 4,736
  • 4
  • 35
  • 47
  • 1
    [This article](http://blog.codeleak.pl/2013/11/controlleradvice-improvements-in-spring.html) that describes a similar concept does not define any parameters to default controller advice (i.e. there's no `annotations` property in it). Have you tried if that works? – M. Prokhorov Apr 10 '17 at 14:31
  • @M.Prokhorov: This is exactly what I'm trying to achieve. And as I look at the article I'm doing exactly the same. but for me this does not work for some reason :/ – walkeros Apr 11 '17 at 07:36
  • @M.Prokhorov: I figured it out. You're article was helpful for digging into this problem. I ended up with a bug in spring (see my own answer). Thanks again. – walkeros Apr 11 '17 at 08:31

5 Answers5

16

After some deeper investigation it seems that the alphabetical order matters :/.

When I renamed my RestControllerExceptionHandler to ARestControllerExceptionHandler (which alphabetically precedes ControllerExceptionHandler) all works as expected! ARestControllerExceptionHandler correctly handles exceptions from RestControllers and ControllerExceptionHandler handles exception from other controllers.

I created a bug in spring for this: https://jira.spring.io/browse/SPR-15432

--- EDIT:

I received the answer for SPR-15432 where it is suggested that this case can be solved with @Order (org.springframework.core.annotation.Order) annotation or by implementing Ordered interface.

This did not work for me before, but it seems that I have imported wrong @Order annotation. (from log4j2 instead of spring). After fixing this it works. Fixed version is following:

// should handle all exception for classes annotated with         
@ControllerAdvice(annotations = RestController.class)
@Order(1) // NOTE: order 1 here
public class RestControllerExceptionHandler {


  @ExceptionHandler(Exception.class)
  public ResponseEntity<ErrorResponse> handleUnexpectedException(Exception e) {

    // below object should be serialized to json
    ErrorResponse errorResponse = new ErrorResponse("asdasd"); 

    return new ResponseEntity<ErrorResponse>(errorResponse, HttpStatus.INTERNAL_SERVER_ERROR);
  }
}
// should handle exceptions for all the other controllers
@ControllerAdvice(annotations = Controller.class)
@Order(2)  // NOTE: order 2 here
public class ControllerExceptionHandler {


  @ExceptionHandler(Exception.class)
  public ResponseEntity<String> handleUnexpectedException(Exception e) {
    return new ResponseEntity<String>("Unexpected exception, HttpStatus.INTERNAL_SERVER_ERROR);
  }
}
walkeros
  • 4,736
  • 4
  • 35
  • 47
  • That may indeed be a bug, but it also very well may not be, it may be just an implementation detail of Spring reflection facilities that won't warrant a fix in your case. Try to look into ordering your advices, which may be done as suggested in [this answer](http://stackoverflow.com/a/19500823/7470253). – M. Prokhorov Apr 11 '17 at 11:34
11

That's happening because @RestController annotation is itself an @Controller as well so Spring is considering the @ControllerAdvice with annotation = Controller.class for both.

You may try another method to define the subset of Controllers that the @ControllerAdvice will have effect, so here are some solutions:


Solution 1

  1. Create a new Annotation @NotRestController:

    @Target({ElementType.TYPE})
    @Retention(RetentionPolicy.RUNTIME)
        public @interface NotRestController {
    }
    
  2. Mark controllers that are not @RestController with both @Controller and @NotRestController:

    @Controller
    @NotRestController
    @RequestMapping("/controller")
    public class SampleController {
    }
    
  3. Use NotRestController.class on ControllerExceptionHandler:

    @ControllerAdvice(annotations = NotRestController.class)
    public class ControllerExceptionHandler {
    

I've created a sample project with Solution 1 and shared it on Github:

https://github.com/brunocleite/springexceptionhandlertest


Solution 2

  1. Move your @RestController classes into a package foo.bar and your @Controller classes into another package foo.nuk.

  2. Use basePackages property of @ControllerAdvice instead of annotations:

    @ControllerAdvice(basePackages={"foo.bar"})
    public class RestControllerExceptionHandler {
    
    @ControllerAdvice(basePackages={"foo.nuk"})
    public class ControllerExceptionHandler {
    

Best regards!

Bruno Leite
  • 542
  • 4
  • 12
  • 3
    +1, however, I'm aware of other options for @Controller advice (as described in solution 1), but I do not want to use them because I want to utilize existing RestController anotation. The solution 1 is ok , but sill I do not want to add extra unnecessary annotations. – walkeros Apr 11 '17 at 07:37
1

This was my solution. It eliminates the need to create additional annotations.

  1. Create 2 base controllers.
@Controller
public class BaseController

@Controller
public class BaseRestController
  1. Extend each controller with chosen base controller.
@Controller
public class UserController extends BaseController

@RestController
public class UserRestController extends BaseRestController
  1. Create 2 global controllers and include only the extended base class in each.
@ControllerAdvice(assignableTypes = BaseController.class)
public class GlobalController

@ControllerAdvice(assignableTypes = BaseRestController.class)
public class GlobalRestController
danna
  • 11
  • 2
1

To further build on the solution 1 of @bruno-leite:

You can define a meta-annotation so you only need to 1 annotation each time insted of 2:

import org.springframework.core.annotation.AliasFor;
import org.springframework.stereotype.Controller;

import java.lang.annotation.*;

/**
 * Alias annotation for @Controller so that @ControllerAdvice instances can
 * target the web controllers separately from the rest controllers.
 */
@Target({ElementType.TYPE})
@Retention(RetentionPolicy.RUNTIME)
@Documented
@Controller
public @interface WebController {
    @AliasFor(
            annotation = Controller.class
    )
    String value() default "";
}

Use it on the web controller(s):

@WebController
@RequestMapping("/customers")
public class CustomerController {

Finally define a @ControllerAdvice that only targets the web controllers like this:

@ControllerAdvice(annotations = WebController.class)
public class GlobalControllerAdvice {

    @ModelAttribute("displayName")
    public String getDisplayName(@AuthenticationPrincipal KeycloakAuthenticationToken principal) {
        if (principal != null) {
            String givenName = principal.getAccount().getKeycloakSecurityContext().getToken().getGivenName();
            String familyName = principal.getAccount().getKeycloakSecurityContext().getToken().getFamilyName();
            return givenName + " " + familyName;
        } else {
            return null;
        }
    }
}

(This example ensures that my Thymeleaf templates always have the displayName attribute, but can be easily adjusted for the exception handlers as well)

Wim Deblauwe
  • 25,113
  • 20
  • 133
  • 211
-4

Use order to fix error:

@Order(1)
@ControllerAdvice(annotations = RestController.class)
public class ErrorRestControllerAdvice {
}

@Order(2)
@ControllerAdvice(annotations = Controller.class)
public class ErrorControllerAdvice {
}