Merge pull request #703 from shazow/cleanup

Tests cleanup and deflake-ing, added NewConnectionError exception

12 files changed, 244 insertions, 203 deletions

diff --git a/CHANGES.rst b/CHANGES.rst
index 58e0d677..73f55d67 100644
--- a/CHANGES.rst
+++ b/CHANGES.rst
@@ -7,8 +7,8 @@ dev (master)
 * ... [Short description of non-trivial change.] (Issue #)
 
 
-1.11.1 (2015-09-03)
-+++++++++++++++++++
+1.12 (2015-09-03)
++++++++++++++++++
 
 * Rely on ``six`` for importing ``httplib`` to work around
   conflicts with other Python 3 shims. (Issue #688)
@@ -16,6 +16,9 @@ dev (master)
 * Add support for directories of certificate authorities, as supported by
   OpenSSL. (Issue #701)
 
+* New exception: ``NewConnectionError``, raised when we fail to establish
+  a new connection, usually ``ECONNREFUSED`` socket error.
+
 
 1.11 (2015-07-21)
 +++++++++++++++++
diff --git a/dev-requirements.txt b/dev-requirements.txt
index a5e405d0..b371cd65 100644
--- a/dev-requirements.txt
+++ b/dev-requirements.txt
@@ -1,8 +1,8 @@
-nose==1.3.3
-nose-exclude==0.2.0
-mock==1.0.1
+nose==1.3.7
+nose-exclude==0.4.1
+mock==1.3.0
 coverage==3.7.1
-tox==1.7.1
-twine==1.3.1
+tox==2.1.1
+twine==1.5.0
 wheel==0.24.0
-tornado==4.1
+tornado==4.2.1
diff --git a/dummyserver/handlers.py b/dummyserver/handlers.py
index ffa1dd36..43398cdf 100644
--- a/dummyserver/handlers.py
+++ b/dummyserver/handlers.py
@@ -168,6 +168,8 @@ class TestingApp(RequestHandler):
 
     def sleep(self, request):
         "Sleep for a specified amount of ``seconds``"
+        # DO NOT USE THIS, IT'S DEPRECATED.
+        # FIXME: Delete this once appengine tests are fixed to not use this handler.
         seconds = float(request.params.get('seconds', '1'))
         time.sleep(seconds)
         return Response()
diff --git a/dummyserver/testcase.py b/dummyserver/testcase.py
index de6aedd4..e5ae51bd 100644
--- a/dummyserver/testcase.py
+++ b/dummyserver/testcase.py
@@ -14,6 +14,11 @@ from dummyserver.handlers import TestingApp
 from dummyserver.proxy import ProxyHandler
 
 
+def consume_socket(sock, chunks=65536):
+    while not sock.recv(chunks).endswith(b'\r\n\r\n'):
+        pass
+
+
 class SocketDummyServerTestCase(unittest.TestCase):
     """
     A simple socket-based server is created for this class that is good for
@@ -35,6 +40,32 @@ class SocketDummyServerTestCase(unittest.TestCase):
         cls.port = cls.server_thread.port
 
     @classmethod
+    def start_response_handler(cls, response, num=1, block_send=None):
+        ready_event = threading.Event()
+        def socket_handler(listener):
+            for _ in range(num):
+                ready_event.set()
+                ready_event.clear()
+
+                sock = listener.accept()[0]
+                consume_socket(sock)
+                if block_send:
+                    block_send.wait()
+                    block_send.clear()
+                sock.send(response)
+                sock.close()
+
+        cls._start_server(socket_handler)
+        return ready_event
+
+    @classmethod
+    def start_basic_handler(cls, **kw):
+        return cls.start_response_handler(
+           b'HTTP/1.1 200 OK\r\n'
+           b'Content-Length: 0\r\n'
+           b'\r\n', **kw)
+
+    @classmethod
     def tearDownClass(cls):
         if hasattr(cls, 'server_thread'):
             cls.server_thread.join(0.1)
diff --git a/test/with_dummyserver/test_connectionpool.py b/test/with_dummyserver/test_connectionpool.py
index 741ae7bb..9294adfc 100644
--- a/test/with_dummyserver/test_connectionpool.py
+++ b/test/with_dummyserver/test_connectionpool.py
@@ -29,22 +29,181 @@ from urllib3.exceptions import (
     MaxRetryError,
     ReadTimeoutError,
     ProtocolError,
+    NewConnectionError,
 )
 from urllib3.packages.six import b, u
 from urllib3.util.retry import Retry
 from urllib3.util.timeout import Timeout
 
-import tornado
-from dummyserver.testcase import HTTPDummyServerTestCase
+from dummyserver.testcase import HTTPDummyServerTestCase, SocketDummyServerTestCase
 from dummyserver.server import NoIPv6Warning, HAS_IPV6_AND_DNS
 
-from nose.tools import timed
+from threading import Event
 
 log = logging.getLogger('urllib3.connectionpool')
 log.setLevel(logging.NOTSET)
 log.addHandler(logging.StreamHandler(sys.stdout))
 
 
+SHORT_TIMEOUT = 0.001
+LONG_TIMEOUT = 0.01
+
+
+class TestConnectionPoolTimeouts(SocketDummyServerTestCase):
+
+    def test_timeout_float(self):
+        block_event = Event()
+        ready_event = self.start_basic_handler(block_send=block_event, num=2)
+
+        # Pool-global timeout
+        pool = HTTPConnectionPool(self.host, self.port, timeout=SHORT_TIMEOUT, retries=False)
+        self.assertRaises(ReadTimeoutError, pool.request, 'GET', '/')
+        block_event.set() # Release block
+
+        # Shouldn't raise this time
+        ready_event.wait()
+        block_event.set() # Pre-release block
+        pool.request('GET', '/')
+
+    def test_conn_closed(self):
+        block_event = Event()
+        self.start_basic_handler(block_send=block_event, num=1)
+
+        pool = HTTPConnectionPool(self.host, self.port, timeout=SHORT_TIMEOUT, retries=False)
+        conn = pool._get_conn()
+        pool._put_conn(conn)
+        try:
+            pool.urlopen('GET', '/')
+            self.fail("The request should fail with a timeout error.")
+        except ReadTimeoutError:
+            if conn.sock:
+                self.assertRaises(socket.error, conn.sock.recv, 1024)
+        finally:
+            pool._put_conn(conn)
+
+        block_event.set()
+
+    def test_timeout(self):
+        # Requests should time out when expected
+        block_event = Event()
+        ready_event = self.start_basic_handler(block_send=block_event, num=6)
+
+        # Pool-global timeout
+        timeout = Timeout(read=SHORT_TIMEOUT)
+        pool = HTTPConnectionPool(self.host, self.port, timeout=timeout, retries=False)
+
+        conn = pool._get_conn()
+        self.assertRaises(ReadTimeoutError, pool._make_request, conn, 'GET', '/')
+        pool._put_conn(conn)
+        block_event.set() # Release request
+
+        ready_event.wait()
+        block_event.clear()
+        self.assertRaises(ReadTimeoutError, pool.request, 'GET', '/')
+        block_event.set() # Release request
+
+        # Request-specific timeouts should raise errors
+        pool = HTTPConnectionPool(self.host, self.port, timeout=LONG_TIMEOUT, retries=False)
+
+        conn = pool._get_conn()
+        ready_event.wait()
+        now = time.time()
+        self.assertRaises(ReadTimeoutError, pool._make_request, conn, 'GET', '/', timeout=timeout)
+        delta = time.time() - now
+        block_event.set() # Release request
+
+        self.assertTrue(delta < LONG_TIMEOUT, "timeout was pool-level LONG_TIMEOUT rather than request-level SHORT_TIMEOUT")
+        pool._put_conn(conn)
+
+        ready_event.wait()
+        now = time.time()
+        self.assertRaises(ReadTimeoutError, pool.request, 'GET', '/', timeout=timeout)
+        delta = time.time() - now
+
+        self.assertTrue(delta < LONG_TIMEOUT, "timeout was pool-level LONG_TIMEOUT rather than request-level SHORT_TIMEOUT")
+        block_event.set() # Release request
+
+        # Timeout int/float passed directly to request and _make_request should
+        # raise a request timeout
+        ready_event.wait()
+        self.assertRaises(ReadTimeoutError, pool.request, 'GET', '/', timeout=SHORT_TIMEOUT)
+        block_event.set() # Release request
+
+        ready_event.wait()
+        conn = pool._new_conn()
+        # FIXME: This assert flakes sometimes. Not sure why.
+        self.assertRaises(ReadTimeoutError, pool._make_request, conn, 'GET', '/', timeout=SHORT_TIMEOUT)
+        block_event.set() # Release request
+
+    def test_connect_timeout(self):
+        def noop_handler(listener):
+            return
+
+        self._start_server(noop_handler)
+
+        url = '/'
+        host, port = self.host, self.port
+        timeout = Timeout(connect=SHORT_TIMEOUT)
+
+        # Pool-global timeout
+        pool = HTTPConnectionPool(host, port, timeout=timeout)
+        conn = pool._get_conn()
+        self.assertRaises(ConnectTimeoutError, pool._make_request, conn, 'GET', url)
+
+        # Retries
+        retries = Retry(connect=0)
+        self.assertRaises(MaxRetryError, pool.request, 'GET', url, retries=retries)
+
+        # Request-specific connection timeouts
+        big_timeout = Timeout(read=LONG_TIMEOUT, connect=LONG_TIMEOUT)
+        pool = HTTPConnectionPool(host, port, timeout=big_timeout, retries=False)
+        conn = pool._get_conn()
+        self.assertRaises(ConnectTimeoutError, pool._make_request, conn, 'GET', url, timeout=timeout)
+
+        pool._put_conn(conn)
+        self.assertRaises(ConnectTimeoutError, pool.request, 'GET', url, timeout=timeout)
+
+    def test_total_applies_connect(self):
+        def noop_handler(listener):
+            return
+
+        self._start_server(noop_handler)
+
+        timeout = Timeout(total=None, connect=SHORT_TIMEOUT)
+        pool = HTTPConnectionPool(self.host, self.port, timeout=timeout)
+        conn = pool._get_conn()
+        self.assertRaises(ConnectTimeoutError, pool._make_request, conn, 'GET', '/')
+
+        timeout = Timeout(connect=3, read=5, total=SHORT_TIMEOUT)
+        pool = HTTPConnectionPool(self.host, self.port, timeout=timeout)
+        conn = pool._get_conn()
+        self.assertRaises(ConnectTimeoutError, pool._make_request, conn, 'GET', '/')
+
+    def test_total_timeout(self):
+        block_event = Event()
+        ready_event = self.start_basic_handler(block_send=block_event, num=2)
+
+        # This will get the socket to raise an EAGAIN on the read
+        timeout = Timeout(connect=3, read=SHORT_TIMEOUT)
+        pool = HTTPConnectionPool(self.host, self.port, timeout=timeout, retries=False)
+        self.assertRaises(ReadTimeoutError, pool.request, 'GET', '/')
+
+        block_event.set()
+        ready_event.wait()
+        block_event.clear()
+
+        # The connect should succeed and this should hit the read timeout
+        timeout = Timeout(connect=3, read=5, total=SHORT_TIMEOUT)
+        pool = HTTPConnectionPool(self.host, self.port, timeout=timeout, retries=False)
+        self.assertRaises(ReadTimeoutError, pool.request, 'GET', '/')
+
+    def test_create_connection_timeout(self):
+        timeout = Timeout(connect=SHORT_TIMEOUT, total=LONG_TIMEOUT)
+        pool = HTTPConnectionPool(TARPIT_HOST, self.port, timeout=timeout, retries=False)
+        conn = pool._new_conn()
+        self.assertRaises(ConnectTimeoutError, conn.connect)
+
+
 class TestConnectionPool(HTTPDummyServerTestCase):
 
     def setUp(self):
@@ -124,26 +283,6 @@ class TestConnectionPool(HTTPDummyServerTestCase):
         r = self.pool.request('POST', '/upload', fields=fields)
         self.assertEqual(r.status, 200, r.data)
 
-    def test_timeout_float(self):
-        url = '/sleep?seconds=0.005'
-        # Pool-global timeout
-        pool = HTTPConnectionPool(self.host, self.port, timeout=0.001, retries=False)
-        self.assertRaises(ReadTimeoutError, pool.request, 'GET', url)
-
-    def test_conn_closed(self):
-        pool = HTTPConnectionPool(self.host, self.port, timeout=0.001, retries=False)
-        conn = pool._get_conn()
-        pool._put_conn(conn)
-        try:
-            url = '/sleep?seconds=0.005'
-            pool.urlopen('GET', url)
-            self.fail("The request should fail with a timeout error.")
-        except ReadTimeoutError:
-            if conn.sock:
-                self.assertRaises(socket.error, conn.sock.recv, 1024)
-        finally:
-            pool._put_conn(conn)
-
     def test_nagle(self):
         """ Test that connections have TCP_NODELAY turned on """
         # This test needs to be here in order to be run. socket.create_connection actually tries to
@@ -152,10 +291,7 @@ class TestConnectionPool(HTTPDummyServerTestCase):
         conn = pool._get_conn()
         pool._make_request(conn, 'GET', '/')
         tcp_nodelay_setting = conn.sock.getsockopt(socket.IPPROTO_TCP, socket.TCP_NODELAY)
-        assert tcp_nodelay_setting > 0, ("Expected TCP_NODELAY to be set on the "
-                                         "socket (with value greater than 0) "
-                                         "but instead was %s" %
-                                         tcp_nodelay_setting)
+        self.assertTrue(tcp_nodelay_setting)
 
     def test_socket_options(self):
         """Test that connections accept socket options."""
@@ -194,79 +330,6 @@ class TestConnectionPool(HTTPDummyServerTestCase):
         self.assertTrue(nagle_disabled)
         self.assertTrue(using_keepalive)
 
-    @timed(0.5)
-    def test_timeout(self):
-        """ Requests should time out when expected """
-        url = '/sleep?seconds=0.003'
-        timeout = Timeout(read=0.001)
-
-        # Pool-global timeout
-        pool = HTTPConnectionPool(self.host, self.port, timeout=timeout, retries=False)
-
-        conn = pool._get_conn()
-        self.assertRaises(ReadTimeoutError, pool._make_request,
-                          conn, 'GET', url)
-        pool._put_conn(conn)
-
-        time.sleep(0.02) # Wait for server to start receiving again. :(
-
-        self.assertRaises(ReadTimeoutError, pool.request, 'GET', url)
-
-        # Request-specific timeouts should raise errors
-        pool = HTTPConnectionPool(self.host, self.port, timeout=0.1, retries=False)
-
-        conn = pool._get_conn()
-        self.assertRaises(ReadTimeoutError, pool._make_request,
-                          conn, 'GET', url, timeout=timeout)
-        pool._put_conn(conn)
-
-        time.sleep(0.02) # Wait for server to start receiving again. :(
-
-        self.assertRaises(ReadTimeoutError, pool.request,
-                          'GET', url, timeout=timeout)
-
-        # Timeout int/float passed directly to request and _make_request should
-        # raise a request timeout
-        self.assertRaises(ReadTimeoutError, pool.request,
-                          'GET', url, timeout=0.001)
-        conn = pool._new_conn()
-        self.assertRaises(ReadTimeoutError, pool._make_request, conn,
-                          'GET', url, timeout=0.001)
-        pool._put_conn(conn)
-
-        # Timeout int/float passed directly to _make_request should not raise a
-        # request timeout if it's a high value
-        pool.request('GET', url, timeout=1)
-
-    @requires_network
-    @timed(0.5)
-    def test_connect_timeout(self):
-        url = '/sleep?seconds=0.005'
-        timeout = Timeout(connect=0.001)
-
-        # Pool-global timeout
-        pool = HTTPConnectionPool(TARPIT_HOST, self.port, timeout=timeout)
-        conn = pool._get_conn()
-        self.assertRaises(ConnectTimeoutError, pool._make_request, conn, 'GET', url)
-
-        # Retries
-        retries = Retry(connect=0)
-        self.assertRaises(MaxRetryError, pool.request, 'GET', url,
-                          retries=retries)
-
-        # Request-specific connection timeouts
-        big_timeout = Timeout(read=0.2, connect=0.2)
-        pool = HTTPConnectionPool(TARPIT_HOST, self.port,
-                                  timeout=big_timeout, retries=False)
-        conn = pool._get_conn()
-        self.assertRaises(ConnectTimeoutError, pool._make_request, conn, 'GET',
-                          url, timeout=timeout)
-
-        pool._put_conn(conn)
-        self.assertRaises(ConnectTimeoutError, pool.request, 'GET', url,
-                          timeout=timeout)
-
-
     def test_connection_error_retries(self):
         """ ECONNREFUSED error should raise a connection error, with retries """
         port = find_unused_port()
@@ -275,50 +338,7 @@ class TestConnectionPool(HTTPDummyServerTestCase):
             pool.request('GET', '/', retries=Retry(connect=3))
             self.fail("Should have failed with a connection error.")
         except MaxRetryError as e:
-            self.assertTrue(isinstance(e.reason, ProtocolError))
-            self.assertEqual(e.reason.args[1].errno, errno.ECONNREFUSED)
-
-    def test_timeout_reset(self):
-        """ If the read timeout isn't set, socket timeout should reset """
-        url = '/sleep?seconds=0.005'
-        timeout = Timeout(connect=0.001)
-        pool = HTTPConnectionPool(self.host, self.port, timeout=timeout)
-        conn = pool._get_conn()
-        try:
-            pool._make_request(conn, 'GET', url)
-        except ReadTimeoutError:
-            self.fail("This request shouldn't trigger a read timeout.")
-
-    @requires_network
-    @timed(5.0)
-    def test_total_timeout(self):
-        url = '/sleep?seconds=0.005'
-
-        timeout = Timeout(connect=3, read=5, total=0.001)
-        pool = HTTPConnectionPool(TARPIT_HOST, self.port, timeout=timeout)
-        conn = pool._get_conn()
-        self.assertRaises(ConnectTimeoutError, pool._make_request, conn, 'GET', url)
-
-        # This will get the socket to raise an EAGAIN on the read
-        timeout = Timeout(connect=3, read=0)
-        pool = HTTPConnectionPool(self.host, self.port, timeout=timeout)
-        conn = pool._get_conn()
-        self.assertRaises(ReadTimeoutError, pool._make_request, conn, 'GET', url)
-
-        # The connect should succeed and this should hit the read timeout
-        timeout = Timeout(connect=3, read=5, total=0.002)
-        pool = HTTPConnectionPool(self.host, self.port, timeout=timeout)
-        conn = pool._get_conn()
-        self.assertRaises(ReadTimeoutError, pool._make_request, conn, 'GET', url)
-
-    @requires_network
-    def test_none_total_applies_connect(self):
-        url = '/sleep?seconds=0.005'
-        timeout = Timeout(total=None, connect=0.001)
-        pool = HTTPConnectionPool(TARPIT_HOST, self.port, timeout=timeout)
-        conn = pool._get_conn()
-        self.assertRaises(ConnectTimeoutError, pool._make_request, conn, 'GET',
-                          url)
+            self.assertEqual(type(e.reason), NewConnectionError)
 
     def test_timeout_success(self):
         timeout = Timeout(connect=3, read=5, total=None)
@@ -372,7 +392,7 @@ class TestConnectionPool(HTTPDummyServerTestCase):
             pool.request('GET', '/', retries=5)
             self.fail("should raise timeout exception here")
         except MaxRetryError as e:
-            self.assertTrue(isinstance(e.reason, ProtocolError), e.reason)
+            self.assertEqual(type(e.reason), NewConnectionError)
 
     def test_keepalive(self):
         pool = HTTPConnectionPool(self.host, self.port, block=True, maxsize=1)
@@ -607,16 +627,13 @@ class TestConnectionPool(HTTPDummyServerTestCase):
             pool = HTTPConnectionPool(self.host, self.port,
                     source_address=addr, retries=False)
             r = pool.request('GET', '/source_address')
-            assert r.data == b(addr[0]), (
-                "expected the response to contain the source address {addr}, "
-                "but was {data}".format(data=r.data, addr=b(addr[0])))
+            self.assertEqual(r.data, b(addr[0]))
 
     def test_source_address_error(self):
         for addr in INVALID_SOURCE_ADDRESSES:
-            pool = HTTPConnectionPool(self.host, self.port,
-                    source_address=addr, retries=False)
-            self.assertRaises(ProtocolError,
-                    pool.request, 'GET', '/source_address')
+            pool = HTTPConnectionPool(self.host, self.port, source_address=addr, retries=False)
+            # FIXME: This assert flakes sometimes. Not sure why.
+            self.assertRaises(NewConnectionError, pool.request, 'GET', '/source_address?{0}'.format(addr))
 
     def test_stream_keepalive(self):
         x = 2
@@ -669,6 +686,8 @@ class TestConnectionPool(HTTPDummyServerTestCase):
             self.assertEqual(http.pool.qsize(), http.pool.maxsize)
 
 
+
+
 class TestRetry(HTTPDummyServerTestCase):
     def setUp(self):
         self.pool = HTTPConnectionPool(self.host, self.port)
@@ -695,7 +714,7 @@ class TestRetry(HTTPDummyServerTestCase):
         self.assertEqual(r.status, 303)
 
         pool = HTTPConnectionPool('thishostdoesnotexist.invalid', self.port, timeout=0.001)
-        self.assertRaises(ProtocolError, pool.request, 'GET', '/test', retries=False)
+        self.assertRaises(NewConnectionError, pool.request, 'GET', '/test', retries=False)
 
     def test_read_retries(self):
         """ Should retry for status codes in the whitelist """
diff --git a/test/with_dummyserver/test_https.py b/test/with_dummyserver/test_https.py
index 068531fe..862ebd9b 100644
--- a/test/with_dummyserver/test_https.py
+++ b/test/with_dummyserver/test_https.py
@@ -318,8 +318,6 @@ class TestHTTPS(HTTPSDummyServerTestCase):
         https_pool.ca_certs = DEFAULT_CA
         https_pool.assert_fingerprint = 'CC:45:6A:90:82:F7FF:C0:8218:8e:' \
                                         '7A:F2:8A:D7:1E:07:33:67:DE'
-        url = '/sleep?seconds=0.005'
-        self.assertRaises(ReadTimeoutError, https_pool.request, 'GET', url)
 
         timeout = Timeout(total=None)
         https_pool = HTTPSConnectionPool(self.host, self.port, timeout=timeout,
diff --git a/test/with_dummyserver/test_proxy_poolmanager.py b/test/with_dummyserver/test_proxy_poolmanager.py
index c593f2da..b2894a87 100644
--- a/test/with_dummyserver/test_proxy_poolmanager.py
+++ b/test/with_dummyserver/test_proxy_poolmanager.py
@@ -287,7 +287,7 @@ class TestHTTPProxyManager(HTTPDummyProxyTestCase):
             https.request('GET', self.http_url, timeout=0.001)
             self.fail("Failed to raise retry error.")
         except MaxRetryError as e:
-            assert isinstance(e.reason, ConnectTimeoutError)
+           self.assertEqual(type(e.reason), ConnectTimeoutError)
 
 
     @timed(0.5)
@@ -298,7 +298,7 @@ class TestHTTPProxyManager(HTTPDummyProxyTestCase):
             https.request('GET', self.http_url)
             self.fail("Failed to raise retry error.")
         except MaxRetryError as e:
-            assert isinstance(e.reason, ConnectTimeoutError)
+            self.assertEqual(type(e.reason), ConnectTimeoutError)
 
 
 class TestIPv6HTTPProxyManager(IPv6HTTPDummyProxyTestCase):
diff --git a/test/with_dummyserver/test_socketlevel.py b/test/with_dummyserver/test_socketlevel.py
index 5af00e0b..d09002b6 100644
--- a/test/with_dummyserver/test_socketlevel.py
+++ b/test/with_dummyserver/test_socketlevel.py
@@ -6,6 +6,7 @@ from urllib3.poolmanager import proxy_from_url
 from urllib3.exceptions import (
         MaxRetryError,
         ProxyError,
+        ConnectTimeoutError,
         ReadTimeoutError,
         SSLError,
         ProtocolError,
@@ -629,42 +630,23 @@ class TestSSL(SocketDummyServerTestCase):
         self.assertRaises(SSLError, request)
 
 
-def consume_socket(sock, chunks=65536):
-    while not sock.recv(chunks).endswith(b'\r\n\r\n'):
-        pass
-
-
-def create_response_handler(response, num=1):
-    def socket_handler(listener):
-        for _ in range(num):
-            sock = listener.accept()[0]
-            consume_socket(sock)
-
-            sock.send(response)
-            sock.close()
-
-    return socket_handler
-
-
 class TestErrorWrapping(SocketDummyServerTestCase):
 
     def test_bad_statusline(self):
-        handler = create_response_handler(
+        self.start_response_handler(
            b'HTTP/1.1 Omg What Is This?\r\n'
            b'Content-Length: 0\r\n'
            b'\r\n'
         )
-        self._start_server(handler)
         pool = HTTPConnectionPool(self.host, self.port, retries=False)
         self.assertRaises(ProtocolError, pool.request, 'GET', '/')
 
     def test_unknown_protocol(self):
-        handler = create_response_handler(
+        self.start_response_handler(
            b'HTTP/1000 200 OK\r\n'
            b'Content-Length: 0\r\n'
            b'\r\n'
         )
-        self._start_server(handler)
         pool = HTTPConnectionPool(self.host, self.port, retries=False)
         self.assertRaises(ProtocolError, pool.request, 'GET', '/')
 
@@ -672,13 +654,12 @@ class TestHeaders(SocketDummyServerTestCase):
 
     @onlyPy3
     def test_httplib_headers_case_insensitive(self):
-        handler = create_response_handler(
+        self.start_response_handler(
            b'HTTP/1.1 200 OK\r\n'
            b'Content-Length: 0\r\n'
            b'Content-type: text/plain\r\n'
            b'\r\n'
         )
-        self._start_server(handler)
         pool = HTTPConnectionPool(self.host, self.port, retries=False)
         HEADERS = {'Content-Length': '0', 'Content-type': 'text/plain'}
         r = pool.request('GET', '/')
@@ -727,14 +708,13 @@ class TestBrokenHeaders(SocketDummyServerTestCase):
         super(TestBrokenHeaders, self).setUp()
 
     def _test_broken_header_parsing(self, headers):
-        handler = create_response_handler((
+        self.start_response_handler((
            b'HTTP/1.1 200 OK\r\n'
            b'Content-Length: 0\r\n'
            b'Content-type: text/plain\r\n'
            ) + b'\r\n'.join(headers) + b'\r\n'
         )
 
-        self._start_server(handler)
         pool = HTTPConnectionPool(self.host, self.port, retries=False)
 
         with LogRecorder() as logs:
@@ -767,13 +747,12 @@ class TestBrokenHeaders(SocketDummyServerTestCase):
 
 class TestHEAD(SocketDummyServerTestCase):
     def test_chunked_head_response_does_not_hang(self):
-        handler = create_response_handler(
+        self.start_response_handler(
            b'HTTP/1.1 200 OK\r\n'
            b'Transfer-Encoding: chunked\r\n'
            b'Content-type: text/plain\r\n'
            b'\r\n'
         )
-        self._start_server(handler)
         pool = HTTPConnectionPool(self.host, self.port, retries=False)
         r = pool.request('HEAD', '/', timeout=1, preload_content=False)
 
@@ -781,13 +760,12 @@ class TestHEAD(SocketDummyServerTestCase):
         self.assertEqual([], list(r.stream()))
 
     def test_empty_head_response_does_not_hang(self):
-        handler = create_response_handler(
+        self.start_response_handler(
            b'HTTP/1.1 200 OK\r\n'
            b'Content-Length: 256\r\n'
            b'Content-type: text/plain\r\n'
            b'\r\n'
         )
-        self._start_server(handler)
         pool = HTTPConnectionPool(self.host, self.port, retries=False)
         r = pool.request('HEAD', '/', timeout=1, preload_content=False)
 
diff --git a/urllib3/connection.py b/urllib3/connection.py
index 7a02f326..3eab1e28 100644
--- a/urllib3/connection.py
+++ b/urllib3/connection.py
@@ -1,7 +1,7 @@
 import datetime
 import sys
 import socket
-from socket import timeout as SocketTimeout
+from socket import error as SocketError, timeout as SocketTimeout
 import warnings
 from .packages import six
 
@@ -36,9 +36,10 @@ except NameError:  # Python 2:
 
 
 from .exceptions import (
+    NewConnectionError,
     ConnectTimeoutError,
-    SystemTimeWarning,
     SubjectAltNameWarning,
+    SystemTimeWarning,
 )
 from .packages.ssl_match_hostname import match_hostname
 
@@ -133,11 +134,15 @@ class HTTPConnection(_HTTPConnection, object):
             conn = connection.create_connection(
                 (self.host, self.port), self.timeout, **extra_kw)
 
-        except SocketTimeout:
+        except SocketTimeout as e:
             raise ConnectTimeoutError(
                 self, "Connection to %s timed out. (connect timeout=%s)" %
                 (self.host, self.timeout))
 
+        except SocketError as e:
+            raise NewConnectionError(
+                self, "Failed to establish a new connection: %s" % e)
+
         return conn
 
     def _prepare_conn(self, conn):
diff --git a/urllib3/connectionpool.py b/urllib3/connectionpool.py
index bc402ddc..b38ac68d 100644
--- a/urllib3/connectionpool.py
+++ b/urllib3/connectionpool.py
@@ -22,10 +22,12 @@ from .exceptions import (
     LocationValueError,
     MaxRetryError,
     ProxyError,
+    ConnectTimeoutError,
     ReadTimeoutError,
     SSLError,
     TimeoutError,
     InsecureRequestWarning,
+    NewConnectionError,
 )
 from .packages.ssl_match_hostname import CertificateError
 from .packages import six
@@ -592,13 +594,13 @@ class HTTPConnectionPool(ConnectionPool, RequestMethods):
             release_conn = True
             raise
 
-        except (TimeoutError, HTTPException, SocketError, ConnectionError) as e:
+        except (TimeoutError, HTTPException, SocketError, ProtocolError) as e:
             # Discard the connection for these exceptions. It will be
             # be replaced during the next _get_conn() call.
             conn = conn and conn.close()
             release_conn = True
 
-            if isinstance(e, SocketError) and self.proxy:
+            if isinstance(e, (SocketError, NewConnectionError)) and self.proxy:
                 e = ProxyError('Cannot connect to proxy.', e)
             elif isinstance(e, (SocketError, HTTPException)):
                 e = ProtocolError('Connection aborted.', e)
diff --git a/urllib3/exceptions.py b/urllib3/exceptions.py
index 36ce0d1e..9607d65f 100644
--- a/urllib3/exceptions.py
+++ b/urllib3/exceptions.py
@@ -112,6 +112,9 @@ class ConnectTimeoutError(TimeoutError):
     "Raised when a socket timeout occurs while connecting to a server"
     pass
 
+class NewConnectionError(ConnectTimeoutError, PoolError):
+    "Raised when we fail to establish a new connection. Usually ECONNREFUSED."
+    pass
 
 class EmptyPoolError(PoolError):
     "Raised when a pool runs out of connections and no more are allowed."
diff --git a/urllib3/util/connection.py b/urllib3/util/connection.py
index 9ed5a64c..4f2f0f18 100644
--- a/urllib3/util/connection.py
+++ b/urllib3/util/connection.py
@@ -80,16 +80,16 @@ def create_connection(address, timeout=socket._GLOBAL_DEFAULT_TIMEOUT,
             sock.connect(sa)
             return sock
 
-        except socket.error as _:
-            err = _
+        except socket.error as e:
+            err = e
             if sock is not None:
                 sock.close()
                 sock = None
 
     if err is not None:
         raise err
-    else:
-        raise socket.error("getaddrinfo returns an empty list")
+
+    raise socket.error("getaddrinfo returns an empty list")
 
 
 def _set_socket_options(sock, options):