lucab / libsystemd-rs

A pure-Rust client library to work with systemd
https://crates.io/crates/libsystemd
Other
105 stars 19 forks source link

Use anyhow::Result as error type? #127

Closed dfreese closed 1 year ago

dfreese commented 1 year ago

It looks like much of what happens with the SdError type is adding context to an underlying error. I've used anyhow, and found it super useful for this sort of pattern; particularly using the Context trait.

Is this something you would be open to? I can open up an example PR if it would make it more concrete.

lucab commented 1 year ago

Thanks for the report. I agree that the way we are currently using thiserror and how we add context details in intermediate layers resembles anyhow a lot. However this is a library crate so thiserror is still a better fit compared to anyhow: https://github.com/dtolnay/thiserror#comparison-to-anyhow. Specifically, I do make use of the features listed at https://github.com/dtolnay/thiserror#details, in the sense that thiserror details do not appear in libsystemd API.

If you are annoyed (and rightly so!) by all the map_err() + format!() sprinkled through the codebase, I think a better course of action is to add a pub(crate) fn with_context() to SdError. That way adding intermediate context become less verbose, and we get most of the benefits of anyhow while still properly using thiserror.

dfreese commented 1 year ago

Absolutely makes sense. Wanted to check before looking into changing the handling of errors, since anyhow would be the easiest way of doing that. I'll update #128 to use a custom Context for SdError.