Skip to content

Commit

Permalink
Improve error reporting for I/O and subprocess errors (#34)
Browse files Browse the repository at this point in the history
Signed-off-by: Thane Thomson <connect@thanethomson.com>
  • Loading branch information
thanethomson committed Jun 23, 2022
1 parent 20fc94d commit 3777df9
Show file tree
Hide file tree
Showing 7 changed files with 55 additions and 39 deletions.
21 changes: 14 additions & 7 deletions src/bin/cli.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(());
Expand All @@ -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(());
Expand Down
26 changes: 15 additions & 11 deletions src/changelog.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down Expand Up @@ -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)))?;
Expand Down Expand Up @@ -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 =
Expand Down Expand Up @@ -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(())
}
Expand Down Expand Up @@ -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),
Expand All @@ -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(())
}
Expand All @@ -417,15 +421,15 @@ fn entry_id_to_filename<S: AsRef<str>>(config: &Config, id: S) -> String {
format!("{}.{}", id.as_ref(), config.change_sets.entry_ext)
}

fn release_dir_filter(config: &Config, e: fs::DirEntry) -> Option<crate::Result<PathBuf>> {
let file_name = e.file_name();
fn release_dir_filter(config: &Config, entry: fs::DirEntry) -> Option<crate::Result<PathBuf>> {
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
}
Expand Down
8 changes: 4 additions & 4 deletions src/changelog/change_set.rs
Original file line number Diff line number Diff line change
Expand Up @@ -76,13 +76,13 @@ impl ChangeSet {
}
}

fn change_set_section_filter(e: fs::DirEntry) -> Option<Result<PathBuf>> {
let meta = match e.metadata() {
fn change_set_section_filter(entry: fs::DirEntry) -> Option<Result<PathBuf>> {
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
}
Expand Down
8 changes: 4 additions & 4 deletions src/changelog/component_section.rs
Original file line number Diff line number Diff line change
Expand Up @@ -81,13 +81,13 @@ impl ComponentSection {
}
}

pub(crate) fn package_section_filter(e: fs::DirEntry) -> Option<Result<PathBuf>> {
let meta = match e.metadata() {
pub(crate) fn package_section_filter(entry: fs::DirEntry) -> Option<Result<PathBuf>> {
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
}
Expand Down
2 changes: 1 addition & 1 deletion src/changelog/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(())
}
Expand Down
6 changes: 4 additions & 2 deletions src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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}\"")]
Expand Down
23 changes: 13 additions & 10 deletions src/fs_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,8 @@ pub fn path_to_str<P: AsRef<Path>>(path: P) -> String {
}

pub fn read_to_string<P: AsRef<Path>>(path: P) -> Result<String> {
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<P: AsRef<Path>>(path: P) -> Result<Option<String>> {
Expand All @@ -23,10 +24,11 @@ pub fn read_to_string_opt<P: AsRef<Path>>(path: P) -> Result<Option<String>> {

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(())
Expand All @@ -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(())
Expand All @@ -45,20 +47,21 @@ pub fn read_and_filter_dir<F>(path: &Path, filter: F) -> Result<Vec<PathBuf>>
where
F: Fn(fs::DirEntry) -> Option<Result<PathBuf>>,
{
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::<Result<Vec<PathBuf>>>()
}

pub fn entry_filter(config: &Config, e: fs::DirEntry) -> Option<Result<PathBuf>> {
let meta = match e.metadata() {
pub fn entry_filter(config: &Config, entry: fs::DirEntry) -> Option<Result<PathBuf>> {
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))
Expand Down

0 comments on commit 3777df9

Please sign in to comment.