Result and Option handling improvements
Discussion within
Merge Request 67
as well as goals stated for
Issue 114
and
Issue 165
all show a common theme regarding how Result::Error
, and Option::None
,
states are currently handled.
TLDR
Library crates need alternatives for methods such as; unwrap
, expect
,
context
, with_context
, and macros like panic
. With the end-goal of not
having any panic prone code within Tor libraries.
Pain Points
After some time with the source code I've identified two patterns that seem to be the root of most bits in need of improvement;
-
Code with more than one source of
Error
, eg.tor-dirclient
andtor-rtcompat
-
Functions that handle
Option
but returnResult
Possible Solutions
So far I've found two well written articles on these subjects;
TLDR
The #[error(transparent)]
option from thiserror
can be used to extend/wrap
existing error kinds, eg.
pub enum Error {
/// ...
#[error(transparent)]
IOError(#[from] std::io::Error),
}
Note, in some cases a similar pattern is already in use; check
tor-dirclient/src/err.rs
Where Option::None
and Result
coexist I currently believe adding None
to
Result
enums may be the least disruptive route, eg.
pub enum Error {
/// Allow `Option::None` to _bubble up_ within methods returning `Result`
None
}
// ...
impl Thing {
fn task(x: Option<T>) -> Result<T, Error> {
match x {
Some(v) => Ok(v),
None => Error::None,
}
}
}
... The above feels like a hack that'll eventually need to be refactored in the
future, such as stating which kind of Option
returned None
, but I also
think at present it'll allow for faster prototyping and greater flexibility.
Because these proposed changes will effect much of the code-base feedback is certainly welcome!
-
Can anyone think of issues that may be caused by converting
Option::None
toResult::None
? -
Are there better methods of handling multiple error types?