Implement custom Plugin error, clean up lossy use, fix matching on bool

Signed-off-by: Kate Goldenring <kate.goldenring@fermyon.com>

Author: kate-goldenring

Cargo.lock

@@ -16,5 +16,6 @@ serde_json = "1.0"
 sha2 = "0.10.2"
 tar = "0.4.38"
 tempfile = "3.3.0"
+thiserror = "1"
 tokio = { version = "1.10", features = [ "fs", "process", "rt", "macros" ] }
 url = "2.2.2"

crates/plugins/Cargo.toml

@@ -0,0 +1,89 @@
+pub type PluginLookupResult<T> = std::result::Result<T, Error>;
+
+/// Error message during plugin lookup or deserializing
+#[derive(Debug, thiserror::Error)]
+pub enum Error {
+    #[error("{0}")]
+    NotFound(NotFoundError),
+
+    #[error("{0}")]
+    ConnectionFailed(ConnectionFailedError),
+
+    #[error("{0}")]
+    InvalidManifest(InvalidManifestError),
+
+    #[error("URL parse error {0}")]
+    UrlParseError(#[from] url::ParseError),
+}
+
+/// Contains error details for when a plugin resource cannot be found at expected location
+#[derive(Debug)]
+pub struct NotFoundError {
+    name: Option<String>,
+    addr: String,
+    err: String,
+}
+
+impl NotFoundError {
+    pub fn new(name: Option<String>, addr: String, err: String) -> Self {
+        Self { name, addr, err }
+    }
+}
+
+impl std::fmt::Display for NotFoundError {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        f.write_str(&format!(
+            "plugin '{}' not found at expected location {}: {}",
+            self.name.as_deref().unwrap_or_default(),
+            self.addr,
+            self.err
+        ))
+    }
+}
+
+/// Contains error details for when a plugin manifest cannot be properly serialized
+#[derive(Debug)]
+pub struct InvalidManifestError {
+    name: Option<String>,
+    addr: String,
+    err: String,
+}
+
+impl InvalidManifestError {
+    pub fn new(name: Option<String>, addr: String, err: String) -> Self {
+        Self { name, addr, err }
+    }
+}
+
+impl std::fmt::Display for InvalidManifestError {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        f.write_str(&format!(
+            "invalid manifest for plugin '{}' at {}: {}",
+            self.name.clone().unwrap_or_default(),
+            self.addr,
+            self.err
+        ))
+    }
+}
+
+/// Contains error details for when there is an error getting a plugin resource from an address.
+#[derive(Debug)]
+pub struct ConnectionFailedError {
+    addr: String,
+    err: String,
+}
+
+impl ConnectionFailedError {
+    pub fn new(addr: String, err: String) -> Self {
+        Self { addr, err }
+    }
+}
+
+impl std::fmt::Display for ConnectionFailedError {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        f.write_str(&format!(
+            "failed to connect to endpoint {}: {}",
+            self.addr, self.err
+        ))
+    }
+}

crates/plugins/src/error.rs

@@ -1,4 +1,4 @@
-use anyhow::{anyhow, Result};
+use anyhow::Result;
 use std::path::{Path, PathBuf};
 use tokio::process::Command;
 use url::Url;
@@ -18,54 +18,48 @@ pub struct GitSource {
 
 impl GitSource {
     /// Creates a new git source
-    pub fn new(
-        source_url: &Url,
-        branch: Option<String>,
-        git_root: impl AsRef<Path>,
-    ) -> Result<GitSource> {
-        Ok(Self {
+    pub fn new(source_url: &Url, branch: Option<String>, git_root: impl AsRef<Path>) -> GitSource {
+        Self {
             source_url: source_url.clone(),
             branch: branch.unwrap_or_else(|| DEFAULT_BRANCH.to_owned()),
             git_root: git_root.as_ref().to_owned(),
-        })
+        }
     }
 
     /// Clones a contents of a git repository to a local directory
-    pub async fn clone(&self) -> Result<()> {
+    pub async fn clone_repo(&self) -> Result<()> {
         let mut git = Command::new("git");
         git.args([
             "clone",
             self.source_url.as_ref(),
             "--branch",
             &self.branch,
             "--single-branch",
-            &self.git_root.to_string_lossy(),
-        ]);
+        ])
+        .arg(&self.git_root);
         let clone_result = git.output().await?;
-        match clone_result.status.success() {
-            true => Ok(()),
-            false => Err(anyhow!(
+        if !clone_result.status.success() {
+            anyhow::bail!(
                 "Error cloning Git repo {}: {}",
                 self.source_url,
-                String::from_utf8(clone_result.stderr)
-                    .unwrap_or_else(|_| "(cannot get error)".to_owned())
-            )),
+                String::from_utf8_lossy(&clone_result.stderr)
+            )
         }
+        Ok(())
     }
 
     /// Fetches the latest changes from the source repository
     pub async fn pull(&self) -> Result<()> {
         let mut git = Command::new("git");
-        git.args(["-C", &self.git_root.to_string_lossy(), "pull"]);
+        git.arg("-C").arg(&self.git_root).arg("pull");
         let pull_result = git.output().await?;
-        match pull_result.status.success() {
-            true => Ok(()),
-            false => Err(anyhow!(
+        if !pull_result.status.success() {
+            anyhow::bail!(
                 "Error updating Git repo at {}: {}",
                 self.git_root.display(),
-                String::from_utf8(pull_result.stderr)
-                    .unwrap_or_else(|_| "(cannot update error)".to_owned())
-            )),
+                String::from_utf8_lossy(&pull_result.stderr)
+            )
         }
+        Ok(())
     }
 }

crates/plugins/src/git.rs

@@ -1,10 +1,10 @@
+pub mod error;
 mod git;
-mod lookup;
+pub mod lookup;
 pub mod manager;
 pub mod manifest;
 mod prompt;
 mod store;
-pub use lookup::{fetch_plugins_repo, PluginLookup, PLUGIN_NOT_FOUND_ERROR_MSG};
 pub use prompt::prompt_confirm_install;
 pub use store::PluginStore;
 

crates/plugins/src/lib.rs

@@ -1,5 +1,4 @@
-use crate::{git::GitSource, manifest::PluginManifest, store::manifest_file_name};
-use anyhow::{Context, Result};
+use crate::{error::*, git::GitSource, manifest::PluginManifest, store::manifest_file_name};
 use semver::Version;
 use std::{
     fs::File,
@@ -14,9 +13,6 @@ const PLUGINS_REPO_LOCAL_DIRECTORY: &str = ".spin-plugins";
 // Name of directory containing the installed manifests
 const PLUGINS_REPO_MANIFESTS_DIRECTORY: &str = "manifests";
 
-// Error message indicating plugin could not be found in plugins repository.
-pub const PLUGIN_NOT_FOUND_ERROR_MSG: &str = "plugin not found";
-
 const SPIN_PLUGINS_REPO: &str = "https://github.com/fermyon/spin-plugins/";
 
 /// Looks up plugin manifests in centralized spin plugin repository.
@@ -33,49 +29,53 @@ impl PluginLookup {
         }
     }
 
-    fn version_string(&self) -> String {
-        self.version
-            .as_ref()
-            .map(|s| s.to_string())
-            .unwrap_or_else(|| String::from("latest"))
-    }
-
-    pub async fn get_manifest_from_repository(&self, plugins_dir: &Path) -> Result<PluginManifest> {
-        log::info!(
-            "Pulling manifest for plugin {} from {}",
-            self.name,
-            SPIN_PLUGINS_REPO
-        );
-        fetch_plugins_repo(plugins_dir, false).await?;
-        let file = File::open(spin_plugins_repo_manifest_path(
-            &self.name,
-            &self.version,
-            plugins_dir,
-        ))
-        .with_context(|| {
-            format!(
-                "{} {} {} in centralized repository",
-                self.name,
-                self.version_string(),
-                PLUGIN_NOT_FOUND_ERROR_MSG,
-            )
+    pub async fn get_manifest_from_repository(
+        &self,
+        plugins_dir: &Path,
+    ) -> PluginLookupResult<PluginManifest> {
+        let url = plugins_repo_url()?;
+        log::info!("Pulling manifest for plugin {} from {url}", self.name);
+        fetch_plugins_repo(&url, plugins_dir, false)
+            .await
+            .map_err(|e| {
+                Error::ConnectionFailed(ConnectionFailedError::new(url.to_string(), e.to_string()))
+            })?;
+        let expected_path = spin_plugins_repo_manifest_path(&self.name, &self.version, plugins_dir);
+        let file = File::open(&expected_path).map_err(|e| {
+            Error::NotFound(NotFoundError::new(
+                Some(self.name.clone()),
+                expected_path.display().to_string(),
+                e.to_string(),
+            ))
+        })?;
+        let manifest: PluginManifest = serde_json::from_reader(file).map_err(|e| {
+            Error::InvalidManifest(InvalidManifestError::new(
+                Some(self.name.clone()),
+                expected_path.display().to_string(),
+                e.to_string(),
+            ))
         })?;
-        let manifest: PluginManifest = serde_json::from_reader(file)
-            .with_context(|| format!("Failed to deserialize {} plugin manifest", self.name))?;
         Ok(manifest)
     }
 }
 
-pub async fn fetch_plugins_repo(plugins_dir: &Path, update: bool) -> Result<()> {
-    let repo_url = Url::parse(SPIN_PLUGINS_REPO)?;
+pub fn plugins_repo_url() -> Result<Url, url::ParseError> {
+    Url::parse(SPIN_PLUGINS_REPO)
+}
+
+pub async fn fetch_plugins_repo(
+    repo_url: &Url,
+    plugins_dir: &Path,
+    update: bool,
+) -> anyhow::Result<()> {
     let git_root = plugin_manifests_repo_path(plugins_dir);
-    let git_source = GitSource::new(&repo_url, None, &git_root)?;
+    let git_source = GitSource::new(repo_url, None, &git_root);
     if git_root.join(".git").exists() {
         if update {
             git_source.pull().await?;
         }
     } else {
-        git_source.clone().await?;
+        git_source.clone_repo().await?;
     }
     Ok(())
 }