From c2aeaa3959b5754f5b39a45bceff91b196b6c986 Mon Sep 17 00:00:00 2001 From: Kevin Burke Date: Sat, 23 Aug 2014 11:52:51 -0700 Subject: [PATCH 1/5] Add support for connect timeouts Modifies the timeout interface to also accept a tuple (connect, read) which would be used to set individual connect and read timeouts for Requests. Adds Advanced documentation explaining the interface and providing guidance for timeout values. --- docs/api.rst | 2 +- docs/user/advanced.rst | 38 ++++++++++++++++++++++++++++++++ requests/adapters.py | 33 +++++++++++++++++++++------- requests/exceptions.py | 17 +++++++++++++- requests/structures.py | 4 ++-- test_requests.py | 50 ++++++++++++++++++++++++++++++++++++++++-- 6 files changed, 130 insertions(+), 14 deletions(-) diff --git a/docs/api.rst b/docs/api.rst index 42f7c5a0..69f138a2 100644 --- a/docs/api.rst +++ b/docs/api.rst @@ -5,7 +5,7 @@ Developer Interface .. module:: requests -This part of the documentation covers all the interfaces of Requests. For +This part of the documentation covers all the interfaces of Requests. For parts where Requests depends on external libraries, we document the most important right here and provide links to the canonical documentation. diff --git a/docs/user/advanced.rst b/docs/user/advanced.rst index 8eb888b1..65970daf 100644 --- a/docs/user/advanced.rst +++ b/docs/user/advanced.rst @@ -707,3 +707,41 @@ Two excellent examples are `grequests`_ and `requests-futures`_. .. _`grequests`: https://github.com/kennethreitz/grequests .. _`requests-futures`: https://github.com/ross/requests-futures + +Timeouts +-------- + +Most requests to external servers should have a timeout attached, in case the +server is not responding in a timely manner. Without a timeout, your code may +hang for minutes or more. + +The **connect** timeout is the number of seconds Requests will wait for your +client to establish a connection to a remote machine (corresponding to the +`connect()`_) call on the socket. It's a good practice to set connect timeouts +to slightly larger than a multiple of 3, which is the default `TCP packet +retransmission window `_. + +Once your client has connected to the server and sent the HTTP request, the +**read** timeout is the number of seconds the client will wait for the server +to send a response. (Specifically, it's the number of seconds that the client +will wait *between* bytes sent from the server. In 99.9% of cases, this is the +time before the server sends the first byte). + +If you specify a single value for the timeout, like this:: + + r = requests.get('https://github.com', timeout=5) + +The timeout value will be applied to both the ``connect`` and the ``read`` +timeouts. Specify a tuple if you would like to set the values separately:: + + r = requests.get('https://github.com', timeout=(3.05, 27)) + +If the remote server is very slow, you can tell Requests to wait forever for +a response, by passing None as a timeout value and then retrieving a cup of +coffee. + +.. code-block:: python + + r = requests.get('https://github.com', timeout=None) + +.. _`connect()`: http://linux.die.net/man/2/connect diff --git a/requests/adapters.py b/requests/adapters.py index 1ce54470..3c1e979f 100644 --- a/requests/adapters.py +++ b/requests/adapters.py @@ -15,17 +15,19 @@ from .packages.urllib3 import Retry from .packages.urllib3.poolmanager import PoolManager, proxy_from_url from .packages.urllib3.response import HTTPResponse from .packages.urllib3.util import Timeout as TimeoutSauce -from .compat import urlparse, basestring, urldefrag, unquote +from .compat import urlparse, basestring, urldefrag from .utils import (DEFAULT_CA_BUNDLE_PATH, get_encoding_from_headers, prepend_scheme_if_needed, get_auth_from_url) from .structures import CaseInsensitiveDict -from .packages.urllib3.exceptions import MaxRetryError -from .packages.urllib3.exceptions import TimeoutError -from .packages.urllib3.exceptions import SSLError as _SSLError +from .packages.urllib3.exceptions import ConnectTimeoutError from .packages.urllib3.exceptions import HTTPError as _HTTPError +from .packages.urllib3.exceptions import MaxRetryError from .packages.urllib3.exceptions import ProxyError as _ProxyError +from .packages.urllib3.exceptions import ReadTimeoutError +from .packages.urllib3.exceptions import SSLError as _SSLError from .cookies import extract_cookies_to_jar -from .exceptions import ConnectionError, Timeout, SSLError, ProxyError +from .exceptions import (ConnectionError, ConnectTimeout, ReadTimeout, SSLError, + ProxyError) from .auth import _basic_auth_str DEFAULT_POOLBLOCK = False @@ -315,6 +317,7 @@ class HTTPAdapter(BaseAdapter): :param request: The :class:`PreparedRequest ` being sent. :param stream: (optional) Whether to stream the request content. :param timeout: (optional) The timeout on the request. + :type timeout: float or tuple (connect timeout, read timeout), eg (3.1, 20) :param verify: (optional) Whether to verify SSL certificates. :param cert: (optional) Any user-provided SSL certificate to be trusted. :param proxies: (optional) The proxies dictionary to apply to the request. @@ -328,7 +331,18 @@ class HTTPAdapter(BaseAdapter): chunked = not (request.body is None or 'Content-Length' in request.headers) - timeout = TimeoutSauce(connect=timeout, read=timeout) + if isinstance(timeout, tuple): + try: + connect, read = timeout + timeout = TimeoutSauce(connect=connect, read=read) + except ValueError as e: + # this may raise a string formatting error. + err = ("Invalid timeout {0}. Pass a (connect, read) " + "timeout tuple, or a single float to set " + "both timeouts to the same value".format(timeout)) + raise ValueError(err) + else: + timeout = TimeoutSauce(connect=timeout, read=timeout) try: if not chunked: @@ -390,6 +404,9 @@ class HTTPAdapter(BaseAdapter): raise ConnectionError(sockerr, request=request) except MaxRetryError as e: + if isinstance(e.reason, ConnectTimeoutError): + raise ConnectTimeout(e, request=request) + raise ConnectionError(e, request=request) except _ProxyError as e: @@ -398,8 +415,8 @@ class HTTPAdapter(BaseAdapter): except (_SSLError, _HTTPError) as e: if isinstance(e, _SSLError): raise SSLError(e, request=request) - elif isinstance(e, TimeoutError): - raise Timeout(e, request=request) + elif isinstance(e, ReadTimeoutError): + raise ReadTimeout(e, request=request) else: raise diff --git a/requests/exceptions.py b/requests/exceptions.py index a4ee9d63..d59637a3 100644 --- a/requests/exceptions.py +++ b/requests/exceptions.py @@ -44,7 +44,22 @@ class SSLError(ConnectionError): class Timeout(RequestException): - """The request timed out.""" + """The request timed out. + + Catching this error will catch both :exc:`ConnectTimeout` and + :exc:`ReadTimeout` errors. + """ + + +class ConnectTimeout(Timeout): + """ The request timed out while trying to connect to the server. + + Requests that produce this error are safe to retry + """ + + +class ReadTimeout(Timeout): + """The server did not send any data in the allotted amount of time.""" class URLRequired(RequestException): diff --git a/requests/structures.py b/requests/structures.py index 66cdad86..5563d1f8 100644 --- a/requests/structures.py +++ b/requests/structures.py @@ -23,7 +23,7 @@ class CaseInsensitiveDict(collections.MutableMapping): case of the last key to be set, and ``iter(instance)``, ``keys()``, ``items()``, ``iterkeys()``, and ``iteritems()`` will contain case-sensitive keys. However, querying and contains - testing is case insensitive: + testing is case insensitive:: cid = CaseInsensitiveDict() cid['Accept'] = 'application/json' @@ -35,7 +35,7 @@ class CaseInsensitiveDict(collections.MutableMapping): of how the header name was originally stored. If the constructor, ``.update``, or equality comparison - operations are given keys that have equal ``.lower()``s, the + operations are given keys that have equal ``.lower()`` s, the behavior is undefined. """ diff --git a/test_requests.py b/test_requests.py index 34ebd8ca..21efe667 100755 --- a/test_requests.py +++ b/test_requests.py @@ -18,7 +18,8 @@ from requests.auth import HTTPDigestAuth, _basic_auth_str from requests.compat import ( Morsel, cookielib, getproxies, str, urljoin, urlparse, is_py3, builtin_str) from requests.cookies import cookiejar_from_dict, morsel_to_cookie -from requests.exceptions import InvalidURL, MissingSchema, ConnectionError +from requests.exceptions import (InvalidURL, MissingSchema, ConnectTimeout, + ReadTimeout) from requests.models import PreparedRequest from requests.structures import CaseInsensitiveDict from requests.sessions import SessionRedirectMixin @@ -38,6 +39,9 @@ else: return s.decode('unicode-escape') +# Requests to this URL should always fail with a connection timeout (nothing +# listening on that port) +TARPIT = "http://10.255.255.1" HTTPBIN = os.environ.get('HTTPBIN_URL', 'http://httpbin.org/') # Issue #1483: Make sure the URL always has a trailing slash HTTPBIN = HTTPBIN.rstrip('/') + '/' @@ -1308,10 +1312,52 @@ class TestMorselToCookieMaxAge(unittest.TestCase): class TestTimeout: def test_stream_timeout(self): try: - requests.get('https://httpbin.org/delay/10', timeout=5.0) + requests.get('https://httpbin.org/delay/10', timeout=2.0) except requests.exceptions.Timeout as e: assert 'Read timed out' in e.args[0].args[0] + def test_invalid_timeout(self): + with pytest.raises(ValueError) as e: + requests.get(httpbin('get'), timeout=(3, 4, 5)) + assert '(connect, read)' in str(e) + + with pytest.raises(ValueError) as e: + requests.get(httpbin('get'), timeout="foo") + assert 'must be an int or float' in str(e) + + def test_none_timeout(self): + """ Check that you can set None as a valid timeout value. + + To actually test this behavior, we'd want to check that setting the + timeout to None actually lets the request block past the system default + timeout. However, this would make the test suite unbearably slow. + Instead we verify that setting the timeout to None does not prevent the + request from succeeding. + """ + r = requests.get(httpbin('get'), timeout=None) + assert r.status_code == 200 + + def test_read_timeout(self): + try: + requests.get(httpbin('delay/10'), timeout=(None, 0.1)) + assert False, "The recv() request should time out." + except ReadTimeout: + pass + + def test_connect_timeout(self): + try: + requests.get(TARPIT, timeout=(0.1, None)) + assert False, "The connect() request should time out." + except ConnectTimeout: + pass + + def test_total_timeout_connect(self): + try: + requests.get(TARPIT, timeout=(0.1, 0.1)) + assert False, "The connect() request should time out." + except ConnectTimeout: + pass + SendCall = collections.namedtuple('SendCall', ('args', 'kwargs')) From f0b9b60f6298ec743f2b470819cd249a041845d3 Mon Sep 17 00:00:00 2001 From: Kevin Burke Date: Sun, 24 Aug 2014 19:46:46 -0700 Subject: [PATCH 2/5] revert change --- requests/structures.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/requests/structures.py b/requests/structures.py index 5563d1f8..3e5f2faa 100644 --- a/requests/structures.py +++ b/requests/structures.py @@ -35,7 +35,7 @@ class CaseInsensitiveDict(collections.MutableMapping): of how the header name was originally stored. If the constructor, ``.update``, or equality comparison - operations are given keys that have equal ``.lower()`` s, the + operations are given keys that have equal ``.lower()``s, the behavior is undefined. """ From 8f9ce13e433013cd9bd4dbe9cf61c5b5f14b65ed Mon Sep 17 00:00:00 2001 From: Kevin Burke Date: Sun, 24 Aug 2014 19:56:57 -0700 Subject: [PATCH 3/5] ConnectTimeout multiple inheritance --- requests/exceptions.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/requests/exceptions.py b/requests/exceptions.py index d59637a3..6dbd98a9 100644 --- a/requests/exceptions.py +++ b/requests/exceptions.py @@ -51,8 +51,8 @@ class Timeout(RequestException): """ -class ConnectTimeout(Timeout): - """ The request timed out while trying to connect to the server. +class ConnectTimeout(ConnectionError, Timeout): + """The request timed out while trying to connect to the server. Requests that produce this error are safe to retry """ From 39b3a436d3058975ac8e05735825da03e755a3b0 Mon Sep 17 00:00:00 2001 From: Kevin Burke Date: Sun, 24 Aug 2014 20:18:05 -0700 Subject: [PATCH 4/5] assert connect timeout inheritance --- test_requests.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/test_requests.py b/test_requests.py index 21efe667..2df97368 100755 --- a/test_requests.py +++ b/test_requests.py @@ -19,7 +19,7 @@ from requests.compat import ( Morsel, cookielib, getproxies, str, urljoin, urlparse, is_py3, builtin_str) from requests.cookies import cookiejar_from_dict, morsel_to_cookie from requests.exceptions import (InvalidURL, MissingSchema, ConnectTimeout, - ReadTimeout) + ReadTimeout, ConnectionError, Timeout) from requests.models import PreparedRequest from requests.structures import CaseInsensitiveDict from requests.sessions import SessionRedirectMixin @@ -1348,8 +1348,10 @@ class TestTimeout: try: requests.get(TARPIT, timeout=(0.1, None)) assert False, "The connect() request should time out." - except ConnectTimeout: + except ConnectTimeout as e: pass + assert isinstance(e, ConnectionError) + assert isinstance(e, Timeout) def test_total_timeout_connect(self): try: From 7f236fcc40ae3597bfc944c704bff2b8d907cd93 Mon Sep 17 00:00:00 2001 From: Kevin Burke Date: Sun, 24 Aug 2014 20:22:55 -0700 Subject: [PATCH 5/5] woops --- test_requests.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/test_requests.py b/test_requests.py index 2df97368..716c0dcf 100755 --- a/test_requests.py +++ b/test_requests.py @@ -1349,9 +1349,8 @@ class TestTimeout: requests.get(TARPIT, timeout=(0.1, None)) assert False, "The connect() request should time out." except ConnectTimeout as e: - pass - assert isinstance(e, ConnectionError) - assert isinstance(e, Timeout) + assert isinstance(e, ConnectionError) + assert isinstance(e, Timeout) def test_total_timeout_connect(self): try: