Consider making Timeout option required or have a default

I have a feeling I'm about to get a swift education on this topic, but I've been thinking about the pros/cons of changing `requests` so that somehow there is a timeout value configured for every request.

I think there are two ways to do this:
1. Provide a default value. I know browsers have a default, so that may be a simple place to begin.
2. Make every user configure this in every request -- bigger API breakage. Probably not the way to go.

The reason I'm thinking about this is because I've used requests for a few years now and until now I didn't realize the importance of providing a timeout. It took one of my programs hanging forever for me to realize that the default here isn't really very good for my purposes. (I'm in the process of updating all my code...)

I also see that a lot of people want `Session` objects to have a timeout parameter, and this might be a way to do that as well. 

If a large default were provided to all requests and all sessions, what negative impact would that have? The only thing I can think of is that some programs will get timeout exceptions where they previously hung, which seems like an improvement to me.
## Caveat, added May 13, 2016:

Please don't use this issue to discuss adding a timeout attribute to requests. There are a number of discussions about this elsewhere (search closed issues), and we don't want that conversation to muddy this issue too. Thanks.


This is an entirely reasonable suggestion.

Honestly, I'm not averse to doing it: there are definitely worse things to do than this. I'd be open to providing a default timeout. However, I don't think we can do it until 3.0.0.

Of course @kennethreitz as keeper of the spirit of requests has got the final say on this, and should definitely weigh in.


Changing the default would definitely need to wait until 3.0.0.

I'm not against having a well-reasoned (and therefore, also well-documented) default timeout parameter.

I'm still pretty strongly against sessions having a timeout attribute though and I think that shouldn't muddy the waters of this discussion because it will sidetrack things and prevent an otherwise productive discussion.


@sigmavirus24 I don't know the argument pro/con sessions having a timeout, but I'm happy to defer to other's judgement and experience on that, keeping these waters clean, like you say. 

I'm just happy that I haven't gotten a swift education about timeouts.


I'm definitely in favor of this in light of dealing with a package that doesn't provide a way to set a timeout, but does accept a fully configured session. My current fix is a specialized `HTTPAdapter` that sets a default timeout if none is provided:

``` python
class TimeoutHTTPAdapter(HTTPAdapter):
    def __init__(self, timeout, *args, **kwargs):
        self._timeout = timeout
        super().__init__(*args, **kwargs)

    def send(self, request, timeout=False, ...):
        if timeout is None:
            timeout = self._timeout
        return super().send(request, timeout=timeout, ...)
```

I'd much prefer something like:

``` python
session = Session(timeout=...)

# or even

session.default_timeout = ...
```

However, this current fix isn't too cumbersome either. 


:+1: for `session = Session(timeout=...)`. Would you merge a patch?


@kuraga No. Per @sigmavirus24, and in many many previous discussions:

> I think I could actually still implement it using hooks - create a method which uses the prepared request workflow, and in the hook, just call it again. What would be your suggested solution?


@Lukasa ok, but which discussion did you cite? Was it private? Which previous discussions?


Argh, sorry, copy-paste fail:

> I'm still pretty strongly against sessions having a timeout attribute though and I think that shouldn't muddy the waters of this discussion because it will sidetrack things and prevent an otherwise productive discussion.


@kuraga please search _closed_ issues. There are several discussions of this. I really don't want this diversion to distract from the topic of a _default timeout_ though. So can we **please** stop discussing this now as I asked nicely before. You and @justanr are distracting from the important and _attainable_ portion of this issue.


@sigmavirus24 I added a caveat to [my initial comment, above](https://github.com/kennethreitz/requests/issues/3070#issue-144324837). Hopefully that should get this discussion focused.


I'm (as a user) slightly against this proposal.
There are following caveats:
1. There should be several different time-outs: resolve, connect, request, read. What should be defaults for each of them?
2. If there will be default read time-out, then (for streaming interfaces) there should also be keep-alive packets. Is this implemented?
3. There must be way to override default and disable time-outs completely.

What I've missed?


At the very least, how about we include the `timeout` arg in the first introductory examples in the documentation? It's bad when intro examples are subtly wrong for the sake of brevity.


It's extremely unclear to me what "wrong" means here. They aren't wrong: they work, as designed. The introductory examples are all clearly typed into the Python interactive interpreter, so they have a timeout: the user sitting there can hit ^C anytime they like.

I'd welcome enhancements to the "timeouts" section of the documentation to be more emphatic, that certainly does seem reasonable. But I don't think we need to go through and add the `timeout` arg. 


> It's extremely unclear to me what "wrong" means here. They aren't wrong: they work, as designed.

I think the point that's generally acknowledged by this bug is that the design _was_ wrong. There's a general consensus here that adding a default timeout or requiring it as an argument is a good idea. The  docs could address that, and that seems like a simple step to take until this issue is resolved in the next major version.

What happens in practice is that people grab the examples from the docs, don't read the timeout section carefully (or don't understand its implications -- I didn't for years until I got bit), and then wind up with programs that can hang forever. I completely agree with @chris-martin that until this issue is fixed, all examples in the docs should provide the timeout argument. Otherwise, we're providing examples that can (and probably will) break your programs.


There is a substantial difference between "is a good idea in production code" and "should be used in every example". For example, all production code should use Sessions for everything, but the documentation doesn't do that because it doesn't help teach the specific lessons that the documentation is intended to teach.

While I appreciate the concern that users will blindly copy code out of the documentation without further consideration, anyone doing that in their product is necessarily going to have sub-par results. This is true in all programming tools. 


That's fair enough, @Lukasa. What about making the Timeout section more explicit then? Right now it says:

> You can tell Requests to stop waiting for a response after a given number of seconds with the timeout parameter.

And then goes on with a long, somewhat complicated warning. Could the first part say:

> You can tell Requests to stop waiting for a response after a given number of seconds with the timeout parameter. Nearly all production code should use this parameter in nearly all requests. Failure to do so can cause your program to hang indefinitely:

Something like that? 


Yup, I'd be happy to merge a PR with that change in it. :smile:


I think that we've addressed this through documentation. I don't think any of the core team is going to change the API to require a timeout and I think we've done our diligence here.

I doubt this will change anybody's mind, but I *vehemently* disagree with closing this bug without a fix. We improved the documentation via the PR that I filed, but in practice I regularly run into programs that hang because they do not have a timeout. It's gotten bad enough that it's one of the first things I think of when a program hangs. 

Requests is a wonderful library but you can't document your way out of a problem like this. Look at the list of issues referencing this one. Documentation is just not enough, or else issues would stop referencing this one.

I respect the requests maintainers greatly, but I hope you'll reconsider closing this. If it causes API changes, that's what version bumps are for. But I'm not even sure API changes would be needed.

Please reconsider.

We could consider making a default in 3.0, but I don't know what it would be... 120s?

Idk, I like our current design. You really shouldn't be hitting the internet without proper timeouts in place ever in production. Any sane engineer knows that — it's not Requests' job to do your job for you. 

Here are a few example idle timeout defaults from other libraries, in case that helps:

* The Haskell [http-client](https://hackage.haskell.org/package/http-client-0.5.7.0/docs/Network-HTTP-Client.html#v:managerResponseTimeout) package, 30 seconds.
* Ruby's [standard library](https://ruby-doc.org/stdlib-2.4.1/libdoc/net/http/rdoc/Net/HTTP.html), 60 seconds.
* The Scala [http4s](https://github.com/http4s/http4s/blob/7468dc30f6cce9f304ed4d1b988a40f4e6d2988b/blaze-client/src/main/scala/org/http4s/client/blaze/bits.scala#L15) library, 60 seconds.

Those seem reasonable to me. I just just think this should be a connect-only timeout, not a download timeout. 

It would also be overridable via environment variable. 

Hooray for reopening this bug. I like the solution of providing a default with an env override. Seems simple enough to me.

> You really shouldn't be hitting the internet without proper timeouts in place ever in production. Any sane engineer knows that — it's not Requests' job to do your job for you.

I do disagree with this though. The thing I love about Requests (and Python in general) is that these kinds of details are usually taken care of for me. For me, I think that's what so jarring about this issue. A super simple GET request has a gotcha that could bite you way later.

Anyway, thanks for reopening. I really appreciate it and I think the community will greatly benefit from a default timeout.

@mlissner I don't disagree with your statement — as long as we apply this only to connect timeouts, not download timeouts. Download timeouts should be unlimited by default (people downloading large files). 

Environment overrides are already causing us to pull our hair out. I'd like to not add anymore, for what that's worth.

And as a user of software, I'd like to not have yet more mysterious inputs to programs, for what that's worth. Nobody's going to bother documenting that their daemon uses `requests` and therefore has behavior that depends on this environment variable.

Don't disagree. Will think about it. 

> Download timeouts should be unlimited by default (people downloading large files).

That's not how Requests timeouts work. They apply per socket I/O syscall, not over the total execution time. That is, they govern how much each call to `read()` blocks. Large file downloads shouldn't be an issue.

Has there been any progress on this?

@mlissner To add to your statement, I think that the current situation kind-of contradicts the stated project goal "HTTP Requests for Humans".

Humans need sensible defaults. Not just in graphical user interfaces, but also in command-line user interfaces and software libraries. And "no timeout" is almost never a sensible default. I was very surprised to read about that trap in the manual. This note should be more prominent in the manual, perhaps being the first note in the Quickstart (rather than the last note), because this is surprising and not what you expect from a library that performs network calls.

(Even better than a more prominent place in the manual, of course, would be to fix this once and for all.)

@sigmavirus24 @chris-martin I fully agree, and I don't need timeout environment variables, either. All I need is either a sane default timeout, or making the timeout argument mandatory. No more, no less.