Lifetime timeouts?

[rthalley]

I had a bug reported against dnspython's https() code, both sync and async versions, for not honoring the timeout. Dnspython is basically making an httpx client and doing a post() or get(). If I instrument the code, I see that it successfully connects to the peer, and then I see it trying to fetch the content, which seems to come in a large number of 16KiB blocks and lasts for far longer than the maximum of 6 seconds we wanted to spend on the operation. (In this case, the remote server apparently doesn't actually understand DNS-over-HTTPS, and I have no idea what it is returning!).

Dnspython's timeout concept is based on "lifetimes"; you say how long you want to spend on everything, and then it arranges for that to happen. The HTTPX concept of timeouts appears to be for individual network operations, but this does not bound the total request lifetime in a case like the one above where you have a responding but slow server that goes on for a long time.

I can work around this in async code by using asyncio.wait_for() or trio.move_on_after(), and limiting how long I wait for the future, but fixing this for the sync API seems hard. So far my best idea would be to run the query in a thread and close its fd after the timeout, but this is complicated and not very efficient! Probably I would just not fix things for sync.

I would like to suggest that the httpx Timeout object be extended to add a "lifetime" timeout which all the code would enforce. This could be done with backwards compatibility by initializing the lifetime to None, and not setting it if a user did something simple like pass a float to to get get() / post(). Only if the user instantiated the Timeout object and set the field would it take effect. In dnspython we convert the relative timeout to an "expiration time" which is an absolute clock time, and then adjust the timeout for any operation we do to take no longer than the remaining time before the expiration (if any). Httpx could do the min(timeout_for_operation, remaining_timeout).

Probably it would be less surprising for ordinary users if specifying a timeout to get()/post() as a float actually did impose the lifetime too, but I am sensitive to backwards compatibility concerns and so I proposed a compatible solution in case that was important.

If I've missed some way to do this, I apologize! Also, I understand this could be a good bit of work and will understand if it does not appeal.

/Bob

[lovelydinosaur]

Thanks for raising this. We've got an issue that discusses this, which you've prompted me to re-title more clearly... #1450

Let's continue the conversation there.