From 1ca56f91e474ad477b3c0b6808ee5b5d4150f64f Mon Sep 17 00:00:00 2001 From: Frando Date: Tue, 3 Jun 2025 14:18:34 +0200 Subject: [PATCH 1/5] feat: print sources on alternate display --- src/error.rs | 36 +++++++++++++++++++++++++++++++----- 1 file changed, 31 insertions(+), 5 deletions(-) diff --git a/src/error.rs b/src/error.rs index 83ec6bd..ca43910 100644 --- a/src/error.rs +++ b/src/error.rs @@ -470,23 +470,47 @@ impl snafu::ErrorCompat for Error { } } +fn write_sources_if_alternate( + f: &mut core::fmt::Formatter, + mut source: Option<&dyn std::error::Error>, +) -> core::fmt::Result { + if !f.alternate() { + return Ok(()); + } + while let Some(inner) = source { + write!(f, ": {}", inner)?; + source = inner.source(); + } + Ok(()) +} + impl core::fmt::Display for Error { fn fmt(&self, f: &mut core::fmt::Formatter) -> core::fmt::Result { match self { Self::Source { source, .. } => { - write!(f, "{}", source) + write!(f, "{}", source)?; + write_sources_if_alternate(f, source.source())?; + Ok(()) } Self::Whatever { message, source, .. } => match (source, message) { (Some(source), Some(message)) => { - write!(f, "{}: {}", message, source) + if f.alternate() { + write!(f, "{}: {:#}", message, source) + } else { + write!(f, "{}: {}", message, source) + } } (None, Some(message)) => { write!(f, "{}", message) } (Some(source), None) => { - write!(f, "{}", source) + if f.alternate() { + write!(f, "{:#}", source) + } else { + write!(f, "{}", source) + } } (None, None) => { write!(f, "Error") @@ -496,10 +520,12 @@ impl core::fmt::Display for Error { message, source, .. } => { if let Some(message) = message { - write!(f, "{}: {}", message, source) + write!(f, "{}: {}", message, source)?; } else { - write!(f, "{}", source) + write!(f, "{}", source)?; } + write_sources_if_alternate(f, source.source())?; + Ok(()) } Self::Anyhow { source, .. } => source.fmt(f), } From 8465ba8a5452b6d4abbe90156962cccb4c31726a Mon Sep 17 00:00:00 2001 From: Frando Date: Sun, 6 Jul 2025 20:22:46 +0200 Subject: [PATCH 2/5] change: add newlines to separate sources --- src/error.rs | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/error.rs b/src/error.rs index ca43910..d522c70 100644 --- a/src/error.rs +++ b/src/error.rs @@ -477,9 +477,16 @@ fn write_sources_if_alternate( if !f.alternate() { return Ok(()); } + let mut i = 0; + let mut first = true; while let Some(inner) = source { - write!(f, ": {}", inner)?; + if first { + write!(f, "\n\nCaused by:")?; + first = false; + } + write!(f, "\n {i}: {}", inner)?; source = inner.source(); + i += 1; } Ok(()) } From c1cb62a085b40af0d1b8b7d11c54b29fc7f56ed3 Mon Sep 17 00:00:00 2001 From: Frando Date: Mon, 7 Jul 2025 11:10:34 +0200 Subject: [PATCH 3/5] fix: don't double-print sources --- src/error.rs | 119 ++++++++++++++++++++++++++++++++++++++++----------- 1 file changed, 93 insertions(+), 26 deletions(-) diff --git a/src/error.rs b/src/error.rs index d522c70..a80fe3c 100644 --- a/src/error.rs +++ b/src/error.rs @@ -470,57 +470,103 @@ impl snafu::ErrorCompat for Error { } } +trait ErrorSource<'a>: std::fmt::Display + std::fmt::Debug { + fn source(&'a self) -> Option>; +} + +impl<'a> ErrorSource<'a> for Error { + fn source(&'a self) -> Option> { + match self { + Error::Source { source, .. } => source.source().map(SourceWrapper::Std), + Error::Anyhow { source, .. } => source.source().map(SourceWrapper::Std), + Error::Message { ref source, .. } => Some(SourceWrapper::Box(source)), + Error::Whatever { ref source, .. } => source.as_ref().map(|s| SourceWrapper::Crate(s)), + } + } +} + +#[derive(Debug, Clone, Copy)] +enum SourceWrapper<'a> { + Std(&'a dyn std::error::Error), + Box(&'a Box), + Crate(&'a Error), +} + +impl<'a> std::fmt::Display for SourceWrapper<'a> { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + SourceWrapper::Std(error) => write!(f, "{error}"), + SourceWrapper::Crate(error) => match error { + Error::Message { message, .. } => { + write!(f, "{}", message.as_deref().unwrap_or("Error")) + } + _ => write!(f, "{error}"), + }, + SourceWrapper::Box(error) => write!(f, "{error}"), + } + } +} + +impl<'a> ErrorSource<'a> for SourceWrapper<'a> { + fn source(&'a self) -> Option> { + match self { + SourceWrapper::Std(error) => std::error::Error::source(error).map(SourceWrapper::Std), + SourceWrapper::Crate(error) => error.source(), + SourceWrapper::Box(error) => error.source().map(SourceWrapper::Std), + } + } +} + fn write_sources_if_alternate( f: &mut core::fmt::Formatter, - mut source: Option<&dyn std::error::Error>, + source: Option>, +) -> core::fmt::Result { + write_sources_if_alternate_inner(f, source, 0)?; + Ok(()) +} + +fn write_sources_if_alternate_inner( + f: &mut core::fmt::Formatter, + source: Option>, + i: usize, ) -> core::fmt::Result { if !f.alternate() { return Ok(()); } - let mut i = 0; - let mut first = true; - while let Some(inner) = source { - if first { - write!(f, "\n\nCaused by:")?; - first = false; - } - write!(f, "\n {i}: {}", inner)?; - source = inner.source(); - i += 1; + if let Some(current) = source { + write!(f, "\n {i}: {}", current)?; + write_sources_if_alternate_inner(f, current.source(), i + 1)?; } Ok(()) } impl core::fmt::Display for Error { fn fmt(&self, f: &mut core::fmt::Formatter) -> core::fmt::Result { + if f.alternate() { + write!(f, "Error: ")?; + } match self { Self::Source { source, .. } => { - write!(f, "{}", source)?; - write_sources_if_alternate(f, source.source())?; - Ok(()) + write!(f, "{source}")?; } Self::Whatever { message, source, .. } => match (source, message) { (Some(source), Some(message)) => { if f.alternate() { - write!(f, "{}: {:#}", message, source) + write!(f, "{message}")?; } else { - write!(f, "{}: {}", message, source) + write!(f, "{message}: {source}")?; } } (None, Some(message)) => { - write!(f, "{}", message) + write!(f, "{message}")?; } (Some(source), None) => { - if f.alternate() { - write!(f, "{:#}", source) - } else { - write!(f, "{}", source) - } + write!(f, "{source}")?; } (None, None) => { - write!(f, "Error") + write!(f, "Error")?; } }, Self::Message { @@ -531,11 +577,10 @@ impl core::fmt::Display for Error { } else { write!(f, "{}", source)?; } - write_sources_if_alternate(f, source.source())?; - Ok(()) } - Self::Anyhow { source, .. } => source.fmt(f), + Self::Anyhow { source, .. } => source.fmt(f)?, } + write_sources_if_alternate(f, self.source()) } } @@ -650,4 +695,26 @@ mod tests { let stack = err.stack(); assert_eq!(stack.len(), 2); } + + #[test] + fn test_sources() { + let err = std::io::Error::new(std::io::ErrorKind::NotFound, "file not found"); + let file_name = "foo.txt"; + let res: Result<(), _> = Err(err).with_context(|| format!("failed to read {file_name}")); + let res: Result<(), _> = res.context("read error"); + + let err = res.err().unwrap(); + let fmt = format!("{err}"); + println!("short:\n{fmt}"); + assert_eq!(&fmt, "read error: failed to read foo.txt: file not found"); + + let fmt = format!("{err:#}"); + println!("alternate:\n{fmt}"); + assert_eq!( + &fmt, + r#"Error: read error + 0: failed to read foo.txt + 1: file not found"# + ); + } } From 8c74b20d3c107305cda37843c729c8878e522297 Mon Sep 17 00:00:00 2001 From: Frando Date: Mon, 7 Jul 2025 11:15:48 +0200 Subject: [PATCH 4/5] reuse alternate display in debug formatter --- src/error.rs | 35 ++++++++++++++++++++--------------- 1 file changed, 20 insertions(+), 15 deletions(-) diff --git a/src/error.rs b/src/error.rs index a80fe3c..b1888c3 100644 --- a/src/error.rs +++ b/src/error.rs @@ -288,13 +288,7 @@ impl std::fmt::Debug for Error { let stack = self.stack(); writeln!(f)?; - for (i, (_, source)) in stack.iter().skip(1).enumerate() { - match source { - Source::Root => {} - _ => writeln!(f, " {}: {}", i, source)?, - } - } - + write!(f, "{self:#}")?; writeln!(f)?; // Span Trace @@ -521,21 +515,29 @@ fn write_sources_if_alternate( f: &mut core::fmt::Formatter, source: Option>, ) -> core::fmt::Result { - write_sources_if_alternate_inner(f, source, 0)?; + if !f.alternate() { + return Ok(()); + } + write_sources(f, source)?; + Ok(()) +} + +fn write_sources( + f: &mut core::fmt::Formatter, + source: Option>, +) -> core::fmt::Result { + write_sources_inner(f, source, 0)?; Ok(()) } -fn write_sources_if_alternate_inner( +fn write_sources_inner( f: &mut core::fmt::Formatter, source: Option>, i: usize, ) -> core::fmt::Result { - if !f.alternate() { - return Ok(()); - } if let Some(current) = source { write!(f, "\n {i}: {}", current)?; - write_sources_if_alternate_inner(f, current.source(), i + 1)?; + write_sources_inner(f, current.source(), i + 1)?; } Ok(()) } @@ -705,16 +707,19 @@ mod tests { let err = res.err().unwrap(); let fmt = format!("{err}"); - println!("short:\n{fmt}"); + println!("short:\n{fmt}\n"); assert_eq!(&fmt, "read error: failed to read foo.txt: file not found"); let fmt = format!("{err:#}"); - println!("alternate:\n{fmt}"); + println!("alternate:\n{fmt}\n"); assert_eq!( &fmt, r#"Error: read error 0: failed to read foo.txt 1: file not found"# ); + + let fmt = format!("{err:?}"); + println!("debug:\n{fmt}\n"); } } From 3094f1fda5e93bf0f36e6c7cddf4dc6bdd2be612 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Philipp=20Kr=C3=BCger?= Date: Fri, 29 Aug 2025 09:35:34 +0200 Subject: [PATCH 5/5] Clippy fixes --- src/error.rs | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/error.rs b/src/error.rs index b1888c3..e7617c5 100644 --- a/src/error.rs +++ b/src/error.rs @@ -302,7 +302,7 @@ impl std::fmt::Debug for Error { for (bt, _) in stack.into_iter() { let bt = bt.unwrap_or(Backtrace::Crate(&empty_bt)); let s = printer.format_trace_to_string(&bt).unwrap(); - writeln!(f, "\n{}", s)?; + writeln!(f, "\n{s}")?; } Ok(()) @@ -337,7 +337,7 @@ impl Error { } } - pub fn stack(&self) -> Vec<(Option, Source<'_>)> { + pub fn stack(&self) -> Vec<(Option>, Source<'_>)> { let mut traces = Vec::new(); match self { @@ -482,6 +482,7 @@ impl<'a> ErrorSource<'a> for Error { #[derive(Debug, Clone, Copy)] enum SourceWrapper<'a> { Std(&'a dyn std::error::Error), + #[allow(clippy::borrowed_box)] Box(&'a Box), Crate(&'a Error), } @@ -536,7 +537,7 @@ fn write_sources_inner( i: usize, ) -> core::fmt::Result { if let Some(current) = source { - write!(f, "\n {i}: {}", current)?; + write!(f, "\n {i}: {current}")?; write_sources_inner(f, current.source(), i + 1)?; } Ok(()) @@ -575,9 +576,9 @@ impl core::fmt::Display for Error { message, source, .. } => { if let Some(message) = message { - write!(f, "{}: {}", message, source)?; + write!(f, "{message}: {source}")?; } else { - write!(f, "{}", source)?; + write!(f, "{source}")?; } } Self::Anyhow { source, .. } => source.fmt(f)?,