[Experiment] Integrate error-stack for better error and stacktrace reporting

Cargo.toml

@@ -6,6 +6,7 @@ edition = "2021"
 [dependencies]
 clap = { version = "4.2.1", features = ["derive"] }
 clap_derive = "4.2.0"
+error-stack = "0.3.1"
 glob-match = "0.2.1"
 itertools = "0.10.5"
 jwalk = "0.8.1"
@@ -14,6 +15,7 @@ rayon = "1.7.0"
 regex = "1.7.3"
 serde = { version = "1.0.159", features = ["derive"] }
 serde_yaml = "0.9.19"
+thiserror = "1.0.40"
 tracing = "0.1.37"
 tracing-subscriber = { version = "0.3.16", features = ["env-filter"] }
 

src/main.rs

@@ -1,11 +1,11 @@
-use ownership::{Ownership, ValidationErrors};
-use tracing::debug;
+use ownership::Ownership;
 
 use crate::project::Project;
 use clap::{Parser, Subcommand};
+use core::fmt;
+use error_stack::{Context, IntoReport, Result, ResultExt};
 use path_clean::PathClean;
 use std::{
- error::Error,
  fs::File,
  path::{Path, PathBuf},
  process,
@@ -47,64 +47,87 @@ struct Args {
 }
 
 impl Args {
- fn absolute_project_root(&self) -> Result<PathBuf, std::io::Error> {
- self.project_root.canonicalize()
+ fn absolute_project_root(&self) -> Result<PathBuf, Error> {
+ self.project_root.canonicalize().into_report().change_context(Error::Io)
  }
 
- fn absolute_config_path(&self) -> Result<PathBuf, std::io::Error> {
+ fn absolute_config_path(&self) -> Result<PathBuf, Error> {
  Ok(self.absolute_path(&self.config_path)?.clean())
  }
 
- fn absolute_codeowners_path(&self) -> Result<PathBuf, std::io::Error> {
+ fn absolute_codeowners_path(&self) -> Result<PathBuf, Error> {
  Ok(self.absolute_path(&self.codeowners_file_path)?.clean())
  }
 
- fn absolute_path(&self, path: &Path) -> Result<PathBuf, std::io::Error> {
+ fn absolute_path(&self, path: &Path) -> Result<PathBuf, Error> {
  Ok(self.absolute_project_root()?.join(path))
  }
 }
 
-fn main() -> Result<(), Box<dyn Error>> {
+#[derive(Debug)]
+enum Error {
+ CannotBuildProject,
+ Io,
+ ValidationErrors,
+}
+
+impl fmt::Display for Error {
+ fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result {
+ match self {
+ Error::CannotBuildProject => fmt.write_str("Error::CannotBuildProject"),
+ Error::Io => fmt.write_str("Error::Io"),
+ Error::ValidationErrors => fmt.write_str("Error::ValidationErrors"),
+ }
+ }
+}
+
+impl Context for Error {}
+
+fn main() -> Result<(), Error> {
  install_logger();
  print_validation_errors_to_stdout(cli())?;
 
  Ok(())
 }
 
-fn cli() -> Result<(), Box<dyn Error>> {
+fn cli() -> Result<(), Error> {
  let args = Args::parse();
 
  let config_path = args.absolute_config_path()?;
  let codeowners_file_path = args.absolute_codeowners_path()?;
  let project_root = args.absolute_project_root()?;
 
-debug!(
-config_path = &config_path.to_str(),
-codeowners_file_path = &codeowners_file_path.to_str(),
-project_root = &project_root.to_str(),
-);
+let config_file = File::open(&config_path)
+.into_report()
+.change_context(Error::Io)
+.attach_printable(format!("{}", config_path.to_string_lossy()))?;
+let config = serde_yaml::from_reader(config_file).into_report().change_context(Error::Io)?;
 
- let config = serde_yaml::from_reader(File::open(config_path)?)?;
-let ownership = Ownership::build(Project::build(&project_root, &codeowners_file_path, &config)?);
+ let ownership =
+Ownership::build(Project::build(&project_root, &codeowners_file_path, &config).change_context(Error::CannotBuildProject)?);
  let command = args.command;
 
  match command {
- Command::Validate => ownership.validate()?,
+ Command::Validate => ownership.validate().change_context(Error::ValidationErrors)?,
  Command::Generate => {
- std::fs::write(codeowners_file_path, ownership.generate_file())?;
+ std::fs::write(codeowners_file_path, ownership.generate_file())
+ .into_report()
+ .change_context(Error::Io)?;
  }
  Command::GenerateAndValidate => {
- std::fs::write(codeowners_file_path, ownership.generate_file())?;
- ownership.validate()?
+ std::fs::write(codeowners_file_path, ownership.generate_file())
+ .into_report()
+ .change_context(Error::Io)?;
+ ownership.validate().change_context(Error::ValidationErrors)?
  }
  }
 
  Ok(())
 }
 
-fn print_validation_errors_to_stdout(result: Result<(), Box<dyn Error>>) -> Result<(), Box<dyn Error>> {
+fn print_validation_errors_to_stdout(result: Result<(), Error>) -> Result<(), Error> {
  if let Err(error) = result {
- if let Some(validation_errors) = error.downcast_ref::<ValidationErrors>() {
+ if let Some(validation_errors) = error.downcast_ref::<ownership::ValidationErrors>() {
  println!("{}", validation_errors);
  process::exit(-1);
  } else {

src/ownership.rs

@@ -10,7 +10,8 @@ mod tests;
 
 use crate::project::Project;
 
-pub use validator::ValidationErrors;
+use error_stack::Result;
+pub use validator::Errors as ValidationErrors;
 
 use self::{
  file_generator::FileGenerator,

src/ownership/validator.rs

@@ -1,6 +1,5 @@
 use core::fmt;
 use std::collections::HashMap;
-use std::error::Error;
 use std::fmt::Display;
 use std::path::Path;
 
@@ -24,25 +23,27 @@ pub struct Validator {
  pub file_generator: FileGenerator,
 }
 
+use error_stack::{Context, Report, Result};
+
 #[derive(Debug)]
 struct Owner {
  pub sources: Vec<String>,
  pub team_name: String,
 }
 
 #[derive(Debug)]
-enum ValidationError {
+enum Error {
  FileWithoutOwner { path: PathBuf },
  FileWithMultipleOwners { path: PathBuf, owners: Vec<Owner> },
  CodeownershipFileIsStale,
 }
 
 #[derive(Debug)]
-pub struct ValidationErrors(Vec<ValidationError>);
+pub struct Errors(Vec<Error>);
 
 impl Validator {
  #[instrument(level = "debug", skip_all)]
- pub fn validate(&self) -> Result<(), ValidationErrors> {
+ pub fn validate(&self) -> Result<(), Errors> {
  let mut validation_errors = Vec::new();
 
  debug!("validate_file_ownership");
@@ -54,20 +55,20 @@ impl Validator {
  if validation_errors.is_empty() {
  Ok(())
  } else {
- Err(ValidationErrors(validation_errors))
+ Err(Report::new(Errors(validation_errors)))
  }
  }
 
- fn validate_file_ownership(&self) -> Vec<ValidationError> {
+ fn validate_file_ownership(&self) -> Vec<Error> {
  let mut validation_errors = Vec::new();
 
  for (file, owners) in self.file_to_owners() {
  let relative_path = self.project.relative_path(&file.path).to_owned();
 
  if owners.is_empty() {
- validation_errors.push(ValidationError::FileWithoutOwner { path: relative_path })
+ validation_errors.push(Error::FileWithoutOwner { path: relative_path })
  } else if owners.len() > 1 {
- validation_errors.push(ValidationError::FileWithMultipleOwners {
+ validation_errors.push(Error::FileWithMultipleOwners {
  path: relative_path,
  owners,
  })
@@ -77,11 +78,11 @@ impl Validator {
  validation_errors
  }
 
- fn validate_codeowners_file(&self) -> Vec<ValidationError> {
+ fn validate_codeowners_file(&self) -> Vec<Error> {
  let generated_file = self.file_generator.generate_file();
 
  if generated_file != self.project.codeowners_file {
- vec![ValidationError::CodeownershipFileIsStale]
+ vec![Error::CodeownershipFileIsStale]
  } else {
  vec![]
  }
@@ -127,21 +128,21 @@ impl Validator {
  }
 }
 
-impl ValidationError {
+impl Error {
  pub fn error_category_message(&self) -> String {
  match self {
-ValidationError::FileWithoutOwner { path: _ } => "Some files are missing ownership:".to_owned(),
-ValidationError::FileWithMultipleOwners { path: _, owners: _ } => "Code ownership should only be defined for each file in one way. The following files have declared ownership in multiple ways.".to_owned(),
-ValidationError::CodeownershipFileIsStale => {
+Error::FileWithoutOwner { path: _ } => "Some files are missing ownership:".to_owned(),
+Error::FileWithMultipleOwners { path: _, owners: _ } => "Code ownership should only be defined for each file in one way. The following files have declared ownership in multiple ways.".to_owned(),
+Error::CodeownershipFileIsStale => {
  "CODEOWNERS out of date. Run `codeownership generate` to update the CODEOWNERS file".to_owned()
  }
  }
  }
 
  pub fn error_message(&self) -> String {
  match self {
-ValidationError::FileWithoutOwner { path } => format!("- {}", path.to_string_lossy()),
-ValidationError::FileWithMultipleOwners { path, owners } => owners
+Error::FileWithoutOwner { path } => format!("- {}", path.to_string_lossy()),
+Error::FileWithMultipleOwners { path, owners } => owners
  .iter()
  .flat_map(|owner| {
  owner
@@ -150,12 +151,29 @@ impl ValidationError {
  .map(|source| format!("- {} (owner: {}, source: {})", path.to_string_lossy(), owner.team_name, &source))
  })
  .join("\n"),
-ValidationError::CodeownershipFileIsStale => "".to_owned(),
+Error::CodeownershipFileIsStale => "".to_owned(),
  }
  }
 }
 
-impl Display for ValidationErrors {
+impl Display for Error {
+ fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+ f.write_str(&self.error_message())?;
+ Ok(())
+ }
+}
+
+impl std::error::Error for Error {
+ fn description(&self) -> &str {
+ match self {
+ Error::FileWithoutOwner { path: _ } => "FileWithoutOwner",
+ Error::FileWithMultipleOwners { path: _, owners: _ } => "FileWithMultipleOwners",
+ Error::CodeownershipFileIsStale => "FileWithMultipleOwners",
+ }
+ }
+}
+
+impl Display for Errors {
  fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
  let grouped_errors = self.0.iter().into_group_map_by(|error| error.error_category_message());
  for (error_category_message, errors) in grouped_errors {
@@ -174,8 +192,4 @@ impl Display for ValidationErrors {
  }
 }
 
-impl Error for ValidationErrors {
- fn description(&self) -> &str {
- "ValidationError"
- }
-}
+impl Context for Errors {}