Upgrade to 4.0.2 causes timeouts in Spring Framework's WebClient

[poutsma]

Upgrading to jetty-reactive-httpclient:4.0.2 causes timeouts in Spring Framework's WebClient when using Jetty's Reactive HTTP Client. We believe that this might be caused by the changes done as part of #323 or #194.

To reproduce the timeouts, use this branch of Spring framework: https://github.com/poutsma/spring-framework/tree/gh-31931 (the main branch is kept on 4.0.1 until this is resolved), and run the org.springframework.web.reactive.function.client.WebClientIntegrationTests in the spring-webflux module. All tests in this class result in timeouts when using JettyClientHttpConnector, while other connectors (Reactor Netty, JDK HttpClient, Apache HTTP Components) run fine.

Note that the WebClientIntegrationTests do not connect to a "real" web server, but use MockWebServer from OkHttp 3 instead. Using the WebClient with the JettyClientHttpConnector to connect to a real host, such as http://example.com, seems to work fine. So this could very well be an issue in MockWebServer, if it wasn't for the fact that other connectors work fine, and that we have not made any recent changes in the code that uses the Jetty's reactive HTTP client.

Additionally, it could very well be that we are using Jetty's reactive HTTP client in an incorrect manner. If so, please suggest any changes we can make. All relevant code is in JettyClientHttpConnector, JettyClientHttpRequest and JettyClientHttpResponse in the org.springframework.http.client.reactive package of the spring-webflux module.

[poutsma]

See spring-projects/spring-framework#31931.

[sbordet]

@poutsma I cannot build the branch because:

Could not GET 'https://repo.spring.io/plugins-release/io/spring/gradle/propdeps-plugin/0.0.9.RELEASE/propdeps-plugin-0.0.9.RELEASE.pom'. Received status code 401 from server

Apparently repo.spring.io is behind authentication.

[snicoll]

@sbordet how did you end up with that issue? We don't have any reference to propdeps-plugin that I am aware of.

[sbordet]

@snicoll I don't recall how propdeps-plugin became an issue... tried to import the project in IJ and building locally with gradle gave that error.
In any case it's gone now, so I can build and test.

I suspect the internal wiring performed by Spring is not 100% correct.
For example, both with Jetty RHC 4.0.1 and the JDK HttpClient, the code ends up into an IllegalStateException because the same Publisher is subscribed twice, and the Publisher only supports 1 Subscriber -- this is the response content Publisher.
For the JDK, the IllegalStateException is thrown from ResponseSubscribers.PublishingBodySubscriber.subscribe().

With Jetty RHC 4.0.2 we are more lenient for double subscriptions, but I can see that the second subscription happens in the context of the call to onComplete() to the first Subscriber.

I suspect it is not intended to subscribe twice to the response content because the second subscription will be pointless as in many cases the response content is not reproducible: it should be read only once.

The WebClientIntegrationTests only fails the tests that have a non-200 response status code.
I guess this has to do with the fact that non-200 are converted into exceptions, and the Mono is built in a way where the error event is converted to the complete event, and this possibly triggers the second subscription.

Jetty RHC reports a complete event after the response content has been read. This event is clearly reported to Spring, to the Flux created here: JettyClientHttpConnector.java:137.

I suspect that Flux is subscribed multiple times, but it is the second subscriber that would make things work; however the notification is sent to the first subscriber.
The second subscriber would obviously not find any content and therefore won't emit any event.

If I manually modify the code during debugging, so that an exception is thrown when the second subscription happens, the test passes 😄

Would you mind to have a look at why Spring subscribes twice?

My guess is that fixing that would fix the JDK client (that now works only because it throws an IllegalStateException like Jetty RHC 4.0.1 was doing), and also fix RHC 4.0.2.

Thanks!

[poutsma]

Thanks for the feedback, @sbordet. I will investigate, and get back to you.

[poutsma]

Thanks for the pointers, @sbordet. After making sure that response bodies can only be subscribed to once, we no longer see the timeouts we saw before. Closing this issue.