line / armeria

Your go-to microservice framework for any situation, from the creator of Netty et al. You can build any type of microservice leveraging your favorite technologies, including gRPC, Thrift, Kotlin, Retrofit, Reactive Streams, Spring Boot and Dropwizard.
https://armeria.dev
Apache License 2.0
4.83k stars 919 forks source link

Making Spring WebClient calls causes RequestContext.current() to throw IllegalStateException #3346

Open michaelpearce-at opened 3 years ago

michaelpearce-at commented 3 years ago

We are running a Spring Boot WebFlux service written in Kotlin with coroutines. The Armeria Spring Boot starter has been used, with Armeria providing the org.springframework.web.reactive.function.client.WebClient implementation.

In any handler function, we're seeing that making any HTTP call with the Armeria WebClient causes any subsequent calls to RequestContext.current() throw IllegalStateException with "RequestContext unavailable".

We noticed the issue as we have a decorator to pass on some specific request headers from the originating request to our gRPC endpoints. When we added a WebClient call before making the gRPC calls, the decorator was no longer able to grab the headers as the context is not found.

Here's a minimal service which reproduces the issue. The first log prints the context, the second one throws IllegalStateException.

import com.linecorp.armeria.common.RequestContext
import org.slf4j.LoggerFactory
import org.springframework.boot.autoconfigure.SpringBootApplication
import org.springframework.boot.runApplication
import org.springframework.context.annotation.Bean
import org.springframework.context.annotation.Configuration
import org.springframework.web.reactive.function.client.WebClient
import org.springframework.web.reactive.function.client.awaitExchange
import org.springframework.web.reactive.function.server.ServerResponse
import org.springframework.web.reactive.function.server.bodyValueAndAwait
import org.springframework.web.reactive.function.server.coRouter

@SpringBootApplication
class ContextBugApplication

fun main(args: Array<String>) {
    runApplication<ContextBugApplication>(*args)
}

@Configuration
class ContextClobberingConfiguration(
    webClientBuilder: WebClient.Builder
) {
    private val webClient: WebClient = webClientBuilder.baseUrl("https://httpstat.us").build()
    private val logger = LoggerFactory.getLogger(this::class.java)

    @Bean
    fun routes() = coRouter {
        GET("/").invoke {
            RequestContext.current<RequestContext>().also {
                logger.debug("Request context: $it") // Request context: [sreqId=..., chanId=..., ...]
            }

            webClient.get().uri("/200").awaitExchange()

            RequestContext.current<RequestContext>().also {
                logger.debug("Request context: $it") // throws IllegalStateException
            }

            ServerResponse.ok().bodyValueAndAwait("ok")
        }
    }
}
minwoox commented 3 years ago

Hi @michaelpearce-at, I think the request context is gone after awaitExchange is called so we should do the request scoping by ourselves. Could you try that using the coroutin dispatcher? https://github.com/line/armeria/blob/master/examples/context-propagation/kotlin/src/main/kotlin/example/armeria/contextpropagation/kotlin/MainService.kt#L43

michaelpearce-at commented 3 years ago

Hi @minwoox, thanks for the quick response!

I can confirm that when I explicitly use the coroutine dispatcher (by wrapping the handler code with withContext), I am then able to successfully access the root context before and after making the WebClient call.

val dispatcher = RequestContext.current<RequestContext>().eventLoop().asCoroutineDispatcher()
withContext(dispatcher) {
    webClient.get().uri("/200").awaitExchange { }

    RequestContext.current<RequestContext>().also {
        logger.debug("Request context: $it") // Success
    }

    ServerResponse.ok().bodyValueAndAwait("ok")
}

Is this expected behaviour? We don't see the same thing when making gRPC calls (using a FutureStub with .await()) - the original request context is preserved afterwards.

minwoox commented 3 years ago

using a FutureStub with .await()

Could you share your code, please?

Is this expected behaviour?

I believe so. ๐Ÿ˜„ The coroutine does not have the request context after it's resumed from the suspended function so we should do the request scoping by ourselves. ๐Ÿ˜„

ikhoon commented 3 years ago

Is this expected behaviour?

Yes, it is possible. If the current CoroutineContext is an EmptyCoroutineContext, a coroutine could be changed after executing a suspend function.

ikhoon commented 3 years ago

/cc @okue who is good at Armeria with Kotlin Coroutine ๐Ÿ˜€

michaelpearce-at commented 3 years ago

Could you share your code, please?

We have a grpc client generated with io.grpc:protoc-gen-grpc-java, and instantiated like, for example,

val grpcClient = Clients.builder("gproto-web+http://localhost:3001")
                .build(SomeServiceGrpc.SomeServiceFutureStub::class.java)

When I replace the webClient.get().awaitExchange() in the above sample code with a grpc call grpcClient.someCall.await() (here await is provided by org.jetbrains.kotlinx:kotlinx-coroutines-guava https://github.com/Kotlin/kotlinx.coroutines/blob/66fe1c8eb99ef1464248e291adfaa647e85a4e24/integration/kotlinx-coroutines-guava/src/ListenableFuture.kt#L228), the Armeria root request context is still accessible in the coroutine afterwards where it is not after a WebClient call.

ikhoon commented 3 years ago

And if you are using Armeriaโ€™s annotated service with a suspend function, Armeria automatically injects RequestContext-aware CoroutineContext for you. https://armeria.dev/docs/server-annotated-service#kotlin-coroutines-support

michaelpearce-at commented 3 years ago

And if you are using Armeriaโ€™s annotated service with a suspend function

For background, we added Armeria as the underlying web server to an existing, established, Spring Webflux application built with functional routes, not annotations. While we could switch to Armeria annotations, it would be a larger refactor. If it turns out that is the most effective approach, we can tackle that work in the future.

Right now though, I'm just trying to understand the reason why the Armeria request context is accessible from a suspending function before I make a Spring WebClient call, but not available (in the same function) after making the call. I couldn't find anything in Armeria's documentation about having to manually manage scopes, I was expecting that a coroutine launched from a functional webflux endpoint served from Armeria would maintain the context throughout its life. I may well be fundamentally misunderstanding something about coroutines though ๐Ÿ˜…

minwoox commented 3 years ago

While we could switch to Armeria annotations, it would be a larger refactor. If it turns out that is the most effective approach, we can tackle that work in the future

I think you don't have to. ๐Ÿ˜„ I guess @ikhoon is just telling an option.

Armeria's documentation about having to manually manage scopes,

Sorry, we need one. ๐Ÿ˜… You might want to refer to a slide though. https://github.com/minwoox/deview2020-requestscoping/blob/master/DEVIEW2020_RequestScoping.pdf