Opened 7 years ago

Last modified 6 years ago

#3228 enhancement new

Conch SFTP should translate errors in ISFTPServer implementation, not in SFTP protocol

Reported by: jml Owned by:
Priority: normal Milestone:
Component: conch Keywords:
Cc: Branch:
Author: Launchpad Bug:

Description

Currently, Conch has a great deal of error translation code in _ebStatus on FileTransferServer. Much of this code translates OS errors into SFTP error messages. This is kind of OK, but it mixes concerns. The ISFTPServer implementation is the thing that knows about the filesystem.

Thus,

  • the ISFTPServer implementation should be responsible for translating errors from IOError, OSError etc.
  • FileTransferServer should expect only SFTPErrors to be raised by ISFTPServer (along with good handling for unexpected errors).
  • The unix SFTP implementation should do its own error translation.
  • (if we care), The ISFTPServer adapter for twisted.vfs should translate its errors to SFTPError not IOError.

Change History (4)

comment:1 Changed 7 years ago by jml

Here's the approach I was thinking of taking:

  • Make the UNIX SFTP implementation raise SFTPErrors. Add unittests while we're at it.
  • Split up the error handling in FileTransferServer so that the SFTPError handling is in a separate errback to the OSError / IOError handling.
  • Place deprecation warnings in the OSError errback.

The first step could probably be its own branch. Any objections?

comment:2 Changed 7 years ago by exarkun

A step that's missing is documenting the error behavior of these methods. ISFTPServer currently documents no errors for some methods, documents other methods as having "implementation-dependent exceptions", and explicitly document EOFError (as opposed to SFTPError) in another case.

Clarifying this would be a big help. The documentation should probably include the information that these methods were allowed to raise some random unknown exception previously, so anyone calling them will know that if they're lucky they'll get SFTPError, but that they should be prepared to handle anything else, too.

comment:3 Changed 6 years ago by jml

Yes. Good call.

Really itching to get to this ticket.

comment:4 Changed 4 years ago by <automation>

  • Owner z3p deleted
Note: See TracTickets for help on using tickets.