From 882ce8c8ce74d393b6d95dcede6e69a1a2e86c9b Mon Sep 17 00:00:00 2001 From: Ian Jackson Date: Tue, 18 Jul 2023 11:09:54 +0100 Subject: [PATCH] retry-error: Provide fmt_error_with_sources in retry-error This code came from tor-error. So now tor-error depends on retry-error. --- Cargo.lock | 2 ++ crates/retry-error/Cargo.toml | 1 + crates/retry-error/semver.md | 1 + crates/retry-error/src/lib.rs | 62 +++++++++++++++++++++++++++++++++- crates/tor-error/Cargo.toml | 1 + crates/tor-error/src/report.rs | 19 ++--------- 6 files changed, 69 insertions(+), 17 deletions(-) create mode 100644 crates/retry-error/semver.md diff --git a/Cargo.lock b/Cargo.lock index 7fbc67cb8..0eb301f8d 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3065,6 +3065,7 @@ name = "retry-error" version = "0.4.2" dependencies = [ "anyhow", + "thiserror", ] [[package]] @@ -4421,6 +4422,7 @@ dependencies = [ "futures", "once_cell", "paste", + "retry-error", "static_assertions", "strum", "thiserror", diff --git a/crates/retry-error/Cargo.toml b/crates/retry-error/Cargo.toml index 8d1bb3f8b..1358915d9 100644 --- a/crates/retry-error/Cargo.toml +++ b/crates/retry-error/Cargo.toml @@ -14,6 +14,7 @@ repository = "https://gitlab.torproject.org/tpo/core/arti.git/" [dev-dependencies] anyhow = "1.0.23" +thiserror = "1" [dependencies] diff --git a/crates/retry-error/semver.md b/crates/retry-error/semver.md new file mode 100644 index 000000000..1d9d65046 --- /dev/null +++ b/crates/retry-error/semver.md @@ -0,0 +1 @@ +ADDED: fn fmt_error_with_sources diff --git a/crates/retry-error/src/lib.rs b/crates/retry-error/src/lib.rs index 3605361ae..a9fb35f5b 100644 --- a/crates/retry-error/src/lib.rs +++ b/crates/retry-error/src/lib.rs @@ -41,7 +41,8 @@ //! use std::error::Error; -use std::fmt::{Debug, Display, Error as FmtError, Formatter}; +use std::fmt::{self, Debug, Display, Error as FmtError, Formatter}; +use std::iter; /// An error type for use when we're going to do something a few times, /// and they might all fail. @@ -227,6 +228,65 @@ impl Display for RetryError { } } +/// Helper: formats a [`std::error::Error`] and its sources (as `"error: source"`) +/// +/// Avoids duplication in messages by not printing messages which are +/// wholly-contained (textually) within already-printed messages. +/// +/// Offered as a `fmt` function: +/// this is for use in more-convenient higher-level error handling functionality, +/// rather than directly in application/functional code. +/// +/// This is used by `RetryError`'s impl of `Display`, +/// but will be useful for other error-handling situations. +/// +/// # Example +/// +/// ``` +/// use std::fmt::{self, Display}; +/// +/// #[derive(Debug, thiserror::Error)] +/// #[error("some pernickety problem")] +/// struct Pernickety; +/// +/// #[derive(Debug, thiserror::Error)] +/// enum ApplicationError { +/// #[error("everything is terrible")] +/// Terrible(#[source] Pernickety), +/// } +/// +/// struct Wrapper(Box); +/// impl Display for Wrapper { +/// fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { +/// retry_error::fmt_error_with_sources(&*self.0, f) +/// } +/// } +/// +/// let bad = Pernickety; +/// let err = ApplicationError::Terrible(bad); +/// +/// let printed = Wrapper(err.into()).to_string(); +/// assert_eq!(printed, "everything is terrible: some pernickety problem"); +/// ``` +pub fn fmt_error_with_sources(mut e: &dyn Error, f: &mut fmt::Formatter) -> fmt::Result { + let mut last = String::new(); + let mut sep = iter::once("").chain(iter::repeat(": ")); + loop { + let this = e.to_string(); + if !last.contains(&this) { + write!(f, "{}{}", sep.next().expect("repeat ended"), &this)?; + } + last = this; + + if let Some(ne) = e.source() { + e = ne; + } else { + break; + } + } + Ok(()) +} + #[cfg(test)] mod test { // @@ begin test lint list maintained by maint/add_warning @@ diff --git a/crates/tor-error/Cargo.toml b/crates/tor-error/Cargo.toml index 09387dd45..dea7f7026 100644 --- a/crates/tor-error/Cargo.toml +++ b/crates/tor-error/Cargo.toml @@ -30,6 +30,7 @@ derive_more = "0.99.3" futures = "0.3" once_cell = "1" paste = "1" +retry-error = { path = "../retry-error", version = "0.4.2" } # WRONG should be 0.4.3 static_assertions = { version = "1", optional = true } strum = { version = "0.25", features = ["derive"] } thiserror = "1" diff --git a/crates/tor-error/src/report.rs b/crates/tor-error/src/report.rs index 93f0a30b7..68c094fe4 100644 --- a/crates/tor-error/src/report.rs +++ b/crates/tor-error/src/report.rs @@ -16,22 +16,9 @@ where { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { /// Non-generic inner function avoids code bloat - fn inner(mut e: &dyn StdError, f: &mut fmt::Formatter) -> fmt::Result { - write!(f, "error")?; - let mut last = String::new(); - loop { - let this = e.to_string(); - if !last.contains(&this) { - write!(f, ": {}", &this)?; - } - last = this; - - if let Some(ne) = e.source() { - e = ne; - } else { - break; - } - } + fn inner(e: &dyn StdError, f: &mut fmt::Formatter) -> fmt::Result { + write!(f, "error: ")?; + retry_error::fmt_error_with_sources(e, f)?; Ok(()) }