Spring Kotlin code that uses WebClient fails after upgrade to Spring 5.2

Affects: 5.2

Consider the following usage of WebClient:

suspend fun getApiResponse(): String {
    val webClient = WebClient.create(apiUrl)

    val response = webClient.get()
            .uri("/resource")
            .exchange().awaitFirst()

    // inspect response, like response.statusCode(); maybe return different result based on response attributes

    return response.bodyToMono(String::class.java).awaitFirst()
}

This code is leveraging kotlinx-coroutines-reactor extensions to use WebClient in imperative manner. It works fine on Spring Framework 5.1.

After upgrade to 5.2, this code starts failing on response.bodyToMono(String::class.java).awaitFirst() with the following exception:

The client response body has been released already due to cancellation.
java.lang.IllegalStateException: The client response body has been released already due to cancellation.
    (Coroutine boundary)
    at com.example.demo.ApiConsumer.getApiResponse$suspendImpl(ApiConsumer.kt:21)
...
Caused by: java.lang.IllegalStateException: The client response body has been released already due to cancellation.
    at org.springframework.http.client.reactive.ReactorClientHttpResponse.lambda$getBody$0(ReactorClientHttpResponse.java:98)
    Suppressed: reactor.core.publisher.FluxOnAssembly$OnAssemblyException: 
Error has been observed at the following site(s):
    |_ checkpoint ⇢ Body from GET http://localhost:37917/resource [DefaultClientResponse]
...

It looks like the fix for #25216 changes the behaviour of WebClient in a way this code starts failing.

Please find the full reproducer at orange-buffalo/spring-5.2-web-client-kotlin-reproducer@6ffd25af2a64fc27b37890d35f698e5f37df7b52 . CancelledResponseReproducer fails with 5.2, downgrading the dependency to 5.1 makes the test pass.

Comment From: jhoeller

@rstoyanchev @sdeleuze Seems like one to look at for 5.2.10...

Comment From: rstoyanchev

awaitFirst() cancels the Publisher after the first item. In this case when exchange() returns the ClientResponse the cancel leads to the response being drained and the connection closed to avoid leaks. awaitSingle() on the other hand only consumes the value and does not cancel, i.e. expects a single item only and lets the Publisher complete without a proactive cancellation. This makes awaitSingle() the right choice.

So I think this is expected behavior, essentially avoid using awaitFirst where only a single value is expected (and guaranteed) based on a Mono return value. It is true the above did work prior to 5.2 and that makes it look like a regression but the cancellation was only ignored due to a bug and we can't just revert to the previous behavior.

As an aside, the exchange() method which returns a ClientResponse is deprecated altogether in 5.3 with #25751 because it allows the responses which contains resources that can be leaked to be used without any possibility for Spring to ensure the resources are properly consumed or released in all circumstances.

Comment From: orange-buffalo

Thank you for the detailed explanation and for pointing to the proper method, @rstoyanchev. We all appreciate the efforts made to provide the high-quality resource handling.

Just a small suggestion regarding exchangeToMono introduced in 5.3. Current documentation does not provide any code sample for Kotlin. It would be helpful for the community to see what is the canonical way to use this functionality in Kotlin suggested by the Core Spring team.

Comment From: rstoyanchev

@orange-buffalo thanks for pointing that out. I am actually in communication with @sdeleuze about that but in the mean time I've re-opened #25751.