From 2b7160ebdf950ef872f785336fcfe17e0c4cb347 Mon Sep 17 00:00:00 2001 From: Lars Wirzenius Date: Fri, 6 May 2022 08:39:14 +0300 Subject: refactor! split SubplotError::IoError into more specific errors Replace SubplotError::IoError with ::Spawn, ::WriteToChild, ::WaitForChild, ::ReadFile, ::CreateFile, ::Writefile. IoError was a catchall error and as such, so generic that it didn't help the user to figure out what actually is wrong. For example, there was no indication what operation was attempted or on what file. The new error variants are specific. Sponsored-by: author --- src/codegen.rs | 13 +++++++++---- src/diagrams.rs | 29 +++++++++++++++++++++-------- src/doc.rs | 3 ++- src/error.rs | 33 +++++++++++++++++++++++---------- src/templatespec.rs | 3 ++- 5 files changed, 57 insertions(+), 24 deletions(-) diff --git a/src/codegen.rs b/src/codegen.rs index b940b4d..5c4255f 100644 --- a/src/codegen.rs +++ b/src/codegen.rs @@ -62,12 +62,15 @@ fn tera(tmplspec: &TemplateSpec, templatename: &str) -> Result Result Result<(), SubplotError> { - let mut f: File = File::create(filename)?; - f.write_all(content.as_bytes())?; + let mut f: File = File::create(filename) + .map_err(|err| SubplotError::CreateFile(filename.to_path_buf(), err))?; + f.write_all(content.as_bytes()) + .map_err(|err| SubplotError::WriteFile(filename.to_path_buf(), err))?; Ok(()) } diff --git a/src/diagrams.rs b/src/diagrams.rs index 264bd3f..2bdd0ed 100644 --- a/src/diagrams.rs +++ b/src/diagrams.rs @@ -131,15 +131,21 @@ impl DotMarkup { impl DiagramMarkup for DotMarkup { fn as_svg(&self) -> Result, SubplotError> { - let mut child = Command::new(DOT_PATH.lock().unwrap().clone()) + let path = DOT_PATH.lock().unwrap().clone(); + let mut child = Command::new(&path) .arg("-Tsvg") .stdin(Stdio::piped()) .stdout(Stdio::piped()) .stderr(Stdio::piped()) - .spawn()?; + .spawn() + .map_err(|err| SubplotError::Spawn(path.clone(), err))?; if let Some(stdin) = child.stdin.as_mut() { - stdin.write_all(self.markup.as_bytes())?; - let output = child.wait_with_output()?; + stdin + .write_all(self.markup.as_bytes()) + .map_err(SubplotError::WriteToChild)?; + let output = child + .wait_with_output() + .map_err(SubplotError::WaitForChild)?; if output.status.success() { Ok(output.stdout) } else { @@ -192,7 +198,8 @@ impl PlantumlMarkup { impl DiagramMarkup for PlantumlMarkup { fn as_svg(&self) -> Result, SubplotError> { - let mut cmd = Command::new(JAVA_PATH.lock().unwrap().clone()); + let path = JAVA_PATH.lock().unwrap().clone(); + let mut cmd = Command::new(&path); cmd.arg("-Djava.awt.headless=true") .arg("-jar") .arg(PLANTUML_JAR_PATH.lock().unwrap().clone()) @@ -208,10 +215,16 @@ impl DiagramMarkup for PlantumlMarkup { if let Some(path) = Self::build_java_path() { cmd.env("PATH", path); } - let mut child = cmd.spawn()?; + let mut child = cmd + .spawn() + .map_err(|err| SubplotError::Spawn(path.clone(), err))?; if let Some(stdin) = child.stdin.as_mut() { - stdin.write_all(self.markup.as_bytes())?; - let output = child.wait_with_output()?; + stdin + .write_all(self.markup.as_bytes()) + .map_err(SubplotError::WriteToChild)?; + let output = child + .wait_with_output() + .map_err(SubplotError::WaitForChild)?; if output.status.success() { Ok(output.stdout) } else { diff --git a/src/doc.rs b/src/doc.rs index e65aff5..37f32d6 100644 --- a/src/doc.rs +++ b/src/doc.rs @@ -204,7 +204,8 @@ impl<'a> Document { ) -> Result { trace!("Parsing document with pullmark-cmark from {:?}", filename); let filename = filename.to_path_buf(); - let markdown = std::fs::read_to_string(&filename)?; + let markdown = std::fs::read_to_string(&filename) + .map_err(|err| SubplotError::ReadFile(filename.clone(), err))?; let ast = ast::AbstractSyntaxTree::from_str(&markdown)?; trace!("Parsed document OK"); diff --git a/src/error.rs b/src/error.rs index f9608a8..f0d76e3 100644 --- a/src/error.rs +++ b/src/error.rs @@ -259,16 +259,29 @@ pub enum SubplotError { #[error("no scenarios were found matching the `{0}` template")] NoScenariosMatched(String), - /// I/O error - /// - /// Subplot did some I/O, and it failed. This is a generic wrapper - /// for any kind of I/O error. - #[error(transparent)] - IoError { - /// The wrapped error. - #[from] - source: std::io::Error, - }, + /// Failed to invoke a program. + #[error("Failed to invoke {0}")] + Spawn(PathBuf, #[source] std::io::Error), + + /// Failed to write to stdin of child process. + #[error("Failed to write to stdin of child process")] + WriteToChild(#[source] std::io::Error), + + /// Error when waiting for child process to finish. + #[error("Error when waiting for child process to finish")] + WaitForChild(#[source] std::io::Error), + + /// Error when reading a file. + #[error("Error when reading {0}")] + ReadFile(PathBuf, #[source] std::io::Error), + + /// Error when creating a file. + #[error("Error when creating {0}")] + CreateFile(PathBuf, #[source] std::io::Error), + + /// Error when writing to a file. + #[error("Error when writing to {0}")] + WriteFile(PathBuf, #[source] std::io::Error), /// Pandoc error /// diff --git a/src/templatespec.rs b/src/templatespec.rs index dbb39dc..7b8723b 100644 --- a/src/templatespec.rs +++ b/src/templatespec.rs @@ -40,7 +40,8 @@ impl TemplateSpec { /// Read a template.yaml file and create the corresponding TemplateSpec. pub fn from_file(filename: &Path) -> Result { - let yaml = resource::read_as_string(filename, None)?; + let yaml = resource::read_as_string(filename, None) + .map_err(|err| SubplotError::ReadFile(filename.to_path_buf(), err))?; let spec = TemplateSpec::from_yaml(&yaml)?; let dirname = match filename.parent() { Some(x) => x, -- cgit v1.2.1