Fix: handle timeout error in client

[gquittet]

Sometimes, we can't connect to a specific device because of a timeout error.

You can see the stacktrace below:

Traceback (most recent call last):
  File "", line 51, in <module>
    main()
  File "", line 34, in main
    client.connect(
  File "venv/lib/site-packages/paramiko/client.py", line 349, in connect
    retry_on_signal(lambda: sock.connect(addr))
  File "venv/lib/site-packages/paramiko/util.py", line 283, in retry_on_signal
    return function()
  File "venv/lib/site-packages/paramiko/client.py", line 349, in <lambda>
    retry_on_signal(lambda: sock.connect(addr))
TimeoutError: A connection attempt failed because the connected party did not properly respond after a period of time, or established connection failed because connected host has failed to respond

Simple code to reproduce the issue:

with SSHClient() as client:
    client.set_missing_host_key_policy(paramiko.AutoAddPolicy())
    client.connect(
        hostname=ip,
        username=username,
        password=password,
        port=port,
    )
    stdin, stdout, stderr = client.exec_command(command)
    output = stdout.read().decode("ascii").strip("\n")
    return output

Related to: #279

[gquittet]

Hello @bitprophet 👋

Can you check this PR?

Thanks a lot ❤

[bskinn]

Hi, @gquittet -- thanks for putting this together! A couple of questions:

• Is this meant to be a fix for Connect timeout without effect on windows #279?
• Are you running into this problem yourself?
• Can you explain in more detail why handling for a TimeoutError should be identical to the handling for socket.error?

[bskinn]

@jun66j5 If I read #279 (comment) correctly, TimeoutError is the error raised instead of socket.error for Python 3.3+ -- and thus once bitprophet removes Python 2 and < 3.6, this try-except can just be changed to an except TimeoutError:?

[gquittet]

Hello @bskinn 👋

• Yes it is a fix for the issue I linked
• I'm running this problem myself so, I decided to fix it and send a PR
• I handle the TimeoutError in the same try/except because the actual code in the except allow me to do that.

Have a nice day/night 😉

[jun66j5]

f I read #279 (comment) correctly, TimeoutError is the error raised instead of socket.error for Python 3.3+ -- and thus once bitprophet removes Python 2 and < 3.6, this try-except can just be changed to an except TimeoutError:?

I don't consider that it is needed to add/change from socket.error in order to catch TimeoutError. Because socket.error is an alias of OSError and TimeoutError is a subclass of OSError in Python 3.3+. See https://docs.python.org/3/library/socket.html#socket.error

Actually, catch socket.error is able to catch TimeoutError in Python 3. Also, in Python 2, socket.error with ETIMEDOUT (linux 110, windows 10060) is raised and catch as well.

$ python3.6
Python 3.6.15 (default, Apr 25 2022, 01:55:53)
[GCC 9.4.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import socket
>>> s = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
>>> s.setsockopt(socket.IPPROTO_TCP, socket.TCP_USER_TIMEOUT, 1000)  # 1,000 ms
>>> try:
...   s.connect(('192.0.2.1', 22))
... except socket.error as e:
...   print('socket.error: %s %s' % (type(e), e))
...
socket.error: <class 'TimeoutError'> [Errno 110] Connection timed out

$ python2.7
Python 2.7.18 (default, Mar  8 2021, 13:02:45)
[GCC 9.3.0] on linux2
Type "help", "copyright", "credits" or "license" for more information.
>>> import socket
>>> s = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
>>> TCP_USER_TIMEOUT = 18
>>> s.setsockopt(socket.IPPROTO_TCP, TCP_USER_TIMEOUT, 1000)  # 1,000 ms
>>> try:
...   s.connect(('192.0.2.1', 22))
... except socket.error as e:
...   print('socket.error: %s %s' % (type(e), e))
...
socket.error: <class 'socket.error'> [Errno 110] Connection timed out
>>> import errno
>>> errno.ETIMEDOUT
110

[jun66j5]

SSHClient tries to connect to next address when the raised socket.error is ECONNREFUSED or EHOSTUNREACH. I consider we could add ETIMEDOUT to the errno list in order to fix the connection timed out.

diff --git a/paramiko/client.py b/paramiko/client.py
index 92feaa1fb..372fbeb21 100644
--- a/paramiko/client.py
+++ b/paramiko/client.py
@@ -26,7 +26,7 @@ import inspect
 import os
 import socket
 import warnings
-from errno import ECONNREFUSED, EHOSTUNREACH
+from errno import ECONNREFUSED, EHOSTUNREACH, ETIMEDOUT

 from paramiko.agent import Agent
 from paramiko.common import DEBUG
@@ -356,7 +356,7 @@ class SSHClient(ClosingContextManager):
                         sock.close()
                     # Raise anything that isn't a straight up connection error
                     # (such as a resolution error)
-                    if e.errno not in (ECONNREFUSED, EHOSTUNREACH):
+                    if e.errno not in (ECONNREFUSED, EHOSTUNREACH, ETIMEDOUT):
                         raise
                     # Capture anything else so we know how the run looks once
                     # iteration is complete. Retain info about which attempt

After the changes:

>>> import paramiko
>>> client = paramiko.SSHClient()
>>> client.connect('192.0.2.1', timeout=3600)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/home/jun66j5/src/paramiko/paramiko/client.py", line 372, in connect
    raise NoValidConnectionsError(errors)
paramiko.ssh_exception.NoValidConnectionsError: [Errno None] Unable to connect to port 22 on 192.0.2.1

[gquittet]

@jun66j5 @bskinn I just made the change 😊

[gquittet]

I also sort the import but I can drop the commit if you don't like ☺️

[jun66j5]

I noticed socket.connect() raises socket.timeout without errno if timeout parameter is shorter than TCP timeout. The socket.timeout is a subclass of socket.error on both Python 2 and 3.

>>> client.connect('192.0.2.1', timeout=1)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/home/jun66j5/src/paramiko/paramiko/client.py", line 349, in connect
    retry_on_signal(lambda: sock.connect(addr))
  File "/home/jun66j5/src/paramiko/paramiko/util.py", line 279, in retry_on_signal
    return function()
  File "/home/jun66j5/src/paramiko/paramiko/client.py", line 349, in <lambda>
    retry_on_signal(lambda: sock.connect(addr))
socket.timeout: timed out

We should check whether the exception is a socket.timeout before accessing errno attribute, and add unit tests if possible.

@@ -356,7 +356,11 @@ class SSHClient(ClosingContextManager):
                         sock.close()
                     # Raise anything that isn't a straight up connection error
                     # (such as a resolution error)
-                    if e.errno not in (ECONNREFUSED, EHOSTUNREACH):
+                    if not isinstance(e, socket.timeout) and e.errno not in (
+                        ECONNREFUSED,
+                        EHOSTUNREACH,
+                        ETIMEDOUT,
+                    ):
                         raise
                     # Capture anything else so we know how the run looks once
                     # iteration is complete. Retain info about which attempt