overall timeout

We already make great use of the timeout parameter which allows setting per TCP transaction timeouts. This is very helpful! However, we also need to support an overall timeout across the connection. Reading the [docs on timeouts](http://docs.python-requests.org/en/master/user/quickstart/#timeouts) I see this isn't currently supported, and searching through the issues at least a bit back I didn't see another request for this feature -- excuse me if there is.

I realize we can set timers in our library to accomplish this, but I'm concerned about the additional overhead (one per thread, and we may have many) as well as any adverse effects to connection pooling if we end up needing to abort a request. Is there a good way to abort a request in the first place? I didn't see anything obvious in the docs.

So: Long term, it would be great if we could add overall timeout to the requests library. Short term, is there a recommended way of implementing this on my end?


Hi @emgerner-msft,

For reference, the following are all variations on this theme if not this exact feature request:
- https://github.com/kennethreitz/requests/issues/2327
- https://github.com/kennethreitz/requests/issues/2685
- https://github.com/kennethreitz/requests/issues/1928
- (and I'm sure there are more)

We've also discussed this over on https://github.com/sigmavirus24/requests-toolbelt/issues/51

You'll notice the last link discusses [this package](https://github.com/pnpnpn/timeout-decorator) which should handle this for you without adding it to requests. The reality, is that there's no need for requests to do this when another package already does it very well.


The package you reference does it by forking a separate process to run the web request. That is a very heavyweight way of achieving the simple goal of a timeout, and in my view is not in any way a substitute for requests itself having a native timeout feature.


@jribbens If you can come up with a way that uses neither threads nor processes, that would be amazing. Until then, if you want a wall clock timeout your best bet is that package as it's the most reliable way of achieving that at the moment.


I don't think @jribbens is saying no threads nor processes. Just that a process _per_ web request is excessive. Many languages have a way of multiple timers sharing a single additional thread or process. I'm just not aware of how to do that best in Python. 

It seems like #1928 has the most discussion of alternatives, but most come with a lot of caveats (this won't work for your use case, etc). I'm fine with having some custom code in my library and writing my own custom solution if this really doesn't belong in requests but I think I need a little more information on what that would look like. The whole reason we use requests is to get away from the low level TCP connection pooling logic but it seems like reading that thread that in order to write this custom code I need to know that logic, and that's what I'm having some trouble with.


@emgerner-msft is correct. I am bit confused by @sigmavirus24's comment, having a "total timeout" without using threads or processes seems quite pedestrian and not at all "amazing". Just calculate the deadline at the start of the whole process (e.g. `deadline = time.time() + total_timeout`) and then on any individual operation set the timeout to be `deadline - time.time()`.


> having a "total timeout" without using threads or processes seems quite pedestrian and not at all "amazing". 

And your solution is rather primitive. The reason _most_ people want a total (or wall clock) timeout is to prevent a read from "hanging", in other words a case like the following:

``` py
r = requests.get(url, stream=True)
for chunk in r.iter_content(chunksize):
    process_data(chunk)
```

Where each read takes a long time in the middle of `iter_content` but it's less than the read timeout (I'm assuming we apply that when streaming, but it still may be the case that we don't) they specified. Certainly it would seem like this should be simply handled by your solution @jribbens until you remember how clocks drift and daylight savings time works and them `time.time()` is woefully insufficient.

Finally, it's important to keep in mind that Requests' API is frozen. There is no good or consistent API for specifying a total timeout. And if we implemented a timeout like you suggest, we would have countless bugs that they specified a one minute long total timeout but it took longer because the last time we checked we were under a minute but their configured read timeout was long enough that their timeout error was raised around a minute and a half. That's a _very_ rough wall timeout that would be slightly better for people looking for this, but no different from the person implementing this themselves.


Apologies if I was unclear @sigmavirus24 , you seem to have critiqued my pseudocode illustration of principle as if you thought it was a literal patch. I should point out though that `time.time()` does not work the way you apparently think - daylight savings time is not relevant, and neither is clock skew on the timescales we're talking about here. Also you have misunderstood the suggestion if you think the bug you describe would occur. Finally I am not sure what you mean by the Requests API being "frozen" as the API was changed as recently as version 2.9.0 so clearly whatever you mean it's not what I would normally understand by the word.


Just to separate my discussion: I'm actually not arguing this is easy. If it were totally simple, I would just write it and stop bugging you. :) 

My problems are:
1) Everything on the threads you listed was monkey patches. That's fine, but I'm using this in a production quality library and can't take the caveat of internal changes breaking everything.
2) The timeout decorator in the link you gave is great, but I'm not clear on how that affects the connection. Even if we accept that the only good way of doing timeouts is with a bunch of threads, how does this library enforce that the socket gets shut down, the connection dropped, etc. We're doing a lot of connections and this seems potentially quite leak prone. requests doesn't have an 'abort' method that I can find (correct me if I'm wrong) so how is the shutdown of the connection happening?

All I'm looking for is a clear 'blessed' version of how to solve this problem on my own, or if there's not a perfect solution, a couple solutions with the caveats discussed. Does that make sense?


@emgerner-msft Assuming you're using CPython, connection shutdown will happen when the request is no longer continuing. At that point all references to the underlying connection will be lost and the socket will be closed and disposed of.


@Lukasa Okay, thanks! How does the library determine the request is no longer continuing? For example, if I used the timeout decorator route and cut off in the middle of the download, when would the download actually stop? Do I need to do anything special with the streaming options?


If you use the timeout decorator, the download will stop when the timeout fires. This is because signals interrupt syscalls, which means that there will be no further calls into the socket. Once the request is no longer in scope (e.g. the stack has unwound to outside of your `requests.*` function), that's in: CPython will clean up the connection object and tear the connection down. No special streaming options are required there.


Perfect. I'm good to close the  thread then, unless others have more to say.


Actually, sorry, one more concern. Was looking at the timeout decorator code more closely since you said that it uses signals was relevant, as opposed to something like Python Timers (presumably). It looks like it [calls signal with SIGALRM](https://github.com/pnpnpn/timeout-decorator/blob/master/timeout_decorator/timeout_decorator.py#L73) which is documented in [Python Signal](https://docs.python.org/3.5/library/signal.html#signal.signal) not to work on Windows. I need this to work in both Unix and Windows environments, as well as in Python 2.7 and 3.3+ (much like requests itself). I'll poke around a bit more and see if this will actually work given that.


@emgerner-msft That's frustrating. =( 


@Lukasa Yup, tried the basic [usage snippet](https://github.com/pnpnpn/timeout-decorator#usage) and it doesn't work on Windows. I read some more of the code/examples and fiddled and it looks like if we don't use signals the package might work, but everything has to be pickable which is not the case for my application. So as far as I can tell, timeout decorator won't solve my problem. Any other ideas?


@emgerner-msft Are you confident that none of the Windows-specific signals are suitable?


@Lukasa To be blunt, I simply don't know. I haven't used signals before, and much like I didn't realize until you told me that they'd interrupt the request I'm not sure what's appropriate. I'm also not trying to get this just to work on Windows. I need full crossplat support (Windows and Unix) and both Python 2 and Python 3 support. So much of signals looks platform specific it's throwing me. [Timer](https://docs.python.org/2/library/threading.html#timer-objects) was one of the solutions I was looking at that looked less low level and thus might take care of my constraints, but I'm not sure then how I could close the connection. I can do more reading, but this is why I was hoping to get additional guidance from you guys. :)


So this is a really tricky place to be.

The reality is that there is more-or-less no cross-platform way to kill a thread except by interrupting it, which is basically what a signal is. That means, I think, that signals are the only route you really have to making this work across platforms. I'm inclined to try to ping in a Windowsy Pythony expert: @brettcannon, do you have a good suggestion here?


Out of interest, is there a reason to not implement "total timeout" in Requests other than that implementing and testing it requires work? I mean, if a patch to implement it magically appeared today would it in theory be rejected or accepted? I appreciate and agree with the "eliminate unnecessary complexity" point of view, but "you can do it by forking a separate process" does not make this feature unnecessary in my opinion.


@jribbens There are a few problems with this.

Part 1 is that the complexity of such a patch is very high. To get it to behave correctly you need to repeatedly change timeouts at the socket level. This means that the patch needs to be passed pervasively though httplib, which we've already patched more than we'd like to. Essentially, we'd need to be reaching into httplib and reimplementing about 50% of its more complex methods in order to achieve this functional change.

Part 2 is that the maintenance of such a patch is relatively burdensome. We'd likely need to start maintaining what amounts to a parallel fork of httplib (more properly http.client at this time) in order to successfully do it. Alternatively, we'd need to take on the maintenance burden of a different HTTP stack that is more amenable to this kind of change. This part is, I suspect, commonly missed by those who wish to have such a feature: the cost of implementing it is high, but that is _nothing_ compared to the ongoing maintenance costs of supporting such a feature on all platforms.

Part 3 is that the advantage of such a patch is unclear. It has been my experience that most people who want a total timeout patch are not thinking entirely clearly about what they want. In most cases, total timeout parameters end up having the effect of killing perfectly good requests for no reason.

For example, suppose you've designed a bit of code that downloads files, and you'd like to handle hangs. While it's initially tempting to want to set a flat total timeout ("no request may take more than 30 seconds!"), such a timeout misses the point. For example, if a file changes from being 30MB to being 30GB in size, such a file can _never_ download in that kind of time interval, even though the download may be entirely healthy.

Put another way, total timeouts are an attractive nuisance: they appear to solve a problem, but they don't do it effectively. A more useful approach, in my opinion, is to take advantage of the per-socket-action timeout, combined with `stream=True` and `iter_content`, and assign yourself timeouts for chunks of data. The way `iter_content` works, flow of control will be returned to your code in a somewhat regular interval. That means that you can set yourself socket-level timeouts (e.g. 5s) and then `iter_content` over fairly small chunks (e.g. 1KB of data) and be relatively confident that unless you're being actively attacked, no denial of service is possible here. If you're really worried about denial of service, set your socket-level timeout much lower and your chunk size smaller (0.5s and 512 bytes) to ensure that you're regularly having control flow handed back to you.

The upshot of all this is that I believe that total timeouts are a misfeature in a library like this one. The best kind of timeout is one that is tuned to allow large responses enough time to download in peace, and such a timeout is best served by socket-level timeouts and `iter_content`.


Maybe @zooba has an idea as he actually knows how Windows works. :)


(Unrelatedly, one of my favourite things to do is to set up a daisy-chain of experts in a GitHub issue.)


Haha, I already know @zooba and @brettcannon. I can discuss with them here or internally as a solution to this would probably help them too.


@emgerner-msft I figured you might, but didn't want to presume: MSFT is a big organisation!


@Lukasa Just reading through the wall of text you just wrote above -- interesting! On the discussion of stream=True and iter_content to time downloads, what is the equivalent way of handling larger uploads?

_PS_: The paragraph above starting with 'Put another way,..' is the kind of guidance I looked for in the docs. Given the number of requests you get for maximum timeout (and your valid reasons for not doing it), maybe the best thing to do is add some of that information in the [timeout docs](http://docs.python-requests.org/en/master/user/advanced/#timeouts)?


lol @lukasa I take your point about maintenance, which was already in my mind, but on "feature vs misfeature" I'm afraid I'm completely opposite to you. I think anyone who _doesn't_ want a total timeout isn't thinking clearly about what they want, and I'm having difficulty imagining a situation where what you describe as a bug  "30MB download changes to 30GB and therefore fails" isn't in fact a beneficial feature!

You can as you say do something a bit similar (but I suspect without most of the benefits of a total timeout) using `stream=True` but I thought the point of requests was that it handled things for you...


> I thought the point of requests was that it handled things for you

It handles HTTP for you. The facts that we already handle connection and read timeouts and that we have had a couple exemptions to our feature freeze of several years are tangential to the discussion of utility, desirability, consistency (across multiple platforms), and maintainability. We appreciate your feedback and your opinion. If you have new information to present, we'd appreciate that.

It may also be telling that requests doesn't handle everything, by the number of rejected feature requests on this project and the fact that there's a separate project implementing common usage patterns for users (the requests toolbelt). If a total timeout belongs anywhere, it would be there, but again, it would have to work on Windows, BSD, Linux, and OSX with excellent test coverage and without it being a nightmare to maintain.


> On the discussion of stream=True and iter_content to time downloads, what is the equivalent way of handling larger uploads?

Define a generator for your upload, and pass that to `data`. Or, if chunked encoding isn't a winner for you, define a file-like object with a magic `read` method and pass _that_ to `data`.

Let me elaborate a bit. If you pass a generator to `data`, requests will iterate over it, and will send each chunk in turn. This means that to send data we'll necessarily have to hand flow of control to your code for each chunk. This lets you do whatever you want in that time, including throw exceptions to abort the request altogether.

If for some reason you can't use chunked transfer encoding for your uploads (unlikely, but possible if the server in question is real bad), you can do the same by creating a file-like object that has a length and then doing your magic in the `read` call, which will be repeatedly called for 8192-byte chunks. Again, this ensures the flow of control goes through your code intermittently, which lets you use your own logic.

> PS: The paragraph above starting with 'Put another way,..' is the kind of guidance I looked for in the docs. Given the number of requests you get for maximum timeout (and your valid reasons for not doing it), maybe the best thing to do is add some of that information in the timeout docs?

I _suppose_. Generally speaking, though, I'm always nervous about putting somewhat defensive text into documentation. It could go into an FAQ I guess, but text that explains why we _don't_ have something is rarely useful in documentation. The space in the docs would be better served, I suspect, by a recipe for doing something.

> I think anyone who doesn't want a total timeout isn't thinking clearly about what they want, and I'm having difficulty imagining a situation where what you describe as a bug "30MB download changes to 30GB and therefore fails" isn't in fact a beneficial feature!

Heh, I'm not:
- package manager (e.g. pip, which uses requests), where packages can vary wildly in data size
- web scraper, which may run against multiple sites that vary wildly in size
- a log aggregator that downloads log files from hosts which have wildly varying levels of us (and therefore log file sizes)
- video downloader (videos can vary wildly in size)

In actuality, I think the case that the developer knows to within one order of magnitude what file sizes they'll be dealing with is the uncommon case. In most cases developers have no idea. And generally I'd say that making assumptions about those sizes is unwise. If you have constraints on download size then your code should deliberately encode those assumptions (e.g. in the form of checks on content-length), rather than implicitly encode them and mix them in with the bandwidth of the user's network so that other people reading the code can see them clearly.

> but I thought the point of requests was that it handled things for you...

Requests very deliberately does not handle everything for users. Trying to do everything is an impossible task, and it's impossible to build a good library that does that. We regularly tell users to drop down to urllib3 in order to achieve something.

We only put code into requests if we can do it better or cleaner than most users will be able to do. If not, there's no value. I'm really not yet sold on total timeout being one of those things, especially given what I perceive to be the relatively marginal utility when aggregated across our user-base.

That said, I'm open to being convinced I'm wrong: I just haven't seen a convincing argument for it yet (and, to head you off at the pass, "I need it!" is not a convincing argument: gotta give some reasons!).


@sigmavirus24 

> If a total timeout belongs anywhere, it would be there, but again, it would have to work on Windows, BSD, Linux, and OSX with excellent test coverage and without it being a nightmare to maintain.

Agreed!


@lukasa I suppose my thinking is that not only do I want it, in fact nearly all users would want it if they thought about it (or they don't realise it's not already there). Half of your usage scenarios above where you say it should be avoided I would say it's vital (web scraper and log aggregator) - the other two it's less necessary as there's likely to be a user waiting for the result who can cancel the download manually if they want. Anything that runs in the background without a UI and doesn't use an overall timeout is buggy in my view!