HttpHandlerConnector does not propagate errors once response is committed

[bclozel]

Since gh-23936, the HttpHandlerConnector uses a non-blocking thread for handling requests, thanks to Mono.defer and Schedulers.parallel().

With this new arrangement, any error happening after the response is committed is completely ignored and dropped. The resulting Mono<ClientHttpResponse> returned by the connector never completes and hangs.

Here is a test reproducing this issue:

@Test
public void handlerThrowingException() {
  TestHttpHandler handler = new TestHttpHandler(response ->
      response.setComplete()
          .then(Mono.error(new IllegalStateException("error while writing response body"))));

  Mono<ClientHttpResponse> connect = new HttpHandlerConnector(handler)
      .connect(HttpMethod.POST, URI.create("/path"), request -> request.writeWith(Mono.empty()));
  StepVerifier.create(connect)
      .expectError(IllegalStateException.class).verify();
}

This can happen if the server encounters an error while writing the response body (and the response has been completed already.

We've hit that problem in Spring Boot while upgrading Spring Framework, see spring-projects/spring-boot#19076.

I'm not sure how we're suppose to handle such errors and if WebTestClient should react. We might need to fix the test itself in Spring Boot, but I'd like to clarify that point first.

[rstoyanchev]

The MonoProcessor<ClientHttpResponse> in HttpHandlerConnector which is used to pass the return value down the chain can be set 1) when the response is completed, or 2) when handling completes or errors. In this scenario both 1) and 2) are taking place.

After the change in #23936 to use a non-blocking thread, this is all now wrapped with Mono#defer and put on a thread marked "non-blocking". That seems to be causing the connect to never complete, and there is also this "Operator called default onErrorDropped" in the logs.

I suspect there may be a Reactor bug for the hanging part. That said we are responsible for trying to set a MonoProcessor twice which should not be taking place. We have to decide which result we want to complete the test with -- the completed response or the error that happened after it was completed?

I can see arguments for both sides. The simplest thing for a "mock" server to do is to assume the completed response is the final thing, since a real server may or may not have actually committed the response yet, and we would just log the error that is effectively dropped.

On the other hand it's easy to miss such an error in the logs arguably it might be more useful for a test to allow them to be "seen" and handled. This would also preserve current behavior.