python2: regression in connect() error handling

This autoformatting PR actually changed the behavior of the error handling:
https://github.com/httplib2/httplib2/pull/105/files#diff-c6669c781a2dee1b2d2671cab4e21c66L985

"raise socket.err, msg" is not the same as "raise socket.error(msg)" in the case where msg is an exception. For instance consider:
msg = socket.timeout("foo")
raise socket.error, msg  # => socket.timeout: foo
raise socket.error(msg)  # => socket.error: foo

This has the effect of potentially changing the type of the error raised by connect(). Interestingly the PY3 copy of the code doesn't have this problem (probably cause it doesn't have msg at all; not sure if it's actually needed but I figured might as well keep it for PY2 in case it is).

The change I propose here will restore the original behavior prior to the autoformat change, and will ensure that both the PY2 and PY3 copies of the code raise the same error type in the event of e.g. a socket.timeout.

python2/httplib2/__init__.py

@@ -1162,7 +1162,6 @@ def connect(self):
             raise ProxiesUnavailableError(
                 "Proxy support missing but proxy use was requested!"
             )
-        msg = "getaddrinfo returns an empty list"
         if self.proxy_info and self.proxy_info.isgood():
             use_proxy = True
             proxy_type, proxy_host, proxy_port, proxy_rdns, proxy_user, proxy_pass, proxy_headers = (
@@ -1176,7 +1175,9 @@ def connect(self):
 
             host = self.host
             port = self.port
-
+
+        socket_err = None
+
         for res in socket.getaddrinfo(host, port, 0, socket.SOCK_STREAM):
             af, socktype, proto, canonname, sa = res
             try:
@@ -1218,7 +1219,8 @@ def connect(self):
                     self.sock.connect((self.host, self.port) + sa[2:])
                 else:
                     self.sock.connect(sa)
-            except socket.error as msg:
+            except socket.error as e:
+                socket_err = e
                 if self.debuglevel > 0:
                     print("connect fail: (%s, %s)" % (self.host, self.port))
                     if use_proxy:
@@ -1241,7 +1243,7 @@ def connect(self):
                 continue
             break
         if not self.sock:
-            raise socket.error(msg)
+            raise socket_err or socket.error("getaddrinfo returns an empty list")
 
 
 class HTTPSConnectionWithTimeout(httplib.HTTPSConnection):
@@ -1338,7 +1340,6 @@ def _ValidateCertificateHostname(self, cert, hostname):
     def connect(self):
         "Connect to a host on a given (SSL) port."
 
-        msg = "getaddrinfo returns an empty list"
         if self.proxy_info and self.proxy_info.isgood():
             use_proxy = True
             proxy_type, proxy_host, proxy_port, proxy_rdns, proxy_user, proxy_pass, proxy_headers = (
@@ -1352,7 +1353,9 @@ def connect(self):
 
             host = self.host
             port = self.port
-
+
+        socket_err = None
+
         address_info = socket.getaddrinfo(host, port, 0, socket.SOCK_STREAM)
         for family, socktype, proto, canonname, sockaddr in address_info:
             try:
@@ -1435,7 +1438,8 @@ def connect(self):
                     raise
             except (socket.timeout, socket.gaierror):
                 raise
-            except socket.error as msg:
+            except socket.error as e:
+                socket_err = e
                 if self.debuglevel > 0:
                     print("connect fail: (%s, %s)" % (self.host, self.port))
                     if use_proxy:
@@ -1458,7 +1462,7 @@ def connect(self):
                 continue
             break
         if not self.sock:
-            raise socket.error(msg)
+            raise socket_err or socket.error("getaddrinfo returns an empty list")
 
 
 SCHEME_TO_CONNECTION = {

tests/test_other.py

@@ -242,3 +242,14 @@ def test_close():
         assert len(http.connections) == 1
         http.close()
         assert len(http.connections) == 0
+
+
+def test_connect_exception_type():
+    # This autoformatting PR actually changed the behavior of error handling:
+    # https://github.com/httplib2/httplib2/pull/105/files#diff-c6669c781a2dee1b2d2671cab4e21c66L985
+    # potentially changing the type of the error raised by connect()
+    # https://github.com/httplib2/httplib2/pull/150
+    http = httplib2.Http()
+    with mock.patch("httplib2.socket.socket.connect", side_effect=socket.timeout("foo")):
+        with tests.assert_raises(socket.timeout):
+            http.request(tests.DUMMY_URL)