Improve errors for missing mapped commands

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<S: ToString>(src: S) -> Self {
-        Self::new(src, "unknown version")
+        Self::new(src, "Unknown version.")
     }
 
     pub fn unfulfillable_version<S: ToString>(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<S: ToString>(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<OsString>, 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::<Vec<_>>()
+                .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,
+}

src/mapper/error.rs

@@ -1,25 +0,0 @@
-use std::{io, path::PathBuf};
-
-use miette::Diagnostic;
-use thiserror::Error;
-
-use super::mapped_command::CommandError;
-
-pub type MapperResult<T> = Result<T, MapperError>;
-
-#[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,
-    },
-}

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<OsString>,
 }
 
-pub type CommandResult<T> = Result<T, CommandError>;
-
-#[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<OsString>) -> Self {
-        Self { path, args }
+    pub fn new(name: String, path: PathBuf, args: Vec<OsString>) -> Self {
+        Self { name, path, args }
     }
 
     #[tracing::instrument(skip_all, level = "debug")]
-    pub async fn run(mut self) -> CommandResult<ExitStatus> {
+    pub async fn run(mut self) -> Result<ExitStatus> {
         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())
     }
 }

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<Vec<NodeApp>> {
+async fn get_applications(path: &Path) -> Result<Vec<NodeApp>> {
     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) {

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)

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);
         }