[issue29757] The loop in utility `socket.create_connection()` swallows previous errors

[Irit Katriel]

Change by Irit Katriel :


--
resolution:  -> duplicate
superseder:  -> OSError: multiple exceptions should preserve the exception type 
if it is common

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue29757] The loop in utility `socket.create_connection()` swallows previous errors

[Cheryl Sabella]

Change by Cheryl Sabella :


--
versions: +Python 3.9 -Python 3.7, Python 3.8

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue29757] The loop in utility `socket.create_connection()` swallows previous errors

[STINNER Victor]

STINNER Victor  added the comment:

> I thought that local-variables were deterministically (ref-countering) 
> destructed. What is happening?

IT's a reference cycle. Exception => traceback => frame => exception. The 'err' 
variable kept the frame alive which kept the exception alive which kept 
everything alive. The GC is supposed to be able to break ref cycles, but GC 
collections are irregular and sometimes the GC fails to break complex cycles.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue29757] The loop in utility `socket.create_connection()` swallows previous errors

[Kostis Anagnostopoulos]

Kostis Anagnostopoulos  added the comment:

The problem of not fixing this (and just add a suggestion in the docs saying 
that the user may re-implement this method) is that frequently this call is 
hidden at the core of many networking libraries.  So monkey-patching is needed 
instead, which is not always nice or easy.


> I just found an old bug in socket.create_connection():
> https://github.com/python/cpython/pull/3546

Regarding #3546, i'm surprised that a function-local variable keeps its 
contents alive, long after the frame of that function has gone.  I thought that 
local-variables were deterministically (ref-countering) destructed.  
What is happening?

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue29757] The loop in utility `socket.create_connection()` swallows previous errors

[Brett Cannon]

Change by Brett Cannon :


--
nosy:  -brett.cannon

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue29757] The loop in utility `socket.create_connection()` swallows previous errors

[STINNER Victor]

STINNER Victor  added the comment:

I propose the close the issue as WONTFIX. I don't see how the current behavior 
is an issue.

I expect a very large traceback just because a server is down/reject 
connections. I'm not sure that I want that by default.

> ## 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.

It's not so hard to reimplement create_connection(): it's a loop on 
getaddrinfo(). So you can decide how to deal with errors.

--

I recall that Yury Selivanov, Nathnaniel Smith and Guido van Rossum discussed 
the idea of "MultiError": an object which contains multiple exceptions, object 
which would render properly all exceptions when a traceback is displayed.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue29757] The loop in utility `socket.create_connection()` swallows previous errors

[Cheryl Sabella]

Change by Cheryl Sabella :


--
stage:  -> patch review
type:  -> enhancement
versions: +Python 3.8

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue29757] The loop in utility `socket.create_connection()` swallows previous errors

[STINNER Victor]

STINNER Victor added the comment:

I'm not excited to keep multiple exception objects alive because the 
Exception.__traceback__ causes many hairy reference cycle issues.

I just found an old bug in socket.create_connection():
https://github.com/python/cpython/pull/3546

See https://bugs.python.org/issue29639#msg302087 for my story of the bug.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue29757] The loop in utility `socket.create_connection()` swallows previous errors

[Kostis Anagnostopoulos]

Kostis Anagnostopoulos added the comment:

Just to remind that as it stands now(b39d4b1c6) it contains no API changes at 
all, so it should be considered for merge into 3.6.x line.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue29757] The loop in utility `socket.create_connection()` swallows previous errors

[Kostis Anagnostopoulos]

Kostis Anagnostopoulos added the comment:

> This is a new feature, so we can only push it to 3.7.

As it stands now(b39d4b1c6) it hardly contains any change - just in the case of 
multiple intermediate errors AND final failure, the exception raised is a bit 
different.

AFAICS it would be a "change" for 3.7 if my one of the 3 options of my last 
comment gets implemented.

Any ideas which one to implement?

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue29757] The loop in utility `socket.create_connection()` swallows previous errors

[Kostis Anagnostopoulos]

Kostis Anagnostopoulos added the comment:

> When the list of errors is passed as a second argument to the exception, how 
> is it rendered?  

This is how my latest ec887c0c3 looks on Linux:

>>> import socket
>>> socket.create_connection(('localhost', 12345))
Traceback (most recent call last):
  File "", line 1, in 
  File "/usr/lib/python3.5/socket.py", line 714, in create_connection
raise error("no connection possible due to %d errors" % nerr, errors)
OSError: [Errno no connection possible due to 2 errors] 
[ConnectionRefusedError(111, 'Connection refused'), ConnectionRefusedError(111, 
'Connection refused')]

And this is on Windows:

>>> socket.create_connection(('localhost', 12345), 1)
Traceback (most recent call last):
  File 
"D:\Apps\WinPython-64bit-3.5.3.0Qt5\python-3.5.3.amd64\lib\site-packages\IPython\core\interactiveshell.py",
 line 2881, in run_code
exec(code_obj, self.user_global_ns, self.user_ns)
  File "", line 1, in 
socket.create_connection(('localhost', 12345), 1)
  File 
"D:\Apps\WinPython-64bit-3.5.3.0Qt5\python-3.5.3.amd64\lib\socket.py", line 
714, in create_connection
raise error("no connection possible due to %d errors" % nerr, errors)
OSError: [Errno no connection possible due to 2 errors] [timeout('timed 
out',), timeout('timed out',)]


> Would it make sense to concatenate all error messages:

But then the user will not receive a list of errors to inspect, but just a big 
string.
The biggest problem in my latest ec887c0c3 is that I'm abusing the 1st arg to 
OSError() constructor, instead of being an `errno` it is a string.
But I got that from the existing code.[1]

And still, this PR is not yer finished because there is no feedback on any 
intermediate errors in the case of success.
As suggested on the OP, this may happen (in my order of preference):

1. with a new argument for the user to provide the list to collect the errors 
(changes the API backward-compatiblly);
2. with logging logs;
3. with warnings;
4. do nothing.

I prefer logging over warnings because they are more configurable.

[1] https://github.com/python/cpython/blob/master/Lib/socket.py#L724

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue29757] The loop in utility `socket.create_connection()` swallows previous errors

[Yury Selivanov]

Yury Selivanov added the comment:

This is a new feature, so we can only push it to 3.7.

--
nosy: +brett.cannon, haypo
versions:  -Python 2.7, Python 3.3, Python 3.4, Python 3.5, Python 3.6

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue29757] The loop in utility `socket.create_connection()` swallows previous errors

[Yury Selivanov]

Yury Selivanov added the comment:

> Case (c) sidesteps the above questions.

I like the (c) option. I don't think we should use logging/warnings here, as 
they can produce unwanted noise that few know how to silence.

When the list of errors is passed as a second argument to the exception, how is 
it rendered?  Would it make sense to concatenate all error messages:


  if errs:
  errors_msg = '  \n'.join(str(e) for e in errs)
  raise error(f"connection failed:\n{errors_msg}", errs)

Or maybe we should track which addr caused which exception?

  for res in getaddrinfo(host, port, 0, SOCK_STREAM):
  af, socktype, proto, canonname, sa = res
  try:
  ...
  except error as e:
  errs.append((canonname, e))
  ...

  if errs:
  error_msg = '  \n'.join(f'{addr}: {e}' for addr, e in errs)
  raise error('connection failed:\n{error_msg}', errs)

--
nosy: +yselivanov

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue29757] The loop in utility `socket.create_connection()` swallows previous errors

[Kostis Anagnostopoulos]

New submission from Kostis Anagnostopoulos:

## 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) sidesteps the above questions.

--
components: Library (Lib)
messages: 289238
nosy: ankostis
priority: normal
pull_requests: 463
severity: normal
status: open
title: The loop in utility `socket.create_connection()` swallows previous errors
versions: Python 2.7, Python 3.3, Python 3.4, Python 3.5, Python 3.6, Python 3.7

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com