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

Allow @ControllerAdvice in WebFlux to handle exceptions before a handler is selected #22991

Closed
wangxing-git opened this issue May 18, 2019 · 9 comments
Assignees
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) type: enhancement A general enhancement
Milestone

Comments

@wangxing-git
Copy link

Affects: 5.1.7.RELEASE


When I tried to handle the http 405 error with @ExceptionHandler I found that I couldn't catch the MethodNotAllowedException exception in webflux, which can be done in the web mvc by catching the HttpRequestMethodNotSupportedException exception. Is this a bug or is it not supported or is there another way to do it?

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label May 18, 2019
@wangxing-git
Copy link
Author

I found another way to do this, using WebExceptionHandler.

        @Bean
        @ConditionalOnMissingBean(name = "globalWebExceptionHandler")
        @Order(Ordered.HIGHEST_PRECEDENCE + 1000)
        public WebExceptionHandler globalWebExceptionHandler() {
            return ReactiveExceptionUtils::handleException;
        }

I delegated the exception to the @ExceptionHandler bean.
But I think this is not very good. Is there any better advice?

@rstoyanchev
Copy link
Contributor

An @ExceptionHandler only handles errors from the controller, while the 405 happens earlier as part of request mapping. The WebExceptionHandler mechanism is global enough to handle all errors so that's the right place to do it.

As you are in a Spring Boot application there are options to customize error handling at that level.

@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 labels May 20, 2019
@gianielsevier
Copy link

I'm having the same issue I was expecting the spring framework to be agnostic of the server and I could centralize the exception handling from my ControllerAdvice as @wangxing-git mentioned we can do that when running the application with tomcat. Is there a way to bring this convenience to the framework?

@rstoyanchev
Copy link
Contributor

rstoyanchev commented Oct 12, 2022

The handling of exceptions before a controller method is selected is a relatively recent Spring MVC feature, added in 5.3 with #22619. We could align the two and provide an equivalent feature in WebFlux.

@rstoyanchev rstoyanchev reopened this Oct 12, 2022
@rstoyanchev rstoyanchev added in: web Issues in web modules (web, webmvc, webflux, websocket) type: enhancement A general enhancement and removed status: invalid An issue that we don't feel is valid labels Oct 12, 2022
@rstoyanchev rstoyanchev added this to the 6.x Backlog milestone Oct 12, 2022
@rstoyanchev rstoyanchev changed the title MethodNotAllowedException cannot be handled by @ExceptionHandler on webflux Allow @ControllerAdvice in WebFlux to handle exceptions before a handler is selected Oct 12, 2022
@M-Whitaker
Copy link

Given the new addition of ResponseEntityExceptionHandler as a part of #27052 I would expect there this to work. As of the moment I can't get a subclass (annotated with ContollerAdvice) to handle exceptions like method not supported even though the code implies this should work. Maybe be missing a configuration item though...

@rstoyanchev rstoyanchev modified the milestones: 6.x Backlog, 6.0.0-RC4 Nov 7, 2022
@rstoyanchev rstoyanchev self-assigned this Nov 7, 2022
rstoyanchev added a commit that referenced this issue Nov 8, 2022
Commit #2878ad added the DispatchExceptionHandler contract for
mapping an error before a handler is selected to a HandlerResult.
The same is also convenient for use in HandlerResult itself which
currently uses a java.util.Function essentially for the same.

See gh-22991
rstoyanchev added a commit that referenced this issue Nov 9, 2022
@M-Whitaker
Copy link

Hi, this is still happening with a 404 as it comes back with the original response:

{
    "timestamp": "2022-11-10T09:40:11.551+00:00",
    "path": "/",
    "status": 404,
    "error": "Not Found",
    "message": null,
    "requestId": "3da6254c-1"
}

I would have thought this would have been fixed with 307247b is the intended behaviour?

@rstoyanchev
Copy link
Contributor

Odd, we have a test for that.

@M-Whitaker
Copy link

Ah I see, that is not what is being displayed. Do you want a zip project file with the issue?

@rstoyanchev
Copy link
Contributor

Sure, or a github repo.

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) type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

5 participants