Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix PY2 regression in connect() error handling. #150

Merged
merged 4 commits into from Nov 13, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
20 changes: 12 additions & 8 deletions python2/httplib2/__init__.py
Expand Up @@ -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 = (
Expand All @@ -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:
Expand Down Expand Up @@ -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:
Expand All @@ -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):
Expand Down Expand Up @@ -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 = (
Expand All @@ -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:
Expand Down Expand Up @@ -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:
Expand All @@ -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 = {
Expand Down
11 changes: 11 additions & 0 deletions tests/test_other.py
Expand Up @@ -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)