Returning boolean/primitive/other status instead of SFTPException for a failing command ?
- 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.