Backwards-incompatible change in 1.15.5+: NoValidConnectionsError

[nchammas]

When trying to connect to an EC2 instance that is still coming up, client.connect() used to raise socket.timeout or socket.error. At least, that was the case up until 1.15.4.

I just upgraded to 2.0.1, and now the same client.connect() call raises NoValidConnectionsError. Looking at the change log, it appears that this change was introduced in 1.15.5 and cherry-picked into 1.16.1.

I believe this is a backwards-incompatible change. Was it intentional?

[bitprophet]

@nchammas Not quite, that changelog entry was re: pickling instances of that exception, it looks like it was added in #22, according to git log -GNoValid.

NoValidConnectionsError is a subclass of socket.error, however, so it should still be passing isinstance tests - that's why it was not listed as being backwards incompatible, IIRC.

Can you clarify the exact issue you're having? Maybe NoValidConnectionsError being raised is a red herring?

[nchammas]

Sorry about the late reply @bitprophet. Here's some more detail on what I'm seeing.

I have this block of code which I use to try connecting to an EC2 instance as it comes up.

    while tries > 0:
        try:
            tries -= 1
            client.connect(
                username=user,
                hostname=host,
                key_filename=identity_file,
                look_for_keys=False,
                timeout=3)
            if print_status:
                print("[{h}] SSH online.".format(h=host))
            break
        except socket.timeout as e:
            time.sleep(5)
        except socket.error as e:
            if e.errno != errno.ECONNREFUSED:
                raise
            time.sleep(5)

On Paramiko 1.15.4, this works fine. On Paramiko 2.0.1, however, I get this:

[snipped]
  File ".../flintrock/flintrock/core.py", line 452, in provision_node
    wait=True)
  File ".../flintrock/flintrock/ssh.py", line 71, in get_ssh_client
    timeout=3)
  File ".../flintrock/venv/lib/python3.5/site-packages/paramiko/client.py", line 324, in connect
    raise NoValidConnectionsError(errors)
paramiko.ssh_exception.NoValidConnectionsError: [Errno None] Unable to connect to port 22 on 54.172.255.27

It doesn't look like from the traceback that I am actually catching the socket.error, but even if I was I would immediately re-raise the exception because errno has a different value. Whereas in 1.15.4 the errno was set to ECONNREFUSED, in 2.0.1 it is set to None.

Perhaps this is a bug?

[bitprophet]

Gotcha, that lines up with what I was guessing, kinda. The issue here is that the change breaking your code went from "we don't touch socket errors" to "we have to hide/massage socket errors because we're trying >1 possible address (IPv4 and IPv6 both)".

So we went from raising(/not capturing/at least not altering) a single real socket.error to raising an aggregate exception which still subclasses socket.error.

See http://docs.paramiko.org/en/2.0/api/ssh_exception.html#paramiko.ssh_exception.NoValidConnectionsError for the API details of the aggregate exception - you should be able to alter your if errno test to deal with the aggregate exception. Slapdash example of something you could try that IMO would have the same meaning as your current code:

except socket.error as e:
    if any(x.errno != errno.ECONNREFUSED for x in e.errors.values()):
        raise
    time.sleep(5)

(Only altered the 2nd line there...)

Alternately, you could expand that a bit so you preserve a reference to the 'problematic' sub-exception, then raise that specifically. If your outer code cares deeply about handling a single socket.error w/ a real errno etc, it would not have to change any then.

Either way, now that you've got the API doc for the new exception, I assume you can figure out the best/most correct change to make on your end. Going to close this as "nothing for us to really do" but obviously feel free to continue the discussion. thanks!

[nchammas]

So I guess the answer to my original question is "yes, there is a backwards-incompatible change" but thankfully it appears to be very easy to fix. No biggie, just wanted to make sure it was all by design!