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

Comment From: poutsma

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.

Comment From: poutsma

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