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<String> 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 <noreply@anthropic.com>
This commit is contained in:
Bryan Helmkamp 2026-09-11 14:56:34 -06:00
parent 99d226250e
commit f69a6779c8
No known key found for this signature in database
5 changed files with 19 additions and 70 deletions

View file

@ -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",

View file

@ -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]

View file

@ -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<sandbox_driver::Error>),
}
@ -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<sandbox_driver::Error> for Error {
fn from(value: sandbox_driver::Error) -> Self {
Self::driver_error(value)
}
}
impl From<String> 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]

View file

@ -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<String>) -> crate::Result<ExecResult> {
@ -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)));

View file

@ -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,
};