PR#1203: Fix `is_conn_error()` for Python 3.3+ change to `socket.error` -

koji

#1203 Fix `is_conn_error()` for Python 3.3+ change to `socket.error`

Merged Jan 15, 2019 by mikem. Opened Jan 8, 2019 by adamwill.

adamwill/koji py3-conn-error into master

adamwill commented Jan 8, 2019

In Python 3.3+, socket.error is no longer a distinct exception.
It is - as the docs say - "A deprecated alias of OSError". This
means that this check:

isinstance(e, socket.error)

is effectively equivalent to:

isinstance(e, OSError)

This is a problem, because requests.exceptions.ConnectionError
(another exception type we handle later in is_conn_error()) is
a subclass of OSError - so on Python 3 we never actually reach
the block that's intended to handle that exception type. We hit
the isinstance(e, socket.error) block at the start instead, and
of course the exception doesn't have an errno attribute, so we
just return False at that point.

There are a few different ways we could fix this; this commit
does it by ditching the isinstance checks, and dropping the
shortcut return False bailout from the early block. We'll still
ultimately return False unless the error is actually one of the
other types we handle; it just costs a couple more isinstance
checks.

I don't think replacing the isinstance socket.error checks is
really necessary at all. We can just check for an errno attr,
and if there is one and it's one of the values we check for...
that seems safe enough to treat as a connection error.

This also changes the second check to be a check of e2, not
e - I'm fairly sure this is what's actually intended, and
the current code was a copy-paste error.

Signed-off-by: Adam Williamson awilliam@redhat.com

adamwill commented Jan 8, 2019

Note, it's pleasantly simple to demonstrate the fundamental problem here for yourself:

[adamw@adam koji (master)]$ python3
Python 3.7.2 (default, Jan  2 2019, 11:32:51) 
[GCC 8.2.1 20181215 (Red Hat 8.2.1-6)] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import requests.exceptions
>>> import socket
>>> err = requests.exceptions.ConnectionError()
>>> isinstance(err, socket.error)
True
>>> 
[adamw@adam koji (master)]$ python2
Python 2.7.15 (default, Sep 21 2018, 17:53:38) 
[GCC 8.2.1 20180905 (Red Hat 8.2.1-3)] on linux2
Type "help", "copyright", "credits" or "license" for more information.
>>> import requests.exceptions
>>> import socket
>>> err = requests.exceptions.ConnectionError()
>>> isinstance(err, socket.error)
False
>>>