From f69a6779c85bf5c4bd8faa6bcacbb1cd7d78cb79 Mon Sep 17 00:00:00 2001 From: Bryan Helmkamp Date: Fri, 11 Sep 2026 14:56:34 -0600 Subject: [PATCH] Shrink fabro-sandbox's error module to what callers use After the driver rounds, fabro-sandbox's Error keeps four variants: Message, Context, AnyhowContext, and Driver. The module still carried helpers written for callers that never arrived: incomplete_operation, is_transport, and is_unsupported classified driver variants nothing in fabro branches on; From and From<&str> let a bare string become an error, which no call site did; driver_error duplicated the From impl; exec_failure and is_not_found had only test callers, and those tests can match on the driver error directly. This removes them. Callers build a Driver error through Error::from, and the two tests that inspected a failure now match on Error::driver(). The Driver variant's doc names the driver variants fabro does act on: Exec, Git, and NotFound. The redaction and log-rendering helpers stay; they are what the error module is for. Co-Authored-By: Claude Fable 5.1 --- lib/components/fabro-sandbox/src/clone.rs | 4 +- .../fabro-sandbox/src/driver_sandbox.rs | 5 +- lib/components/fabro-sandbox/src/error.rs | 70 +++---------------- lib/components/fabro-sandbox/src/exec.rs | 6 +- lib/components/fabro-sandbox/src/sandbox.rs | 4 +- 5 files changed, 19 insertions(+), 70 deletions(-) diff --git a/lib/components/fabro-sandbox/src/clone.rs b/lib/components/fabro-sandbox/src/clone.rs index 0b2b0e6ab..0d0f38226 100644 --- a/lib/components/fabro-sandbox/src/clone.rs +++ b/lib/components/fabro-sandbox/src/clone.rs @@ -118,7 +118,7 @@ pub(crate) async fn clone_github_repo( .await .map_err(|failure| { clone_failure_error( - crate::Error::driver_error(failure.error), + crate::Error::from(failure.error), CloneStep::Network, has_app, ) @@ -331,7 +331,7 @@ mod tests { } fn git_failure(exit_code: i32, stderr: &str) -> crate::Error { - crate::Error::driver_error(sandbox_driver::Error::Git(GitFailure::from_command( + crate::Error::from(sandbox_driver::Error::Git(GitFailure::from_command( "git clone", ExecFailure::new( "git clone", diff --git a/lib/components/fabro-sandbox/src/driver_sandbox.rs b/lib/components/fabro-sandbox/src/driver_sandbox.rs index c72f3d56e..aadca5d0b 100644 --- a/lib/components/fabro-sandbox/src/driver_sandbox.rs +++ b/lib/components/fabro-sandbox/src/driver_sandbox.rs @@ -1189,7 +1189,10 @@ mod tests { .read_file("nonexistent.txt", None, None) .await .unwrap_err(); - assert!(read.is_not_found(), "{read}"); + assert!( + matches!(read.driver(), Some(sandbox_driver::Error::NotFound { .. })), + "{read}" + ); } #[tokio::test] diff --git a/lib/components/fabro-sandbox/src/error.rs b/lib/components/fabro-sandbox/src/error.rs index 45c6b4f01..eabd62f32 100644 --- a/lib/components/fabro-sandbox/src/error.rs +++ b/lib/components/fabro-sandbox/src/error.rs @@ -25,8 +25,8 @@ pub enum Error { /// A sandbox-driver failure: provider, transport, or an operation whose /// outcome is unknown. The driver's own variants stay reachable through - /// [`Error::driver`] so callers can act on `NotFound`, `Unsupported`, - /// `Transport`, and `Incomplete` without string matching. + /// [`Error::driver`] so callers can act on `Exec`, `Git`, and `NotFound` + /// without string matching. #[error(transparent)] Driver(Box), } @@ -61,10 +61,6 @@ impl Error { collect_causes(self) } - pub fn driver_error(source: sandbox_driver::Error) -> Self { - Self::Driver(Box::new(source)) - } - /// The underlying sandbox-driver error, when this error carries one /// anywhere in its chain. pub fn driver(&self) -> Option<&sandbox_driver::Error> { @@ -81,46 +77,6 @@ impl Error { None } - /// The command that ran and failed, when this error reports one: a - /// non-zero exit fabro turned into an error, or a driver operation whose - /// command failed underneath it. - pub fn exec_failure(&self) -> Option<&sandbox_driver::ExecFailure> { - match self.driver()? { - sandbox_driver::Error::Exec(failure) => Some(failure), - _ => None, - } - } - - /// The facts established when a driver operation ended without a - /// complete outcome. A caller that sees `Some` must not replay the - /// operation: its effects may already have happened. - pub fn incomplete_operation(&self) -> Option<&sandbox_driver::IncompleteOperation> { - match self.driver()? { - sandbox_driver::Error::Incomplete(incomplete) => Some(incomplete), - _ => None, - } - } - - /// True when communication with an out-of-process provider failed. The - /// operation may or may not have run; fabro rebuilds handles through - /// `attach` rather than retrying blind. - pub fn is_transport(&self) -> bool { - matches!(self.driver(), Some(sandbox_driver::Error::Transport(_))) - } - - /// True when the driver reported the resource missing. - pub fn is_not_found(&self) -> bool { - matches!(self.driver(), Some(sandbox_driver::Error::NotFound { .. })) - } - - /// True when the provider does not support the requested capability. - pub fn is_unsupported(&self) -> bool { - matches!( - self.driver(), - Some(sandbox_driver::Error::Unsupported { .. }) - ) - } - pub fn display_with_causes(&self) -> String { render_with_causes(&self.to_string(), &self.causes()) } @@ -128,19 +84,7 @@ impl Error { impl From for Error { fn from(value: sandbox_driver::Error) -> Self { - Self::driver_error(value) - } -} - -impl From for Error { - fn from(value: String) -> Self { - Self::Message(value) - } -} - -impl From<&str> for Error { - fn from(value: &str) -> Self { - Self::Message(value.to_string()) + Self::Driver(Box::new(value)) } } @@ -259,14 +203,16 @@ mod tests { } #[test] - fn exec_failure_is_reachable_through_the_context_chain() { + fn the_driver_error_is_reachable_through_the_context_chain() { let error = Error::context("metadata push failed", failed_push("", "boom")); - let failure = error.exec_failure().expect("exec failure"); + let Some(sandbox_driver::Error::Exec(failure)) = error.driver() else { + panic!("expected an exec failure, got {error:?}"); + }; assert_eq!(failure.label(), "git push origin refs/heads/run"); assert_eq!(failure.exit_code(), Some(128)); assert_eq!(failure.termination(), Termination::Exited); - assert!(Error::message("plain").exec_failure().is_none()); + assert!(Error::message("plain").driver().is_none()); } #[test] diff --git a/lib/components/fabro-sandbox/src/exec.rs b/lib/components/fabro-sandbox/src/exec.rs index 0216d0a2d..87b52000f 100644 --- a/lib/components/fabro-sandbox/src/exec.rs +++ b/lib/components/fabro-sandbox/src/exec.rs @@ -265,7 +265,7 @@ impl ExecResultExt for ExecResult { self.stderr, ) .with_duration(self.duration); - crate::Error::driver_error(failure.into()) + crate::Error::from(sandbox_driver::Error::from(failure)) } fn into_result(self, label: impl Into) -> crate::Result { @@ -631,7 +631,9 @@ mod tests { 42, ); let error = result.into_result("git push").unwrap_err(); - let failure = error.exec_failure().expect("exec failure"); + let Some(sandbox_driver::Error::Exec(failure)) = error.driver() else { + panic!("expected an exec failure, got {error:?}"); + }; assert_eq!(failure.label(), "git push"); assert_eq!(failure.exit_code(), Some(128)); assert_eq!(failure.duration(), Some(Duration::from_millis(42))); diff --git a/lib/components/fabro-sandbox/src/sandbox.rs b/lib/components/fabro-sandbox/src/sandbox.rs index d4d3f04e9..8b64ea0e1 100644 --- a/lib/components/fabro-sandbox/src/sandbox.rs +++ b/lib/components/fabro-sandbox/src/sandbox.rs @@ -387,9 +387,7 @@ fn push_attempts( .map(|(index, attempt)| { let is_last = index + 1 == last; let exec_output_tail = match (attempt.failure, &outcome) { - (Some(failure), _) => { - crate::Error::driver_error(failure).default_redacted_output_tail() - } + (Some(failure), _) => crate::Error::from(failure).default_redacted_output_tail(), (None, Err(error)) if is_last => error.default_redacted_output_tail(), (None, _) => None, };