reqwest icon indicating copy to clipboard operation
reqwest copied to clipboard

Feature: A better way to read the response body, without losing access to the other data of the response

Open d4h0 opened this issue 2 years ago • 1 comments

Hi,

I like reqwest quite a lot, however there is often one significant pain point for me: If I read the response body (e.g. via json or text), the Response is consumed.

I've read why Response is implemented as it is, and I'm wondering if there is anything that can be done to improve the situation.

In end-user code, it is possible to work around this limitation (but I feel, it is still painful).

However, when designing an HTTP client that wraps reqwest, it seems my options are all suboptimal. Basically, this design choice influences my API design significantly.

Currently, I see the following options:

  1. I can use Response::chunk to read the body, without invalidating the Response. In this case, I have to reimplement what text, json, etc. does. text uses an unsafe block, so then my crate can't use #![forbid(unsafe_code)] (I believe. I've never actually used any unsafe code, so far. Which is another reason, why I'd like to avoid it, but performance is important to me).

  2. I can mem::take and clone the data I need from the Response, which for my wrapper crate, is everything. This adds some unnecessary allocation to my client, and adds a lot of additional code (I believe. Basically, I'd have to duplicate most of the API of Response).

As far as I understand the situation, Request anyway doesn't protect users completely from misuse. Or what happens, if I read the response body via chunk and then call text? To me, it seems, that is exactly what Response tries to prevent, but it seems easily doable (and there doesn't seem to be a note of what will happen, if text or similar is called afterwards).

I thought about, what could be done, and I came up with following possible solutions:

  1. Providing methods that can be used to take all needed values, without additional allocations. But how would that look for cookies without allocating? Url probably also can't be replaced without allocating (but I'm not sure). And this would mean, duplicating a lot of Responses code (in my case).

  2. Adding a way to provide text to variants of text and json that don't consume the Response (i.e. text_from_string). Then we could use chunk to get the body, and then call text_from_string to get the same behavior that text has.

  3. Just adding variants that don't consume the Resonse. This shouldn't be any different from the already existing chunk method.

  4. Adding variants of text and json that consume the Response and return a type that has all the data and methods of Response, but without the methods that read the body. Something like:

pub async fn into_response_with_body(self) -> ResponseWithBody {
  let body = self.read_body_via_chunk_or_similar();
  let body = self.text_from_string(body);
  // ^ `text_from_string` could be private, in this scenario
  ResponseWithBody::new(self, body)
}

I think, (4) is by far the best end-result. Ideally, there would be something similar that allows streaming the body (e.g. ResponseWithBody<String> and ResponseWithBody<Stream>).

(2) also would be okay, but wouldn't prevent misuses as (4) does (which basically prevents any misuse, especially, if chunk could be deprecated/removed from Response). On the other hand, in this case (3) would have the same trade-offs, and maybe would be easier to add.

Does anybody have any thoughts on this?

d4h0 avatar May 10 '22 18:05 d4h0

Thinking of submitting a PR for this but seeing the vast amount of unclosed PR's waiting for approval, I don't know if I should...

franklinblanco avatar Aug 04 '22 18:08 franklinblanco