-
Notifications
You must be signed in to change notification settings - Fork 196
Add connection/read timeout for requests adapter #342
Changes from 6 commits
153cfac
1e7739a
7be26b6
8c310c7
28c5699
1417c51
09dfa86
f0bc05c
ead832b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -101,7 +101,7 @@ class HTTP11Connection(object): | |
|
|
||
| def __init__(self, host, port=None, secure=None, ssl_context=None, | ||
| proxy_host=None, proxy_port=None, proxy_headers=None, | ||
| **kwargs): | ||
| timeout=None, **kwargs): | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We should probably add this to the common
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ok,I just added |
||
| if port is None: | ||
| self.host, self.port = to_host_port_tuple(host, default_port=80) | ||
| else: | ||
|
|
@@ -150,6 +150,14 @@ def __init__(self, host, port=None, secure=None, ssl_context=None, | |
| #: the standard hyper parsing interface. | ||
| self.parser = Parser() | ||
|
|
||
| # timeout | ||
| if isinstance(timeout, tuple): | ||
| self._connect_timeout = timeout[0] | ||
| self._read_timeout = timeout[1] | ||
| else: | ||
| self._connect_timeout = timeout | ||
| self._read_timeout = timeout | ||
|
|
||
| def connect(self): | ||
| """ | ||
| Connect to the server specified when the object was created. This is a | ||
|
|
@@ -172,10 +180,11 @@ def connect(self): | |
| # Simple http proxy | ||
| sock = socket.create_connection( | ||
| (self.proxy_host, self.proxy_port), | ||
| 5 | ||
| timeout=self._connect_timeout | ||
| ) | ||
| else: | ||
| sock = socket.create_connection((self.host, self.port), 5) | ||
| sock = socket.create_connection((self.host, self.port), | ||
| timeout=self._connect_timeout) | ||
| proto = None | ||
|
|
||
| if self.secure: | ||
|
|
@@ -184,6 +193,9 @@ def connect(self): | |
| log.debug("Selected protocol: %s", proto) | ||
| sock = BufferedSocket(sock, self.network_buffer_size) | ||
|
|
||
| # Set read timeout | ||
| sock.settimeout(self._read_timeout) | ||
|
|
||
| if proto not in ('http/1.1', None): | ||
| raise TLSUpgrade(proto, sock) | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -110,6 +110,18 @@ def test_initialization_with_ipv6_addresses_proxy_inline_port(self): | |
| assert c.proxy_host == 'ffff:aaaa::1' | ||
| assert c.proxy_port == 8443 | ||
|
|
||
| def test_initialization_timeout(self): | ||
| c = HTTP11Connection('httpbin.org', timeout=30) | ||
|
|
||
| assert c._connect_timeout == 30 | ||
| assert c._read_timeout == 30 | ||
|
|
||
| def test_initialization_tuple_timeout(self): | ||
| c = HTTP11Connection('httpbin.org', timeout=(5, 60)) | ||
|
|
||
| assert c._connect_timeout == 5 | ||
| assert c._read_timeout == 60 | ||
|
|
||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We need these tests for HTTP/2 as well.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ok,I add this in test/test_hyper.py |
||
| def test_basic_request(self): | ||
| c = HTTP11Connection('httpbin.org') | ||
| c._sock = sock = DummySocket() | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -13,6 +13,7 @@ | |
| import hyper | ||
| import hyper.http11.connection | ||
| import pytest | ||
| from socket import timeout as SocketTimeout | ||
| from contextlib import contextmanager | ||
| from mock import patch | ||
| from concurrent.futures import ThreadPoolExecutor, TimeoutError | ||
|
|
@@ -1230,6 +1231,114 @@ def do_connect(conn): | |
|
|
||
| self.tear_down() | ||
|
|
||
| def test_connection_timeout(self): | ||
| self.set_up(timeout=0.5) | ||
|
|
||
| def socket_handler(listener): | ||
| time.sleep(1) | ||
| sock = listener.accept()[0] | ||
|
||
| sock.close() | ||
|
|
||
| self._start_server(socket_handler) | ||
| conn = self.get_connection() | ||
| try: | ||
| conn.connect() | ||
| assert False | ||
|
||
| except (SocketTimeout, ssl.SSLError): | ||
| # Py2 raises this as a BaseSSLError, | ||
| # Py3 raises it as socket timeout. | ||
| # assert 'timed out' in e.message | ||
|
||
| pass | ||
|
|
||
| self.tear_down() | ||
|
|
||
| def test_read_timeout(self): | ||
| self.set_up(timeout=0.5) | ||
|
|
||
| req_event = threading.Event() | ||
| recv_event = threading.Event() | ||
|
|
||
| def socket_handler(listener): | ||
| sock = listener.accept()[0] | ||
|
|
||
| # We get two messages for the connection open and then a HEADERS | ||
| # frame. | ||
| receive_preamble(sock) | ||
| sock.recv(65535) | ||
|
|
||
| # Wait for request | ||
| req_event.wait(5) | ||
| # Now, send the headers for the response. This response has no body | ||
|
||
| time.sleep(1) | ||
|
|
||
| # Now, send the headers for the response. This response has no body | ||
| f = build_headers_frame( | ||
|
||
| [(':status', '204'), ('content-length', '0')] | ||
| ) | ||
| f.flags.add('END_STREAM') | ||
| f.stream_id = 1 | ||
| sock.send(f.serialize()) | ||
|
|
||
| # Wait for the message from the main thread. | ||
| recv_event.wait(5) | ||
| sock.close() | ||
|
|
||
| self._start_server(socket_handler) | ||
| conn = self.get_connection() | ||
| conn.request('GET', '/') | ||
| req_event.set() | ||
|
|
||
| try: | ||
| conn.get_response() | ||
| assert False | ||
|
||
| except (SocketTimeout, ssl.SSLError): | ||
| # Py2 raises this as a BaseSSLError, | ||
| # Py3 raises it as socket timeout. | ||
| # assert 'timed out' in e.message | ||
| pass | ||
|
|
||
| # Awesome, we're done now. | ||
| recv_event.set() | ||
| self.tear_down() | ||
|
|
||
| def test_default_connection_timeout(self): | ||
| self.set_up(timeout=None) | ||
|
|
||
| # Confirm that we send the connection upgrade string and the initial | ||
| # SettingsFrame. | ||
| data = [] | ||
| send_event = threading.Event() | ||
|
|
||
| def socket_handler(listener): | ||
| time.sleep(1) | ||
| sock = listener.accept()[0] | ||
|
|
||
| # We should get one big chunk. | ||
| first = sock.recv(65535) | ||
| data.append(first) | ||
|
|
||
| # We need to send back a SettingsFrame. | ||
| f = SettingsFrame(0) | ||
| sock.send(f.serialize()) | ||
|
|
||
| send_event.set() | ||
| sock.close() | ||
|
|
||
| self._start_server(socket_handler) | ||
| conn = self.get_connection() | ||
| try: | ||
| conn.connect() | ||
| except (SocketTimeout, ssl.SSLError): | ||
| # Py2 raises this as a BaseSSLError, | ||
| # Py3 raises it as socket timeout. | ||
| assert False | ||
|
||
|
|
||
| send_event.wait(5) | ||
|
|
||
| assert data[0].startswith(b'PRI * HTTP/2.0\r\n\r\nSM\r\n\r\n') | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How can this possibly pass? If the connection fails, we're never going to see this data.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. default timeout
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah, sorry, I see that. Thanks! |
||
|
|
||
| self.tear_down() | ||
|
|
||
|
|
||
| @patch('hyper.http20.connection.H2_NPN_PROTOCOLS', PROTOCOLS) | ||
| class TestRequestsAdapter(SocketLevelTest): | ||
|
|
@@ -1290,7 +1399,8 @@ def socket_handler(listener): | |
|
|
||
| s = requests.Session() | ||
| s.mount('https://%s' % self.host, HTTP20Adapter()) | ||
| r = s.get('https://%s:%s/some/path' % (self.host, self.port)) | ||
| r = s.get('https://%s:%s/some/path' % (self.host, self.port), | ||
| timeout=(10, 60)) | ||
|
|
||
| # Assert about the received values. | ||
| assert r.status_code == 200 | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's just add
timeoutto our list of kwargs.