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 Routing DSL's onError filters execute in unexpected order #25541

Closed
Megamiun opened this issue Aug 6, 2020 · 2 comments
Closed

Webflux Routing DSL's onError filters execute in unexpected order #25541

Megamiun opened this issue Aug 6, 2020 · 2 comments
Assignees
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) type: bug A general bug
Milestone

Comments

@Megamiun
Copy link

Megamiun commented Aug 6, 2020

Affects: Spring Framework 5.2.7

Hello. I am trying to use the onError functions of the Kotlin DSL coRouter to create a global error handler on the root of my routes.

Everything was looking right at first, but after some times, we perceived some errors were not being captured by the right handler:

    @Bean
    fun rootRoute() = coRouter {
        GET("/error") { throw IOException("") }

        onError<IOException> { e, _ ->  badRequest().bodyValueAndAwait("IOException") }
        onError<Exception> { e, _ -> badRequest().bodyValueAndAwait("Exception") }
    }

In this MWE, it seems that every time we hit "/error", and therefore have an IOException, the exception is captured by the generic Exception handler, instead of by the IOException one. After some investigation, we discovered that if we switched the order of the onErrors, the correct filter start being applied.

It seems that although the error filters are arranged in the correct order inside RouterFunctionBuilder, this ordering causes the last error handlers being verified before.

I found no documentation explaining how the onError handlers worked when using the CoRouterFunctionDsl and RouterFunctionDsl, but I am almost certain this is not the desired behaviour, and if it is, we have to document it better.


As an extra example:

    @Bean
    fun rootRoute() = coRouter {
        GET("/error") { throw IOException("") }

        onError({ println("first"); false }) { e, _ -> badRequest().buildAndAwait() }
        onError({ println("second"); false }) { e, _ -> badRequest().buildAndAwait() }
    }

This example, when doing a GET at "/errors", prints the following lines:

second
first
@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Aug 6, 2020
@poutsma poutsma self-assigned this Sep 10, 2020
@poutsma poutsma added in: web Issues in web modules (web, webmvc, webflux, websocket) type: bug A general bug and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels Sep 11, 2020
@poutsma poutsma modified the milestones: 5.2.9, 5.3 RC2 Sep 11, 2020
@poutsma
Copy link
Contributor

poutsma commented Sep 14, 2020

You are correct, there is an issue here, and the order of onError (and also after) filters should be revised. But because there is also existing behavior, and any change in this area might break existing applications, I am setting the milestone to 5.3 RC2.

@poutsma poutsma closed this as completed in 392895e Oct 6, 2020
@poutsma
Copy link
Contributor

poutsma commented Oct 6, 2020

After team discussion, it was decided to leave the after filter order as is, but only change the onError order.

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: bug A general bug
Projects
None yet
Development

No branches or pull requests

3 participants