Retries on `response.read` and `response.aread` can't be handled inside `RetryTransport`

[vgavro]

Currently i got ReadTimeout and RemoteProtocolError("peer closed connection without sending complete message body ...") on httpx.Client.get, which is not handled on Transport level because errors raised during reading response, not returning response from Transport (which is expected by httpx API).
Transport is only returning response on receiving headers, but body fetching on Response.read/Response.aread is happening inside client logic.

Obviously, this should be mentioned in README, getting ReadTimeout on Client(transport=RetryTransport()).get() without any retries in logs looks like a bug (while it's expected behaviour).

How this could be solved:

1. class httpx.RetryClient(httpx.Client) and wrap stream/get/... there, or maybe RetryClient(original_client)
   We should use response.extensions["retries"] to pass retry object with left attempts.

2. resp = retry_response_read(client, "GET", url, **kwargs) (same as above, more explicit)

3. Maybe create wrapper and apply it on transport level like: response.stream = RetryStreamWrapper(response.stream, self.retry)
   Huge downsides i'm seeing here - we need to re-initiate request to get new response(?), which can be totally unexpected for cases like: resp = client.get(url); print(resp.headers()); resp.read()
   here either we update resp object, either return body of new (other) response - both approaches are bad if we need to send new request, of course.

4. other ideas?

[will-ockmore]

Hey @vgavro, thanks for raising this, it's a great point.

You're right that this is expected behaviour - transports aren't involved in reading a request body. At minimum this should be mentioned in the documentation.

I'll have a think on what the best solution is here. This is a subset of cases, but it's an important subset (where there is a slow or unresponsive server).

[Abdelgha-4]

I'm +1 on class httpx.RetryClient(httpx.Client) with a @classmethod from_httpx_client(cls, httpx_client: httpx.Client) -> Self.

[will-ockmore]

I've added an explanation for this behaviour in the FAQ on the docs site. I am planning on implementing the RetryClient soon. I agree that it is the best approach, and this limitation needs to be addressed.

[vgavro]

@will-ockmore tbh I really think that RetryClient approach is last resort, RetryTransport approach allows to inject retry logic to customized httpx clients, which is great benefit from my point of view.

More important, I don't see clean way to have auto-retries inside stream() method. Client receive response, user reads response.headers, user invoke response.read() - if it fails and client retries it automatically - response.headers will be changed and this is too unexpected for user.. not to say how to implement iterative reading with timeouts...

What do you think about the idea to implement stream=False option to RetryTransport? (Defaults to true).
This allows to 1) read and retry response on transport level before returning to client's interfaces (including stream() interface), also 2) gives clean way to explicitly say - you're using this transport not in stream level, so even Client.stream isn't broken, it's not really stream anymore.
Also - we can have request(extensions={stream: False}) to have this retry logic per-request, and allow user to use Client.stream anyway and handle read retries on their own.