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

Using @ControllerAdvice with WebFlux [SPR-16554] #21097

Closed
spring-projects-issues opened this issue Mar 6, 2018 · 9 comments
Closed

Using @ControllerAdvice with WebFlux [SPR-16554] #21097

spring-projects-issues opened this issue Mar 6, 2018 · 9 comments
Assignees
Labels
in: web status: declined

Comments

@spring-projects-issues
Copy link
Collaborator

@spring-projects-issues spring-projects-issues commented Mar 6, 2018

Enzo Bonggio opened SPR-16554 and commented

I have a project that I want to migrate from Tomcat to Netty but I found that one particular error is not being catch by my ControllerAdvice class.

To reproduce the problem:

  1. create one project with webflux dependency
  2. create one class that have ControllerAdvice annotation
  3. add this code to the class:
    @ExceptionHandler(Throwable.class)
    @ResponseBody
    Complex handleAll(Throwable ex) {
        return new Complex();
    }

    @Data
    public static class Complex {
        String something;
    }
  1. on Application.java add :
@GetMapping("/something")
Mono<Void> something() {
     return Mono.error(new Throwable("mensaje"));
}
  1. bootRun the app
  2. make a POST call to localhost:8080/something

At this point I expect that the ExceptionHandler catch Method Not Allowed exception and return my Complex object but is not happening. I don't understand also why this is working as expected if I add compile('org.springframework.boot:spring-boot-starter-web') to the project.


Reference URL: https://gist.github.com/enzobonggio/8807cfc7e63c73c38fad9018a5c76702

Issue Links:

  • #20940 [docs] Add WebFlux content on exception handling
  • #21109 Support @ResponseStatus-annotated exceptions on WebFlux

0 votes, 5 watchers

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Mar 6, 2018

Enzo Bonggio commented

With this exceptions is the same problem UnsupportedMediaTypeStatusException, NotAcceptableStatusException.

When I have starter-web dependency the exception that I'm catching is :
org.springframework.web.HttpRequestMethodNotSupportedException: Request method 'POST' not supported

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Mar 6, 2018

Rossen Stoyanchev commented

I've changed the title since I think the question is specific to WebFlux, not Netty in particular.

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Mar 6, 2018

Brian Clozel commented

Hello Enzo Bonggio!

Thanks for raising this issue and starting the conversation. I've reproduced the behavior you're describing with a sample application.

In Spring MVC, both mapping and handling errors can be indeed handled by @ExceptionHandler methods globally defined in @ControllerAdvice classes: whether something happen during the mapping phase or the actual controller handler execution, you can handle errors there.

In Spring WebFlux the situation is a bit different, since mapping errors (content negotiation and HTTP mapping errors) are not handled by that mechanism. One could argue that since a @ControllerAdvice can be restricted to a subset of Controllers, then you need to select a controller handler in the first place. This can lead to quite a few confusing issues. We'll document this better with #20940.

Now we'd still like to understand a bit more about what's driving this use case in your applications:

  • are you trying to build a global mapping error handling in your application? Which format? How?
  • are you restricting this error handling to a subset of controller using @ControllerAdvice specifics?
  • is this about Spring Boot apps only or vanilla Spring Framework apps as well? Did you take a look at the Spring Boot error handling feature? Is there something missing there?

Now about your last question: adding spring-boot-starter-web to a Spring Boot application turns your app into a Spring MVC application. As of Spring Framework 5.0, Spring MVC applications do support reactive return types (see the reference documentation on that). We're doing that because we think many developers will want to import spring-boot-starter-webflux in an existing spring-boot-starter-web application in order to use the new WebClient.

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Mar 6, 2018

Enzo Bonggio commented

Thanks a lot for the response Brian Clozel,
So I can still managing the errors the way I used to just by having both dependencies.

You can close this now

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Mar 7, 2018

Brian Clozel commented

Thanks Enzo Bonggio,

Just note that if you're confused about Spring MVC vs. Spring WebFlux, you can learn more about the runtime model differences and pros/cons in this talk.

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Mar 7, 2018

Andreas Schilling commented

As I was also following the discussion on gitter I just want to make a small addition.
I think another pitfall when moving to the reactive web stack is that people (incl. ourselves ;-)) expect @ResponseStatus annotations to work via methods of @ControllerAdvice classes or directly on custom exceptions. This is not 100% related to this issue, but quite close.
Maybe you already plan to give more insight into this topic through your documentation improvement, but if not it might be worth thinking about doing so? For example it took us a while to find out about ResponseStatusException which then can be used to gain the expected results you are used to from the servlet stack.

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Mar 7, 2018

Rossen Stoyanchev commented

Andreas Schilling thanks for the extra comment. It's true we don't support @ResponseStatus and there is no reason not to via ResponseStatusExceptionHandler. If you don't mind creating a separate ticket?

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Mar 7, 2018

Andreas Schilling commented

Sure thing! I put it on my TODO list for tomorrow.

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Mar 8, 2018

Andreas Schilling commented

And here we go: #21109

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: web status: declined
Projects
None yet
Development

No branches or pull requests

2 participants