Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Throw exceptions on IFileStorage instead of returning false or null. #281

Open
frabe1579 opened this issue Nov 2, 2022 · 4 comments
Open

Comments

@frabe1579
Copy link

Almost all the methods in IFileStorage, in the different implementations, catches exception without analyzing errors and returns null or false.
This can lead to mis-interpretation of the result.
For example, takes the methods ExistsAsync and GetFileInfoAsync: they return false/null if the file does not exist, but also for any error (look at S3FileStorage). So if something goes wrong in the network, the main program can potentially have destructive bugs.
I think the best solution is to throw an exception if the result is not one of the expected results, and not returning false/null for any exception.
Also, SaveFileAsync, RenameFileAsync, CopyFileAsync, DeleteFileAsync and DeleteFilesAsync shoudn't have a boolean return value, but instead throw an exception if something goes wrong, to ensure that the main program is informed of the correct result of the operation.
Finally, GetFileStreamAsync should throw an exception instaead of returning null if an error occur, and maybe if the file does not exist.
What do you think about this?

@ejsmith
Copy link
Contributor

ejsmith commented Nov 2, 2022

@frabe1579 I think you are probably right. I don't want to go crazy with throwing exceptions, but you are right that network errors and services being down should not be going through and returning a result like everything was fine.

@niemyjski
Copy link
Member

@frabe1579 would you mind submitting a pr for this.

@frabe1579
Copy link
Author

Ok, I will try some solutions, and present them as PRs.

@U52284
Copy link

U52284 commented Aug 14, 2023

I just came about that one that GetFileStream returns null and this is a weird one for me. I expected it to create a new file I can write data in since a stream is both way and the name does not indicate its only a reading stream or similar.

I get that there is a SaveFileAsync but now I need to setup a memory stream, write everything in that one and in the end I can write it to the file.
This is very unpractical for writing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants