From 6c7054973041b34981e2dd536d707f262fd58b09 Mon Sep 17 00:00:00 2001 From: Kostis Anagnostopoulos Date: Wed, 8 Mar 2017 15:43:43 +0100 Subject: [PATCH 1/2] feat(socket): don't swallows errors in `socket.create_connection()` loop ## Context The utility method `socket.create_connection()` currently works like that: 1. resolve the destination-address into one or more IP(v4 & v6) addresses; 2. loop on each IP address and stop to the 1st one to work; 3. if none works, re-raise the last error. ## The problem So currently the loop in `socket.create_connection()` ignores all intermediate errors and reports only the last connection failure, which might be irrelevant. For instance, when both IPv4 & IPv6 networks are supported, usually the last address is a IPv6 address and it frequently fails with an irrelevant error - the actual cause have already been ignored. ## Possible solutions & open questions To facilitate network debugging, there are at least 3 options: a. log each failure [as they happen](/python/cpython/blob/6f0eb93183519024cb360162bdd81b9faec97ba6/Lib/socket.py#L717), but that would get the final failure twice: once as a (warning?) message, and once as an exception . b. collect all failures and log them only when connection fails to collect the errors, but that might miss important infos to the user; c. collect and return all failures in list attached to the raised exception. A question for cases (a) & (b) is what logging "means" to use: the `warnings` or `logging` module? And if `logging` is chosen, log them in `'DEBUG'` or `'WARNING'` level? Case (c), implemented in this PR sidesteps the above questions. --- Lib/socket.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/Lib/socket.py b/Lib/socket.py index 740e71782af2c3..3a185bf3f71cc2 100644 --- a/Lib/socket.py +++ b/Lib/socket.py @@ -700,7 +700,7 @@ def create_connection(address, timeout=_GLOBAL_DEFAULT_TIMEOUT, """ host, port = address - err = None + errs = [] for res in getaddrinfo(host, port, 0, SOCK_STREAM): af, socktype, proto, canonname, sa = res sock = None @@ -714,12 +714,12 @@ def create_connection(address, timeout=_GLOBAL_DEFAULT_TIMEOUT, return sock except error as _: - err = _ + errs.append(_) if sock is not None: sock.close() - if err is not None: - raise err + if errs: + raise error("connection failed", errs) else: raise error("getaddrinfo returns an empty list") From b39d4b1c699e6bcb2ffcaf661bac6d0625e32b9e Mon Sep 17 00:00:00 2001 From: Kostis Anagnostopoulos Date: Fri, 17 Mar 2017 17:49:33 +0100 Subject: [PATCH 2/2] refact(socket): corrections according to #562 - Rename `_` and `errs` variable names. - Raise original error if a single one. - Improve a bit error message, but still no Errno provided for when multiple errors are included. --- Lib/socket.py | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/Lib/socket.py b/Lib/socket.py index 3a185bf3f71cc2..59201c414fccbd 100644 --- a/Lib/socket.py +++ b/Lib/socket.py @@ -700,7 +700,7 @@ def create_connection(address, timeout=_GLOBAL_DEFAULT_TIMEOUT, """ host, port = address - errs = [] + errors = [] for res in getaddrinfo(host, port, 0, SOCK_STREAM): af, socktype, proto, canonname, sa = res sock = None @@ -713,13 +713,15 @@ def create_connection(address, timeout=_GLOBAL_DEFAULT_TIMEOUT, sock.connect(sa) return sock - except error as _: - errs.append(_) + except error as ex: + errors.append(ex) if sock is not None: sock.close() - - if errs: - raise error("connection failed", errs) + nerr = len(errors) + if nerr == 1: + raise errors[0] + elif nerr > 1: + raise error("no connection possible due to %d errors" % nerr, errors) else: raise error("getaddrinfo returns an empty list")