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.

2 files changed, 23 insertions, 8 deletions

diff --git a/python2/httplib2/__init__.py b/python2/httplib2/__init__.py
index e89e859..affc057 100644
--- a/python2/httplib2/__init__.py
+++ b/python2/httplib2/__init__.py
@@ -1162,7 +1162,6 @@ class HTTPConnectionWithTimeout(httplib.HTTPConnection):
             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 @@ class HTTPConnectionWithTimeout(httplib.HTTPConnection):
 
             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 @@ class HTTPConnectionWithTimeout(httplib.HTTPConnection):
                     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 @@ class HTTPConnectionWithTimeout(httplib.HTTPConnection):
                 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 @@ class HTTPSConnectionWithTimeout(httplib.HTTPSConnection):
     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 @@ class HTTPSConnectionWithTimeout(httplib.HTTPSConnection):
 
             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 @@ class HTTPSConnectionWithTimeout(httplib.HTTPSConnection):
                     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 @@ class HTTPSConnectionWithTimeout(httplib.HTTPSConnection):
                 continue
             break
         if not self.sock:
-            raise socket.error(msg)
+            raise socket_err or socket.error("getaddrinfo returns an empty list")
 
 
 SCHEME_TO_CONNECTION = {
diff --git a/tests/test_other.py b/tests/test_other.py
index eefd8e3..0f450ab 100644
--- a/tests/test_other.py
+++ b/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)