Warn user once about missing timeouts

[earonesty]

Not sure if this is the best way to do it, but some sort of warning that "hanging forever" is a possibility is probably important - given that this lib is for "humans".

[sigmavirus24]

A) print isn't how you warn users
B) warnings end up bubbling to people not controlling requests and really aren't a great user experience
C) __timeout_warned is not going to work well here
D) Not using a timeout is totally valid

[earonesty]

A) OK, I made a warning.
B) Well, it's either that or severe intermittent bugs bubbling up. I'll take the former
C) ? An instance variable wouldn't work, but as a class var, you get warned once and that's it.
D) Valid? Under what circumstances should a request wait longer for a packet more than, say, 2 hours? This isn't the timeout for the whole download... just the time between any response bytes at all.

[sigmavirus24]

B) Well, it's either that or severe intermittent bugs bubbling up. I'll take the former

Yeah, of course you will. You won't be dealing with the fallout from users. The warnings already in this project and in urllib3 have caused years long issue generation and comments complaining about them.

C) ? An instance variable wouldn't work, but as a class var, you get warned once and that's it.

This won't work across multiple processes or threads. It also doesn't properly provide any context. Imagine an application using celery and N API clients that all use Requests under the hood. Which one isn't providing a timeout? How does the user fix it? There's absolutely no value in this for the majority of the users who will actually end up seeing this.

D) Valid? Under what circumstances should a request wait longer for a packet more than, say, 2 hours? This isn't the timeout for the whole download... just the time between any response bytes at all.

There's little value in responding to hyperbole, so I won't.

[earonesty]

You won't be dealing with the fallout from users.

What we do now is patch requests to raise an exception if a timeout isn't present - which is fine. This allows us to quickly file bug reports with downstream libraries, and/or then up those libs.

I agree, there might be less fallout if a default timeout is in there - instead of a warning. I'm just not sure how having requests with infinite timeouts is helpful to anyone except those trying to deliberately produce bugs.

Bear in mind... the effect of this is to break, at runtime, in rare and potentially dangerous production conditions.

Maybe there's more fallout from not having something?

This won't work across multiple processes or threads

It will work fine across threads. Processes, no.

I doubt anyone would be too upset if a potentially fatal bug is in their system.

hyperbole

The lack of a timeout, given the right conditions can cause applications that seem to work fine in production to suddenly break catastrophically and permanently. That's because it's rather rare that a TCP FIN_WAIT will never go through - but when that happens, the requests lib will never return.

2 hours was just an example of a default timeout that would be perfectly fine - if you're opposed to a warning.

I think it's sort of irresponsible to leave it without both a warning and a default timeout, simply because the conditions that cause an infinite wait are hard to simulate for users.

I noticed there's an open issue to have a default timeout too. That would be fine.

context

Yes, it would be nicer, maybe, to have a bit of a trace on the warning.

[nateprewitt]

I don't think we're going to merge a warning for this. It produces a considerable amount of noise in people's systems and we've generally stopped using warnings due to how often Requests is bundled in other projects. Often code the user has no control over is generating these warnings in their logs.

There's currently a paragraph in the quickstart guide with guidance at the top of the timeout section. That's probably still our best approach here.

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: