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

RestControllerAdvice with custom annotations is not working properly [SPR-17569] #22101

Closed
spring-projects-issues opened this issue Dec 5, 2018 · 7 comments
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket)

Comments

@spring-projects-issues
Copy link
Collaborator

spring-projects-issues commented Dec 5, 2018

volkanto opened SPR-17569 and commented

We have two different controllers for two different versions of the API. We are trying to distinguish the exception handling too. Regarding this need, we created custom annotations to distinguish the controllers.

 

Here are the custom annotations for the version definitions:

@Target(ElementType.TYPE)
@Retention(RetentionPolicy.RUNTIME)
public @interface ApiV1 { }

@Target(ElementType.TYPE)
@Retention(RetentionPolicy.RUNTIME)
public @interface ApiV2 { }

 

Here are the controller implementations with the custom version annotations:

@ApiV1
@RequestMapping("/path")
@RestController
public class XYZControllerImpl implements com.xyz.XYZController { ... }

@ApiV2
@RequestMapping("/v2/path")
@RestController
public class XYZControllerV2Impl implements com.xyz.v2.XYZController { ... }

 

And here are the custom exception handlers regarding the version declarations:

@RestControllerAdvice(annotations = ApiV1.class)
public class ExceptionHandlerForV1 {      
    private static final String ONLY_APPLICATION_JSON_SUPPORTED = "only 'application/json' supported";     
    ...        
    @ExceptionHandler(HttpMediaTypeNotSupportedException.class)    
    @ResponseStatus(HttpStatus.BAD_REQUEST)    
    public CustomResponse handleHttpMediaTypeNotSupportedException(HttpMediaTypeNotSupportedException e) {        
        return new CustomResponse(CustomResponse.Status.INVALID_REQUEST, ONLY_APPLICATION_JSON_SUPPORTED);    
    }     
    ... 
}

@RestControllerAdvice(annotations = ApiV2.class) 
public class ExceptionHandlerForV2 { ... }

 

If a HttpMediaTypeNotSupportedException occurs, it is not handled by ExceptionHandlerForV1. We are getting Spring's exception instead of our custom exception. But if I remove annotations = ApiV1.class part from the ExceptionHandlerForV1, this class is getting a global exception handler and related exceptions caught by this handler in a correct way. And also same behavior happens for MethodNotAllowedException too. But other types of exceptions handled properly.

We are wondering why this two exceptions (we tried these two exceptions so far) not handled by our custom exception handlers. Is it a design decision or a bug? If this is a design decision, how can we handle all these in our custom exception handlers?

We also tried all these cases with Spring 5.1.3.RELEASE too.


Affects: 5.0.6

Reference URL: #21502

@spring-projects-issues
Copy link
Collaborator Author

spring-projects-issues commented Dec 6, 2018

Rossen Stoyanchev commented

Please fill out the description to explain what you're requesting. I've read #21502 and it sounded like you want to see a Javadoc improvement, but the subject of this ticket sounds unclear.

@spring-projects-issues
Copy link
Collaborator Author

volkanto commented

Hi Rossen Stoyanchev, I've updated the description.

@spring-projects-issues
Copy link
Collaborator Author

Rossen Stoyanchev commented

Thanks, @ExceptionHandler methods, including those from @ControllerAdvice should be used before the DefaultHandlerExceptionResolver, assuming you're using something like @EnableWebMvc. So it should work as expected. I'm not sure what the issue is. Could you extract a small sample that demonstrates the issue?

@spring-projects-issues spring-projects-issues added type: bug A general bug status: waiting-for-triage An issue we've not yet triaged or decided on in: web Issues in web modules (web, webmvc, webflux, websocket) and removed type: bug A general bug labels Jan 11, 2019
@rstoyanchev rstoyanchev added status: waiting-for-feedback We need additional information before we can continue and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels Jan 25, 2019
@spring-projects-issues
Copy link
Collaborator Author

If you would like us to look at this issue, please provide the requested information. If the information is not provided within the next 7 days this issue will be closed.

@spring-projects-issues spring-projects-issues added the status: feedback-reminder We've sent a reminder that we need additional information before we can continue label Feb 1, 2019
@spring-projects-issues
Copy link
Collaborator Author

Closing due to lack of requested feedback. If you would like us to look at this issue, please provide the requested information and we will re-open the issue.

@spring-projects-issues spring-projects-issues removed status: waiting-for-feedback We need additional information before we can continue status: feedback-reminder We've sent a reminder that we need additional information before we can continue labels Feb 8, 2019
@matdelong
Copy link

I'm having this issue as well. It seems to me that certain exceptions that occur while deciding if an endpoint in a controller class match the request (correct http method, correct Accept and Content-Type headers, etc.) then the endpoint is never chosen, so the Controller class is never chosen, so the "annotation" property of the ControllerAdvice annotation won't be matched. This seems counter-intuitive to me, because the path has been matched well enough to know, for example, the client sent the wrong HTTP method, so it stands to reason the request should be attached to that endpoint's controller, and the advice class should be able to target an annotation from that controller.

@EghliDos
Copy link

I'm having this issue as well. It seems to me that certain exceptions that occur while deciding if an endpoint in a controller class match the request (correct http method, correct Accept and Content-Type headers, etc.) then the endpoint is never chosen, so the Controller class is never chosen, so the "annotation" property of the ControllerAdvice annotation won't be matched. This seems counter-intuitive to me, because the path has been matched well enough to know, for example, the client sent the wrong HTTP method, so it stands to reason the request should be attached to that endpoint's controller, and the advice class should be able to target an annotation from that controller.

It seems to me, if annotations are explicitly specified on the controller handler, e.g. media type consumed or HTTP method is used, then Spring MVC takes it as part of its signature, and during the determination stage to which controller the request belongs to, it wouldn't find the match, and therefore it delegates the exception handling to the default Spring MVC handler. Hence none of the custom exception handlers will be called. It's a bit annoying, but if that's how their Servlet implementation behaves, then we just have to understand the behavior.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket)
Projects
None yet
Development

No branches or pull requests

4 participants