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

WebFlux: @ControllerAdvice should handle all the exceptions including from WebFilter #32924

Closed
kitkars opened this issue May 29, 2024 · 7 comments
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) status: invalid An issue that we don't feel is valid

Comments

@kitkars
Copy link

kitkars commented May 29, 2024

Provide a mechanism to handle all the exceptions via @ControllerAdvice including the errors emitted from the WebFilter.

I have attached a simple reproducible project with latest spring boot 3.3.0 + WebFlux.

demo.zip

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label May 29, 2024
@kitkars kitkars changed the title @ControllerAdvice should handle all the exceptions including from WebFilter WebFlux: @ControllerAdvice should handle all the exceptions including from WebFilter May 29, 2024
@simonbasle simonbasle self-assigned this May 31, 2024
@simonbasle
Copy link
Contributor

The repro case is indeed quite straightforward. Although I'm not sure about the way you simulate a WebFilter error (by using exchange.getResponse().writeWith(Mono.error(...))), there's also the simpler case of doing return Mono.error(...) commented out in your repro case, which seems more admissible but doesn't work either.

AFAIU filter errors are supposed to be processable by @ControllerAdvices in both webmvc and webflux since 6.0 (see gh-22991), with the introduction of DispatchExceptionHandler and the fact that RequestMappingHandlerAdapter implements that interface 🤔

But here from what I've seen the error reaches an HttpWebHandlerAdapter rather than a RequestMappingHandlerAdapter.

@rstoyanchev I would need your help in investigating this a bit further and determining if it a setup issue or if the repro case is indeed supposed to work as-is and this is an actual bug.

@simonbasle simonbasle added the status: waiting-for-internal-feedback An issue that needs input from a member or another Spring Team label May 31, 2024
@kitkars
Copy link
Author

kitkars commented May 31, 2024

@simonbasle , Thanks. I tried all these options - assuming at least one option should work.. none of them works! :(

exchange.getResponse().writeWith(Mono.error(...))
Mono.error(...)
throw MyException()

@bclozel
Copy link
Member

bclozel commented Jun 3, 2024

I think this behavior should be by design.

I believe the intent behind #22991 (aligning WebFlux with #22619) was for @ExceptionHandler methods to be involved for all WebHandler (WebFlux) and HttpRequestHandler (MVC).

On the other hand, I think we can compare WebFilter to Servlet filters as they operate at a lower level. Commit 2878ade does mention the WebFilterChain as a possible source of handled exceptions, but I don't think this is effectively the case. Filters in WebFlux are managed at the HttpHandler level, see:

https://github.com/spring-projects/spring-framework/blob/main/spring-web/src/main/java/org/springframework/web/server/adapter/WebHttpHandlerBuilder.java#L406-L411

I believe HttpHandler is the HTTP adapter layer, where Spring and other Frameworks/libraries can be plugged in the HTTP server. Handling errors at this level involves the WebExceptionHandler contract, which does not know about controllers or controller advices.

The MVC documentation points to other handlers, not Sevlet Filters:

Moreover, as of 5.3, @ExceptionHandler methods in @ControllerAdvice can be used to handle exceptions from any @Controller or any other handler.

@bclozel bclozel added the in: web Issues in web modules (web, webmvc, webflux, websocket) label Jun 3, 2024
@rstoyanchev
Copy link
Contributor

rstoyanchev commented Jun 3, 2024

Indeed this is by design. WebFilters are ahead of the DispatcherHandler and outside of higher level, web framework handling through annotated controller methods, and controller advice.

Depending on what you need to do, Boot error handling also could be helpful for errors that happen outside, as well as errors that escape and remain not handled.

@rstoyanchev rstoyanchev added status: waiting-for-feedback We need additional information before we can continue and removed status: waiting-for-internal-feedback An issue that needs input from a member or another Spring Team labels Jun 3, 2024
@bclozel
Copy link
Member

bclozel commented Jun 4, 2024

@kitkars you can implement your own WebExceptionHandler, and declare it as a bean in your application to handle such exceptions. Ordering it with @Order(-2) should place it right before the one from Spring Boot. See the error handling section of the Spring Boot docs.

Let us know how it works for you.

@simonbasle simonbasle removed their assignment Jun 6, 2024
@kitkars
Copy link
Author

kitkars commented Jun 7, 2024

Thanks @bclozel . I am already using workarounds. The controller-advice is one such elegant way to keep centralized error handling for an app. Thanks. We can close this as there are work arounds.

@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 Jun 7, 2024
@bclozel
Copy link
Member

bclozel commented Jun 7, 2024

Thanks for letting us know. I'm glad this works for your app.

@bclozel bclozel closed this as not planned Won't fix, can't repro, duplicate, stale Jun 7, 2024
@bclozel bclozel 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 Jun 7, 2024
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

5 participants