SFTPStorage: keep trying to connect when timeout (in a loop)

[SorianoMarmol]

I was implementing saving a file in SFTP and, when testing time out, it was never launched.
I have debugged and it goes into a loop trying to connect.

Specifically:

• https://github.com/jschneier/django-storages/blob/master/storages/backends/sftpstorage.py#L67

        try:
            self._ssh.connect(self._host, **self._params)
        except paramiko.AuthenticationException as e:

the exception thrown is socket "timeout" (from socket import timeout)

I may not have done something correctly, but I have done this workaround (maybe it helps someone, or to illustrate the problem)

from socket import timeout
from storages.backends.sftpstorage import SFTPStorage

class SFTPStorageWithException(SFTPStorage):

    @property
    def sftp(self):
        try:
            return super().sftp
        except timeout as exc:
            log_message(f'Timeout connecting to SFTP: {exc}', 'error')
            raise paramiko.AuthenticationException(exc)

thanks

[amolinaalvarez]

I came across the same issue and was able to work around it by using @SorianoMarmol's approach.

[jschneier]

Sorry, I don't understand. What do you mean by so it returns "None"?

[SorianoMarmol]

Sorry, I don't understand. What do you mean by so it returns "None"?

i mean, instead of raise an exception (timeout or paramiko.AuthenticationException) _connect are returning none

I think the expected behavior should be to throw the exception (since it can't connect due to timeout). In my workaround I throw the exception to handle it. Without it, it tries to always connect, in a loop, when saving the file

maybe it can be easily replicated with very little timeout

thanks

[jschneier]

Is the problem that socket.timeout is a subclass of OSError and delete and exists catch them? In general, socket.timeout is raised out of _connect so I'm not sure how you see it is returning None.

Assuming I have access to an SFTP server and can set the timeout very low, can you give me example code?

[SorianoMarmol]

eliminated the part of "None" since it could have been a confusion and with the example the problem is illustrated

Is the problem that socket.timeout is a subclass of OSError and delete and exists catch them?

could be , please, check next comment

Assuming I have access to an SFTP server and can set the timeout very low, can you give me example code?

is a field of type filefield with storage and upload_to methods, so, for example

def _get_storage(*args, **kwargs):
        from storages.backends.sftpstorage import SFTPStorage
        return SFTPStorage()

def _get_dir_path(instance, filename):
    if edr_sftp_enabled():
        return f'{filename}'

sftp_file = models.FileField(
        blank=True, null=True,
        max_length=250,
        storage=_get_storage,
        upload_to=_get_dir_path)

thanks for your answer, I can try some more debugging or try whatever if needed

[SorianoMarmol]

A bit of debugging ( all calls have the "exists" part in common )

first .sftp call

  /home/rsoriano/.local/lib/python3.7/site-packages/django/db/models/fields/files.py(302)pre_save()
    301             # Commit the file to storage prior to saving the model
--> 302             file.save(file.name, file.file, save=False)
    303         return file

  /home/rsoriano/.local/lib/python3.7/site-packages/django/db/models/fields/files.py(89)save()
     88         name = self.field.generate_filename(self.instance, name)
---> 89         self.name = self.storage.save(name, content, max_length=self.field.max_length)
     90         setattr(self.instance, self.field.attname, self.name)

  /home/rsoriano/.local/lib/python3.7/site-packages/django/core/files/storage.py(53)save()
     52 
---> 53         name = self.get_available_name(name, max_length=max_length)
     54         return self._save(name, content)

  /home/rsoriano/.local/lib/python3.7/site-packages/django/core/files/storage.py(87)get_available_name()
     86         # exceed the max_length.
---> 87         while self.exists(name) or (max_length and len(name) > max_length):
     88             # file_ext includes the dot.

  /home/rsoriano/.local/lib/python3.7/site-packages/storages/backends/sftpstorage.py(150)exists()
    149         try:
--> 150             self.sftp.stat(self._remote_path(name))
    151             return True

second

  /home/rsoriano/.local/lib/python3.7/site-packages/django/db/models/fields/files.py(302)pre_save()
    301             # Commit the file to storage prior to saving the model
--> 302             file.save(file.name, file.file, save=False)
    303         return file

  /home/rsoriano/.local/lib/python3.7/site-packages/django/db/models/fields/files.py(89)save()
     88         name = self.field.generate_filename(self.instance, name)
---> 89         self.name = self.storage.save(name, content, max_length=self.field.max_length)
     90         setattr(self.instance, self.field.attname, self.name)

  /home/rsoriano/.local/lib/python3.7/site-packages/django/core/files/storage.py(54)save()
     53         name = self.get_available_name(name, max_length=max_length)
---> 54         return self._save(name, content)
     55 

  /home/rsoriano/.local/lib/python3.7/site-packages/storages/backends/sftpstorage.py(128)_save()
    127         dirname = posixpath.dirname(path)
--> 128         if not self.exists(dirname):
    129             self._mkdir(dirname)

  /home/rsoriano/.local/lib/python3.7/site-packages/storages/backends/sftpstorage.py(150)exists()
    149         try:
--> 150             self.sftp.stat(self._remote_path(name))
    151             return True

third

  /home/rsoriano/.local/lib/python3.7/site-packages/django/db/models/fields/files.py(302)pre_save()
    301             # Commit the file to storage prior to saving the model
--> 302             file.save(file.name, file.file, save=False)
    303         return file

  /home/rsoriano/.local/lib/python3.7/site-packages/django/db/models/fields/files.py(89)save()
     88         name = self.field.generate_filename(self.instance, name)
---> 89         self.name = self.storage.save(name, content, max_length=self.field.max_length)
     90         setattr(self.instance, self.field.attname, self.name)

  /home/rsoriano/.local/lib/python3.7/site-packages/django/core/files/storage.py(54)save()
     53         name = self.get_available_name(name, max_length=max_length)
---> 54         return self._save(name, content)
     55 

  /home/rsoriano/.local/lib/python3.7/site-packages/storages/backends/sftpstorage.py(129)_save()
    128         if not self.exists(dirname):
--> 129             self._mkdir(dirname)
    130 

  /home/rsoriano/.local/lib/python3.7/site-packages/storages/backends/sftpstorage.py(113)_mkdir()
    112         parent = posixpath.dirname(path)
--> 113         if not self.exists(parent):
    114             self._mkdir(parent)

  /home/rsoriano/.local/lib/python3.7/site-packages/storages/backends/sftpstorage.py(150)exists()
    149         try:
--> 150             self.sftp.stat(self._remote_path(name))
    151             return True

q

  File "/home/rsoriano/.local/lib/python3.7/site-packages/django/db/models/sql/compiler.py", line 1310, in pre_save_val
    return field.pre_save(obj, add=True)
  File "/home/rsoriano/.local/lib/python3.7/site-packages/django/db/models/fields/files.py", line 302, in pre_save
    file.save(file.name, file.file, save=False)
  File "/home/rsoriano/.local/lib/python3.7/site-packages/django/db/models/fields/files.py", line 89, in save
    self.name = self.storage.save(name, content, max_length=self.field.max_length)
  File "/home/rsoriano/.local/lib/python3.7/site-packages/django/core/files/storage.py", line 54, in save
    return self._save(name, content)
  File "/home/rsoriano/.local/lib/python3.7/site-packages/storages/backends/sftpstorage.py", line 129, in _save
    self._mkdir(dirname)
  File "/home/rsoriano/.local/lib/python3.7/site-packages/storages/backends/sftpstorage.py", line 114, in _mkdir
    self._mkdir(parent)
  File "/home/rsoriano/.local/lib/python3.7/site-packages/storages/backends/sftpstorage.py", line 114, in _mkdir
    self._mkdir(parent)
  File "/home/rsoriano/.local/lib/python3.7/site-packages/storages/backends/sftpstorage.py", line 113, in _mkdir
    if not self.exists(parent):
  File "/home/rsoriano/.local/lib/python3.7/site-packages/storages/backends/sftpstorage.py", line 150, in exists
    self.sftp.stat(self._remote_path(name))

[jschneier]

@SorianoMarmol can you try with this patch

diff --git a/storages/backends/sftpstorage.py b/storages/backends/sftpstorage.py
index 529daf2..643685c 100644
--- a/storages/backends/sftpstorage.py
+++ b/storages/backends/sftpstorage.py
@@ -150,7 +150,7 @@ class SFTPStorage(BaseStorage):
         try:
             self.sftp.stat(self._remote_path(name))
             return True
-        except OSError:
+        except FileNotFoundError:
             return False
 
     def _isdir_attr(self, item):

[jschneier]

@SorianoMarmol can you also confirm that .exists() works correctly for a non-existent file in that case (or tell me what error is returned).

[jschneier]

The patch does work, I will merge the fix shortly.

[SorianoMarmol]

thanks a lot!