From 3777df9799f85402c52966e24bef7699baff4955 Mon Sep 17 00:00:00 2001 From: Thane Thomson Date: Thu, 23 Jun 2022 18:05:44 -0400 Subject: [PATCH] Improve error reporting for I/O and subprocess errors (#34) Signed-off-by: Thane Thomson --- src/bin/cli.rs | 21 ++++++++++++++------- src/changelog.rs | 26 +++++++++++++++----------- src/changelog/change_set.rs | 8 ++++---- src/changelog/component_section.rs | 8 ++++---- src/changelog/config.rs | 2 +- src/error.rs | 6 ++++-- src/fs_utils.rs | 23 +++++++++++++---------- 7 files changed, 55 insertions(+), 39 deletions(-) diff --git a/src/bin/cli.rs b/src/bin/cli.rs index 966704e..f366847 100644 --- a/src/bin/cli.rs +++ b/src/bin/cli.rs @@ -275,17 +275,21 @@ fn add_unreleased_entry_with_editor( return Err(Error::FileExists(entry_path.display().to_string())); } - let tmpdir = tempfile::tempdir()?; + let tmpdir = + tempfile::tempdir().map_err(|e| Error::Io(Path::new("tempdir").to_path_buf(), e))?; let tmpfile_path = tmpdir.path().join("entry.md"); - std::fs::write(&tmpfile_path, ADD_CHANGE_TEMPLATE)?; + std::fs::write(&tmpfile_path, ADD_CHANGE_TEMPLATE) + .map_err(|e| Error::Io(tmpfile_path.clone(), e))?; // Run the user's editor and wait for the process to exit let _ = std::process::Command::new(editor) .arg(&tmpfile_path) - .status()?; + .status() + .map_err(|e| Error::Subprocess(editor.to_str().unwrap().to_string(), e))?; // Check if the temporary file's content's changed, and that it's not empty - let tmpfile_content = std::fs::read_to_string(&tmpfile_path)?; + let tmpfile_content = std::fs::read_to_string(&tmpfile_path) + .map_err(|e| Error::Io(tmpfile_path.to_path_buf(), e))?; if tmpfile_content.is_empty() || tmpfile_content == ADD_CHANGE_TEMPLATE { log::info!("No changes to entry - not adding new entry to changelog"); return Ok(()); @@ -302,17 +306,20 @@ fn prepare_release(config: &Config, editor: &Path, path: &Path, version: &str) - .join(&config.change_sets.summary_filename); // If the summary doesn't exist, try to create it if std::fs::metadata(&summary_path).is_err() { - std::fs::write(&summary_path, RELEASE_SUMMARY_TEMPLATE)?; + std::fs::write(&summary_path, RELEASE_SUMMARY_TEMPLATE) + .map_err(|e| Error::Io(summary_path.clone(), e))?; } // Run the user's editor and wait for the process to exit let _ = std::process::Command::new(editor) .arg(&summary_path) - .status()?; + .status() + .map_err(|e| Error::Subprocess(editor.to_str().unwrap().to_string(), e))?; // Check if the file's contents have changed - if not, don't continue with // the release - let summary_content = std::fs::read_to_string(&summary_path)?; + let summary_content = + std::fs::read_to_string(&summary_path).map_err(|e| Error::Io(summary_path.clone(), e))?; if summary_content.is_empty() || summary_content == RELEASE_SUMMARY_TEMPLATE { log::info!("No changes to release summary - not creating a new release"); return Ok(()); diff --git a/src/changelog.rs b/src/changelog.rs index da2c0df..8bfbca1 100644 --- a/src/changelog.rs +++ b/src/changelog.rs @@ -107,7 +107,8 @@ impl Changelog { if let Some(ep) = maybe_epilogue_path { let new_epilogue_path = path.join(&config.epilogue_filename); if !fs_utils::file_exists(&new_epilogue_path) { - fs::copy(ep, &new_epilogue_path)?; + fs::copy(ep, &new_epilogue_path) + .map_err(|e| Error::Io(ep.as_ref().to_path_buf(), e))?; info!( "Copied epilogue from {} to {}", path_to_str(ep), @@ -149,7 +150,8 @@ impl Changelog { } } - let path = fs::canonicalize(path.as_ref())?; + let path = fs::canonicalize(path.as_ref()) + .map_err(|e| Error::Io(path.as_ref().to_path_buf(), e))?; let parent = path .parent() .ok_or_else(|| Error::NoParentFolder(path_to_str(&path)))?; @@ -178,7 +180,8 @@ impl Changelog { "Attempting to load changelog from directory: {}", path.display() ); - if !fs::metadata(path)?.is_dir() { + let meta = fs::metadata(path).map_err(|e| Error::Io(path.to_path_buf(), e))?; + if !meta.is_dir() { return Err(Error::ExpectedDir(fs_utils::path_to_str(path))); } let unreleased = @@ -237,7 +240,7 @@ impl Changelog { if fs::metadata(&entry_path).is_ok() { return Err(Error::FileExists(path_to_str(&entry_path))); } - fs::write(&entry_path, content.as_ref())?; + fs::write(&entry_path, content.as_ref()).map_err(|e| Error::Io(entry_path.clone(), e))?; info!("Wrote entry to: {}", path_to_str(&entry_path)); Ok(()) } @@ -391,7 +394,8 @@ impl Changelog { return Err(Error::ExpectedDir(path_to_str(&unreleased_path))); } - fs::rename(&unreleased_path, &version_path)?; + fs::rename(&unreleased_path, &version_path) + .map_err(|e| Error::Io(unreleased_path.clone(), e))?; info!( "Moved {} to {}", path_to_str(&unreleased_path), @@ -407,7 +411,7 @@ impl Changelog { let unreleased_dir = path.join(&config.unreleased.folder); ensure_dir(&unreleased_dir)?; let unreleased_gitkeep = unreleased_dir.join(".gitkeep"); - fs::write(&unreleased_gitkeep, "")?; + fs::write(&unreleased_gitkeep, "").map_err(|e| Error::Io(unreleased_gitkeep.clone(), e))?; debug!("Wrote {}", path_to_str(&unreleased_gitkeep)); Ok(()) } @@ -417,15 +421,15 @@ fn entry_id_to_filename>(config: &Config, id: S) -> String { format!("{}.{}", id.as_ref(), config.change_sets.entry_ext) } -fn release_dir_filter(config: &Config, e: fs::DirEntry) -> Option> { - let file_name = e.file_name(); +fn release_dir_filter(config: &Config, entry: fs::DirEntry) -> Option> { + let file_name = entry.file_name(); let file_name = file_name.to_string_lossy(); - let meta = match e.metadata() { + let meta = match entry.metadata() { Ok(m) => m, - Err(e) => return Some(Err(Error::Io(e))), + Err(e) => return Some(Err(Error::Io(entry.path(), e))), }; if meta.is_dir() && file_name != config.unreleased.folder { - Some(Ok(e.path())) + Some(Ok(entry.path())) } else { None } diff --git a/src/changelog/change_set.rs b/src/changelog/change_set.rs index 09a234e..6183a7b 100644 --- a/src/changelog/change_set.rs +++ b/src/changelog/change_set.rs @@ -76,13 +76,13 @@ impl ChangeSet { } } -fn change_set_section_filter(e: fs::DirEntry) -> Option> { - let meta = match e.metadata() { +fn change_set_section_filter(entry: fs::DirEntry) -> Option> { + let meta = match entry.metadata() { Ok(m) => m, - Err(e) => return Some(Err(Error::Io(e))), + Err(e) => return Some(Err(Error::Io(entry.path(), e))), }; if meta.is_dir() { - Some(Ok(e.path())) + Some(Ok(entry.path())) } else { None } diff --git a/src/changelog/component_section.rs b/src/changelog/component_section.rs index 895c54e..de04195 100644 --- a/src/changelog/component_section.rs +++ b/src/changelog/component_section.rs @@ -81,13 +81,13 @@ impl ComponentSection { } } -pub(crate) fn package_section_filter(e: fs::DirEntry) -> Option> { - let meta = match e.metadata() { +pub(crate) fn package_section_filter(entry: fs::DirEntry) -> Option> { + let meta = match entry.metadata() { Ok(m) => m, - Err(e) => return Some(Err(Error::Io(e))), + Err(e) => return Some(Err(Error::Io(entry.path(), e))), }; if meta.is_dir() { - Some(Ok(e.path())) + Some(Ok(entry.path())) } else { None } diff --git a/src/changelog/config.rs b/src/changelog/config.rs index 7e685c0..e377e61 100644 --- a/src/changelog/config.rs +++ b/src/changelog/config.rs @@ -123,7 +123,7 @@ impl Config { path.display() ); let content = toml::to_string_pretty(&self).map_err(Error::TomlSerialize)?; - std::fs::write(path, content)?; + std::fs::write(path, content).map_err(|e| Error::Io(path.to_path_buf(), e))?; info!("Saved configuration to: {}", path.display()); Ok(()) } diff --git a/src/error.rs b/src/error.rs index 7abadb9..216064c 100644 --- a/src/error.rs +++ b/src/error.rs @@ -6,8 +6,10 @@ use thiserror::Error; /// All error variants that can be produced by unclog. #[derive(Debug, Error)] pub enum Error { - #[error("I/O error: {0}")] - Io(#[from] std::io::Error), + #[error("I/O error relating to {0}: {1}")] + Io(PathBuf, std::io::Error), + #[error("failed to execute subprocess {0}: {1}")] + Subprocess(String, std::io::Error), #[error("expected path to be a directory: {0}")] ExpectedDir(String), #[error("unexpected release directory name prefix: \"{0}\"")] diff --git a/src/fs_utils.rs b/src/fs_utils.rs index fe50df6..23b7cf8 100644 --- a/src/fs_utils.rs +++ b/src/fs_utils.rs @@ -10,7 +10,8 @@ pub fn path_to_str>(path: P) -> String { } pub fn read_to_string>(path: P) -> Result { - Ok(fs::read_to_string(path)?) + let path = path.as_ref(); + fs::read_to_string(path).map_err(|e| Error::Io(path.to_path_buf(), e)) } pub fn read_to_string_opt>(path: P) -> Result> { @@ -23,10 +24,11 @@ pub fn read_to_string_opt>(path: P) -> Result> { pub fn ensure_dir(path: &Path) -> Result<()> { if fs::metadata(path).is_err() { - fs::create_dir(path)?; + fs::create_dir(path).map_err(|e| Error::Io(path.to_path_buf(), e))?; info!("Created directory: {}", path_to_str(path)); } - if !fs::metadata(path)?.is_dir() { + let meta = fs::metadata(path).map_err(|e| Error::Io(path.to_path_buf(), e))?; + if !meta.is_dir() { return Err(Error::ExpectedDir(path_to_str(path))); } Ok(()) @@ -35,7 +37,7 @@ pub fn ensure_dir(path: &Path) -> Result<()> { pub fn rm_gitkeep(path: &Path) -> Result<()> { let path = path.join(".gitkeep"); if fs::metadata(&path).is_ok() { - fs::remove_file(&path)?; + fs::remove_file(&path).map_err(|e| Error::Io(path.to_path_buf(), e))?; debug!("Removed .gitkeep file from: {}", path_to_str(&path)); } Ok(()) @@ -45,20 +47,21 @@ pub fn read_and_filter_dir(path: &Path, filter: F) -> Result> where F: Fn(fs::DirEntry) -> Option>, { - fs::read_dir(path)? + fs::read_dir(path) + .map_err(|e| Error::Io(path.to_path_buf(), e))? .filter_map(|r| match r { Ok(e) => filter(e), - Err(e) => Some(Err(Error::Io(e))), + Err(e) => Some(Err(Error::Io(path.to_path_buf(), e))), }) .collect::>>() } -pub fn entry_filter(config: &Config, e: fs::DirEntry) -> Option> { - let meta = match e.metadata() { +pub fn entry_filter(config: &Config, entry: fs::DirEntry) -> Option> { + let meta = match entry.metadata() { Ok(m) => m, - Err(e) => return Some(Err(Error::Io(e))), + Err(e) => return Some(Err(Error::Io(entry.path(), e))), }; - let path = e.path(); + let path = entry.path(); let ext = path.extension()?.to_str()?; if meta.is_file() && ext == config.change_sets.entry_ext { Some(Ok(path))