httpx: Passing 'timeout=None' should always result in no timeout

Spun up from https://github.com/encode/httpx/issues/411

Currently, the behavior of passing None to the timeout parameter in the various request entrypoints is not consistent:

import httpx

url = "https://httpbin.org/delay/10"

# (1)
httpx.get(url, timeout=None)  # Times out after 5s

# (2)
client = httpx.Client(timeout=None)
client.get(url)  # Does not timeout, returns after 10s

# (3)
client = httpx.Client()
client.get(url, timeout=None)  # Times out after 5s

We should make sure passing None always results in the behavior (2), i.e. no timeout should be applied, as opposed to (1) and (3) where the default timeout configuration is used.


The implementation suggested in #411 is to use a sentinel object:

# models.py
UNSET = object()

# api.py
def get(..., timeout: TimeoutTypes = UNSET, ...):
    ...

We need to switch to using UNSET in all relevant places. For example (though these may not end up being relevant, as I have not solved this myself):

  • Functions in httpx/api.py.
  • Methods of BaseClient, Client and AsyncClient in httpx/client.py.
  • Methods of dispatchers in httpx/dispatch/, esp. .send().
  • Possibly other places that aren’t on the top of my head at the time of writing this. 😃

Next, we need to check for that sentinel instead of None when deciding whether to use the default timeout config.

Notable places affected by this switch are:

  • .send() methods in clients and dispatchers (only use the client’s or dispatcher’s self.timeout if timeout is UNSET, instead of None).
  • The .connect(), .read() and .write() methods on the concurrency backend classes:

https://github.com/encode/httpx/blob/86a0eb0268bdc4beeb90d8136546233cd8401b0e/httpx/concurrency/asyncio.py#L63-L67

About this issue

  • Original URL
  • State: closed
  • Created 5 years ago
  • Comments: 24 (24 by maintainers)

Most upvoted comments

If anyone is up for tackling this (and give the problem another POV), I’d be super happy. I feel like I’m going crazy over the issues that arise when actually trying to make this change, and don’t have the resources to dig any deeper right now unfortunately.

Yeah, I hadn’t yet dove deep enough to see the full extent of the rabbit hole… until just now. Ouch.

Another option could be to just set the default parameter values to 5, leaving timeout=None to mean no timeouts.

Seems like a very sensible suggestion, right? Not sure why it took us such a winding road to get there. 😌

Another option could be to just set the default parameter values to 5, leaving timeout=None to mean no timeouts.