From 5f6cba1469abae368a323e1b63d7248f24e085e2 Mon Sep 17 00:00:00 2001 From: Zykino Date: Wed, 5 Jul 2023 21:09:56 +0200 Subject: [PATCH] Upgrade error handeling --- src/command.rs | 77 ++++++++++++++++++++++++++++++------------------ src/errors.rs | 80 ++++++++++---------------------------------------- src/lib.rs | 4 +-- 3 files changed, 67 insertions(+), 94 deletions(-) diff --git a/src/command.rs b/src/command.rs index e9bc409..46d4d09 100644 --- a/src/command.rs +++ b/src/command.rs @@ -7,12 +7,22 @@ use std::process::{Command, ExitStatus, Stdio}; use std::{fmt, fs, io}; #[derive(Debug, Serialize, Deserialize, PartialEq)] -enum UpdateSteps { +pub enum UpdateSteps { PreInstall, Install, PostInstall, } +impl Display for UpdateSteps { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + UpdateSteps::PreInstall => write!(f, "PreInstall"), + UpdateSteps::Install => write!(f, "Install"), + UpdateSteps::PostInstall => write!(f, "PostInstall"), + } + } +} + // TODO: change the struct’s names for the `topgrade`’s one. They are much better. /// Root of the machine’s dependency graph @@ -43,7 +53,7 @@ pub struct Executor { /// A command to execute on the system as part of an executor #[derive(Debug, Clone, Serialize, Deserialize)] -struct Cmd { +pub struct Cmd { exe: String, params: Option>, current_dir: Option, @@ -52,7 +62,7 @@ struct Cmd { /// The actual (cleaned) command that will be executed on the system as part of an executor #[derive(Debug, Clone, Serialize, Deserialize)] -pub(crate) struct ActualCmd { +pub struct ActualCmd { exe: String, params: Vec, current_dir: Option, @@ -72,6 +82,10 @@ impl From<&String> for UpdateSteps { } pub fn get_packages_folder(opt: &Opt) -> io::Result { + if let Some(p) = opt.config_folder.clone() { + return Ok(p); + } + let config_folder = directories::ProjectDirs::from("net", "ZykiCorp", "System Updater") .ok_or(io::Error::new( io::ErrorKind::NotFound, @@ -154,6 +168,7 @@ impl Updater { } let packages_folder = get_packages_folder(&opt)?; + // TODO: Useless match? Still a risck for "time-of-check to time-of-use" bug match packages_folder.try_exists() { Ok(true) => {} // Ok: Exist and should be readable Ok(false) => { @@ -178,7 +193,7 @@ impl Updater { } Err(..) => false, }) { - let file = packages_folder.join(file?.path()); // "default.yaml"); + let file = file?.path(); let sys = std::fs::read_to_string(&file).unwrap(); let sys = serde_yaml::from_str(&sys).map_err(|err| { io::Error::new( @@ -244,16 +259,18 @@ impl Executor { if let Some(pre_install) = &self.pre_install { for cmd in pre_install { let cmd = cmd.clone().prepare(opt); - let exit_status = cmd - .execute(opt) - .map_err(|err| MyError::new(MyErrorKind::PreInstall, err, cmd.clone()))?; + let exit_status = cmd.execute(opt).map_err(|err| Error::Execution { + source: err, + step: UpdateSteps::PreInstall, + cmd: cmd.clone(), + })?; if !exit_status.success() { - return Err(MyError::new( - MyErrorKind::PreInstall, - io::Error::new(io::ErrorKind::Other, format!("{}", exit_status)), - cmd.clone(), - )); + return Err(Error::Execution { + source: io::Error::new(io::ErrorKind::Other, format!("{}", exit_status)), + step: UpdateSteps::PreInstall, + cmd, + }); } } } @@ -262,16 +279,18 @@ impl Executor { pub fn install(&self, opt: &Opt) -> Result<()> { let cmd = self.install.clone().prepare(opt); - let exit_status = cmd - .execute(opt) - .map_err(|err| MyError::new(MyErrorKind::Install, err, cmd.clone()))?; + let exit_status = cmd.execute(opt).map_err(|err| Error::Execution { + source: err, + step: UpdateSteps::Install, + cmd: cmd.clone(), + })?; if !exit_status.success() { - return Err(MyError::new( - MyErrorKind::Install, - io::Error::new(io::ErrorKind::Other, format!("{}", exit_status)), - cmd.clone(), - )); + return Err(Error::Execution { + source: io::Error::new(io::ErrorKind::Other, format!("{}", exit_status)), + step: UpdateSteps::Install, + cmd, + }); } Ok(()) } @@ -280,16 +299,18 @@ impl Executor { if let Some(post_install) = &self.post_install { for cmd in post_install { let cmd = cmd.clone().prepare(opt); - let exit_status = cmd - .execute(opt) - .map_err(|err| MyError::new(MyErrorKind::PostInstall, err, cmd.clone()))?; + let exit_status = cmd.execute(opt).map_err(|err| Error::Execution { + source: err, + step: UpdateSteps::PostInstall, + cmd: cmd.clone(), + })?; if !exit_status.success() { - return Err(MyError::new( - MyErrorKind::PostInstall, - io::Error::new(io::ErrorKind::Other, format!("{}", exit_status)), - cmd.clone(), - )); + return Err(Error::Execution { + source: io::Error::new(io::ErrorKind::Other, format!("{}", exit_status)), + step: UpdateSteps::PostInstall, + cmd, + }); } } } diff --git a/src/errors.rs b/src/errors.rs index 6fe4599..6d85cfc 100644 --- a/src/errors.rs +++ b/src/errors.rs @@ -1,75 +1,27 @@ +// Use https://kazlauskas.me/entries/errors as a reference + use crate::*; -use std::error::Error; -use std::fmt; -use std::fmt::{Display, Formatter}; use std::io; use std::result; +use thiserror::Error; -pub type Result = result::Result; +pub type Result = result::Result; /// An error that can occur in this crate. /// /// Generally, this error corresponds to problems with underlying process. -#[derive(Debug)] -pub struct MyError { - source: io::Error, - kind: MyErrorKind, - cmd: ActualCmd, -} - -#[derive(Debug)] #[non_exhaustive] -pub enum MyErrorKind { - Config, - PreInstall, - Install, - PostInstall, -} - -impl MyError { - pub(crate) fn new(kind: MyErrorKind, source: io::Error, cmd: ActualCmd) -> MyError { - MyError { source, kind, cmd } - } - - /// Return the kind of this error. - pub fn kind(&self) -> &MyErrorKind { - &self.kind - } -} - -impl Error for MyError { - fn source(&self) -> Option<&(dyn Error + 'static)> { - Some(&self.source) - } -} - -impl Display for MyError { - fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { - match self.kind { - MyErrorKind::Config => write!( - f, - "Could not read configuration file: {}", - self.source().unwrap() - ), - MyErrorKind::PreInstall => write!( - f, - "Could not do the pre_install with command {}: {}", - self.cmd, - self.source().unwrap() - ), - MyErrorKind::Install => write!( - f, - "Could not install with command {}: {}", - self.cmd, - self.source().unwrap() - ), - MyErrorKind::PostInstall => write!( - f, - "Could not do the post_install command {}: {}", - self.cmd, - self.source().unwrap() - ), - } - } +#[derive(Debug, Error)] +pub enum Error { + // #[error("TODO")] + // Config, + #[error( + "Could not achieve the \"{step}\" step. Command `{cmd}` resulted in an exited with: {source}" + )] + Execution { + source: io::Error, + step: UpdateSteps, + cmd: ActualCmd, + }, } diff --git a/src/lib.rs b/src/lib.rs index cb7dac6..e67a3e4 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -12,7 +12,7 @@ use std::path::PathBuf; #[derive(Parser)] pub struct Opt { #[arg(hide = true)] // TODO: hidden option for debug? or usefull for everyone? - pub config_file: Option, + pub config_folder: Option, #[arg(short, long)] pub quiet: bool, // TODO: use clap_verbosity_flag instead @@ -24,7 +24,7 @@ pub struct Opt { enum Status { /// All steps asked were successful Ok, - Err(MyError), + Err(Error), } impl Display for Status {