From 1c5598e3b3f006270693c4dedb2ea16b5208769e Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Sun, 18 Jan 2026 13:35:53 +0100 Subject: [PATCH 1/7] Various improvements to how errors are defined for consistency --- gix-commitgraph/src/file/commit.rs | 2 +- gix-commitgraph/src/file/init.rs | 7 +-- gix-commitgraph/src/file/verify.rs | 9 ++-- gix-commitgraph/src/init.rs | 4 +- gix-commitgraph/src/verify.rs | 52 +++++++++---------- gix/src/revision/spec/parse/delegate/mod.rs | 9 ++-- .../revision/spec/parse/delegate/navigate.rs | 6 +-- .../revision/spec/parse/delegate/revision.rs | 14 ++--- 8 files changed, 45 insertions(+), 58 deletions(-) diff --git a/gix-commitgraph/src/file/commit.rs b/gix-commitgraph/src/file/commit.rs index 9be156f4485..29522bb5caa 100644 --- a/gix-commitgraph/src/file/commit.rs +++ b/gix-commitgraph/src/file/commit.rs @@ -162,7 +162,7 @@ impl Iterator for Parents<'_> { self.next() } else { Some(Err(message!( - "commit {0}'s extra edges overflows the commit-graph file's extra edges list", + "commit {}'s extra edges overflows the commit-graph file's extra edges list", self.commit_data.id() ))) } diff --git a/gix-commitgraph/src/file/init.rs b/gix-commitgraph/src/file/init.rs index b07c69a78b2..b78e77a60a4 100644 --- a/gix-commitgraph/src/file/init.rs +++ b/gix-commitgraph/src/file/init.rs @@ -1,6 +1,5 @@ use std::path::{Path, PathBuf}; -use bstr::ByteSlice; use gix_error::{message, ErrorExt, Exn, Message, ResultExt}; use crate::{ @@ -142,11 +141,7 @@ impl File { } if base_graph_count > 0 && base_graphs_list_offset.is_none() { - return Err(message!( - "Chunk named {:?} was not found in chunk file index", - BASE_GRAPHS_LIST_CHUNK_ID.as_bstr() - ) - .into()); + return Err(message!("Chunk named {BASE_GRAPHS_LIST_CHUNK_ID:?} was not found in chunk file index").into()); } let (fan, _) = read_fan(&data[fan_offset..]); diff --git a/gix-commitgraph/src/file/verify.rs b/gix-commitgraph/src/file/verify.rs index 5b3faaf8d89..60340dbc583 100644 --- a/gix-commitgraph/src/file/verify.rs +++ b/gix-commitgraph/src/file/verify.rs @@ -65,10 +65,9 @@ impl File { .raise()); } return Err(message!( - "commit at file position {} with ID {} is out of order relative to its predecessor with ID {}", + "commit at file position {} with ID {} is out of order relative to its predecessor with ID {prev_id}", commit.position(), - commit.id(), - prev_id + commit.id() ) .raise()); } @@ -114,8 +113,8 @@ impl File { hasher.update(&self.data[..data_len_without_trailer]); let actual = hasher .try_finalize() - .map_err(|e| message!("failed to hash commit graph file: {}", e).raise())?; - actual.verify(self.checksum()).map_err(|e| message!("{}", e).raise())?; + .map_err(|e| message!("failed to hash commit graph file: {e}").raise())?; + actual.verify(self.checksum()).map_err(|e| message!("{e}").raise())?; Ok(actual) } } diff --git a/gix-commitgraph/src/init.rs b/gix-commitgraph/src/init.rs index 6006b07ecf3..d97bff9744e 100644 --- a/gix-commitgraph/src/init.rs +++ b/gix-commitgraph/src/init.rs @@ -57,9 +57,7 @@ impl Graph { let num_commits: u64 = files.iter().map(|f| u64::from(f.num_commits())).sum(); if num_commits > u64::from(MAX_COMMITS) { return Err(message!( - "Commit-graph files contain {} commits altogether, but only {} commits are allowed", - num_commits, - MAX_COMMITS + "Commit-graph files contain {num_commits} commits altogether, but only {MAX_COMMITS} commits are allowed" )); } diff --git a/gix-commitgraph/src/verify.rs b/gix-commitgraph/src/verify.rs index 26ffcc19353..280c170dd48 100644 --- a/gix-commitgraph/src/verify.rs +++ b/gix-commitgraph/src/verify.rs @@ -88,40 +88,38 @@ impl Graph { } let next_file_start_pos = Position(file_start_pos.0 + file.num_commits()); - let file_stats = file - .traverse(|commit| { - let mut max_parent_generation = 0u32; - for parent_pos in commit.iter_parents() { - let parent_pos = parent_pos.map_err(|err| err.raise_erased())?; - if parent_pos >= next_file_start_pos { - return Err(message!( - "Commit {} has parent position {parent_pos} that is out of range (should be in range 0-{})", - commit.id(), - Position(next_file_start_pos.0 - 1) - ) - .raise_erased()); - } - let parent = self.commit_at(parent_pos); - max_parent_generation = max(max_parent_generation, parent.generation()); - } - - // If the max parent generation is GENERATION_NUMBER_MAX, then this commit's - // generation should be GENERATION_NUMBER_MAX too. - let expected_generation = min(max_parent_generation + 1, GENERATION_NUMBER_MAX); - if commit.generation() != expected_generation { + let file_stats = file.traverse(|commit| { + let mut max_parent_generation = 0u32; + for parent_pos in commit.iter_parents() { + let parent_pos = parent_pos.map_err(|err| err.raise_erased())?; + if parent_pos >= next_file_start_pos { return Err(message!( - "Commit {}'s generation should be {expected_generation} but is {}", + "Commit {} has parent position {parent_pos} that is out of range (should be in range 0-{})", commit.id(), - commit.generation() + Position(next_file_start_pos.0 - 1) ) .raise_erased()); } + let parent = self.commit_at(parent_pos); + max_parent_generation = max(max_parent_generation, parent.generation()); + } + + // If the max parent generation is GENERATION_NUMBER_MAX, then this commit's + // generation should be GENERATION_NUMBER_MAX too. + let expected_generation = min(max_parent_generation + 1, GENERATION_NUMBER_MAX); + if commit.generation() != expected_generation { + return Err(message!( + "Commit {}'s generation should be {expected_generation} but is {}", + commit.id(), + commit.generation() + ) + .raise_erased()); + } - processor(commit).or_raise_erased(|| message!("processor failed on commit {id}", id = commit.id()))?; + processor(commit).or_raise_erased(|| message!("processor failed on commit {id}", id = commit.id()))?; - Ok(()) - }) - .map_err(|err| message!("{}: {}", file.path().display(), err).raise())?; + Ok(()) + })?; max_generation = max(max_generation, file_stats.max_generation); stats.num_commits += file_stats.num_commits; diff --git a/gix/src/revision/spec/parse/delegate/mod.rs b/gix/src/revision/spec/parse/delegate/mod.rs index 41c2eda9f0b..6e6bd17d8e9 100644 --- a/gix/src/revision/spec/parse/delegate/mod.rs +++ b/gix/src/revision/spec/parse/delegate/mod.rs @@ -42,14 +42,12 @@ impl<'repo> Delegate<'repo> { match (ambiguous_errors.pop(), ambiguous_errors.pop()) { (Some(one), None) => Some(one), - (Some(one), Some(two)) => { - Some(Exn::raise_all([one, two], message!("Both objects were ambiguous")).erased()) - } + (Some(one), Some(two)) => Some(Exn::raise_all([one, two], message("Both objects were ambiguous")).erased()), _ => (!delayed_errors.is_empty()).then(|| { if delayed_errors.len() == 1 { delayed_errors.pop().expect("it's exactly one") } else { - Exn::raise_all(delayed_errors, message!("one or more delayed errors")).erased() + Exn::raise_all(delayed_errors, message("one or more delayed errors")).erased() } }), } @@ -166,9 +164,8 @@ impl Delegate<'_> { Ok(()) } else { Err(message!( - "Object {oid} was a {actual}, but needed it to be a {expected}", + "Object {oid} was a {actual}, but needed it to be a {kind}", actual = obj.kind, - expected = kind, oid = obj.id.attach(repo).shorten_or_id(), ) .raise_erased()) diff --git a/gix/src/revision/spec/parse/delegate/navigate.rs b/gix/src/revision/spec/parse/delegate/navigate.rs index a5d1227ad75..a1e794060d9 100644 --- a/gix/src/revision/spec/parse/delegate/navigate.rs +++ b/gix/src/revision/spec/parse/delegate/navigate.rs @@ -25,7 +25,7 @@ impl delegate::Navigate for Delegate<'_> { let objs = match self.objs[self.idx].as_mut() { Some(objs) => objs, None => { - bail!(message!("Tried to navigate the commit-graph without providing an anchor first").raise_erased()) + bail!(message("Tried to navigate the commit-graph without providing an anchor first").raise_erased()) } }; let repo = self.repo; @@ -256,7 +256,7 @@ impl delegate::Navigate for Delegate<'_> { .rev_walk( references .peeled() - .or_raise_erased(|| message!("Couldn't configure iterator for peeling"))? + .or_raise_erased(|| message("Couldn't configure iterator for peeling"))? .filter_map(Result::ok) .filter(|r| r.id().header().ok().is_some_and(|obj| obj.kind().is_commit())) .filter_map(|r| r.detach().peeled), @@ -375,7 +375,7 @@ fn handle_errors_and_replacements( delayed_errors.extend(errors.into_iter().map(|(_, err)| err)); Err(delayed_errors .pop() - .unwrap_or_else(|| message!("BUG: Somehow there was no error but one was expected").raise_erased())) + .unwrap_or_else(|| message("BUG: Somehow there was no error but one was expected").raise_erased())) } else { for (obj, err) in errors { if let Some(pos) = objs.iter().position(|o| o == &obj) { diff --git a/gix/src/revision/spec/parse/delegate/revision.rs b/gix/src/revision/spec/parse/delegate/revision.rs index da03dbe4a62..87bbc5d2979 100644 --- a/gix/src/revision/spec/parse/delegate/revision.rs +++ b/gix/src/revision/spec/parse/delegate/revision.rs @@ -18,7 +18,7 @@ impl delegate::Revision for Delegate<'_> { fn find_ref(&mut self, name: &BStr) -> Result<(), Exn> { self.unset_disambiguate_call(); if self.has_delayed_err() && self.refs[self.idx].is_some() { - return Err(message!("Refusing call as there are delayed errors and a ref is available").raise_erased()); + return Err(message("Refusing call as there are delayed errors and a ref is available").raise_erased()); } match self.repo.refs.find(name) { Ok(r) => { @@ -51,7 +51,7 @@ impl delegate::Revision for Delegate<'_> { .or_erased()?; match ok { - None => Err(message!("An object prefixed {} could not be found", prefix).raise_erased()), + None => Err(message!("An object prefixed {prefix} could not be found").raise_erased()), Some(Ok(_) | Err(())) => { assert!(self.objs[self.idx].is_none(), "BUG: cannot set the same prefix twice"); let candidates = candidates.expect("set above"); @@ -111,7 +111,7 @@ impl delegate::Revision for Delegate<'_> { *val = Some(r.clone().detach()); r } - Ok(None) => return Err(message!("Unborn heads do not have a reflog yet").raise_erased()), + Ok(None) => return Err(message("Unborn heads do not have a reflog yet").raise_erased()), Err(err) => return Err(err.raise_erased()), }, }; @@ -134,7 +134,7 @@ impl delegate::Revision for Delegate<'_> { { Some(closest_line) => closest_line.new_oid, None => match last { - None => return Err(message!("Reflog does not contain any entries").raise_erased()), + None => return Err(message("Reflog does not contain any entries").raise_erased()), Some(id) => id, }, }; @@ -184,7 +184,7 @@ impl delegate::Revision for Delegate<'_> { .and_then(|from_to| from_to.find(" to ").map(|pos| &from_to[..pos])) .map(|from_branch| (from_branch.into(), line.previous_oid)) })), - None => Err(message!( + None => Err(message( "Reference HEAD does not have a reference log, cannot search prior checked out branch", ) .raise() @@ -234,7 +234,7 @@ impl delegate::Revision for Delegate<'_> { r } Ok(None) => { - return Err(message!("Unborn heads cannot have push or upstream tracking branches").raise_erased()) + return Err(message("Unborn heads cannot have push or upstream tracking branches").raise_erased()) } Err(err) => { return Err(err.raise_erased()); @@ -271,7 +271,7 @@ impl delegate::Revision for Delegate<'_> { } }, } - Err(message!("Couldn't find sibling of {kind:?}", kind = kind).raise_erased()) + Err(message!("Couldn't find sibling of {kind:?}").raise_erased()) } } From 053c3ee2217480eead3aa7c71fa4b65455444921 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Sun, 18 Jan 2026 14:55:05 +0100 Subject: [PATCH 2/7] feat: anyhow support for `gix-error::Exn` This is mainly useful for `gitoxide-core`, which may call plumbing. --- Cargo.lock | 4 ++ gitoxide-core/Cargo.toml | 1 + gitoxide-core/src/commitgraph/verify.rs | 5 +- .../src/repository/commitgraph/verify.rs | 3 +- gix-error/Cargo.toml | 12 ++++ gix-error/src/{ => concrete}/message.rs | 0 gix-error/src/concrete/mod.rs | 2 + gix-error/src/{ => concrete}/parse.rs | 0 gix-error/src/error.rs | 2 +- gix-error/src/exn/impls.rs | 15 ++++- gix-error/src/lib.rs | 28 ++++++-- gix-error/tests/error/exn.rs | 67 ++++++++++++++++++- 12 files changed, 124 insertions(+), 15 deletions(-) rename gix-error/src/{ => concrete}/message.rs (100%) create mode 100644 gix-error/src/concrete/mod.rs rename gix-error/src/{ => concrete}/parse.rs (100%) diff --git a/Cargo.lock b/Cargo.lock index d8449ae5765..21d919bc648 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1358,6 +1358,7 @@ dependencies = [ "futures-lite", "gix", "gix-archive", + "gix-error", "gix-fsck", "gix-pack", "gix-status", @@ -1753,7 +1754,10 @@ dependencies = [ name = "gix-error" version = "0.0.0" dependencies = [ + "anyhow", "bstr", + "document-features", + "gix-error", "insta", ] diff --git a/gitoxide-core/Cargo.toml b/gitoxide-core/Cargo.toml index 8a2829617ad..5969fdce277 100644 --- a/gitoxide-core/Cargo.toml +++ b/gitoxide-core/Cargo.toml @@ -58,6 +58,7 @@ gix-transport-configuration-only = { package = "gix-transport", version = "^0.52 gix-archive-for-configuration-only = { package = "gix-archive", version = "^0.26.0", path = "../gix-archive", optional = true, features = ["tar", "tar_gz"] } gix-status = { version = "^0.24.0", path = "../gix-status" } gix-fsck = { version = "^0.16.0", path = "../gix-fsck" } +gix-error-for-configuration-only = { package = "gix-error", version = "^0.0.0", path = "../gix-error", features = ["anyhow"] } serde = { version = "1.0.114", optional = true, default-features = false, features = ["derive"] } anyhow = "1.0.100" thiserror = "2.0.17" diff --git a/gitoxide-core/src/commitgraph/verify.rs b/gitoxide-core/src/commitgraph/verify.rs index 40169e24c65..91a629a6794 100644 --- a/gitoxide-core/src/commitgraph/verify.rs +++ b/gitoxide-core/src/commitgraph/verify.rs @@ -25,7 +25,6 @@ pub(crate) mod function { use crate::OutputFormat; use anyhow::Result; use gix::commitgraph::{verify::Outcome, Graph}; - use gix::Exn; pub fn verify( path: impl AsRef, @@ -39,13 +38,13 @@ pub(crate) mod function { W1: io::Write, W2: io::Write, { - let g = Graph::at(path.as_ref()).map_err(Exn::into_error)?; + let g = Graph::at(path.as_ref())?; #[allow(clippy::unnecessary_wraps, unknown_lints)] fn noop_processor(_commit: &gix::commitgraph::file::Commit<'_>) -> std::result::Result<(), std::fmt::Error> { Ok(()) } - let stats = g.verify_integrity(noop_processor).map_err(Exn::into_error)?; + let stats = g.verify_integrity(noop_processor)?; #[cfg_attr(not(feature = "serde"), allow(clippy::single_match))] match output_statistics { diff --git a/gitoxide-core/src/repository/commitgraph/verify.rs b/gitoxide-core/src/repository/commitgraph/verify.rs index d3c72aaf4fb..65eadc6db4f 100644 --- a/gitoxide-core/src/repository/commitgraph/verify.rs +++ b/gitoxide-core/src/repository/commitgraph/verify.rs @@ -14,7 +14,6 @@ pub(crate) mod function { use crate::{repository::commitgraph::verify::Context, OutputFormat}; use anyhow::Result; - use gix::Exn; pub fn verify( repo: gix::Repository, @@ -34,7 +33,7 @@ pub(crate) mod function { fn noop_processor(_commit: &gix::commitgraph::file::Commit<'_>) -> std::result::Result<(), std::fmt::Error> { Ok(()) } - let stats = g.verify_integrity(noop_processor).map_err(Exn::into_error)?; + let stats = g.verify_integrity(noop_processor)?; #[cfg_attr(not(feature = "serde"), allow(clippy::single_match))] match output_statistics { diff --git a/gix-error/Cargo.toml b/gix-error/Cargo.toml index 8d05c1a34ce..5969fe1c1b4 100644 --- a/gix-error/Cargo.toml +++ b/gix-error/Cargo.toml @@ -11,9 +11,21 @@ edition = "2021" include = ["src/**/*", "LICENSE-*"] rust-version = "1.82" +[features] +## If enabled, the `Exn` error type converts to `anyhow::Error` natively so `?` can be used directly. +anyhow = ["dep:anyhow"] + [dependencies] bstr = { version = "1.12.0", default-features = false, features = ["std"] } +anyhow = { version = "1.0.100", optional = true } +document-features = { version = "0.2.0", optional = true } + [dev-dependencies] +gix-error = { path = ".", features = ["anyhow"] } insta = "1.46.0" +[package.metadata.docs.rs] +all-features = true +features = ["document-features"] + diff --git a/gix-error/src/message.rs b/gix-error/src/concrete/message.rs similarity index 100% rename from gix-error/src/message.rs rename to gix-error/src/concrete/message.rs diff --git a/gix-error/src/concrete/mod.rs b/gix-error/src/concrete/mod.rs new file mode 100644 index 00000000000..e2dee024119 --- /dev/null +++ b/gix-error/src/concrete/mod.rs @@ -0,0 +1,2 @@ +pub(super) mod message; +pub(super) mod parse; diff --git a/gix-error/src/parse.rs b/gix-error/src/concrete/parse.rs similarity index 100% rename from gix-error/src/parse.rs rename to gix-error/src/concrete/parse.rs diff --git a/gix-error/src/error.rs b/gix-error/src/error.rs index b82fe2a9915..509654d956e 100644 --- a/gix-error/src/error.rs +++ b/gix-error/src/error.rs @@ -12,7 +12,7 @@ impl Error { /// Return an iterator over all errors in the tree in breadth-first order, starting with this one. pub fn iter_frames(&self) -> impl Iterator + '_ { - self.inner.frame().iter() + self.inner.frame().iter_frames() } } diff --git a/gix-error/src/exn/impls.rs b/gix-error/src/exn/impls.rs index 202e702ebc1..07f2758acc6 100644 --- a/gix-error/src/exn/impls.rs +++ b/gix-error/src/exn/impls.rs @@ -186,7 +186,7 @@ impl Exn { /// Iterate over all frames in breadth-first order. The first frame is this instance, /// followed by all of its children. pub fn iter(&self) -> impl Iterator { - self.frame().iter() + self.frame().iter_frames() } /// Iterate over all frames and find one that downcasts into error of type `T`. @@ -380,7 +380,7 @@ impl Frame { /// Iterate over all frames in breadth-first order. The first frame is this instance, /// followed by all of its children. - pub fn iter(&self) -> impl Iterator + '_ { + pub fn iter_frames(&self) -> impl Iterator + '_ { let mut queue = std::collections::VecDeque::new(); queue.push_back(self); BreadthFirstFrames { queue } @@ -413,6 +413,17 @@ where } } +#[cfg(feature = "anyhow")] +impl From> for anyhow::Error +where + E: Error + Send + Sync + 'static, +{ + fn from(err: Exn) -> Self { + // TODO: make this source-chain compatible. + anyhow::Error::from(err.into_box()) + } +} + impl From> for Frame where E: Error + Send + Sync + 'static, diff --git a/gix-error/src/lib.rs b/gix-error/src/lib.rs index 908272f9ce0..c94adbb44fb 100644 --- a/gix-error/src/lib.rs +++ b/gix-error/src/lib.rs @@ -39,6 +39,25 @@ //! //! A side effect of this is that any callee that causes errors needs to be annotated with //! `.or_raise(|| message!("context information"))` or `.or_raise_erased(|| message!("context information"))`. +//! +//! # Feature Flags +#![cfg_attr( + all(doc, feature = "document-features"), + doc = ::document_features::document_features!() +)] +//! # Why not `anyhow`? +//! +//! `anyhow` is a proven and optimized library, and it would certainly suffice for an error-chain based approach +//! where users are expected to downcast to concrete types. +//! +//! What's missing though is `track-caller` which will always capture the location of error instantiation, along with +//! compatibility for error trees, which are happening when multiple calls are in flight during concurrency. +//! +//! Both libraries share the shortcoming of not being able to implement `std::error::Error` on their error type, +//! and both provide workarounds. +//! +//! `exn` is much less optimized, but also costs only a `Box` on the stack, +//! which in any case is a step up from `thiserror` which exposed a lot of heft to the stack. #![deny(missing_docs, unsafe_code)] /// A result type to hide the [Exn] error wrapper. mod exn; @@ -61,8 +80,7 @@ pub struct Error { mod error; -mod message; -pub use message::{message, Message}; - -mod parse; -pub use parse::ParseError; +/// Various kinds of concrete errors that implement [`std::error::Error`]. +mod concrete; +pub use concrete::message::{message, Message}; +pub use concrete::parse::ParseError; diff --git a/gix-error/tests/error/exn.rs b/gix-error/tests/error/exn.rs index a9131f1e977..da44590fc4f 100644 --- a/gix-error/tests/error/exn.rs +++ b/gix-error/tests/error/exn.rs @@ -13,10 +13,10 @@ // limitations under the License. use crate::{debug_string, new_tree_error, Error, ErrorWithSource}; -use gix_error::Exn; use gix_error::OptionExt; use gix_error::ResultExt; use gix_error::{message, ErrorExt}; +use gix_error::{Exn, Message}; #[test] fn raise_chain() { @@ -364,7 +364,7 @@ fn error_tree() { | └─ E7, at gix-error/tests/error/main.rs:22:30 "); - insta::assert_debug_snapshot!(err.frame().iter().map(ToString::to_string).collect::>(), @r#" + insta::assert_debug_snapshot!(err.frame().iter_frames().map(ToString::to_string).collect::>(), @r#" [ "E6", "E5", @@ -502,3 +502,66 @@ fn erased_into_error() { let e = Error("E1").raise().erased(); let _into_error_works = e.into_error(); } + +#[cfg(feature = "anyhow")] +#[test] +fn raise_chain_anyhow() { + let e1 = Error("E1") + .raise() + .chain(Exn::raise_all([Error("E1c1-1"), Error("E1c1-2")], Error("E1-2"))) + .chain(Exn::raise_all([Error("E1c2-1"), Error("E1c2-2")], Error("E1-3"))); + let e2 = e1.raise(Error("E2")); + let root = e2.raise(Message::new("root")); + + // It's a linked list as linked up with the first child, but also has multiple children. + insta::assert_snapshot!(format!("{root:#}"), @r#" + Message("root") + | + └─ Error("E2") + | + └─ Error("E1") + | + └─ Error("E1-2") + | | + | └─ Error("E1c1-1") + | | + | └─ Error("E1c1-2") + | + └─ Error("E1-3") + | + └─ Error("E1c2-1") + | + └─ Error("E1c2-2") + "#); + + // TODO: this should print the complete error chain. + insta::assert_snapshot!(remove_stackstrace(format!("{:?}", anyhow::Error::from(root))), @"root"); +} + +#[cfg(feature = "anyhow")] +#[test] +fn inverse_error_call_chain_anyhow() { + let e1 = Error("E1").raise(); + let e2 = e1.chain(Error("E2")); + let e3 = e2.chain(Error("E3")); + let e4 = e3.chain(Error("E4")); + let e5 = e4.chain(Error("E5")); + insta::assert_debug_snapshot!(e5, @" + E1 + | + └─ E2 + | + └─ E3 + | + └─ E4 + | + └─ E5 + "); + + // TODO: this should print the complete error chain. + insta::assert_snapshot!(remove_stackstrace(format!("{:?}", anyhow::Error::from(e5))), @"E1"); +} + +fn remove_stackstrace(s: String) -> String { + s.find("Stack backtrace:").map_or(s.clone(), |pos| s[..pos].into()) +} From 28f4211afadd91c5b5d2d2a0698f37e660cc0c66 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Sun, 18 Jan 2026 13:53:12 +0100 Subject: [PATCH 3/7] feat: make it possible to produce errors that work well with `anyhow` source-chain display. --- gix-error/src/concrete/chain.rs | 36 ++++ gix-error/src/concrete/mod.rs | 1 + gix-error/src/exn/impls.rs | 47 ++++- gix-error/src/lib.rs | 5 + gix-error/tests/error/error.rs | 54 ++--- gix-error/tests/error/exn.rs | 362 +++++++++++++++++++++----------- gix-error/tests/error/main.rs | 48 ++--- 7 files changed, 367 insertions(+), 186 deletions(-) create mode 100644 gix-error/src/concrete/chain.rs diff --git a/gix-error/src/concrete/chain.rs b/gix-error/src/concrete/chain.rs new file mode 100644 index 00000000000..bf41461f26a --- /dev/null +++ b/gix-error/src/concrete/chain.rs @@ -0,0 +1,36 @@ +use crate::write_location; +use std::fmt::{Debug, Display, Formatter}; +use std::panic::Location; + +/// A generic error which represents a linked-list of errors and exposes it with [source()](std::error::Error::source). +/// It's meant to be the target of a conversion of any [Exn](crate::Exn) error tree. +/// +/// It's useful for inter-op with other error handling crates like `anyhow` which offer simplified access to the error chain, +/// and thus is expected to be wrapped in one of their types intead of being used directly. +pub struct ChainedError { + pub(crate) err: Box, + pub(crate) location: &'static Location<'static>, + pub(crate) source: Option>, +} + +impl Debug for ChainedError { + fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { + Debug::fmt(&self.err, f) + } +} + +impl Display for ChainedError { + fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { + Display::fmt(&self.err, f)?; + if !f.alternate() { + write_location(f, self.location)?; + } + Ok(()) + } +} + +impl std::error::Error for ChainedError { + fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { + self.source.as_ref().map(|e| e as &(dyn std::error::Error + 'static)) + } +} diff --git a/gix-error/src/concrete/mod.rs b/gix-error/src/concrete/mod.rs index e2dee024119..eb73a30acdc 100644 --- a/gix-error/src/concrete/mod.rs +++ b/gix-error/src/concrete/mod.rs @@ -1,2 +1,3 @@ +pub(super) mod chain; pub(super) mod message; pub(super) mod parse; diff --git a/gix-error/src/exn/impls.rs b/gix-error/src/exn/impls.rs index 07f2758acc6..392cecc44f3 100644 --- a/gix-error/src/exn/impls.rs +++ b/gix-error/src/exn/impls.rs @@ -12,13 +12,14 @@ // See the License for the specific language governing permissions and // limitations under the License. +use std::collections::VecDeque; use std::error::Error; use std::fmt; use std::marker::PhantomData; use std::ops::Deref; use std::panic::Location; -use crate::Exn; +use crate::{write_location, ChainedError, Exn}; impl From for Exn { #[track_caller] @@ -178,6 +179,14 @@ impl Exn { self.into() } + /// Convert this error tree into a chain of errors, breadth first, which flattens the tree + /// but retains all type dynamic type information. + /// + /// This is useful for inter-op with `anyhow`. + pub fn into_chain(self) -> crate::ChainedError { + self.into() + } + /// Return the underlying exception frame. pub fn frame(&self) -> &Frame { &self.frame @@ -245,7 +254,7 @@ fn write_frame_recursive( } }?; if !f.alternate() { - write_location(f, frame)?; + write_location(f, frame.location)?; } let children = frame.children(); @@ -269,11 +278,6 @@ fn write_frame_recursive( Ok(()) } -fn write_location(f: &mut fmt::Formatter<'_>, exn: &Frame) -> fmt::Result { - let location = exn.location(); - write!(f, ", at {}:{}:{}", location.file(), location.line(), location.column()) -} - impl fmt::Display for Exn { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { fmt::Display::fmt(&self.frame, f) @@ -419,8 +423,7 @@ where E: Error + Send + Sync + 'static, { fn from(err: Exn) -> Self { - // TODO: make this source-chain compatible. - anyhow::Error::from(err.into_box()) + anyhow::Error::from(err.into_chain()) } } @@ -516,3 +519,29 @@ impl SourceError { } } } + +impl From> for ChainedError +where + E: std::error::Error + Send + Sync + 'static, +{ + fn from(mut err: Exn) -> Self { + let stack: VecDeque<_> = err.frame.children.drain(..).collect(); + let location = err.frame.location; + ChainedError { + err: err.into_box(), + location, + source: recurse_source_frames(stack), + } + } +} + +fn recurse_source_frames(mut stack: VecDeque) -> Option> { + let frame = stack.pop_front()?; + stack.extend(frame.children); + Box::new(ChainedError { + err: frame.error, + location: frame.location, + source: recurse_source_frames(stack), + }) + .into() +} diff --git a/gix-error/src/lib.rs b/gix-error/src/lib.rs index c94adbb44fb..8559f7ee5cd 100644 --- a/gix-error/src/lib.rs +++ b/gix-error/src/lib.rs @@ -82,5 +82,10 @@ mod error; /// Various kinds of concrete errors that implement [`std::error::Error`]. mod concrete; +pub use concrete::chain::ChainedError; pub use concrete::message::{message, Message}; pub use concrete::parse::ParseError; + +pub(crate) fn write_location(f: &mut std::fmt::Formatter<'_>, location: &std::panic::Location) -> std::fmt::Result { + write!(f, ", at {}:{}", location.file(), location.line()) +} diff --git a/gix-error/tests/error/error.rs b/gix-error/tests/error/error.rs index b6089d485a8..7bd39a35d6e 100644 --- a/gix-error/tests/error/error.rs +++ b/gix-error/tests/error/error.rs @@ -1,46 +1,46 @@ -use crate::{debug_string, new_tree_error, Error as Simple, ErrorWithSource}; -use gix_error::{Error, ErrorExt}; +use crate::{debug_string, new_tree_error, ErrorWithSource}; +use gix_error::{message, Error, ErrorExt}; use std::error::Error as _; #[test] fn from_exn_error() { - let err = Error::from(Simple("one").raise()); + let err = Error::from(message("one").raise()); assert_eq!(err.to_string(), "one"); - insta::assert_snapshot!(debug_string(&err), @"one, at gix-error/tests/error/error.rs:7:41"); + insta::assert_snapshot!(debug_string(&err), @"one, at gix-error/tests/error/error.rs:7"); insta::assert_debug_snapshot!(err, @"one"); assert_eq!(err.source().map(debug_string), None); } #[test] fn from_exn_error_tree() { - let err = Error::from(new_tree_error().raise(Simple("topmost"))); + let err = Error::from(new_tree_error().raise(message("topmost"))); assert_eq!(err.to_string(), "topmost"); - insta::assert_snapshot!(debug_string(&err), @r" - topmost, at gix-error/tests/error/error.rs:16:44 + insta::assert_snapshot!(debug_string(&err), @" + topmost, at gix-error/tests/error/error.rs:16 | - └─ E6, at gix-error/tests/error/main.rs:25:9 + └─ E6, at gix-error/tests/error/main.rs:25 | - └─ E5, at gix-error/tests/error/main.rs:17:18 + └─ E5, at gix-error/tests/error/main.rs:17 | | - | └─ E3, at gix-error/tests/error/main.rs:9:21 + | └─ E3, at gix-error/tests/error/main.rs:9 | | | - | | └─ E1, at gix-error/tests/error/main.rs:8:30 + | | └─ E1, at gix-error/tests/error/main.rs:8 | | - | └─ E10, at gix-error/tests/error/main.rs:12:22 + | └─ E10, at gix-error/tests/error/main.rs:12 | | | - | | └─ E9, at gix-error/tests/error/main.rs:11:30 + | | └─ E9, at gix-error/tests/error/main.rs:11 | | - | └─ E12, at gix-error/tests/error/main.rs:15:23 + | └─ E12, at gix-error/tests/error/main.rs:15 | | - | └─ E11, at gix-error/tests/error/main.rs:14:32 + | └─ E11, at gix-error/tests/error/main.rs:14 | - └─ E4, at gix-error/tests/error/main.rs:20:21 + └─ E4, at gix-error/tests/error/main.rs:20 | | - | └─ E2, at gix-error/tests/error/main.rs:19:30 + | └─ E2, at gix-error/tests/error/main.rs:19 | - └─ E8, at gix-error/tests/error/main.rs:23:21 + └─ E8, at gix-error/tests/error/main.rs:23 | - └─ E7, at gix-error/tests/error/main.rs:22:30 + └─ E7, at gix-error/tests/error/main.rs:22 "); insta::assert_debug_snapshot!(err, @r" topmost @@ -88,7 +88,7 @@ fn from_exn_error_tree() { "#); assert_eq!( err.source().map(debug_string).as_deref(), - Some(r#"Error("E6")"#), + Some(r#"Message("E6")"#), "The source is the first child" ); assert_eq!( @@ -100,11 +100,11 @@ fn from_exn_error_tree() { #[test] fn from_any_error() { - let err = Error::from_error(Simple("one")); + let err = Error::from_error(message("one")); assert_eq!(err.to_string(), "one"); - assert_eq!(debug_string(&err), r#"Error("one")"#); + assert_eq!(debug_string(&err), r#"Message("one")"#); insta::assert_debug_snapshot!(err, @r#" - Error( + Message( "one", ) "#); @@ -114,20 +114,20 @@ fn from_any_error() { #[test] fn from_any_error_with_source() { - let err = Error::from_error(ErrorWithSource("main", Simple("one"))); + let err = Error::from_error(ErrorWithSource("main", message("one"))); assert_eq!(err.to_string(), "main", "display is the error itself"); - assert_eq!(debug_string(&err), r#"ErrorWithSource("main", Error("one"))"#); + assert_eq!(debug_string(&err), r#"ErrorWithSource("main", Message("one"))"#); insta::assert_debug_snapshot!(err, @r#" ErrorWithSource( "main", - Error( + Message( "one", ), ) "#); assert_eq!( err.source().map(debug_string).as_deref(), - Some(r#"Error("one")"#), + Some(r#"Message("one")"#), "The source is provided by the wrapped error" ); } diff --git a/gix-error/tests/error/exn.rs b/gix-error/tests/error/exn.rs index da44590fc4f..34785e8f8a3 100644 --- a/gix-error/tests/error/exn.rs +++ b/gix-error/tests/error/exn.rs @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -use crate::{debug_string, new_tree_error, Error, ErrorWithSource}; +use crate::{debug_string, fixup_paths, new_tree_error, ErrorWithSource}; use gix_error::OptionExt; use gix_error::ResultExt; use gix_error::{message, ErrorExt}; @@ -20,11 +20,11 @@ use gix_error::{Exn, Message}; #[test] fn raise_chain() { - let e1 = Error("E1").raise(); - let e2 = e1.raise(Error("E2")); - let e3 = e2.raise(Error("E3")); - let e4 = e3.raise(Error("E4")); - let e5 = e4.raise(Error("E5")); + let e1 = message("E1").raise(); + let e2 = e1.raise(message("E2")); + let e3 = e2.raise(message("E3")); + let e4 = e3.raise(message("E4")); + let e5 = e4.raise(message("E5")); insta::assert_debug_snapshot!(e5, @r" E5 | @@ -36,16 +36,16 @@ fn raise_chain() { | └─ E1 "); - insta::assert_snapshot!(debug_string(&e5), @r" - E5, at gix-error/tests/error/exn.rs:27:17 + insta::assert_snapshot!(debug_string(&e5), @" + E5, at gix-error/tests/error/exn.rs:27 | - └─ E4, at gix-error/tests/error/exn.rs:26:17 + └─ E4, at gix-error/tests/error/exn.rs:26 | - └─ E3, at gix-error/tests/error/exn.rs:25:17 + └─ E3, at gix-error/tests/error/exn.rs:25 | - └─ E2, at gix-error/tests/error/exn.rs:24:17 + └─ E2, at gix-error/tests/error/exn.rs:24 | - └─ E1, at gix-error/tests/error/exn.rs:23:26 + └─ E1, at gix-error/tests/error/exn.rs:23 "); let e = e5.erased(); @@ -61,28 +61,28 @@ fn raise_chain() { └─ E1 "); insta::assert_snapshot!(format!("{e:#}"), @r#" - Error("E5") + Message("E5") | - └─ Error("E4") + └─ Message("E4") | - └─ Error("E3") + └─ Message("E3") | - └─ Error("E2") + └─ Message("E2") | - └─ Error("E1") + └─ Message("E1") "#); insta::assert_snapshot!(format!("{e:}"), @"E5"); - insta::assert_snapshot!(debug_string(&e), @r" - E5, at gix-error/tests/error/exn.rs:27:17 + insta::assert_snapshot!(debug_string(&e), @" + E5, at gix-error/tests/error/exn.rs:27 | - └─ E4, at gix-error/tests/error/exn.rs:26:17 + └─ E4, at gix-error/tests/error/exn.rs:26 | - └─ E3, at gix-error/tests/error/exn.rs:25:17 + └─ E3, at gix-error/tests/error/exn.rs:25 | - └─ E2, at gix-error/tests/error/exn.rs:24:17 + └─ E2, at gix-error/tests/error/exn.rs:24 | - └─ E1, at gix-error/tests/error/exn.rs:23:26 + └─ E1, at gix-error/tests/error/exn.rs:23 "); // Double-erase @@ -100,15 +100,15 @@ fn raise_chain() { "); insta::assert_snapshot!(format!("{e:#}"), @r#" - Error("E5") + Message("E5") | - └─ Error("E4") + └─ Message("E4") | - └─ Error("E3") + └─ Message("E3") | - └─ Error("E2") + └─ Message("E2") | - └─ Error("E1") + └─ Message("E1") "#); assert_eq!( e.into_error().probable_cause().to_string(), @@ -119,7 +119,7 @@ fn raise_chain() { #[test] fn raise_all() { - let e = Error("Top").raise_all( + let e = message("Top").raise_all( (1..5).map(|idx| message!("E{}", idx).raise_all((0..idx).map(|sidx| message!("E{}-{}", idx, sidx)))), ); insta::assert_debug_snapshot!(e, @r" @@ -153,36 +153,36 @@ fn raise_all() { | └─ E4-3 "); - insta::assert_snapshot!(debug_string(&e), @r" - Top, at gix-error/tests/error/exn.rs:122:26 + insta::assert_snapshot!(debug_string(&e), @" + Top, at gix-error/tests/error/exn.rs:122 | - └─ E1, at gix-error/tests/error/exn.rs:123:47 + └─ E1, at gix-error/tests/error/exn.rs:123 | | - | └─ E1-0, at gix-error/tests/error/exn.rs:123:47 + | └─ E1-0, at gix-error/tests/error/exn.rs:123 | - └─ E2, at gix-error/tests/error/exn.rs:123:47 + └─ E2, at gix-error/tests/error/exn.rs:123 | | - | └─ E2-0, at gix-error/tests/error/exn.rs:123:47 + | └─ E2-0, at gix-error/tests/error/exn.rs:123 | | - | └─ E2-1, at gix-error/tests/error/exn.rs:123:47 + | └─ E2-1, at gix-error/tests/error/exn.rs:123 | - └─ E3, at gix-error/tests/error/exn.rs:123:47 + └─ E3, at gix-error/tests/error/exn.rs:123 | | - | └─ E3-0, at gix-error/tests/error/exn.rs:123:47 + | └─ E3-0, at gix-error/tests/error/exn.rs:123 | | - | └─ E3-1, at gix-error/tests/error/exn.rs:123:47 + | └─ E3-1, at gix-error/tests/error/exn.rs:123 | | - | └─ E3-2, at gix-error/tests/error/exn.rs:123:47 + | └─ E3-2, at gix-error/tests/error/exn.rs:123 | - └─ E4, at gix-error/tests/error/exn.rs:123:47 + └─ E4, at gix-error/tests/error/exn.rs:123 | - └─ E4-0, at gix-error/tests/error/exn.rs:123:47 + └─ E4-0, at gix-error/tests/error/exn.rs:123 | - └─ E4-1, at gix-error/tests/error/exn.rs:123:47 + └─ E4-1, at gix-error/tests/error/exn.rs:123 | - └─ E4-2, at gix-error/tests/error/exn.rs:123:47 + └─ E4-2, at gix-error/tests/error/exn.rs:123 | - └─ E4-3, at gix-error/tests/error/exn.rs:123:47 + └─ E4-3, at gix-error/tests/error/exn.rs:123 "); let e = e.chain_all((1..3).map(|idx| message!("SE{}", idx))); @@ -223,7 +223,7 @@ fn raise_all() { "); insta::assert_snapshot!(format!("{:#}", e), @r#" - Error("Top") + Message("Top") | └─ Message("E1") | | @@ -257,7 +257,7 @@ fn raise_all() { | └─ Message("SE2") "#); - let _this_should_compile = Error("Top-untyped").raise_all((1..5).map(|idx| message!("E{}", idx).raise_erased())); + let _this_should_compile = message("Top-untyped").raise_all((1..5).map(|idx| message!("E{}", idx).raise_erased())); assert_eq!( e.into_error().probable_cause().to_string(), @@ -268,11 +268,11 @@ fn raise_all() { #[test] fn inverse_error_call_chain() { - let e1 = Error("E1").raise(); - let e2 = e1.chain(Error("E2")); - let e3 = e2.chain(Error("E3")); - let e4 = e3.chain(Error("E4")); - let e5 = e4.chain(Error("E5")); + let e1 = message("E1").raise(); + let e2 = e1.chain(message("E2")); + let e3 = e2.chain(message("E3")); + let e4 = e3.chain(message("E4")); + let e5 = e4.chain(message("E5")); insta::assert_debug_snapshot!(e5, @r" E1 | @@ -284,28 +284,28 @@ fn inverse_error_call_chain() { | └─ E5 "); - insta::assert_snapshot!(debug_string(&e5), @r" - E1, at gix-error/tests/error/exn.rs:271:26 + insta::assert_snapshot!(debug_string(&e5), @" + E1, at gix-error/tests/error/exn.rs:271 | - └─ E2, at gix-error/tests/error/exn.rs:272:17 + └─ E2, at gix-error/tests/error/exn.rs:272 | - └─ E3, at gix-error/tests/error/exn.rs:273:17 + └─ E3, at gix-error/tests/error/exn.rs:273 | - └─ E4, at gix-error/tests/error/exn.rs:274:17 + └─ E4, at gix-error/tests/error/exn.rs:274 | - └─ E5, at gix-error/tests/error/exn.rs:275:17 + └─ E5, at gix-error/tests/error/exn.rs:275 "); insta::assert_snapshot!(format!("{e5:#}"), @r#" - Error("E1") + Message("E1") | - └─ Error("E2") + └─ Message("E2") | - └─ Error("E3") + └─ Message("E3") | - └─ Error("E4") + └─ Message("E4") | - └─ Error("E5") + └─ Message("E5") "#); assert_eq!(e5.into_error().probable_cause().to_string(), "E5"); @@ -339,30 +339,30 @@ fn error_tree() { | └─ E7 "); - insta::assert_snapshot!(debug_string(&err), @r" - E6, at gix-error/tests/error/main.rs:25:9 + insta::assert_snapshot!(debug_string(&err), @" + E6, at gix-error/tests/error/main.rs:25 | - └─ E5, at gix-error/tests/error/main.rs:17:18 + └─ E5, at gix-error/tests/error/main.rs:17 | | - | └─ E3, at gix-error/tests/error/main.rs:9:21 + | └─ E3, at gix-error/tests/error/main.rs:9 | | | - | | └─ E1, at gix-error/tests/error/main.rs:8:30 + | | └─ E1, at gix-error/tests/error/main.rs:8 | | - | └─ E10, at gix-error/tests/error/main.rs:12:22 + | └─ E10, at gix-error/tests/error/main.rs:12 | | | - | | └─ E9, at gix-error/tests/error/main.rs:11:30 + | | └─ E9, at gix-error/tests/error/main.rs:11 | | - | └─ E12, at gix-error/tests/error/main.rs:15:23 + | └─ E12, at gix-error/tests/error/main.rs:15 | | - | └─ E11, at gix-error/tests/error/main.rs:14:32 + | └─ E11, at gix-error/tests/error/main.rs:14 | - └─ E4, at gix-error/tests/error/main.rs:20:21 + └─ E4, at gix-error/tests/error/main.rs:20 | | - | └─ E2, at gix-error/tests/error/main.rs:19:30 + | └─ E2, at gix-error/tests/error/main.rs:19 | - └─ E8, at gix-error/tests/error/main.rs:23:21 + └─ E8, at gix-error/tests/error/main.rs:23 | - └─ E7, at gix-error/tests/error/main.rs:22:30 + └─ E7, at gix-error/tests/error/main.rs:22 "); insta::assert_debug_snapshot!(err.frame().iter_frames().map(ToString::to_string).collect::>(), @r#" [ @@ -381,7 +381,7 @@ fn error_tree() { ] "#); - let new_e = Error("E-New").raise_all(err.drain_children()); + let new_e = message("E-New").raise_all(err.drain_children()); insta::assert_debug_snapshot!(new_e, @r" E-New | @@ -412,36 +412,36 @@ fn error_tree() { #[test] fn result_ext() { - let result: Result<(), Error> = Err(Error("An error")); - let result = result.or_raise(|| Error("Another error")); - insta::assert_snapshot!(debug_string(result.unwrap_err()), @r" - Another error, at gix-error/tests/error/exn.rs:416:25 + let result: Result<(), Message> = Err(message("An error")); + let result = result.or_raise(|| message("Another error")); + insta::assert_snapshot!(debug_string(result.unwrap_err()), @" + Another error, at gix-error/tests/error/exn.rs:416 | - └─ An error, at gix-error/tests/error/exn.rs:416:25 + └─ An error, at gix-error/tests/error/exn.rs:416 "); } #[test] fn option_ext() { let result: Option<()> = None; - let result = result.ok_or_raise(|| Error("An error")); - insta::assert_snapshot!(debug_string(result.unwrap_err()), @"An error, at gix-error/tests/error/exn.rs:427:25"); + let result = result.ok_or_raise(|| message("An error")); + insta::assert_snapshot!(debug_string(result.unwrap_err()), @"An error, at gix-error/tests/error/exn.rs:427"); } #[test] -fn from_error() { - fn foo() -> Result<(), Exn> { - Err(Error("An error"))?; +fn from_message() { + fn foo() -> Result<(), Exn> { + Err(message("An error"))?; Ok(()) } let result = foo(); - insta::assert_snapshot!(debug_string(result.unwrap_err()),@"An error, at gix-error/tests/error/exn.rs:434:9"); + insta::assert_snapshot!(debug_string(result.unwrap_err()),@"An error, at gix-error/tests/error/exn.rs:434"); } #[test] fn new_with_source() { - let e = Exn::new(ErrorWithSource("top", Error("source"))); + let e = Exn::new(ErrorWithSource("top", message("source"))); insta::assert_debug_snapshot!(e,@r" top | @@ -451,18 +451,18 @@ fn new_with_source() { #[test] fn bail() { - fn foo() -> Result<(), Exn> { - gix_error::bail!(Error("An error")); + fn foo() -> Result<(), Exn> { + gix_error::bail!(message("An error")); } let result = foo(); - insta::assert_snapshot!(debug_string(result.unwrap_err()), @"An error, at gix-error/tests/error/exn.rs:455:9"); + insta::assert_snapshot!(debug_string(result.unwrap_err()), @"An error, at gix-error/tests/error/exn.rs:455"); } #[test] fn ensure_ok() { - fn foo() -> Result<(), Exn> { - gix_error::ensure!(true, Error("An error")); + fn foo() -> Result<(), Exn> { + gix_error::ensure!(true, message("An error")); Ok(()) } @@ -471,81 +471,93 @@ fn ensure_ok() { #[test] fn ensure_fail() { - fn foo() -> Result<(), Exn> { - gix_error::ensure!(false, Error("An error")); + fn foo() -> Result<(), Exn> { + gix_error::ensure!(false, message("An error")); Ok(()) } let result = foo(); - insta::assert_snapshot!(debug_string(result.unwrap_err()), @"An error, at gix-error/tests/error/exn.rs:475:9"); + insta::assert_snapshot!(debug_string(result.unwrap_err()), @"An error, at gix-error/tests/error/exn.rs:475"); } #[test] -fn result_ok() -> Result<(), Exn> { +fn result_ok() -> Result<(), Exn> { Ok(()) } #[test] fn erased_into_inner() { - let e = Error("E1").raise().erased(); + let e = message("E1").raise_erased(); let _into_inner_works = e.into_inner(); } #[test] fn erased_into_box() { - let e = Error("E1").raise().erased(); + let e = message("E1").raise_erased(); let _into_box_works = e.into_box(); } #[test] -fn erased_into_error() { - let e = Error("E1").raise().erased(); +fn erased_into_message() { + let e = message("E1").raise().erased(); let _into_error_works = e.into_error(); } #[cfg(feature = "anyhow")] #[test] fn raise_chain_anyhow() { - let e1 = Error("E1") + let e1 = message("E1") .raise() - .chain(Exn::raise_all([Error("E1c1-1"), Error("E1c1-2")], Error("E1-2"))) - .chain(Exn::raise_all([Error("E1c2-1"), Error("E1c2-2")], Error("E1-3"))); - let e2 = e1.raise(Error("E2")); + .chain(Exn::raise_all([message("E1c1-1"), message("E1c1-2")], message("E1-2"))) + .chain(Exn::raise_all([message("E1c2-1"), message("E1c2-2")], message("E1-3"))); + let e2 = e1.raise(message("E2")); let root = e2.raise(Message::new("root")); // It's a linked list as linked up with the first child, but also has multiple children. insta::assert_snapshot!(format!("{root:#}"), @r#" Message("root") | - └─ Error("E2") + └─ Message("E2") | - └─ Error("E1") + └─ Message("E1") | - └─ Error("E1-2") + └─ Message("E1-2") | | - | └─ Error("E1c1-1") + | └─ Message("E1c1-1") | | - | └─ Error("E1c1-2") + | └─ Message("E1c1-2") | - └─ Error("E1-3") + └─ Message("E1-3") | - └─ Error("E1c2-1") + └─ Message("E1c2-1") | - └─ Error("E1c2-2") + └─ Message("E1c2-2") "#); // TODO: this should print the complete error chain. - insta::assert_snapshot!(remove_stackstrace(format!("{:?}", anyhow::Error::from(root))), @"root"); + insta::assert_snapshot!(remove_stackstrace(format!("{:?}", anyhow::Error::from(root))), @" + root, at gix-error/tests/error/exn.rs:514 + + Caused by: + 0: E2, at gix-error/tests/error/exn.rs:513 + 1: E1, at gix-error/tests/error/exn.rs:510 + 2: E1-2, at gix-error/tests/error/exn.rs:511 + 3: E1-3, at gix-error/tests/error/exn.rs:512 + 4: E1c1-1, at gix-error/tests/error/exn.rs:511 + 5: E1c1-2, at gix-error/tests/error/exn.rs:511 + 6: E1c2-1, at gix-error/tests/error/exn.rs:512 + 7: E1c2-2, at gix-error/tests/error/exn.rs:512 + "); } #[cfg(feature = "anyhow")] #[test] fn inverse_error_call_chain_anyhow() { - let e1 = Error("E1").raise(); - let e2 = e1.chain(Error("E2")); - let e3 = e2.chain(Error("E3")); - let e4 = e3.chain(Error("E4")); - let e5 = e4.chain(Error("E5")); + let e1 = message("E1").raise(); + let e2 = e1.chain(message("E2")); + let e3 = e2.chain(message("E3")); + let e4 = e3.chain(message("E4")); + let e5 = e4.chain(message("E5")); insta::assert_debug_snapshot!(e5, @" E1 | @@ -559,9 +571,115 @@ fn inverse_error_call_chain_anyhow() { "); // TODO: this should print the complete error chain. - insta::assert_snapshot!(remove_stackstrace(format!("{:?}", anyhow::Error::from(e5))), @"E1"); + insta::assert_snapshot!(remove_stackstrace(format!("{:?}", anyhow::Error::from(e5))), @" + E1, at gix-error/tests/error/exn.rs:556 + + Caused by: + 0: E2, at gix-error/tests/error/exn.rs:557 + 1: E3, at gix-error/tests/error/exn.rs:558 + 2: E4, at gix-error/tests/error/exn.rs:559 + 3: E5, at gix-error/tests/error/exn.rs:560 + "); } fn remove_stackstrace(s: String) -> String { - s.find("Stack backtrace:").map_or(s.clone(), |pos| s[..pos].into()) + fixup_paths(s.find("Stack backtrace:").map_or(s.clone(), |pos| s[..pos].into())) +} + +#[test] +fn into_chain() { + let e1 = message("E1") + .raise() + .chain(Exn::raise_all([message("E1c1-1"), message("E1c1-2")], message("E1-2"))) + .chain(Exn::raise_all([message("E1c2-1"), message("E1c2-2")], message("E1-3"))); + let e2 = e1.raise(message("E2")); + let root = e2.raise(Message::new("root")); + + insta::assert_snapshot!(format!("{root:#}"), @r#" + Message("root") + | + └─ Message("E2") + | + └─ Message("E1") + | + └─ Message("E1-2") + | | + | └─ Message("E1c1-1") + | | + | └─ Message("E1c1-2") + | + └─ Message("E1-3") + | + └─ Message("E1c2-1") + | + └─ Message("E1c2-2") + "#); + + // It's a linked list as linked up with the first child, but also has multiple children. + let root = root.into_chain(); + // By default, there is paths displayed, just like everywhere. + insta::assert_debug_snapshot!(causes_display(&root, Style::Normal), @r#" + [ + "root, at gix-error/tests/error/exn.rs:596", + "E2, at gix-error/tests/error/exn.rs:595", + "E1, at gix-error/tests/error/exn.rs:592", + "E1-2, at gix-error/tests/error/exn.rs:593", + "E1-3, at gix-error/tests/error/exn.rs:594", + "E1c1-1, at gix-error/tests/error/exn.rs:593", + "E1c1-2, at gix-error/tests/error/exn.rs:593", + "E1c2-1, at gix-error/tests/error/exn.rs:594", + "E1c2-2, at gix-error/tests/error/exn.rs:594", + ] + "#); + + // But these can alos be turned off + insta::assert_debug_snapshot!(causes_display(&root, Style::Alternate), @r#" + [ + "root", + "E2", + "E1", + "E1-2", + "E1-3", + "E1c1-1", + "E1c1-2", + "E1c2-1", + "E1c2-2", + ] + "#); + + // This should look similar. + #[cfg(feature = "anyhow")] + insta::assert_snapshot!(remove_stackstrace(format!("{:?}", anyhow::Error::from(root))), @" + root, at gix-error/tests/error/exn.rs:596 + + Caused by: + 0: E2, at gix-error/tests/error/exn.rs:595 + 1: E1, at gix-error/tests/error/exn.rs:592 + 2: E1-2, at gix-error/tests/error/exn.rs:593 + 3: E1-3, at gix-error/tests/error/exn.rs:594 + 4: E1c1-1, at gix-error/tests/error/exn.rs:593 + 5: E1c1-2, at gix-error/tests/error/exn.rs:593 + 6: E1c2-1, at gix-error/tests/error/exn.rs:594 + 7: E1c2-2, at gix-error/tests/error/exn.rs:594 + "); +} + +enum Style { + Normal, + Alternate, +} + +fn causes_display(err: &(dyn std::error::Error + 'static), style: Style) -> Vec { + let mut out = Vec::new(); + let mut current = Some(err); + while let Some(err) = current { + out.push(fixup_paths(match style { + Style::Normal => err.to_string(), + Style::Alternate => { + format!("{err:#}") + } + })); + current = err.source(); + } + out } diff --git a/gix-error/tests/error/main.rs b/gix-error/tests/error/main.rs index ffa0c948357..f401fb291cf 100644 --- a/gix-error/tests/error/main.rs +++ b/gix-error/tests/error/main.rs @@ -2,51 +2,43 @@ mod error; mod exn; mod utils { - use gix_error::{ErrorExt, Exn}; + use gix_error::{message, ErrorExt, Exn, Message}; - pub fn new_tree_error() -> Exn { - let e1 = Error("E1").raise(); - let e3 = e1.raise(Error("E3")); + pub fn new_tree_error() -> Exn { + let e1 = message("E1").raise(); + let e3 = e1.raise(message("E3")); - let e9 = Error("E9").raise(); - let e10 = e9.raise(Error("E10")); + let e9 = message("E9").raise(); + let e10 = e9.raise(message("E10")); - let e11 = Error("E11").raise(); - let e12 = e11.raise(Error("E12")); + let e11 = message("E11").raise(); + let e12 = e11.raise(message("E12")); - let e5 = Exn::raise_all([e3, e10, e12], Error("E5")); + let e5 = Exn::raise_all([e3, e10, e12], message("E5")); - let e2 = Error("E2").raise(); - let e4 = e2.raise(Error("E4")); + let e2 = message("E2").raise(); + let e4 = e2.raise(message("E4")); - let e7 = Error("E7").raise(); - let e8 = e7.raise(Error("E8")); + let e7 = message("E7").raise(); + let e8 = e7.raise(message("E8")); - Exn::raise_all([e5, e4, e8], Error("E6")) + Exn::raise_all([e5, e4, e8], message("E6")) } - #[derive(Debug)] - pub struct Error(pub &'static str); - - impl std::fmt::Display for Error { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - write!(f, "{}", self.0) - } + pub fn debug_string(input: impl std::fmt::Debug) -> String { + fixup_paths(format!("{input:?}")) } - impl std::error::Error for Error {} - - pub fn debug_string(input: impl std::fmt::Debug) -> String { + pub fn fixup_paths(input: String) -> String { if cfg!(windows) { - let out = format!("{input:?}"); - out.replace('\\', "/") + input.replace('\\', "/") } else { - format!("{input:?}") + input } } #[derive(Debug)] - pub struct ErrorWithSource(pub &'static str, pub Error); + pub struct ErrorWithSource(pub &'static str, pub Message); impl std::fmt::Display for ErrorWithSource { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { From 461c87667c75a9db0a74c43ef68d71b88a7dd754 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Sun, 18 Jan 2026 17:15:56 +0100 Subject: [PATCH 4/7] feat: Add an `auto-chain-error` feature to let `gix-error::Error` produce error chains suitable for `anyhow`. --- .github/workflows/ci.yml | 2 +- gitoxide-core/src/repository/diff.rs | 2 +- gix-error/Cargo.toml | 15 ++- gix-error/src/error.rs | 178 +++++++++++++++++++-------- gix-error/src/lib.rs | 8 ++ gix-error/tests/auto_chain_error.rs | 95 ++++++++++++++ gix-error/tests/error/error.rs | 2 +- gix-error/tests/error/exn.rs | 48 ++++---- justfile | 1 + 9 files changed, 268 insertions(+), 83 deletions(-) create mode 100644 gix-error/tests/auto_chain_error.rs diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 25ab4e0e06e..52e7a623780 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -341,7 +341,7 @@ jobs: GIX_TEST_IGNORE_ARCHIVES: '1' TEST_ARGS: ${{ matrix.test-args }} run: | - cargo nextest --profile=with-xml run --workspace --no-fail-fast -- ` + cargo nextest --profile=with-xml run --workspace --no-fail-fast --exclude gix-error -- ` (-split $Env:TEST_ARGS) continue-on-error: true - name: Check for errors diff --git a/gitoxide-core/src/repository/diff.rs b/gitoxide-core/src/repository/diff.rs index e90f777bec4..261e6eaf551 100644 --- a/gitoxide-core/src/repository/diff.rs +++ b/gitoxide-core/src/repository/diff.rs @@ -129,7 +129,7 @@ fn resolve_revspec( // When the revspec is just a name, the delegate tries to resolve a reference which fails. // We extract the error from the tree to learn the name, and treat it as file. let not_found = err - .iter_frames() + .sources() .find_map(|f| f.error().downcast_ref::()); if let Some(gix::refs::file::find::existing::Error::NotFound { name }) = not_found { let root = repo.workdir().map(ToOwned::to_owned); diff --git a/gix-error/Cargo.toml b/gix-error/Cargo.toml index 5969fe1c1b4..29605b981b7 100644 --- a/gix-error/Cargo.toml +++ b/gix-error/Cargo.toml @@ -12,8 +12,21 @@ include = ["src/**/*", "LICENSE-*"] rust-version = "1.82" [features] -## If enabled, the `Exn` error type converts to `anyhow::Error` natively so `?` can be used directly. +## The [`Exn`](crate::Exn) type converts to [`anyhow::Error`] natively so `?` can be used directly. +## +## Otherwise, it would have to be manually converted via +## [`into_box()`](crate::Exn::into_box()) or [`into_inner()`](crate::Exn::into_inner()). anyhow = ["dep:anyhow"] +## The [`Error`](crate::Error) type is always flattening the [`Exn`](crate::Exn) error tree +## into a chain of errors, while keeping there locations and runtime type-information. +auto-chain-error = [] +## The opposite of `auto-chain-error` and implicitly enabled by default. Use it to override `auto-chain-error`. +tree-error = [] + +[[test]] +name = "auto-chain-error" +path = "tests/auto_chain_error.rs" +required-features = ["auto-chain-error"] [dependencies] bstr = { version = "1.12.0", default-features = false, features = ["std"] } diff --git a/gix-error/src/error.rs b/gix-error/src/error.rs index 509654d956e..8d8e07346a6 100644 --- a/gix-error/src/error.rs +++ b/gix-error/src/error.rs @@ -1,78 +1,148 @@ -use crate::{exn, Error, Exn}; -use std::fmt::Formatter; - -/// Utilities -impl Error { - /// Return the error that is most likely the root cause, based on heuristics. - /// Note that if there is nothing but this error, i.e. no source or children, this error is returned. - pub fn probable_cause(&self) -> &(dyn std::error::Error + 'static) { - let root = self.inner.frame(); - root.probable_cause().unwrap_or(root).error() +#[cfg(any(feature = "tree-error", not(feature = "auto-chain-error")))] +mod _impl { + use crate::{Error, Exn}; + use std::fmt::Formatter; + + /// Utilities + impl Error { + /// Return the error that is most likely the root cause, based on heuristics. + /// Note that if there is nothing but this error, i.e. no source or children, this error is returned. + pub fn probable_cause(&self) -> &(dyn std::error::Error + 'static) { + let root = self.inner.frame(); + root.probable_cause().unwrap_or(root).error() + } + + /// Return an iterator over all errors in the tree in breadth-first order, starting with this one. + pub fn sources(&self) -> impl Iterator + '_ { + self.inner.frame().iter_frames().map(|f| f.error() as _) + } } - /// Return an iterator over all errors in the tree in breadth-first order, starting with this one. - pub fn iter_frames(&self) -> impl Iterator + '_ { - self.inner.frame().iter_frames() + pub(crate) enum Inner { + ExnAsError(Box), + Exn(Box), } -} -pub(super) enum Inner { - ExnAsError(Box), - Exn(Box), -} + impl Inner { + fn frame(&self) -> &crate::exn::Frame { + match self { + Inner::ExnAsError(f) | Inner::Exn(f) => f, + } + } + } -impl Inner { - fn frame(&self) -> &exn::Frame { - match self { - Inner::ExnAsError(f) | Inner::Exn(f) => f, + impl Error { + /// Create a new instance representing the given `error`. + #[track_caller] + pub fn from_error(error: impl std::error::Error + Send + Sync + 'static) -> Self { + Error { + inner: Inner::ExnAsError(Exn::new(error).into()), + } } } -} -impl Error { - /// Create a new instance representing the given `error`. - #[track_caller] - pub fn from_error(error: impl std::error::Error + Send + Sync + 'static) -> Self { - Error { - inner: Inner::ExnAsError(Exn::new(error).into()), + impl std::fmt::Display for Error { + fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { + match &self.inner { + Inner::ExnAsError(err) => std::fmt::Display::fmt(err.error(), f), + Inner::Exn(frame) => std::fmt::Display::fmt(frame, f), + } } } -} -impl std::fmt::Display for Error { - fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { - match &self.inner { - Inner::ExnAsError(err) => std::fmt::Display::fmt(err.error(), f), - Inner::Exn(frame) => std::fmt::Display::fmt(frame, f), + impl std::fmt::Debug for Error { + fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { + match &self.inner { + Inner::ExnAsError(err) => std::fmt::Debug::fmt(err.error(), f), + Inner::Exn(frame) => std::fmt::Debug::fmt(frame, f), + } } } -} -impl std::fmt::Debug for Error { - fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { - match &self.inner { - Inner::ExnAsError(err) => std::fmt::Debug::fmt(err.error(), f), - Inner::Exn(frame) => std::fmt::Debug::fmt(frame, f), + impl std::error::Error for Error { + /// Return the first source of an [Exn] error, or the source of a boxed error. + fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { + match &self.inner { + Inner::ExnAsError(frame) | Inner::Exn(frame) => frame.children().first().map(|f| f.error() as _), + } } } -} -impl std::error::Error for Error { - /// Return the first source of an [Exn] error, or the source of a boxed error. - fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { - match &self.inner { - Inner::ExnAsError(frame) | Inner::Exn(frame) => frame.children().first().map(|f| f.error() as _), + impl From> for Error + where + E: std::error::Error + Send + Sync + 'static, + { + fn from(err: Exn) -> Self { + Error { + inner: Inner::Exn(err.into()), + } } } } +#[cfg(any(feature = "tree-error", not(feature = "auto-chain-error")))] +pub(super) use _impl::Inner; + +#[cfg(all(feature = "auto-chain-error", not(feature = "tree-error")))] +mod _impl { + use crate::{Error, Exn}; + use std::fmt::Formatter; + + /// Utilities + impl Error { + /// Return the error that is most likely the root cause, based on heuristics. + /// Note that if there is nothing but this error, i.e. no source or children, this error is returned. + pub fn probable_cause(&self) -> &(dyn std::error::Error + 'static) { + use std::error::Error; + self.inner + .source() + .unwrap_or(self as &(dyn std::error::Error + 'static)) + } + + /// Return an iterator over all errors in the tree in breadth-first order, starting with this one. + pub fn sources(&self) -> impl Iterator + '_ { + std::iter::successors(Some(&self.inner as &(dyn std::error::Error + 'static)), |err| { + err.source() + }) + } + } + + impl Error { + /// Create a new instance representing the given `error`. + #[track_caller] + pub fn from_error(error: impl std::error::Error + Send + Sync + 'static) -> Self { + Error { + inner: Exn::new(error).into_chain(), + } + } + } + + impl std::fmt::Display for Error { + fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { + std::fmt::Display::fmt(&self.inner, f) + } + } + + impl std::fmt::Debug for Error { + fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { + std::fmt::Debug::fmt(&self.inner, f) + } + } + + impl std::error::Error for Error { + /// Return the first source of an [Exn] error, or the source of a boxed error. + fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { + self.inner.source() + } + } -impl From> for Error -where - E: std::error::Error + Send + Sync + 'static, -{ - fn from(err: Exn) -> Self { - Error { - inner: Inner::Exn(err.into()), + impl From> for Error + where + E: std::error::Error + Send + Sync + 'static, + { + fn from(err: Exn) -> Self { + Error { + inner: err.into_chain(), + } } } } diff --git a/gix-error/src/lib.rs b/gix-error/src/lib.rs index 8559f7ee5cd..f424c26e9f1 100644 --- a/gix-error/src/lib.rs +++ b/gix-error/src/lib.rs @@ -74,8 +74,16 @@ pub use exn::{ErrorExt, Exn, Frame, OptionExt, ResultExt, Something, Untyped}; /// All `source()` values when created with [`Error::from_error()`] are turned into frames, /// but lose their type information completely. /// This is because they are only seen as reference and thus can't be stored. +/// +/// # The `auto-chain-error` feature +/// +/// If it's enabled, this type is merely a wrapper around [`ChainedError`]. This happens automatically +/// so applications that require this don't have to go through an extra conversion. pub struct Error { + #[cfg(any(feature = "tree-error", not(feature = "auto-chain-error")))] inner: error::Inner, + #[cfg(all(feature = "auto-chain-error", not(feature = "tree-error")))] + inner: ChainedError, } mod error; diff --git a/gix-error/tests/auto_chain_error.rs b/gix-error/tests/auto_chain_error.rs new file mode 100644 index 00000000000..cc46eb951e9 --- /dev/null +++ b/gix-error/tests/auto_chain_error.rs @@ -0,0 +1,95 @@ +use gix_error::{message, Error, ErrorExt, Exn, Message}; +use std::error::Error as _; + +#[test] +fn from_exn_error() { + let err = Error::from(message("one").raise()); + assert_eq!(format!("{err:#}"), "one"); + insta::assert_snapshot!(debug_string(&err), @r#"Message("one")"#); + insta::assert_debug_snapshot!(err, @r#" + Message( + "one", + ) + "#); + assert_eq!(err.source().map(debug_string), None); +} + +#[test] +fn from_exn_error_tree() { + let err = Error::from(new_tree_error().raise(message("topmost"))); + assert_eq!(format!("{err:#}").to_string(), "topmost"); + insta::assert_debug_snapshot!(err.sources().map(|err| fixup_paths(err.to_string())).collect::>(), @r#" + [ + "topmost, at gix-error/tests/auto_chain_error.rs:19", + "E6, at gix-error/tests/auto_chain_error.rs:82", + "E5, at gix-error/tests/auto_chain_error.rs:74", + "E4, at gix-error/tests/auto_chain_error.rs:77", + "E8, at gix-error/tests/auto_chain_error.rs:80", + "E3, at gix-error/tests/auto_chain_error.rs:66", + "E10, at gix-error/tests/auto_chain_error.rs:69", + "E12, at gix-error/tests/auto_chain_error.rs:72", + "E2, at gix-error/tests/auto_chain_error.rs:76", + "E7, at gix-error/tests/auto_chain_error.rs:79", + "E1, at gix-error/tests/auto_chain_error.rs:65", + "E9, at gix-error/tests/auto_chain_error.rs:68", + "E11, at gix-error/tests/auto_chain_error.rs:71", + ] + "#); + assert_eq!( + err.source().map(debug_string).as_deref(), + Some(r#"Message("E6")"#), + "The source is the first child" + ); + assert_eq!( + format!("{:#}", err.probable_cause()), + "E6", + "we get the top-most error that has most causes" + ); +} + +#[test] +fn from_any_error() { + let err = Error::from_error(message("one")); + assert_eq!(format!("{err:#}"), "one"); + assert_eq!(debug_string(&err), r#"Message("one")"#); + insta::assert_debug_snapshot!(err, @r#" + Message( + "one", + ) + "#); + assert_eq!(err.source().map(debug_string), None); + assert_eq!(format!("{:#}", err.probable_cause()), "one"); +} + +pub fn new_tree_error() -> Exn { + let e1 = message("E1").raise(); + let e3 = e1.raise(message("E3")); + + let e9 = message("E9").raise(); + let e10 = e9.raise(message("E10")); + + let e11 = message("E11").raise(); + let e12 = e11.raise(message("E12")); + + let e5 = Exn::raise_all([e3, e10, e12], message("E5")); + + let e2 = message("E2").raise(); + let e4 = e2.raise(message("E4")); + + let e7 = message("E7").raise(); + let e8 = e7.raise(message("E8")); + + Exn::raise_all([e5, e4, e8], message("E6")) +} + +pub fn debug_string(input: impl std::fmt::Debug) -> String { + fixup_paths(format!("{input:?}")) +} + +fn fixup_paths(input: String) -> String { + if cfg!(windows) { + input.replace('\\', "/") + } else { + input + } +} diff --git a/gix-error/tests/error/error.rs b/gix-error/tests/error/error.rs index 7bd39a35d6e..bde73fb403f 100644 --- a/gix-error/tests/error/error.rs +++ b/gix-error/tests/error/error.rs @@ -69,7 +69,7 @@ fn from_exn_error_tree() { | └─ E7 "); - insta::assert_debug_snapshot!(err.iter_frames().map(ToString::to_string).collect::>(), @r#" + insta::assert_debug_snapshot!(err.sources().map(ToString::to_string).collect::>(), @r#" [ "topmost", "E6", diff --git a/gix-error/tests/error/exn.rs b/gix-error/tests/error/exn.rs index 34785e8f8a3..a7df071015d 100644 --- a/gix-error/tests/error/exn.rs +++ b/gix-error/tests/error/exn.rs @@ -534,7 +534,6 @@ fn raise_chain_anyhow() { └─ Message("E1c2-2") "#); - // TODO: this should print the complete error chain. insta::assert_snapshot!(remove_stackstrace(format!("{:?}", anyhow::Error::from(root))), @" root, at gix-error/tests/error/exn.rs:514 @@ -570,15 +569,14 @@ fn inverse_error_call_chain_anyhow() { └─ E5 "); - // TODO: this should print the complete error chain. insta::assert_snapshot!(remove_stackstrace(format!("{:?}", anyhow::Error::from(e5))), @" - E1, at gix-error/tests/error/exn.rs:556 + E1, at gix-error/tests/error/exn.rs:555 Caused by: - 0: E2, at gix-error/tests/error/exn.rs:557 - 1: E3, at gix-error/tests/error/exn.rs:558 - 2: E4, at gix-error/tests/error/exn.rs:559 - 3: E5, at gix-error/tests/error/exn.rs:560 + 0: E2, at gix-error/tests/error/exn.rs:556 + 1: E3, at gix-error/tests/error/exn.rs:557 + 2: E4, at gix-error/tests/error/exn.rs:558 + 3: E5, at gix-error/tests/error/exn.rs:559 "); } @@ -620,15 +618,15 @@ fn into_chain() { // By default, there is paths displayed, just like everywhere. insta::assert_debug_snapshot!(causes_display(&root, Style::Normal), @r#" [ - "root, at gix-error/tests/error/exn.rs:596", - "E2, at gix-error/tests/error/exn.rs:595", - "E1, at gix-error/tests/error/exn.rs:592", - "E1-2, at gix-error/tests/error/exn.rs:593", - "E1-3, at gix-error/tests/error/exn.rs:594", - "E1c1-1, at gix-error/tests/error/exn.rs:593", - "E1c1-2, at gix-error/tests/error/exn.rs:593", - "E1c2-1, at gix-error/tests/error/exn.rs:594", - "E1c2-2, at gix-error/tests/error/exn.rs:594", + "root, at gix-error/tests/error/exn.rs:594", + "E2, at gix-error/tests/error/exn.rs:593", + "E1, at gix-error/tests/error/exn.rs:590", + "E1-2, at gix-error/tests/error/exn.rs:591", + "E1-3, at gix-error/tests/error/exn.rs:592", + "E1c1-1, at gix-error/tests/error/exn.rs:591", + "E1c1-2, at gix-error/tests/error/exn.rs:591", + "E1c2-1, at gix-error/tests/error/exn.rs:592", + "E1c2-2, at gix-error/tests/error/exn.rs:592", ] "#); @@ -650,17 +648,17 @@ fn into_chain() { // This should look similar. #[cfg(feature = "anyhow")] insta::assert_snapshot!(remove_stackstrace(format!("{:?}", anyhow::Error::from(root))), @" - root, at gix-error/tests/error/exn.rs:596 + root, at gix-error/tests/error/exn.rs:594 Caused by: - 0: E2, at gix-error/tests/error/exn.rs:595 - 1: E1, at gix-error/tests/error/exn.rs:592 - 2: E1-2, at gix-error/tests/error/exn.rs:593 - 3: E1-3, at gix-error/tests/error/exn.rs:594 - 4: E1c1-1, at gix-error/tests/error/exn.rs:593 - 5: E1c1-2, at gix-error/tests/error/exn.rs:593 - 6: E1c2-1, at gix-error/tests/error/exn.rs:594 - 7: E1c2-2, at gix-error/tests/error/exn.rs:594 + 0: E2, at gix-error/tests/error/exn.rs:593 + 1: E1, at gix-error/tests/error/exn.rs:590 + 2: E1-2, at gix-error/tests/error/exn.rs:591 + 3: E1-3, at gix-error/tests/error/exn.rs:592 + 4: E1c1-1, at gix-error/tests/error/exn.rs:591 + 5: E1c1-2, at gix-error/tests/error/exn.rs:591 + 6: E1c2-1, at gix-error/tests/error/exn.rs:592 + 7: E1c2-2, at gix-error/tests/error/exn.rs:592 "); } diff --git a/justfile b/justfile index 5a23f6317fb..57d2a97ce82 100755 --- a/justfile +++ b/justfile @@ -157,6 +157,7 @@ unit-tests: cargo nextest run -p gix-status-tests --features gix-features-parallel --no-fail-fast cargo nextest run -p gix-worktree-state-tests --features gix-features-parallel --no-fail-fast cargo nextest run -p gix-worktree-tests --features gix-features-parallel --no-fail-fast + cargo nextest run -p gix-error --no-fail-fast --test auto-chain-error --features auto-chain-error cargo nextest run -p gix-hash --no-fail-fast cargo nextest run -p gix-hash --features sha256 --no-fail-fast cargo nextest run -p gix-hash --no-default-features --features sha256 --no-fail-fast # TODO: make this actually work by removing 'sha1' from default features. From 00c053cbc50b61d5168bc56e7808e5229ee5fe8d Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Sun, 18 Jan 2026 20:43:40 +0100 Subject: [PATCH 5/7] Adapt to changes in `gix-error` --- gitoxide-core/src/repository/diff.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gitoxide-core/src/repository/diff.rs b/gitoxide-core/src/repository/diff.rs index 261e6eaf551..bfbe8c28c91 100644 --- a/gitoxide-core/src/repository/diff.rs +++ b/gitoxide-core/src/repository/diff.rs @@ -130,7 +130,7 @@ fn resolve_revspec( // We extract the error from the tree to learn the name, and treat it as file. let not_found = err .sources() - .find_map(|f| f.error().downcast_ref::()); + .find_map(|err| err.downcast_ref::()); if let Some(gix::refs::file::find::existing::Error::NotFound { name }) = not_found { let root = repo.workdir().map(ToOwned::to_owned); let name = gix::path::os_string_into_bstring(name.into())?; From 08d7966bc50eaf5ca4204cb4fe84129fd75cfa32 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Sun, 18 Jan 2026 20:46:08 +0100 Subject: [PATCH 6/7] feat: Enable `auto-chain-error` by default. This way, the behaviour of `gix::Error` is the same as it was before without actually exposing an `Exn` error tree. --- .github/workflows/ci.yml | 4 +++- gix/Cargo.toml | 10 ++++++++-- justfile | 1 + 3 files changed, 12 insertions(+), 3 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 52e7a623780..7495669d1d4 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -280,8 +280,10 @@ jobs: # due to being more in line with some Unix-oriented assumptions. So we need to verify that # the test suite is compatible with being run even outside that environment, including that # `gix-testtools` is still able to run fixture scripts with the `bash` shell as intended. + # `gix-error` is excluded because it gets compiled with the wrong feature toggles here. + # It's tested individually. run: | # zizmor: ignore[template-injection] - cargo nextest run --workspace --no-fail-fast -- ${{ matrix.test-args }} + cargo nextest run --workspace --no-fail-fast --exclude gix-error -- ${{ matrix.test-args }} - name: Check that tracked archives are up to date run: | # If this fails, the fix is usually to commit a regenerated archive. diff --git a/gix/Cargo.toml b/gix/Cargo.toml index 1f7660316ff..0f2a8b8abb0 100644 --- a/gix/Cargo.toml +++ b/gix/Cargo.toml @@ -24,7 +24,7 @@ required-features = ["blocking-network-client"] [features] -default = ["max-performance-safe", "comfort", "basic", "extras"] +default = ["max-performance-safe", "comfort", "basic", "extras", "auto-chain-error"] #! There are various categories of features which help to optimize performance and build times. `gix` comes with 'batteries included' and everything is #! enabled as long as it doesn't sacrifice compatibility. Most users will be fine with that but will pay with higher compile times than necessary as they @@ -272,6 +272,12 @@ zlib-stock = ["gix-features/zlib"] #! #! The catch-all of feature toggles. +## All [`gix::Error`](crate::Error) will produce error chains, which makes them work as expected in conjunction with `anyhow`. +auto-chain-error = ["gix-error/auto-chain-error"] + +## The opposite of `auto-chain-error`. Use it to override `auto-chain-error`. +tree-error = ["gix-error/tree-error"] + ## Enable tracing using the `tracing` crate for coarse tracing. tracing = ["gix-features/tracing"] @@ -407,7 +413,7 @@ document-features = { version = "0.2.0", optional = true } [dev-dependencies] # For additional features that aren't enabled by default due to MSRV gix = { path = ".", default-features = false, features = [ - "need-more-recent-msrv", + "need-more-recent-msrv", "tree-error" ] } pretty_assertions = "1.4.0" gix-testtools = { path = "../tests/tools" } diff --git a/justfile b/justfile index 57d2a97ce82..1514bf95cd9 100755 --- a/justfile +++ b/justfile @@ -158,6 +158,7 @@ unit-tests: cargo nextest run -p gix-worktree-state-tests --features gix-features-parallel --no-fail-fast cargo nextest run -p gix-worktree-tests --features gix-features-parallel --no-fail-fast cargo nextest run -p gix-error --no-fail-fast --test auto-chain-error --features auto-chain-error + cargo nextest run -p gix-error --no-fail-fast cargo nextest run -p gix-hash --no-fail-fast cargo nextest run -p gix-hash --features sha256 --no-fail-fast cargo nextest run -p gix-hash --no-default-features --features sha256 --no-fail-fast # TODO: make this actually work by removing 'sha1' from default features. From 16327efe24e2321e2a4efe5321e9f0483484b10a Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Sun, 18 Jan 2026 22:13:02 +0100 Subject: [PATCH 7/7] Address Copilot review --- .github/workflows/ci.yml | 2 +- gix-error/Cargo.toml | 2 +- gix-error/src/lib.rs | 3 +++ gix-error/tests/error/exn.rs | 2 +- 4 files changed, 6 insertions(+), 3 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 7495669d1d4..ac8dbc5767f 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -429,7 +429,7 @@ jobs: - name: Test (nextest) env: GIX_TEST_IGNORE_ARCHIVES: '1' - run: cargo nextest run --workspace --no-fail-fast + run: cargo nextest run --workspace --no-fail-fast --exclude gix-error test-32bit-windows-size-doc: runs-on: windows-latest diff --git a/gix-error/Cargo.toml b/gix-error/Cargo.toml index 29605b981b7..3ee22be5762 100644 --- a/gix-error/Cargo.toml +++ b/gix-error/Cargo.toml @@ -18,7 +18,7 @@ rust-version = "1.82" ## [`into_box()`](crate::Exn::into_box()) or [`into_inner()`](crate::Exn::into_inner()). anyhow = ["dep:anyhow"] ## The [`Error`](crate::Error) type is always flattening the [`Exn`](crate::Exn) error tree -## into a chain of errors, while keeping there locations and runtime type-information. +## into a chain of errors, while keeping their locations and runtime type-information. auto-chain-error = [] ## The opposite of `auto-chain-error` and implicitly enabled by default. Use it to override `auto-chain-error`. tree-error = [] diff --git a/gix-error/src/lib.rs b/gix-error/src/lib.rs index f424c26e9f1..e78899f52f8 100644 --- a/gix-error/src/lib.rs +++ b/gix-error/src/lib.rs @@ -79,6 +79,9 @@ pub use exn::{ErrorExt, Exn, Frame, OptionExt, ResultExt, Something, Untyped}; /// /// If it's enabled, this type is merely a wrapper around [`ChainedError`]. This happens automatically /// so applications that require this don't have to go through an extra conversion. +/// +/// When both the `tree-error` and `auto-chain-error` features are enabled, the `tree-error` +/// behavior takes precedence and this type uses the tree-based representation. pub struct Error { #[cfg(any(feature = "tree-error", not(feature = "auto-chain-error")))] inner: error::Inner, diff --git a/gix-error/tests/error/exn.rs b/gix-error/tests/error/exn.rs index a7df071015d..f50cd3d69ed 100644 --- a/gix-error/tests/error/exn.rs +++ b/gix-error/tests/error/exn.rs @@ -630,7 +630,7 @@ fn into_chain() { ] "#); - // But these can alos be turned off + // But these can also be turned off insta::assert_debug_snapshot!(causes_display(&root, Style::Alternate), @r#" [ "root",