`Body` should implement `AsyncRead`

[cramertj]

It seems logical for Body to implement the tokio-io AsyncRead trait. Is there a particular reason it does not, or is this just awaiting an implementation?

[cramertj]

In particular, this is nice when using tokio_io::io::copy to copy the body into an AsyncWrite. Currently, this looks like:

res.body().fold(sock, |sock, chunk| {
    tokio_io::io::write_all(sock, chunk).map(|(s, _)| s)
})

With Body: AsyncRead, you can do:

tokio_io::io::copy(res.body(), sock)

[seanmonstar]

The reason it doesn't is it can't be done in the type without some overhead, and it'd basically be an adapter of Stream<Item=impl AsRef<[u8]>> to AsyncRead, which isn't too hyper-specific.

[cramertj]

an adapter of Stream<Item=impl AsRef<[u8]>> to AsyncRead

Yeah, I guess I'd expect such a thing to exist in some form or another. Do you have any thoughts about how to make these types line up better? The fold works, but it's definitely not very intuitive.

[seanmonstar]

If the other end is a Sink, there's stream.forward(sink) or sink.send_all(stream)...

[cramertj]

So, what I want then is probably something like res.body().forward(my_async_rw.framed(bytes())) where bytes() is something like tokio-rs/tokio-io#78 ? Interestingly, even this only works because my AsyncWrite also happens to be an AsyncRead-- if it wasn't, framed wouldn't be available.

Edit: if that were the case, I could still used FramedWrite::new.

[cramertj]

@seanmonstar In order for the stream.forward trick to work, the type has to implement Sink with SinkItem = hyper::Chunk. Maybe this could work in the future when rust-lang/futures-rs#623 is resolved so that AsyncWrites could automatically implement Sink and accept items of all T: AsRef[u8].

[seanmonstar]

I'm going to close this for now, because it can't be done without overhead that would affect anyone who didn't need the AsyncRead implementation.

[cramertj]

@seanmonstar SGTM-- I'll keep this in mind to come back to once rust-lang/futures-rs#623 is resolved.

[ry]

@seanmonstar Can you suggest a way to turn a Body into an AsyncRead ?

I guess the reason why there's overhead in presenting Body as an AsyncRead is that Body promises to delivery a "Chunk" - which I infer corresponds to chunks in transfer-encoding: chunked. These chunks are not bounded in length, and so cannot be guaranteed to be written completely into the buffer supplied by poll_read() - and therefore it would require internal buffering to present an AsyncRead? Do I have this right?

If the above is correct, it seems the problem is that Body is too high-level of a concept. It would be desirable to have lower-level body which didn't promise to deliver complete chunks, but just parts of the body as they arrived.

[seanmonstar]

@ry

I infer corresponds to chunks in transfer-encoding: chunked. [..] Do I have this right?

No, the Chunk type is not actually related to transfer-encoding: chunked. (Maybe the name is confusing and should be something else?) A Chunk is simply a piece of the body as it arrives. hyper does hardly any buffering (the only buffering is that there needs to be some sort of buffer to read at all, and if the headers are smaller than the buffer, or a chunked-encoding body has chunks smaller, than some of the body will still be in the read buffer). Even when chunked encoded, the Chunks are yielded as soon as they are received from the transport.

---

The overhead I was talking about here is around the need for an impl AsyncRead to buffer a single Chunk at times. This is because you could call poll_read(buf_with_1kb_space), and a Chunk from the transport could have been 4kb. In that case, it needs to poll_data for a Chunk, copy over 1kb, and then save the Chunk so the other 3kb can be read.

The reqwest crate has a ReadableChunks type that adapts a Body into an AsyncRead.

[spease]

It looks like ReadableChunks has been removed from reqwest. What’s the recommended method to expose an AsyncRead trait for the incoming response data now? Thanks.

[let4be]

It would be really nice to have this in hyper...