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

ExceptionHandler not called for HttpMediaTypeNotSupportedException #29120

Closed
wimdeblauwe opened this issue Sep 9, 2022 · 5 comments
Closed
Assignees
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) status: invalid An issue that we don't feel is valid

Comments

@wimdeblauwe
Copy link

wimdeblauwe commented Sep 9, 2022

Given a very simple Spring Boot 2.7.3 on Java 17 project with only spring-boot-starter-web as dependency.

If I have this controller:

import org.springframework.web.bind.annotation.ExceptionHandler;
import org.springframework.web.bind.annotation.GetMapping;
import org.springframework.web.bind.annotation.RestController;

@RestController
public class TestRestController {

    @ExceptionHandler
    public void handleException(Exception e) {
        System.out.println("Exception!");
        e.printStackTrace();
    }

    @GetMapping("/")
    public void test() {
        throw new RuntimeException("fake");
    }
}

Then the exception stack trace is printed. However, when I change the @GetMapping to this:

@GetMapping(value = "/", consumes = "application/merge-patch+json")

Then there is a org.springframework.web.HttpMediaTypeNotSupportedException as expected, but the exception handler is not called and no stacktrace is printed.

I also tested with a separate @ControllerAdvice class, but the behavior is the same:

@ControllerAdvice(annotations = RestController.class)
@ConditionalOnWebApplication(type = ConditionalOnWebApplication.Type.SERVLET)
public class MyExceptionHandler {
    @ExceptionHandler
    public void handleException(Exception e) {
        System.out.println("Exception in advice!");
        e.printStackTrace();
    }
}

I would like to handle this exception in my error handling library, but I don't know why it never arrives in my exception handler.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Sep 9, 2022
@sbrannen sbrannen added the in: web Issues in web modules (web, webmvc, webflux, websocket) label Sep 9, 2022
@sbrannen sbrannen added this to the Triage Queue milestone Sep 9, 2022
@rstoyanchev
Copy link
Contributor

rstoyanchev commented Sep 13, 2022

@ExceptionHandler in a controller method applies to exceptions raised when invoking methods of that controller. In this case, however, there is no matching handler. HttpMediaTypeNotSupportedException is raised when no handler fully matches the request, but there is a partial match on the URL and a mismatch in media types that can be consumed.

@ExceptionHandler on an @ControllerAdvice is expected to work and I've confirmed that it does. I suspect there is some difference in your setup, e.g. are you sure that MyExceptionHandler gets registered, given that you have conditions on it?

In general, it is a good idea to provide an actual sample to debug the issue. I'll have to ask for that in this case since I can't reproduce the issue otherwise.

@rstoyanchev rstoyanchev self-assigned this Sep 13, 2022
@rstoyanchev rstoyanchev added the status: waiting-for-feedback We need additional information before we can continue label Sep 13, 2022
@wimdeblauwe
Copy link
Author

I did some more tests. If I change the @ExceptionHandler to not use the annotations argument on @ControllerAdvice anymore, then the handler is called:

@ControllerAdvice
@ConditionalOnWebApplication(type = ConditionalOnWebApplication.Type.SERVLET)
public class MyExceptionHandler {
    @ExceptionHandler
    public void handleException(Exception e) {
        System.out.println("Exception in advice!");
        e.printStackTrace();
    }
}

I also tried with @ControllerAdvice(annotations = Controller.class), but then the handler is also not called. Any idea why restricting the advice like that makes it so that the handler is no longer called?

I created a sample project at https://github.com/wimdeblauwe/spring-issue-29120

If you run it and access http://localhost:8080 with a http client, you will see that the custom exception handler is not called. If you comment out the annotations = RestController.class, you will see that it works.

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Sep 13, 2022
@rstoyanchev
Copy link
Contributor

This comes down once again to the fact there is no matching handler. The ControllerAdvice is set to match @RestController handlers but if there is no matching handler, this criteria cannot be verified. I'm not sure there is much we can do. I realize it looks counter-intuitive in this simple example, but in theory there could be multiple partially matched controller methods.

The only other suggestion I can think of is that you can inject the HandlerMethod associated with the exception into an @ExceptionHandler method and check if that's null in which case at least you know that you're dealing with an exception that occurred before a handler could be selected.

@rstoyanchev rstoyanchev added status: waiting-for-feedback We need additional information before we can continue and removed status: feedback-provided Feedback has been provided labels Sep 16, 2022
@wimdeblauwe
Copy link
Author

I have @ControllerAdvice(annotations = RestController.class) because my error handling library should only be active for @RestController controllers, not for @Controller classes (in case there is a mixed application with a web interface and an API). Is there another way I can have my exception handler only active for rest controllers?

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Sep 19, 2022
@rstoyanchev
Copy link
Contributor

Your exception handler is already active for @RestController, but in this case where HttpMediaTypeNotSupported is raised before a request is matched to a handler, the condition has to fail if strictly followed. You can choose to handle it unconditionally with a late ordered controller advice perhaps, if that works.

@rstoyanchev rstoyanchev closed this as not planned Won't fix, can't repro, duplicate, stale Sep 21, 2022
@rstoyanchev rstoyanchev removed this from the Triage Queue milestone Sep 21, 2022
@rstoyanchev rstoyanchev added status: invalid An issue that we don't feel is valid and removed status: waiting-for-triage An issue we've not yet triaged or decided on status: feedback-provided Feedback has been provided labels Sep 21, 2022
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) status: invalid An issue that we don't feel is valid
Projects
None yet
Development

No branches or pull requests

4 participants