smbj
smbj copied to clipboard
Unchecked exceptions
I'd like to make the suggestion that unchecked exception use be eliminated within this project. My current strategy has been to wrap every call that interfaces with this library in a RuntimeException-catching try block, but this is far from an ideal solution.
I'm also seeing cases where RuntimeExceptions can be thrown in unexpected places, such as the FileInputStream.read() implementation. I'm having to create wrapper classes to deal with these. In my opinion anything going wrong in an InputStream's read() method should be dealt with using an IOException as the Java API specifies.
Also seeing background threads (which are managed entirely within the library) stop the JVM due to unchecked exceptions.
In my project, I absolutely cannot have an unhandled exception.
Agreed, I will revisit the exception handling soon.
@hierynomus What is your current progress on this issue? I was going to take a stab at it but don't want to duplicate any efforts you have already made.
I've made a start with it. The current idea:
-
Buffer.BufferException extends RuntimeException
, as that should not be thrown unless we get malformed/malicious packets -
SMBRuntimeException -> SMBException
and no longer aRuntimeException
-
SMBApiException extends SMBException
- async threads catch all exceptions and propagate them nicely (for instance the
AsyncPacketReader
I'd like to add on to this. There's at least one chicken-and-egg problem caused by what seems like excessive exception-throwing and I think it should be addressed. If DiskShare.folderExists()
is called on a file or DiskShare.fileExists()
is called on a folder, a runtime exception is thrown, so you need to know ahead of time whether you're looking at a file or a folder. However, you get that information from DiskShare.getFileInformation()
, which also throws a runtime exception if you point it at a path where nothing exists. You can't find out if an object exists without knowing its type, and you can't find out its type without knowing it exists (at least, not without some ugly exception handling). The root cause of this interaction may be the way SMB was designed, but it would be nice if the API could abstract this limitation out somehow--perhaps add a public DiskShare.exists()
method which is agnostic to the object type as far as the caller is concerned?
The DiskShare.fileExists()
and DiskShare.folderExists()` methods have been fixed in the current master already.
Ah, I see #235 now; I was only looking at open issues before. Do you have an estimate for when that change will find its way into a release on Maven? Thanks.
Release 0.5.1 has just been released to BinTray, so should be appearing in maven central soon.
Do you still work on it ? Should I catch SMBApiException in my code ? I use AWS FSX. Sometimes the password expires and so my program throws an SMBApiException. How to properly deal with that situation ?