3 ms·

I'm a recent snafu (https://docs.rs/snafu/latest/snafu/ https://docs.rs/snafu/latest/snafu/) convert over thiserror (https://docs.rs/thiserror/latest/thiserror/

by mparis 1y ago

I'm a recent snafu (https://docs.rs/snafu/latest/snafu/ https://docs.rs/snafu/latest/snafu/) convert over thiserror (https://docs.rs/thiserror/latest/thiserror/ https://docs.rs/thiserror/latest/thiserror/). You pay the cost of adding `context` calls at error sites but it leads to great error propagation and enables multiple error variants that reference the same source error type which I always had issues with in `thiserror`. No dogma. If you want an error per module that seems like a good way to start, but for complex cases where you want to break an error down more, we'll often have an error type per function/struct/trait.

shepmaster 1y ago
Thanks for using SNAFU! Any feedback you'd like to share?
Expurple 1y ago
> multiple error variants that reference the same source error type which I always had issues with in `thiserror`. Huh? #[derive(Debug, thiserror::Error)] enum CustomError { #[error("failed to open a: {0}")] A(std::io::Error), #[error("failed to open b: {0}")] B(std::io::Error), } fn main() -> Result<(), CustomError> { std::fs::read_to_string("a").map_err(CustomError::A)?; std::fs::read_to_string("b").map_err(CustomError::B)?; Ok(()) } If I understand correctly, the main feature of snafu is "merely" reducing the boilerplace when adding context: low_level_result.context(ErrorWithContextSnafu { context })?; // vs low_level_result.map_err(|err| ErrorWithContext { err, context })?; But to me, the win seems to small to justify the added complexity.
- shepmaster 1y ago
  You certainly can use thiserror to accomplish the same goals! However, your example does a little subtle slight-of-hand that you probably didn't mean to and leaves off the enum name (or the `use` statement): low_level_result.context(ErrorWithContextSnafu { context })?; low_level_result.map_err(|err| CustomError::ErrorWithContext { err, context })?; Other small details: - You don't need to move the inner error yourself. - You don't need to use a closure, which saves a few characters. This is even true in cases where you have a reference and want to store the owned value in the error: #[derive(Debug, Snafu)] struct DemoError { source: std::io::Error, filename: PathBuf } let filename: &Path = todo!(); result.context(OpenFileSnafu { filename })?; // `context` will change `&Path` to `PathBuf` - You can choose to capture certain values implicitly, such as a source file location, a backtrace, or your own custom data (the current time, a global-ish request ID, etc.) ---- As an aside: #[error("failed to open a: {0}")] It is now discouraged to include the text of the inner error in the `Display` of the wrapping error. Including it leads to duplicated data when printing out chains of errors in a nicer / structured manner. SNAFU has a few types that work to undo this duplication, but it's better to avoid it in the first place.

by mparis 1y ago

shepmaster 1y ago

Thanks for using SNAFU! Any feedback you'd like to share?

Expurple 1y ago

> multiple error variants that reference the same source error type which I always had issues with in `thiserror`. Huh? #[derive(Debug, thiserror::Error)] enum CustomError { #[error("failed to open a: {0}")] A(std::io::Error), #[error("failed to open b: {0}")] B(std::io::Error), } fn main() -> Result<(), CustomError> { std::fs::read_to_string("a").map_err(CustomError::A)?; std::fs::read_to_string("b").map_err(CustomError::B)?; Ok(()) } If I understand correctly, the main feature of snafu is "merely" reducing the boilerplace when adding context: low_level_result.context(ErrorWithContextSnafu { context })?; // vs low_level_result.map_err(|err| ErrorWithContext { err, context })?; But to me, the win seems to small to justify the added complexity.

shepmaster 1y ago
You certainly can use thiserror to accomplish the same goals! However, your example does a little subtle slight-of-hand that you probably didn't mean to and leaves off the enum name (or the `use` statement): low_level_result.context(ErrorWithContextSnafu { context })?; low_level_result.map_err(|err| CustomError::ErrorWithContext { err, context })?; Other small details: - You don't need to move the inner error yourself. - You don't need to use a closure, which saves a few characters. This is even true in cases where you have a reference and want to store the owned value in the error: #[derive(Debug, Snafu)] struct DemoError { source: std::io::Error, filename: PathBuf } let filename: &Path = todo!(); result.context(OpenFileSnafu { filename })?; // `context` will change `&Path` to `PathBuf` - You can choose to capture certain values implicitly, such as a source file location, a backtrace, or your own custom data (the current time, a global-ish request ID, etc.) ---- As an aside: #[error("failed to open a: {0}")] It is now discouraged to include the text of the inner error in the `Display` of the wrapping error. Including it leads to duplicated data when printing out chains of errors in a nicer / structured manner. SNAFU has a few types that work to undo this duplication, but it's better to avoid it in the first place.