Retries

[tomchristie]

Our yardstick for 1.0 has generally been to achieve feature parity with requests, plus the following additional support...

• Sync + Async support.
• HTTP/2 support.
• Strict timeout defaults.
• Type annotations throughtout.
• Streaming multipart uploads.

Something we've so far omitted is retires=..., which are not included in either the requests QuickStart guide, or in the Advanced Usage section. However requests does have some retry support, can be enabled by mounting a custom adapter... https://requests.readthedocs.io/en/master/api/#requests.adapters.HTTPAdapter

I'd suggest that if we do want to add retry support we should start off by doing so in a limited fashion, and only provide a simply retries=<int> argument on the client, which matches the same "only retry on connection failures" behaviour that requests defaults to for integer retry arguments.

That doesn't necessarily preclude that we could consider more complex retry behaviour at some point in the future, but I'm prefer we push back on that for as long as possible.

Having more dials for configuration is something we should generally avoid where possible. Also, we've gone to a great deal of effort over nicely spec'ing our Transport API, and it's perfectly feasible for developers to build against that to deal with any more complex behaviours that they'd like to see.

With all that in mind, I'd suggest something we might consider would be...

class Client(..., retries: int=0):
    ...

With the implementation handled inside our _send_single_request method. Something like...

transport = self._transport_for_url(request.url)
retries = self.retries

while True:
    with map_exceptions(HTTPCORE_EXC_MAP, request=request):
        try:
            (
                http_version,
                status_code,
                reason_phrase,
                headers,
                stream,
            ) = transport.request(
                request.method.encode(),
                request.url.raw,
                headers=request.headers.raw,
                stream=request.stream,
                timeout=timeout.as_dict(),
            )
       except (httpcore.ConnectError, httpcore.ConnectTimeout):
            if retries <= 0:
               raise
            retries -= 1

response = Response(
    status_code,
    http_version=http_version.decode("ascii"),
    headers=headers,
    stream=stream,  # type: ignore
    request=request,
)

self.cookies.extract_cookies(response)

status = f"{response.status_code} {response.reason_phrase}"
response_line = f"{response.http_version} {status}"
logger.debug(f'HTTP Request: {request.method} {request.url} "{response_line}"')

return response

I'm still not 100% sure that we want this, but perhaps this is a decent low-impact feature.
Any thoughts?

[StephenBrown2]

Thanks very much for considering this again. Sorry I've been such a squeaky wheel about it. 🙃 I'm definitely a 👍 on this MVP, which will bring forth closer parity with requests core, and the more advanced/granular control (i.e. retrying on status codes/respecting the "Retry-After" header, inspecting the body, etc) can be done by either implementing a similar class to urllib3 for config:
https://urllib3.readthedocs.io/en/latest/reference/urllib3.util.html#urllib3.util.retry.Retry

class urllib3.util.retry.Retry(
    total=10,
    connect=None,
    read=None,
    redirect=None,
    status=None,
    method_whitelist=frozenset(["HEAD", "TRACE", "GET", "PUT", "OPTIONS", "DELETE"]),
    status_forcelist=None,
    backoff_factor=0,
    raise_on_redirect=True,
    raise_on_status=True,
    history=None,
    respect_retry_after_header=True,
    remove_headers_on_redirect=frozenset(["Authorization"]),
):

OR documenting use of the Middleware implementation that @florimondmanca has done the initial legwork on in #1134

[tomchristie]

Ah good reminder there - I've missed off doing any sleep / back off in that example, which it probably should include.

[StephenBrown2]

Example of use: https://findwork.dev/blog/advanced-usage-python-requests-timeouts-retries-hooks/#retry-on-failure

[florimondmanca]

Reminder of the proposed API from #784 (which I think pretty much fits the bill for what we're trying to do, actually, even implementation wise) - items in bold not present in issue description here:

---

• Retries are off-by-default.
• Configurable both on client and per-request, with "request overrides client value" (default is retries = 0 (None is invalid)).
• They're only applied for on connection failures, and for idempotent HTTP verbs only.
• Allows customizing the max number of retries and a backoff factor for sleep in-between retries.
• Example usage:

client = httpx.Client(retries=<int>)
client = httpx.Client(retries=httpx.Retries(<int>[, backoff_factor=<float>])

---

Notes:

• <int> or <Simple config class> follows our standards for eg timeouts and proxies.
• For <int> usage we'd use a sensible default backoff factor, such as 0.2 (I think that's what Retries for connection failures #784 used?).

(P.S.: @StephenBrown2 Note that I'm not planning we merge the middleware idea just yet. :-))

[florimondmanca]

We also had some discussion around Retry-After support and retry-on-some-status-codes: #784 (comment)

I don't think we should plan for this in core (as noted in the comment, there are some tricky design considerations to prevent users from shooting themselves in the foot), but that's only my current impression.

[StephenBrown2]

As long as further configuration can be added to the httpx.Retries class later (which it looks like it most certainly could without breaking any interfaces), seems reasonable to me, as it meets the current minimal requirements and allows for further configuration customization later.

[florimondmanca]

Yes, we can imagine making a main method for "decide whether we should retry and when" public at some point to allow for extension, a bit similar to our Auth interface. But for now it's probably better to keep things private. :-) (Tom already mentioned this back then: #784 (review))

[StephenBrown2]

Oh I didn't mean user-extensibility. I meant adding more well-thought out knobs to the core httpx.Retries class itself.

[JayH5]

My 2c having used requests and various API client libraries: a couple of things I'd like to see considered, but aren't necessarily have-to-haves:

1. It would be cool if the retry algorithm could include jitter. I know we don't want to add more dials/adjustments to the API, but afaik implementing some kind of default jitter amount shouldn't add a lot of complexity. The go-to reference for this is usually this AWS blog post. I know urllib3 doesn't do this, but it'd be great if httpx could support what is widely considered to be a best-practice out-of-the-box (kinda like how we have timeouts by default).
2. The way timeouts and retries interact should be considered. For example, a client library I've used before for a certain time-series database has timeouts and a very naïve retry implementation. Some queries are long-running and should really timeout, but setting a timeout like 30 seconds with a retry count of 3 (and these are the only adjustments available) results in calls that take up to 90 seconds--which is really not what you want. A decent amount of this is due to their poor implementation, but it'd be good if we could put a little bit of thought into this, even if it's just documentation. e.g. we should only retry on connect timeouts and suggest that separate connect and read timeouts are used with retries.

[tomchristie]

@JayH5 There's a really interesting observation there, in particular wrt. timeouts and retries.

The implication that follows is that for connection retries what we really want is retries managed at the Transport API level,
so that we can have something like this on connects...

attempts = 0
start_time = time.time()
elapsed = 0.0
next_timeout = timeout

while attempts <= retries and next_timeout > 0.0:
    try:
        connect(..., timeout=timeout)
    except ConnectFailed:
        next_timeout = timeout - elapsed
        elapsed = time.time() - start_time
        attempts += 1

That's quite nice because we're always strictly observing the connection timeout, while also adding retry support within the available time window.

The jitter idea is interesting. I guess it's not always going to be as relevant in the HTTP context, because most resources you're accessing will have a whole range of different clients connecting, and you're getting a much more messy skew of connection times, retries, etc. rather than eg. in the database case where you've got a bunch of connections all doing exactly the same thing, and it's easier to see how jitter could help there. However, the same can be true in the HTTP context sometimes. (Say for inter-service communications.)

[tomchristie]

Couple of data points from big open source Python services...

PyPI - Uses a mounted Session with max_retries=int.
ReadTheDocs - Uses a mounted Session with max_retries=<Retry class>

• https://github.com/pypa/warehouse/blob/1fbb4ac752e68b5840b9e09b68e44a165569bfa6/warehouse/http.py
• https://github.com/readthedocs/readthedocs.org/blob/master/readthedocs/api/v2/client.py

At this point in time I'd be leaning towards httpx exposing a single connect_retries=int at the transport level, mirroring requests, and in line with our uds and local_address options. A nice effect of that is that we can ensure that it allow plays properly with the connect timeout.

We can also still expose more complex retry configuration further down the road.

[florimondmanca]

@tomchristie Are you meaning - add a connect_retries=<int> option to the HTTPCore connection pool classes?

I've played with this idea a bit and it's a bit awkward. Eg for testing we'd like to mock the network calls, but since the retry behavior would be tightly coupled with the "open a socket" operation, it seems pretty hard to test, which seems to be a big smell.

Instead I'd be okay with a connect_retries=<int> option only (with a hardcoded backoff), but at the HTTPX level, which as you said we can expand down the road if we want to.

[mvbrn]

Are there any plans to support read/write retries too?

[gerardo-garcia]

It seems to me that retry on failure is not supported. Maybe I misunderstood, but I think that the current mechanism only adds connect retries. I would like to combine timeout with retries on failure, so that a request is retried max_retries with a backoff factor, where every request has a defined timeout. Would it be possible?

As a reference, the behaviour would be similar to what can be achieved in requests, as described in this
link. It uses a TimeoutHTTPAdapter class, child of HTTPAdapter, a Retry class, an a requests session mounting the TimeoutHTTPAdapter with the Retry class. Below a link to a public gist that should work in requests as a reference:
https://gist.github.com/gerardo-garcia/c8e7bd277b43a44b3958e231efea82eb

Any help on this will be welcome.