Unclear IllegalStateException when trying to pipe an HttpResponse that's already been read into another stream

[noobgramming]

Version

Vert.X Java 4.3.3

Context

I encountered this exception while trying to pipe an HttpResponse to another stream after it had already been read

Do you have a reproducer?

No, but the cause is relatively easy to explain

Reproducing

1. Make any HTTP request using Vert.X client that returns a response body
2. Read this response body using body().asJsonObject() or similar
3. Try to pipe the original response into another stream using pipeTo()

Example trace - trying to pipe a response body into the body of a new request (proxying a response body from one request into the request body of another) after previously reading the response body using body().asJsonObject()

java.lang.IllegalStateException
    at io.vertx.core.http.impl.HttpClientResponseImpl.checkEnded(HttpClientResponseImpl.java:150) ~[app.jar:?]
    at io.vertx.core.http.impl.HttpClientResponseImpl.endHandler(HttpClientResponseImpl.java:172) ~[app.jar:?]
    at io.vertx.core.http.impl.HttpClientResponseImpl.endHandler(HttpClientResponseImpl.java:38) ~[app.jar:?]
    at io.vertx.core.streams.impl.PipeImpl.<init>(PipeImpl.java:35) ~[app.jar:?]
    at io.vertx.core.streams.ReadStream.pipeTo(ReadStream.java:134) ~[app.jar:?]
    at io.vertx.core.http.HttpServerResponse.send(HttpServerResponse.java:359) ~[app.jar:?]
    at io.vertx.mutiny.core.http.HttpServerResponse.lambda$send$18(HttpServerResponse.java:677) ~[app.jar:?]
    at io.smallrye.mutiny.vertx.AsyncResultUni.subscribe(AsyncResultUni.java:31) ~[app.jar:?]

Discussion

This took me a while to track down because there's no exception message. It's clear why this happens - once the read cursor for the response body advances beyond the beginning you really shouldn't be able to read it again because some of the Buffers containing it may have already been discarded.

IMO Vert.X should throw an Exception with a message like "Error - Response body has already been read and can only be read once" if you try to read it multiple times.

I would be happy if we just added a useful message to this exception

  private void checkEnded() {
    if (trailers != null) {
      throw new IllegalStateException();
    }
  }

But perhaps it would be even more helpful if HttpClientResponseImpl tracked how many bytes have been read from the response, and if it is "finished" reading. This would prevent people from - example - reading 100 bytes of the response then trying to call body().asJsonObject() on the remainder. Or calling body().asJsonObject() twice. Both of which would hardly, if ever, make sense to do.

We have been using Vert.X HTTP client quite a bit recently, and multiple/partial reads of the response body have been a recurring bug in our codebase that's been hard to track down

[vietj]

I think we can indeed improve error handling here.

Can you elaborate about partial read use cases ? it is not clear to me. Does it happen with WebClient ?

[noobgramming]

It happens when you read the request body in full using one of the convenience methods like io.vertx.ext.web.RequestBody.body().asJsonObject() then try to use pipeTo() to send the body to another stream.

I'm not sure if partial reads would cause the same issue, that's just speculation because the error is thrown when checking if trailers have been sent yet