dirs-dev/directories-rs

Functions should return Result not Option

ijackson opened this issue · 1 comments

ijackson commented

We had a user report a bad error message from some Arti libraries on Android. The root cause of the problem was #83, but additionally, I felt that the error message from our library was too poor. (https://gitlab.torproject.org/tpo/core/arti/-/issues/ 989 999)

I investigated and found that the main reason the message was poor was because ProjectDirs::from simply returned None. So we had to make up our own, rather useless, error message.

IMO all the fallible functions should return Result. (This would be a semver-breaking change, obviously.)

sidju commented

In an attempt to rephrase this slightly more clearly (since I was confused and opposed until I understood):

Functions would better return Result<Option>, so that:

  • Err can represent when some part of the process to construct the path or identify if it is valid for the platform failed
  • Ok(None) can represent when the library successfully concluded there is no such path for the platform
  • Ok(Some(path)) can represent when a valid path for the platform was successfully constructed

This would allow easy error propagation with ? or .expect() while otherwise keeping user code intact, and would make failure causes more clear.

(Edit: Removed semi-related suggestion that was already implemented in the API.)

  • 👍1