From 8b4383aaf3020e04d532a35e16bdf9bf31ade1b6 Mon Sep 17 00:00:00 2001 From: trivernis Date: Sun, 22 Jan 2023 12:44:13 +0100 Subject: [PATCH] Improve errors for missing mapped commands --- src/error.rs | 70 ++++++++++++++++++++++++++++++------ src/mapper/error.rs | 25 ------------- src/mapper/mapped_command.rs | 49 +++++++++++++------------ src/mapper/mapped_dir.rs | 31 ++++++++-------- src/mapper/mod.rs | 13 ++----- src/web_api/mod.rs | 10 +++--- 6 files changed, 111 insertions(+), 87 deletions(-) delete mode 100644 src/mapper/error.rs diff --git a/src/error.rs b/src/error.rs index 9bac734..aaf71ad 100644 --- a/src/error.rs +++ b/src/error.rs @@ -1,8 +1,10 @@ +use std::{ffi::OsString, path::PathBuf}; + use miette::{Diagnostic, NamedSource, SourceSpan}; use thiserror::Error; -use crate::{mapper::error::MapperError, repository::extract::ExtractError}; +use crate::repository::extract::ExtractError; #[derive(Debug, Error, Diagnostic)] pub enum Error { @@ -10,10 +12,6 @@ pub enum Error { #[error("The node archive could not be extracted")] Extract(#[from] ExtractError), - #[diagnostic(code(nenv::mapper))] - #[error("Mapping failed")] - Mapper(#[from] MapperError), - #[diagnostic(code(nenv::version))] #[error("The passed version is invalid")] Version(#[from] VersionError), @@ -45,20 +43,20 @@ impl VersionError { } pub fn unknown_version(src: S) -> Self { - Self::new(src, "unknown version") + Self::new(src, "Unknown version.") } pub fn unfulfillable_version(src: S) -> Self { - Self::new(src, "the version requirement cannot be fulfilled") + Self::new(src, "The version requirement cannot be fulfilled.") } pub fn not_installed(src: S) -> Self { - Self::new(src, "the version is not installed") + Self::new(src, "The version is not installed.") } } #[derive(Debug, Error, Diagnostic)] -#[error("failed to parse json")] +#[error("Failed to parse the contents as JSON.")] #[diagnostic(code(nenv::json::deserialize))] pub struct ParseJsonError { #[source_code] @@ -81,7 +79,7 @@ pub struct SerializeJsonError { #[derive(Debug, Error, Diagnostic)] #[diagnostic(code(nenv::toml::deserialize))] -#[error("failed to parse toml value")] +#[error("Failed to parse the toml file.")] pub struct ParseTomlError { #[source_code] src: NamedSource, @@ -116,7 +114,7 @@ impl ParseTomlError { #[derive(Debug, Error, Diagnostic)] #[diagnostic(code(nenv::toml::serialize))] -#[error("failed to serialize value to toml string")] +#[error("Failed to serialize the value to toml string.")] pub struct SerializeTomlError { #[from] caused_by: toml::ser::Error, @@ -126,3 +124,53 @@ pub struct SerializeTomlError { #[diagnostic(code(nenv::http))] #[error("http request failed")] pub struct ReqwestError(#[from] reqwest::Error); + +#[derive(Debug, Error, Diagnostic)] +#[diagnostic( + code(nenv::exec::command), + help("Make sure you selected the correct node version and check if {path:?} exist.") +)] +#[error("The command `{command}` could not be found for this node version.")] +pub struct CommandNotFoundError { + command: String, + + #[source_code] + full_command: String, + + path: PathBuf, + + #[label("this command")] + pos: SourceSpan, +} + +impl CommandNotFoundError { + pub fn new(command: String, args: Vec, path: PathBuf) -> Self { + let pos = (0, command.len()).into(); + let full_command = format!( + "{command} {}", + args.into_iter() + .map(|a| a.into_string().unwrap_or_default()) + .collect::>() + .join(" ") + ); + Self { + command, + full_command, + path, + pos, + } + } +} + +#[derive(Debug, Error, Diagnostic)] +#[error("Failed to create mappings to directory {dir:?}.")] +#[diagnostic( + code(nenv::map::command), + help("Check if this node version was installed correctly.") +)] +pub struct MapDirError { + pub dir: PathBuf, + + #[source] + pub caused_by: std::io::Error, +} diff --git a/src/mapper/error.rs b/src/mapper/error.rs deleted file mode 100644 index 999f804..0000000 --- a/src/mapper/error.rs +++ /dev/null @@ -1,25 +0,0 @@ -use std::{io, path::PathBuf}; - -use miette::Diagnostic; -use thiserror::Error; - -use super::mapped_command::CommandError; - -pub type MapperResult = Result; - -#[derive(Error, Diagnostic, Debug)] -pub enum MapperError { - #[error("Failed to execute mapped command")] - Command(#[from] CommandError), - - #[error("IO operation failed")] - Io(#[from] io::Error), - - #[error("Failed to map directory {src:?}")] - DirMapping { - src: PathBuf, - - #[source] - err: io::Error, - }, -} diff --git a/src/mapper/mapped_command.rs b/src/mapper/mapped_command.rs index 4b07a39..7227e9a 100644 --- a/src/mapper/mapped_command.rs +++ b/src/mapper/mapped_command.rs @@ -4,56 +4,56 @@ use std::{ process::{ExitStatus, Stdio}, }; -use thiserror::Error; -use tokio::{io, process::Command}; +use crate::error::CommandNotFoundError; +use miette::{Context, IntoDiagnostic, Result}; +use tokio::process::Command; pub struct MappedCommand { + name: String, path: PathBuf, args: Vec, } -pub type CommandResult = Result; - -#[derive(Error, Debug)] -pub enum CommandError { - #[error(transparent)] - Io(#[from] io::Error), - - #[error("The command {0:?} could not be found for this nodejs version")] - NotFound(PathBuf), -} - impl MappedCommand { - pub fn new(path: PathBuf, args: Vec) -> Self { - Self { path, args } + pub fn new(name: String, path: PathBuf, args: Vec) -> Self { + Self { name, path, args } } #[tracing::instrument(skip_all, level = "debug")] - pub async fn run(mut self) -> CommandResult { + pub async fn run(mut self) -> Result { self.adjust_path()?; let exit_status = Command::new(self.path) .args(self.args) .stdin(Stdio::inherit()) .stdout(Stdio::inherit()) .stderr(Stdio::inherit()) - .spawn()? + .spawn() + .into_diagnostic() + .context("Running mapped command")? .wait() - .await?; + .await + .into_diagnostic() + .context("Waiting for command to exit")?; Ok(exit_status) } #[cfg(not(target_os = "windows"))] - fn adjust_path(&mut self) -> CommandResult<()> { + fn adjust_path(&mut self) -> Result<()> { if !self.path.exists() { - Err(CommandError::NotFound(self.path.to_owned())) + Err(CommandNotFoundError::new( + self.name.to_owned(), + self.args.to_owned(), + self.path.to_owned(), + ) + .into()) } else { Ok(()) } } #[cfg(target_os = "windows")] - fn adjust_path(&mut self) -> CommandResult<()> { + fn adjust_path(&mut self) -> Result<()> { let extensions = ["exe", "bat", "cmd", "ps1"]; for extension in &extensions { let joined_path = self.path.with_extension(extension); @@ -63,6 +63,11 @@ impl MappedCommand { return Ok(()); } } - Err(CommandError::NotFound(self.path.to_owned())) + Err(CommandNotFoundError::new( + self.name.to_owned(), + self.args.to_owned(), + self.path.to_owned(), + ) + .into()) } } diff --git a/src/mapper/mapped_dir.rs b/src/mapper/mapped_dir.rs index a5490c7..d8433bf 100644 --- a/src/mapper/mapped_dir.rs +++ b/src/mapper/mapped_dir.rs @@ -2,10 +2,9 @@ use std::{collections::HashSet, io, path::Path}; use tokio::fs::{self, DirEntry}; -use crate::{consts::BIN_DIR, repository::node_path::NodePath}; - -use super::error::{MapperError, MapperResult}; +use crate::{consts::BIN_DIR, error::MapDirError, repository::node_path::NodePath}; +use miette::{Context, IntoDiagnostic, Result}; struct NodeApp { info: DirEntry, name: String, @@ -25,11 +24,12 @@ impl NodeApp { } /// creates wrappers to map this application - pub async fn map_executable(&self) -> MapperResult<()> { + pub async fn map_executable(&self) -> Result<()> { let src_path = BIN_DIR.join(self.info.file_name()); self.write_wrapper_script(&src_path) .await - .map_err(|err| MapperError::DirMapping { src: src_path, err }) + .into_diagnostic() + .context("Creating executable wrapper script") } #[cfg(not(target_os = "windows"))] @@ -55,7 +55,7 @@ impl NodeApp { } } -pub async fn map_node_bin(node_path: NodePath) -> MapperResult<()> { +pub async fn map_node_bin(node_path: NodePath) -> Result<()> { let mapped_app_names = get_applications(&*BIN_DIR) .await? .iter() @@ -71,16 +71,19 @@ pub async fn map_node_bin(node_path: NodePath) -> MapperResult<()> { Ok(()) } -async fn get_applications(path: &Path) -> MapperResult> { +async fn get_applications(path: &Path) -> Result> { let mut files = Vec::new(); - let mut iter = fs::read_dir(path) - .await - .map_err(|err| MapperError::DirMapping { - src: path.to_owned(), - err, - })?; + let mut iter = fs::read_dir(path).await.map_err(|err| MapDirError { + dir: path.to_owned(), + caused_by: err, + })?; - while let Some(entry) = iter.next_entry().await? { + while let Some(entry) = iter + .next_entry() + .await + .into_diagnostic() + .context("Reading directory entries")? + { let entry_path = entry.path(); if entry_path.is_file() && !exclude_path(&entry_path) { diff --git a/src/mapper/mod.rs b/src/mapper/mod.rs index e54f0ee..e74d4f9 100644 --- a/src/mapper/mod.rs +++ b/src/mapper/mod.rs @@ -8,13 +8,9 @@ use crate::{ repository::{NodeVersion, Repository}, }; -use self::{ - error::MapperError, mapped_command::MappedCommand, mapped_dir::map_node_bin, - package_info::PackageInfo, -}; +use self::{mapped_command::MappedCommand, mapped_dir::map_node_bin, package_info::PackageInfo}; use miette::{IntoDiagnostic, Result}; -pub mod error; mod mapped_command; mod mapped_dir; mod package_info; @@ -63,11 +59,8 @@ impl Mapper { .repo .get_version_path(&self.active_version)? .ok_or_else(|| VersionError::not_installed(&self.active_version))?; - let executable = node_path.bin().join(command); - let exit_status = MappedCommand::new(executable, args) - .run() - .await - .map_err(MapperError::from)?; + let executable = node_path.bin().join(&command); + let exit_status = MappedCommand::new(command, executable, args).run().await?; self.map_active_version().await?; Ok(exit_status) diff --git a/src/web_api/mod.rs b/src/web_api/mod.rs index dc4c0c2..502c3eb 100644 --- a/src/web_api/mod.rs +++ b/src/web_api/mod.rs @@ -50,11 +50,11 @@ impl WebApi { .send() .await .map_err(ReqwestError::from) - .context("fetching versions")? + .context("Fetching versions")? .json() .await .map_err(ReqwestError::from) - .context("fetching versions")?; + .context("Parsing versions response")?; Ok(versions) } @@ -76,11 +76,11 @@ impl WebApi { .send() .await .map_err(ReqwestError::from) - .context("downloading nodejs")?; + .context("Downloading nodejs")?; let total_size = res .content_length() - .ok_or_else(|| miette!("missing content_length header"))?; + .ok_or_else(|| miette!("Missing content_length header"))?; let pb = progress_bar(total_size); pb.set_message(format!("Downloading node v{version}")); @@ -93,7 +93,7 @@ impl WebApi { .write_all(&chunk) .await .into_diagnostic() - .context("writing download chunk to file")?; + .context("Writing download chunk to file")?; total_downloaded = min(chunk.len() as u64 + total_downloaded, total_size); pb.set_position(total_downloaded); }