ConnectionError on timeout=0

[Damgaard]

I get the following exception when doing a request with timeout=0.

>>> requests.get('http://github.com', timeout=0)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/usr/local/lib/python2.7/dist-packages/requests/api.py", line 55, in get
    return request('get', url, **kwargs)
  File "/usr/local/lib/python2.7/dist-packages/requests/api.py", line 44, in request
    return session.request(method=method, url=url, **kwargs)
  File "/usr/local/lib/python2.7/dist-packages/requests/sessions.py", line 335, in request
    resp = self.send(prep, **send_kwargs)
  File "/usr/local/lib/python2.7/dist-packages/requests/sessions.py", line 438, in send
    r = adapter.send(request, **kwargs)
  File "/usr/local/lib/python2.7/dist-packages/requests/adapters.py", line 327, in send
    raise ConnectionError(e)
requests.exceptions.ConnectionError: HTTPConnectionPool(host='github.com', port=80): Max retries exceeded with url: / (Caused by <class 'socket.error'>: [Errno 115] Operation now in progress)

I don't see any mention of this in the issues. Shouldn't the expected behavior of this be equivalent to timeout=None?

I'm running requests version 1.2.3 and python 2.7.3

[Lukasa]

Thanks for raising this issue!

I don't think I agree with your assessment of what the 'expected behaviour' of this should be. You've set a timeout of zero seconds. That implies that the connection process should time out after zero seconds, e.g. immediately.

I'm a bit mixed here. On the one hand, we're doing exactly what the code asked for. On the other hand, I'd argue that falsy values for timeout should be treated the same as None. @sigmavirus24, thoughts?

[Damgaard]

If it times out immediately, then it should raise an Timeout exception. Which I'd also be fine with. But it should behave either as None or a very low number.

[Lukasa]

Unfortunately the problem here is in httplib. In httplib, timeout=0 actually causes a call on the socket after the connect() call to throw EINPROGRESS, which is not strictly a timeout error. Still, I think we should say that timeout=0 should be synonymous with timeout=None.

[sigmavirus24]

@Lukasa they are thoroughly distinct values in this context. I would be okay with seeing timeout=0 and immediately raising a Timeout exception before doing anything. Since we don't return a response (or request) this won't affect the API.

While I don't think we're doing anything wrong by passing this along and catching (and raising) the right exception, I think @Damgaard is justified in his confusion.

Regardless, I'm not entirely convinced this justifies a change. Perhaps the documentation could be improved, but if we do change this it will be a breaking API change and will have to be put off until 2.0. There are probably plenty of people properly handling this and changing the exception raised would break their code.

[Lukasa]

Mm, maybe specialcasing timeout=0 is the right thing to do. A bit weirder if we plumb through urllib3's new timeout API though.

[sigmavirus24]

In all candor I'm of the opinion that we already special case too many things in requests. A fair number of them are untested and that means that they can regress at any time.

[piotr-dobrogost]

Related urllib3/urllib3/pull/246

[Lukasa]

Good spot @piotr-dobrogost, that should solve our problems nicely.