BlobReadChannel read trows a non-IOException

[agrinh]

Couldn't find this discussed anywhere and thought it belonged here, I hope you agree, please let me know otherwise.

BlobReadChannel.read is annotated with @throws IOException which fits the interface and expected behaviour with nio (e.g. I get the expected IOException on read in the standard ReadableBytesChannel interface and in an input stream created with Channels.newInputStream)

However, at least one of the current exceptions thrown is StorageException, which is not an IOException, forcing users to know that this particular ReadbleBytesChannel throws another exception.

Could the exceptions thrown here be modified to inherit from IOException instead?

[frankyn]

Hi @agrinh,

Thanks for filing this issue. Looks like this has been an issue for a while now. I think it makes sense to update the exception type, but haven't investigated the implications of making such a change.

How often does this occur?

[dmitry-fa]

What about introducing new StorageIOException extending IOException?

[frankyn]

If it's a bug for the contract to return StorageException, then it should be updated to an IOException to meet expectation. It also avoids the need to support another Exception class.

Additionally, the method has throws IOException so it's a known checked exception for end-users.

[dmitry-fa]

The thrown StorageException used to contain the extra code, fortunately, in case of BlobReadChannel.read() this code is UNKNOWN_CODE, so yes we could safely replace StorageException with IOException without introducing a new exception class.

I'll take care of this.

[frankyn]

Thanks @dmitry-fa !

[agrinh]

Thanks, that sounds great!

Also, @frankyn sorry for not getting back to you sooner, looks like you don't need any further input now.

[frankyn]

Np! Thanks for raising the issue to us @agrinh <https://github.com/agrinh>! Frank Natividad | Developer Relations Engineer

…

On Thu, Apr 2, 2020 at 12:11 PM Agrin ***@***.***> wrote: Thanks, that sounds great! Also, @frankyn <https://github.com/frankyn> sorry for not getting back to you sooner, looks like you don't need any further input now. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#219 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAORXQCEZHYL5NPYVZXU2DDRKTPMXANCNFSM4LXYOIWQ> .