hierynomus / hierynomus/sshj

Returning boolean/primitive/other status instead of SFTPException for a failing command ?

Open
#91 2 comments 0 reactions 0 assignees View on GitHub
improvement
Dominant language
Java
Stars
2.7k
Forks
620
Avg merge
3d 23h
Merged PRs (30d)
11

Description

Hello there,

Here is an API improvement that I'd like to suggest. My focus will be on the SFTP part of sshj as it is the code part that I know the best right now.

When we use the sshj SFTP client (stateless or stateful doesn't matter), every time a command fails, an SFTPException is raised. I believe this is wrong as it doesn't enable us to distinguish the two categories of failures in this case:
1. Regular failure because of human-nature (e.g. cd() into a non existent directory, writing in a directory where writing access are not granted, and so one). This kind of failure happens all the time (after all, we're just humans right ? ;-) ) and are not exceptional. Recovery is often possible and the "SFTP flow" is not broken (i.e. we can technically issue other commands after it).
2. Exceptional failures which kind of say that it's unlikely we're gonna be able to recover from them (e.g. Connection lost).

While raising an exception is well suited for the latter case, I believe it is really wrong to raise them in the former case. Apart from semantics, there is also a performance issue here: creating, throwing, catching and processing an exception is really costly and should be done, as stated, in exceptional circumstances.

Anyway, there is an other good reason about the whole story I think:
1. The API currently states that IOException are being thrown, which SFTPException actually extends. From a pure interface point of view, if we don't look at the code, there is no way to get that SFTPException / Other IOException granularity. So, in a sense, we're already blind without realizing it.

Given the current API, I realize that it might break some things but even if it's a long shot, I'm convinced that it's worth it.

Best Regards,

Pierre Lepropre.

Contributor guide

No contributing guide indexed for this repository

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.