From 8b8c3334c09e272f5c4d588f7347c62c88992f3d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafael=20Fern=C3=A1ndez=20L=C3=B3pez?= Date: Thu, 24 Aug 2023 11:55:16 +0200 Subject: [PATCH] feat: use custom error types in internal and external crates (#182) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat: use custom error types in internal and external crates * chore: be explicit in tests about the expected and returned value * Apply suggestions from code review --------- Co-authored-by: Ángel M --- Cargo.lock | 7 --- Cargo.toml | 3 +- crates/config/Cargo.toml | 1 - crates/config/src/errors.rs | 39 +++++++++++++ crates/config/src/lib.rs | 16 +++--- crates/data-kv/Cargo.toml | 1 - crates/project/Cargo.toml | 1 - crates/project/src/errors.rs | 68 +++++++++++++++++++++++ crates/project/src/fetch.rs | 2 +- crates/project/src/lib.rs | 13 +++-- crates/project/src/metadata.rs | 11 ++-- crates/project/src/types/git.rs | 12 ++-- crates/runtimes/Cargo.toml | 1 - crates/runtimes/src/errors.rs | 40 +++++++++++++ crates/runtimes/src/lib.rs | 12 ++-- crates/runtimes/src/modules/external.rs | 12 ++-- crates/runtimes/src/modules/javascript.rs | 3 +- crates/runtimes/src/modules/native.rs | 5 +- crates/runtimes/src/runtime.rs | 3 +- crates/server/Cargo.toml | 1 - crates/server/src/errors.rs | 19 +++++++ crates/server/src/lib.rs | 13 ++++- crates/store/Cargo.toml | 1 - crates/store/src/errors.rs | 55 ++++++++++++++++++ crates/store/src/lib.rs | 48 ++++++++++------ crates/worker/Cargo.toml | 1 - crates/worker/src/config.rs | 23 +++----- crates/worker/src/errors.rs | 54 ++++++++++++++++++ crates/worker/src/io.rs | 7 ++- crates/worker/src/lib.rs | 60 ++++++++++++++------ src/commands/runtimes.rs | 20 ++++--- src/utils/errors.rs | 60 ++++++++++++++++++++ src/utils/mod.rs | 1 + src/utils/runtimes.rs | 11 ++-- tests/e2e.rs | 5 +- 35 files changed, 502 insertions(+), 127 deletions(-) create mode 100644 crates/config/src/errors.rs create mode 100644 crates/project/src/errors.rs create mode 100644 crates/runtimes/src/errors.rs create mode 100644 crates/server/src/errors.rs create mode 100644 crates/store/src/errors.rs create mode 100644 crates/worker/src/errors.rs create mode 100644 src/utils/errors.rs diff --git a/Cargo.lock b/Cargo.lock index 11a053de..ef4b6ac2 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4077,7 +4077,6 @@ dependencies = [ name = "wws-config" version = "1.4.0" dependencies = [ - "anyhow", "serde", "serde_json", "toml 0.7.4", @@ -4088,7 +4087,6 @@ dependencies = [ name = "wws-data-kv" version = "1.4.0" dependencies = [ - "anyhow", "serde", "serde_json", ] @@ -4106,7 +4104,6 @@ dependencies = [ name = "wws-project" version = "1.4.0" dependencies = [ - "anyhow", "git2", "openssl", "path-slash", @@ -4138,7 +4135,6 @@ dependencies = [ name = "wws-runtimes" version = "1.4.0" dependencies = [ - "anyhow", "serde", "serde_json", "wasmtime-wasi", @@ -4153,7 +4149,6 @@ version = "1.4.0" dependencies = [ "actix-files", "actix-web", - "anyhow", "wws-api-manage", "wws-data-kv", "wws-panel", @@ -4165,7 +4160,6 @@ dependencies = [ name = "wws-store" version = "1.4.0" dependencies = [ - "anyhow", "blake3", ] @@ -4174,7 +4168,6 @@ name = "wws-worker" version = "1.4.0" dependencies = [ "actix-web", - "anyhow", "base64", "reqwest", "serde", diff --git a/Cargo.toml b/Cargo.toml index fbc71466..d349318c 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -23,8 +23,8 @@ path = "src/main.rs" # Main binary (wws CLI dependencies) [dependencies] -anyhow = { workspace = true } actix-web = { workspace = true } +anyhow = "1.0.66" env_logger = "0.10.0" clap = { version = "4.0.10", features = ["derive"] } prettytable-rs = "0.10.0" @@ -74,7 +74,6 @@ exclude = [ [workspace.dependencies] actix-web = "4" -anyhow = "1.0.66" lazy_static = "1.4.0" reqwest = "0.11" serde = { version = "1.0", features = ["derive"] } diff --git a/crates/config/Cargo.toml b/crates/config/Cargo.toml index 173c1dd8..e42e2a34 100644 --- a/crates/config/Cargo.toml +++ b/crates/config/Cargo.toml @@ -7,7 +7,6 @@ license = { workspace = true } repository = { workspace = true } [dependencies] -anyhow = { workspace = true } serde = { workspace = true } serde_json = { workspace = true } toml = { workspace = true } diff --git a/crates/config/src/errors.rs b/crates/config/src/errors.rs new file mode 100644 index 00000000..989a9e7e --- /dev/null +++ b/crates/config/src/errors.rs @@ -0,0 +1,39 @@ +// Copyright 2023 VMware, Inc. +// SPDX-License-Identifier: Apache-2.0 + +pub type Result = std::result::Result; + +#[derive(Debug)] +pub enum ConfigError { + CannotLoadConfig(std::io::Error), + CannotParseConfig(toml::de::Error), + CannotSaveConfig, +} + +impl std::fmt::Display for ConfigError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + Self::CannotLoadConfig(err) => write!(f, "Could not load configuration: {}", err), + Self::CannotParseConfig(err) => write!(f, "Could not parse configuration: {}", err), + Self::CannotSaveConfig => write!(f, "Could not save configuration"), + } + } +} + +impl From for ConfigError { + fn from(error: std::io::Error) -> Self { + ConfigError::CannotLoadConfig(error) + } +} + +impl From for ConfigError { + fn from(error: toml::de::Error) -> ConfigError { + ConfigError::CannotParseConfig(error) + } +} + +impl From for ConfigError { + fn from(_: toml::ser::Error) -> ConfigError { + ConfigError::CannotSaveConfig + } +} diff --git a/crates/config/src/lib.rs b/crates/config/src/lib.rs index fb91a882..e4c229ff 100644 --- a/crates/config/src/lib.rs +++ b/crates/config/src/lib.rs @@ -1,7 +1,9 @@ // Copyright 2022 VMware, Inc. // SPDX-License-Identifier: Apache-2.0 -use anyhow::{anyhow, Result}; +pub mod errors; +pub use errors::Result; + use serde::{Deserialize, Serialize}; use std::{ fs, @@ -42,9 +44,7 @@ impl Config { let config_path = Self::config_path(project_root); if config_path.exists() { - toml::from_str(&fs::read_to_string(config_path)?).map_err(|_| { - anyhow!("Error opening the .wws.toml file. The file format is not correct") - }) + Ok(toml::from_str(&fs::read_to_string(config_path)?)?) } else { Ok(Self::default()) } @@ -111,10 +111,10 @@ impl Config { /// Write the current configuration into the `.wws.toml` file. It will /// store it in the project root folder pub fn save(&self, project_root: &Path) -> Result<()> { - let contents = toml::to_string_pretty(self)?; - - fs::write(Self::config_path(project_root), contents) - .map_err(|_| anyhow!("Error saving the .wws.toml file")) + Ok(fs::write( + Self::config_path(project_root), + toml::to_string_pretty(self)?, + )?) } /// Retrieve the configuration path from the project root diff --git a/crates/data-kv/Cargo.toml b/crates/data-kv/Cargo.toml index 0dcf5db9..d4414685 100644 --- a/crates/data-kv/Cargo.toml +++ b/crates/data-kv/Cargo.toml @@ -7,6 +7,5 @@ license = { workspace = true } repository = { workspace = true } [dependencies] -anyhow = { workspace = true } serde = { workspace = true } serde_json = { workspace = true } \ No newline at end of file diff --git a/crates/project/Cargo.toml b/crates/project/Cargo.toml index aff5ca11..41cae7de 100644 --- a/crates/project/Cargo.toml +++ b/crates/project/Cargo.toml @@ -7,7 +7,6 @@ license = { workspace = true } repository = { workspace = true } [dependencies] -anyhow = { workspace = true } reqwest = { workspace = true } serde = { workspace = true } serde_json = { workspace = true } diff --git a/crates/project/src/errors.rs b/crates/project/src/errors.rs new file mode 100644 index 00000000..4c15f43a --- /dev/null +++ b/crates/project/src/errors.rs @@ -0,0 +1,68 @@ +// Copyright 2023 VMware, Inc. +// SPDX-License-Identifier: Apache-2.0 + +use wws_store::errors::StoreError; + +pub type Result = std::result::Result; + +#[derive(Debug)] +pub enum FetchError { + DefaultBranchMissing, + GitError(git2::Error), + HttpError(reqwest::Error), + InvalidURL, + InvalidReusedRepository, + InvalidRepository, + InvalidChecksum, + MissingPathInFilesystem, + StoreError(StoreError), +} + +impl std::fmt::Display for FetchError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + Self::DefaultBranchMissing => write!( + f, + "Couldn't find the default main branch. Please, set the Git branch you want to use" + ), + Self::GitError(err) => write!(f, "Git error: {}", err), + Self::HttpError(err) => write!(f, "HTTP error: {}", err), + Self::InvalidURL => write!(f, "Invalid URL"), + Self::InvalidReusedRepository => write!(f, "Invalid local copy of repository"), + Self::InvalidRepository => write!(f, "Invalid repository"), + Self::InvalidChecksum => write!(f, "Invalid checksum"), + Self::MissingPathInFilesystem => write!(f, "Missing path in filesystem"), + Self::StoreError(err) => write!(f, "Store error: {}", err), + } + } +} + +impl From for FetchError { + fn from(error: git2::Error) -> Self { + FetchError::GitError(error) + } +} + +impl From for FetchError { + fn from(_error: url::ParseError) -> Self { + FetchError::InvalidURL + } +} + +impl From for FetchError { + fn from(error: reqwest::Error) -> Self { + FetchError::HttpError(error) + } +} + +impl From for FetchError { + fn from(error: StoreError) -> Self { + FetchError::StoreError(error) + } +} + +impl From for FetchError { + fn from(_error: std::string::FromUtf8Error) -> Self { + FetchError::InvalidRepository + } +} diff --git a/crates/project/src/fetch.rs b/crates/project/src/fetch.rs index df8f7c81..3af13921 100644 --- a/crates/project/src/fetch.rs +++ b/crates/project/src/fetch.rs @@ -1,8 +1,8 @@ // Copyright 2022 VMware, Inc. // SPDX-License-Identifier: Apache-2.0 +use crate::errors::Result; use crate::metadata::Checksum; -use anyhow::Result; use reqwest::header::USER_AGENT; /// The current wws version diff --git a/crates/project/src/lib.rs b/crates/project/src/lib.rs index 91f5eb89..fa2fd3ae 100644 --- a/crates/project/src/lib.rs +++ b/crates/project/src/lib.rs @@ -1,12 +1,13 @@ // Copyright 2022-2023 VMware, Inc. // SPDX-License-Identifier: Apache-2.0 +pub mod errors; mod fetch; pub mod metadata; pub mod options; pub mod types; -use anyhow::{bail, Result}; +use errors::Result; use fetch::fetch_and_validate; use metadata::{RemoteFile, Runtime}; use options::Options; @@ -68,7 +69,7 @@ pub fn identify_type(location: &Path) -> Result { if path.exists() { Ok(ProjectType::Local) } else { - bail!("The given path does not exist in the local filesystem.") + Err(errors::FetchError::MissingPathInFilesystem) } } } @@ -138,11 +139,11 @@ pub fn check_runtime(project_root: &Path, repository: &str, runtime: &Runtime) - /// and delete the folder. pub fn uninstall_runtime(project_root: &Path, repository: &str, metadata: &Runtime) -> Result<()> { // Delete the current folder - Store::new( + Ok(Store::new( project_root, &["runtimes", repository, &metadata.name, &metadata.version], ) - .delete_root_folder() + .delete_root_folder()?) } /// Downloads a remote file and saves into the given store. This @@ -150,7 +151,7 @@ pub fn uninstall_runtime(project_root: &Path, repository: &str, metadata: &Runti /// in the metadata. async fn download_file(file: &RemoteFile, store: &Store) -> Result<()> { let contents = fetch_and_validate(&file.url, &file.checksum).await?; - store.write(&[&file.filename], &contents) + Ok(store.write(&[&file.filename], &contents)?) } #[cfg(test)] @@ -190,7 +191,7 @@ mod tests { Ok(_) => { panic!("The folder doesn't exist, so identifying it should fail."); } - Err(err) => assert!(err.to_string().contains("does not exist")), + Err(err) => assert!(err.to_string().contains("Missing path in filesystem")), } } } diff --git a/crates/project/src/metadata.rs b/crates/project/src/metadata.rs index 45c5c425..2793197b 100644 --- a/crates/project/src/metadata.rs +++ b/crates/project/src/metadata.rs @@ -1,8 +1,8 @@ // Copyright 2022 VMware, Inc. // SPDX-License-Identifier: Apache-2.0 +use crate::errors::{self, Result}; use crate::fetch::fetch; -use anyhow::{anyhow, Result}; use serde::{Deserialize, Serialize}; use sha256::digest as sha256_digest; use std::collections::HashMap; @@ -61,15 +61,12 @@ impl Repository { } impl FromStr for Repository { - type Err = anyhow::Error; + type Err = errors::FetchError; /// Reads and parses the metadata from a slice of bytes. It will return /// a result as the deserialization may fail. fn from_str(data: &str) -> Result { - toml::from_str::(data).map_err(|err| { - println!("Err: {err}"); - anyhow!("wws could not deserialize the repository metadata") - }) + toml::from_str::(data).map_err(|_| errors::FetchError::InvalidRepository) } } @@ -169,7 +166,7 @@ impl Checksum { pub fn validate(&self, bytes: &[u8]) -> Result<()> { match self { Checksum::Sha256 { value } if value == &sha256_digest(bytes) => Ok(()), - _ => Err(anyhow!("The checksums don't match")), + _ => Err(errors::FetchError::InvalidChecksum), } } } diff --git a/crates/project/src/types/git.rs b/crates/project/src/types/git.rs index d92a2ddf..ec69ebf1 100644 --- a/crates/project/src/types/git.rs +++ b/crates/project/src/types/git.rs @@ -1,8 +1,8 @@ // Copyright 2023 VMware, Inc. // SPDX-License-Identifier: Apache-2.0 +use crate::errors::{self, Result}; use crate::options::{GitReference, Options}; -use anyhow::{anyhow, bail, Result}; use git2::{build::CheckoutBuilder, FetchOptions, Oid, Repository}; use sha256::digest as sha256_digest; use std::{ @@ -18,19 +18,17 @@ static DEFAULT_REMOTE: &str = "origin"; pub fn prepare_git_project(location: &Path, options: Options) -> Result { let project_url = location .to_str() - .ok_or(anyhow!("The project URL cannot be retrieved"))?; + .ok_or(errors::FetchError::InvalidRepository)?; let (folder, git_ref) = parse_options(options); // By default, we use temporary dirs let mut dir = temp_dir().join(sha256_digest(project_url)); let repo = if dir.exists() { // Reuse the same repository. - Repository::open(&dir) - .map_err(|e| anyhow!("There was an error opening the repository: {e}"))? + Repository::open(&dir).map_err(|_| errors::FetchError::InvalidReusedRepository)? } else { // clone it - Repository::clone(project_url, &dir) - .map_err(|e| anyhow!("There was an error cloning the repository: {e}"))? + Repository::clone(project_url, &dir).map_err(|_| errors::FetchError::InvalidRepository)? }; if let Some(git_ref) = git_ref.as_ref() { @@ -106,7 +104,7 @@ fn detect_main_branch(repo: &Repository) -> Result<&str> { } else if repo.find_branch("master", git2::BranchType::Local).is_ok() { Ok("master") } else { - bail!("Couldn't find the default main branch. Please, set the Git branch you want to use.") + Err(errors::FetchError::DefaultBranchMissing) } } diff --git a/crates/runtimes/Cargo.toml b/crates/runtimes/Cargo.toml index f7d90b73..1542be01 100644 --- a/crates/runtimes/Cargo.toml +++ b/crates/runtimes/Cargo.toml @@ -7,7 +7,6 @@ license = { workspace = true } repository = { workspace = true } [dependencies] -anyhow = { workspace = true } serde = { workspace = true } serde_json = { workspace = true } wasmtime-wasi = { workspace = true } diff --git a/crates/runtimes/src/errors.rs b/crates/runtimes/src/errors.rs new file mode 100644 index 00000000..d6d74ba2 --- /dev/null +++ b/crates/runtimes/src/errors.rs @@ -0,0 +1,40 @@ +// Copyright 2023 VMware, Inc. +// SPDX-License-Identifier: Apache-2.0 + +pub type Result = std::result::Result; + +#[derive(Debug)] +pub enum RuntimeError { + CannotReadModule, + InvalidExtension { extension: Option }, + InvalidWrapper, + IOError(std::io::Error), + MissingRuntime { extension: String }, + StoreError(wws_store::errors::StoreError), + WasiContextError, + WasiError(Option), +} + +impl From for RuntimeError { + fn from(error: wws_store::errors::StoreError) -> Self { + RuntimeError::StoreError(error) + } +} + +impl From for RuntimeError { + fn from(_error: std::string::FromUtf8Error) -> Self { + RuntimeError::InvalidWrapper + } +} + +impl From for RuntimeError { + fn from(error: std::io::Error) -> Self { + RuntimeError::IOError(error) + } +} + +impl From for RuntimeError { + fn from(error: wasmtime_wasi::Error) -> Self { + RuntimeError::WasiError(Some(error)) + } +} diff --git a/crates/runtimes/src/lib.rs b/crates/runtimes/src/lib.rs index 3bf14594..687bb4c1 100644 --- a/crates/runtimes/src/lib.rs +++ b/crates/runtimes/src/lib.rs @@ -1,10 +1,12 @@ //// Copyright 2022 VMware, Inc. // SPDX-License-Identifier: Apache-2.0 +pub mod errors; +use errors::Result; + mod modules; mod runtime; -use anyhow::{anyhow, Result}; use modules::{external::ExternalRuntime, javascript::JavaScriptRuntime, native::NativeRuntime}; use std::path::Path; use wws_config::Config; @@ -33,7 +35,7 @@ pub fn init_runtime( other => init_external_runtime(project_root, config, path, other), } } else { - Err(anyhow!("The given file does not have a valid extension")) + Err(errors::RuntimeError::InvalidExtension { extension: None }) } } @@ -67,8 +69,8 @@ fn init_external_runtime( runtime_config.clone(), )?)) } else { - Err(anyhow!(format!( - "The '{extension}' extension does not have an associated runtime" - ))) + Err(errors::RuntimeError::MissingRuntime { + extension: extension.to_string(), + }) } } diff --git a/crates/runtimes/src/modules/external.rs b/crates/runtimes/src/modules/external.rs index 04770b0e..4ced4544 100644 --- a/crates/runtimes/src/modules/external.rs +++ b/crates/runtimes/src/modules/external.rs @@ -1,8 +1,9 @@ // Copyright 2022 VMware, Inc. // SPDX-License-Identifier: Apache-2.0 +use crate::errors::{self, Result}; + use crate::runtime::Runtime; -use anyhow::Result; use std::{ fs, path::{Path, PathBuf}, @@ -92,15 +93,18 @@ impl Runtime for ExternalRuntime { fn prepare_wasi_ctx(&self, builder: WasiCtxBuilder) -> Result { let dir = Dir::open_ambient_dir(&self.store.folder, ambient_authority())?; - Ok(builder + builder .preopened_dir(dir, "/src")? - .args(&self.metadata.args)?) + .args(&self.metadata.args) + .map_err(|_| errors::RuntimeError::WasiContextError) } /// Returns a reference to the Wasm module that should /// run this worker. It can be a custom (native) or a /// shared module (others). fn module_bytes(&self) -> Result> { - self.runtime_store.read(&[&self.metadata.binary.filename]) + self.runtime_store + .read(&[&self.metadata.binary.filename]) + .map_err(|_| errors::RuntimeError::CannotReadModule) } } diff --git a/crates/runtimes/src/modules/javascript.rs b/crates/runtimes/src/modules/javascript.rs index baa41dc9..0c16a458 100644 --- a/crates/runtimes/src/modules/javascript.rs +++ b/crates/runtimes/src/modules/javascript.rs @@ -1,8 +1,9 @@ // Copyright 2022 VMware, Inc. // SPDX-License-Identifier: Apache-2.0 +use crate::errors::Result; use crate::runtime::Runtime; -use anyhow::Result; + use std::path::{Path, PathBuf}; use wasmtime_wasi::{ambient_authority, Dir, WasiCtxBuilder}; use wws_store::Store; diff --git a/crates/runtimes/src/modules/native.rs b/crates/runtimes/src/modules/native.rs index e945b412..4f3c18c5 100644 --- a/crates/runtimes/src/modules/native.rs +++ b/crates/runtimes/src/modules/native.rs @@ -1,8 +1,9 @@ // Copyright 2022 VMware, Inc. // SPDX-License-Identifier: Apache-2.0 +use crate::errors::Result; use crate::runtime::Runtime; -use anyhow::Result; + use std::{fs, path::PathBuf}; pub struct NativeRuntime { @@ -22,6 +23,6 @@ impl Runtime for NativeRuntime { /// run this worker. It can be a custom (native) or a /// shared module (others). fn module_bytes(&self) -> Result> { - fs::read(&self.path).map_err(anyhow::Error::msg) + fs::read(&self.path).map_err(|_| crate::errors::RuntimeError::CannotReadModule) } } diff --git a/crates/runtimes/src/runtime.rs b/crates/runtimes/src/runtime.rs index 5f337ea3..47d2bdd0 100644 --- a/crates/runtimes/src/runtime.rs +++ b/crates/runtimes/src/runtime.rs @@ -1,7 +1,8 @@ // Copyright 2022 VMware, Inc. // SPDX-License-Identifier: Apache-2.0 -use anyhow::Result; +use crate::errors::Result; + use wasmtime_wasi::WasiCtxBuilder; /// Define the behavior a Runtime must have. This includes methods diff --git a/crates/server/Cargo.toml b/crates/server/Cargo.toml index 96796e6d..af07f7a9 100644 --- a/crates/server/Cargo.toml +++ b/crates/server/Cargo.toml @@ -7,7 +7,6 @@ license = { workspace = true } repository = { workspace = true } [dependencies] -anyhow = { workspace = true } actix-web = { workspace = true } wws-api-manage = { workspace = true } wws-data-kv = { workspace = true } diff --git a/crates/server/src/errors.rs b/crates/server/src/errors.rs new file mode 100644 index 00000000..a8f4b35a --- /dev/null +++ b/crates/server/src/errors.rs @@ -0,0 +1,19 @@ +// Copyright 2023 VMware, Inc. +// SPDX-License-Identifier: Apache-2.0 + +pub type Result = std::result::Result; + +#[derive(Debug)] +pub enum ServeError { + InitializeServerError, +} + +impl std::error::Error for ServeError {} + +impl std::fmt::Display for ServeError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + ServeError::InitializeServerError => write!(f, "Error initializing server"), + } + } +} diff --git a/crates/server/src/lib.rs b/crates/server/src/lib.rs index b211e55b..ecce2418 100644 --- a/crates/server/src/lib.rs +++ b/crates/server/src/lib.rs @@ -1,6 +1,9 @@ // Copyright 2022 VMware, Inc. // SPDX-License-Identifier: Apache-2.0 +mod errors; +use errors::Result; + mod handlers; use actix_files::Files; @@ -10,7 +13,6 @@ use actix_web::{ web::{self, Data}, App, HttpServer, }; -use anyhow::Result; use handlers::assets::handle_assets; use handlers::not_found::handle_not_found; use handlers::worker::handle_worker; @@ -46,7 +48,11 @@ pub async fn serve( // Configure stderr if let Some(path) = stderr { - let file = OpenOptions::new().read(true).write(true).open(path)?; + let file = OpenOptions::new() + .read(true) + .write(true) + .open(path) + .map_err(|_| errors::ServeError::InitializeServerError)?; stderr_file = Data::new(Some(file)); } else { @@ -110,7 +116,8 @@ pub async fn serve( app }) - .bind(format!("{}:{}", hostname, port))?; + .bind(format!("{}:{}", hostname, port)) + .map_err(|_| errors::ServeError::InitializeServerError)?; Ok(server.run()) } diff --git a/crates/store/Cargo.toml b/crates/store/Cargo.toml index d7e51f59..52c967d1 100644 --- a/crates/store/Cargo.toml +++ b/crates/store/Cargo.toml @@ -7,5 +7,4 @@ license = { workspace = true } repository = { workspace = true } [dependencies] -anyhow = { workspace = true } blake3 = "1.3.3" \ No newline at end of file diff --git a/crates/store/src/errors.rs b/crates/store/src/errors.rs new file mode 100644 index 00000000..2bfe5746 --- /dev/null +++ b/crates/store/src/errors.rs @@ -0,0 +1,55 @@ +// Copyright 2023 VMware, Inc. +// SPDX-License-Identifier: Apache-2.0 + +use std::path::PathBuf; + +pub type Result = std::result::Result; + +#[derive(Debug)] +pub enum StoreError { + CannotCreateDirectory { + path: PathBuf, + error: std::io::Error, + }, + CannotDeleteDirectory { + path: PathBuf, + error: std::io::Error, + }, + CannotReadFile { + path: PathBuf, + error: std::io::Error, + }, + CannotWriteFile { + path: PathBuf, + error: std::io::Error, + }, +} + +impl std::fmt::Display for StoreError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + Self::CannotCreateDirectory { path, error } => { + write!( + f, + "Could not create directory {}: {}", + path.display(), + error + ) + } + Self::CannotDeleteDirectory { path, error } => { + write!( + f, + "Could not delete directory {}: {}", + path.display(), + error + ) + } + Self::CannotReadFile { path, error } => { + write!(f, "Could not read file {}: {}", path.display(), error) + } + Self::CannotWriteFile { path, error } => { + write!(f, "Could not write to file {}: {}", path.display(), error) + } + } + } +} diff --git a/crates/store/src/lib.rs b/crates/store/src/lib.rs index 1ccb1414..cf93c23c 100644 --- a/crates/store/src/lib.rs +++ b/crates/store/src/lib.rs @@ -7,12 +7,14 @@ // // This struct provide the basics to interact with that folder // in both Unix and Windows systems. -use anyhow::{anyhow, Result}; use std::{ fs, path::{Path, PathBuf}, }; +pub mod errors; +use errors::{Result, StoreError}; + /// This is a temporary folder in which runtimes can prepare /// and store certain data. For example, the JS runtime have /// to mount a folder with the source code. To avoid moun†ing @@ -46,22 +48,29 @@ impl Store { let folder = Self::build_root_path(project_root, folder); // Try to create the directory - fs::create_dir_all(&folder)?; + fs::create_dir_all(&folder).map_err(|err| StoreError::CannotCreateDirectory { + path: folder.clone(), + error: err, + })?; Ok(Self { folder }) } /// Create the root folder for the current context pub fn create_root_folder(&self) -> Result<()> { - fs::create_dir_all(&self.folder) - .map_err(|err| anyhow!("There was an error creating the a required folder: {}", err)) + fs::create_dir_all(&self.folder).map_err(|err| StoreError::CannotCreateDirectory { + path: self.folder.clone(), + error: err, + }) } /// Delete the root folder from the current context pub fn delete_root_folder(&self) -> Result<()> { if self.folder.exists() { - fs::remove_dir_all(&self.folder) - .map_err(|err| anyhow!("There was an error deleting the folder: {}", err)) + fs::remove_dir_all(&self.folder).map_err(|err| StoreError::CannotDeleteDirectory { + path: self.folder.clone(), + error: err, + }) } else { Ok(()) } @@ -75,28 +84,28 @@ impl Store { /// Write a specific file inside the configured root folder pub fn write(&self, path: &[&str], contents: &[u8]) -> Result<()> { let file_path = self.build_folder_path(path); - fs::write(file_path, contents)?; - - Ok(()) + fs::write(&file_path, contents).map_err(|err| StoreError::CannotWriteFile { + path: file_path, + error: err, + }) } /// Read the file content in the given store pub fn read(&self, path: &[&str]) -> Result> { let file_path = self.build_folder_path(path); - fs::read(&file_path).map_err(|err| { - anyhow!( - "There was an error reading the {} file: {}", - &file_path.display(), - err - ) + fs::read(&file_path).map_err(|err| StoreError::CannotReadFile { + path: file_path, + error: err, }) } /// Copy file inside the configured root folder pub fn copy(&self, source: &Path, dest: &[&str]) -> Result<()> { let file_path = self.build_folder_path(dest); - fs::copy(source, file_path)?; - + fs::copy(source, &file_path).map_err(|err| StoreError::CannotWriteFile { + path: file_path, + error: err, + })?; Ok(()) } @@ -109,7 +118,10 @@ impl Store { /// Generate a file hash based on the blake3 implementation pub fn file_hash(path: &Path) -> Result { - let content = fs::read(path)?; + let content = fs::read(path).map_err(|err| StoreError::CannotReadFile { + path: path.to_path_buf(), + error: err, + })?; Ok(blake3::hash(&content).to_string()) } diff --git a/crates/worker/Cargo.toml b/crates/worker/Cargo.toml index 6110b6bc..f1092b77 100644 --- a/crates/worker/Cargo.toml +++ b/crates/worker/Cargo.toml @@ -11,7 +11,6 @@ doctest = false [dependencies] actix-web = { workspace = true } -anyhow = { workspace = true } reqwest = { workspace = true } serde = { workspace = true } serde_json = { workspace = true } diff --git a/crates/worker/src/config.rs b/crates/worker/src/config.rs index befa3c9c..6af93464 100644 --- a/crates/worker/src/config.rs +++ b/crates/worker/src/config.rs @@ -1,10 +1,10 @@ // Copyright 2022-2023 VMware, Inc. // SPDX-License-Identifier: Apache-2.0 +use crate::errors::Result; use crate::features::http_requests::HttpRequestsConfig; use crate::features::wasi_nn::WasiNnConfig; use crate::features::{data::ConfigData, folders::Folder}; -use anyhow::{anyhow, Result}; use serde::{Deserialize, Deserializer}; use std::collections::HashMap; use std::path::PathBuf; @@ -58,18 +58,9 @@ impl Config { /// namespace = "todos" /// ``` pub fn try_from_file(path: PathBuf) -> Result { - let contents = fs::read_to_string(&path)?; - - let try_config: Result = from_str(&contents); - - match try_config { - Ok(c) => Ok(c), - Err(err) => Err(anyhow!( - "Error reading the configuration file at {}: {}", - &path.to_str().unwrap_or("?"), - err - )), - } + Ok(from_str(&fs::read_to_string(path).map_err(|_| { + crate::errors::WorkerError::CannotLoadConfig + })?)?) } /// Returns a data Key/Value configuration if available @@ -87,11 +78,13 @@ impl Config { /// function won't modify the K or the V of the HashMap. If /// V starts with $, its value will be read from the server /// environment variables -fn read_environment_variables<'de, D>(deserializer: D) -> Result, D::Error> +fn read_environment_variables<'de, D>( + deserializer: D, +) -> core::result::Result, D::Error> where D: Deserializer<'de>, { - let result: Result>, D::Error> = + let result: core::result::Result>, D::Error> = Deserialize::deserialize(deserializer); match result { diff --git a/crates/worker/src/errors.rs b/crates/worker/src/errors.rs new file mode 100644 index 00000000..8c386790 --- /dev/null +++ b/crates/worker/src/errors.rs @@ -0,0 +1,54 @@ +// Copyright 2023 VMware, Inc. +// SPDX-License-Identifier: Apache-2.0 + +use std::path::PathBuf; + +pub type Result = std::result::Result; + +#[derive(Debug)] +pub enum WorkerError { + BadWasmModule, + CannotLoadConfig, + CannotParseConfig { + path: PathBuf, + error: toml::de::Error, + }, + ConfigureRuntimeError, + DeserializeConfigError, + FailedToInitialize, + RuntimeError(wws_runtimes::errors::RuntimeError), + WorkerBodyReadError, +} + +impl From for WorkerError { + fn from(_error: toml::de::Error) -> Self { + WorkerError::CannotLoadConfig + } +} + +impl From for WorkerError { + fn from(error: wws_runtimes::errors::RuntimeError) -> Self { + WorkerError::RuntimeError(error) + } +} + +impl std::fmt::Display for WorkerError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + WorkerError::BadWasmModule => write!(f, "Bad Wasm module"), + WorkerError::CannotLoadConfig => write!(f, "Could not load configuration"), + WorkerError::CannotParseConfig { path, error } => write!( + f, + "Could not parse configuration at {:?}: {:?}", + path, error + ), + WorkerError::ConfigureRuntimeError => write!(f, "Error configuring runtime"), + WorkerError::DeserializeConfigError => write!(f, "Error deserializing configuration"), + WorkerError::FailedToInitialize => write!(f, "Failed to initialize"), + WorkerError::RuntimeError(error) => { + write!(f, "Error on Wasm module runtime: {:?}", error) + } + WorkerError::WorkerBodyReadError => write!(f, "Error reading body from worker"), + } + } +} diff --git a/crates/worker/src/io.rs b/crates/worker/src/io.rs index b126fa5a..8a297414 100644 --- a/crates/worker/src/io.rs +++ b/crates/worker/src/io.rs @@ -1,11 +1,12 @@ // Copyright 2022 VMware, Inc. // SPDX-License-Identifier: Apache-2.0 +use crate::errors::{self, Result}; + use actix_web::{ http::{header::HeaderMap, StatusCode}, HttpRequest, }; -use anyhow::Result; use base64::{engine::general_purpose, Engine as _}; use serde::{Deserialize, Serialize}; use std::collections::HashMap; @@ -126,7 +127,9 @@ impl WasmOutput { /// decode the data if the base64 flag is enabled. pub fn body(&self) -> Result> { if self.base64 { - Ok(general_purpose::STANDARD.decode(&self.data)?) + Ok(general_purpose::STANDARD + .decode(&self.data) + .map_err(|_| errors::WorkerError::WorkerBodyReadError)?) } else { Ok(self.data.as_bytes().into()) } diff --git a/crates/worker/src/lib.rs b/crates/worker/src/lib.rs index 93f858b8..085e7635 100644 --- a/crates/worker/src/lib.rs +++ b/crates/worker/src/lib.rs @@ -3,14 +3,15 @@ mod bindings; pub mod config; +pub mod errors; pub mod features; pub mod io; mod stdio; use actix_web::HttpRequest; -use anyhow::{anyhow, Result}; use bindings::http::{add_to_linker as http_add_to_linker, HttpBindings}; use config::Config; +use errors::Result; use features::wasi_nn::WASI_NN_BACKEND_OPENVINO; use io::{WasmInput, WasmOutput}; use sha256::digest as sha256_digest; @@ -73,7 +74,8 @@ impl Worker { let engine = Engine::default(); let runtime = init_runtime(project_root, path, project_config)?; let bytes = runtime.module_bytes()?; - let module = Module::from_binary(&engine, &bytes)?; + let module = + Module::from_binary(&engine, &bytes).map_err(|_| errors::WorkerError::BadWasmModule)?; // Prepare the environment if required runtime.prepare()?; @@ -102,7 +104,10 @@ impl Worker { let stderr_file; if let Some(file) = stderr { - stderr_file = Some(file.try_clone()?); + stderr_file = Some( + file.try_clone() + .map_err(|_| errors::WorkerError::ConfigureRuntimeError)?, + ); } else { stderr_file = None; } @@ -112,15 +117,19 @@ impl Worker { let mut linker = Linker::new(&self.engine); - http_add_to_linker(&mut linker, |s: &mut WorkerState| &mut s.http)?; - wasmtime_wasi::add_to_linker(&mut linker, |s: &mut WorkerState| &mut s.wasi)?; + http_add_to_linker(&mut linker, |s: &mut WorkerState| &mut s.http) + .map_err(|_| errors::WorkerError::ConfigureRuntimeError)?; + wasmtime_wasi::add_to_linker(&mut linker, |s: &mut WorkerState| &mut s.wasi) + .map_err(|_| errors::WorkerError::ConfigureRuntimeError)?; // I have to use `String` as it's required by WasiCtxBuilder let tuple_vars: Vec<(String, String)> = vars.iter().map(|(k, v)| (k.clone(), v.clone())).collect(); // Create the initial WASI context - let mut wasi_builder = WasiCtxBuilder::new().envs(&tuple_vars)?; + let mut wasi_builder = WasiCtxBuilder::new() + .envs(&tuple_vars) + .map_err(|_| errors::WorkerError::ConfigureRuntimeError)?; // Configure the stdio wasi_builder = stdio.configure_wasi_ctx(wasi_builder); @@ -129,11 +138,13 @@ impl Worker { if let Some(folders) = self.config.folders.as_ref() { for folder in folders { if let Some(base) = &self.path.parent() { - let dir = Dir::open_ambient_dir(base.join(&folder.from), ambient_authority())?; - wasi_builder = wasi_builder.preopened_dir(dir, &folder.to)?; + let dir = Dir::open_ambient_dir(base.join(&folder.from), ambient_authority()) + .map_err(|_| errors::WorkerError::ConfigureRuntimeError)?; + wasi_builder = wasi_builder + .preopened_dir(dir, &folder.to) + .map_err(|_| errors::WorkerError::ConfigureRuntimeError)?; } else { - // TODO: Revisit error management on #73 - return Err(anyhow!("The worker couldn't be initialized")); + return Err(errors::WorkerError::FailedToInitialize); } } } @@ -152,9 +163,18 @@ impl Worker { wasmtime_wasi_nn::add_to_linker(&mut linker, |s: &mut WorkerState| { Arc::get_mut(s.wasi_nn.as_mut().unwrap()) .expect("wasi-nn is not implemented with multi-threading support") + }) + .map_err(|_| { + errors::WorkerError::RuntimeError( + wws_runtimes::errors::RuntimeError::WasiContextError, + ) })?; - Some(Arc::new(WasiNnCtx::new()?)) + Some(Arc::new(WasiNnCtx::new().map_err(|_| { + errors::WorkerError::RuntimeError( + wws_runtimes::errors::RuntimeError::WasiContextError, + ) + })?)) } } else { None @@ -173,22 +193,28 @@ impl Worker { }; let mut store = Store::new(&self.engine, state); - linker.module(&mut store, "", &self.module)?; linker - .get_default(&mut store, "")? - .typed::<(), ()>(&store)? - .call(&mut store, ())?; + .module(&mut store, "", &self.module) + .map_err(|_| errors::WorkerError::ConfigureRuntimeError)?; + linker + .get_default(&mut store, "") + .map_err(|_| errors::WorkerError::ConfigureRuntimeError)? + .typed::<(), ()>(&store) + .map_err(|_| errors::WorkerError::ConfigureRuntimeError)? + .call(&mut store, ()) + .map_err(|_| errors::WorkerError::ConfigureRuntimeError)?; drop(store); let contents: Vec = stdio .stdout .try_into_inner() - .map_err(|_err| anyhow::Error::msg("Nothing to show"))? + .unwrap_or_default() .into_inner(); // Build the output - let output: WasmOutput = serde_json::from_slice(&contents)?; + let output: WasmOutput = serde_json::from_slice(&contents) + .map_err(|_| errors::WorkerError::ConfigureRuntimeError)?; Ok(output) } diff --git a/src/commands/runtimes.rs b/src/commands/runtimes.rs index 4c5676a0..77e24166 100644 --- a/src/commands/runtimes.rs +++ b/src/commands/runtimes.rs @@ -49,12 +49,16 @@ impl Install { pub async fn run(&self, project_root: &Path, args: &Runtimes) -> Result<()> { match (&self.name, &self.version) { (Some(name), Some(version)) => { - install_from_repository(project_root, args, name, version).await + install_from_repository(project_root, args, name, version) + .await + .map_err(|err| err.into()) } (Some(_), None) | (None, Some(_)) => Err(anyhow!( "The name and version are mandatory when installing a runtime from a repository" )), - (None, None) => install_missing_runtimes(project_root).await, + (None, None) => install_missing_runtimes(project_root) + .await + .map_err(|err| err.into()), } } } @@ -71,7 +75,9 @@ impl List { let repo_url = get_repo_url(args); println!("⚙️ Fetching data from the repository..."); - let repo = Repository::from_remote_file(&repo_url).await?; + let repo = Repository::from_remote_file(&repo_url) + .await + .map_err(|err| anyhow!(err))?; let mut table = Table::new(); table.set_format(*format::consts::FORMAT_BOX_CHARS); @@ -115,7 +121,7 @@ impl Check { /// installed in the current project root. pub fn run(&self, project_root: &Path) -> Result<()> { // Retrieve the configuration - let config = Config::load(project_root)?; + let config = Config::load(project_root).map_err(|err| anyhow!(err))?; let mut is_missing = false; let mut total = 0; @@ -187,7 +193,7 @@ impl Uninstall { /// from the .wws.toml file pub fn run(&self, project_root: &Path, args: &Runtimes) -> Result<()> { // Retrieve the configuration - let mut config = Config::load(project_root)?; + let mut config = Config::load(project_root).map_err(|err| anyhow!(err))?; let repo_name = get_repo_name(args); let runtime = config.get_runtime(&repo_name, &self.name, &self.version); @@ -196,9 +202,9 @@ impl Uninstall { "🗑 Uninstalling: {} - {} / {}", &repo_name, &runtime.name, &runtime.version ); - uninstall_runtime(project_root, &repo_name, runtime)?; + uninstall_runtime(project_root, &repo_name, runtime).map_err(|err| anyhow!(err))?; config.remove_runtime(&repo_name, &self.name, &self.version); - config.save(project_root)?; + config.save(project_root).map_err(|err| anyhow!(err))?; } else { println!( "🗑 The runtime was not installed: {} - {} / {}", diff --git a/src/utils/errors.rs b/src/utils/errors.rs new file mode 100644 index 00000000..e510854e --- /dev/null +++ b/src/utils/errors.rs @@ -0,0 +1,60 @@ +// Copyright 2023 VMware, Inc. +// SPDX-License-Identifier: Apache-2.0 + +use anyhow::anyhow; +use wws_config::errors::ConfigError; +use wws_project::errors::FetchError; + +pub type Result = std::result::Result; + +#[derive(Debug)] +pub enum UtilsError { + ConfigError(wws_config::errors::ConfigError), + FetchError(FetchError), + MissingRuntime { runtime: String, version: String }, +} + +impl std::fmt::Display for UtilsError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + Self::ConfigError(err) => write!(f, "Configuration error: {}", err), + Self::FetchError(err) => write!(f, "Fetch error: {}", err), + Self::MissingRuntime { runtime, version } => { + write!(f, "Missing runtime {} with version {}", runtime, version) + } + } + } +} + +impl From for UtilsError { + fn from(error: wws_config::errors::ConfigError) -> Self { + UtilsError::ConfigError(error) + } +} + +impl From for UtilsError { + fn from(error: FetchError) -> Self { + UtilsError::FetchError(error) + } +} + +impl From for anyhow::Error { + fn from(error: UtilsError) -> Self { + match error { + UtilsError::ConfigError(error) => match error { + ConfigError::CannotLoadConfig(error) => anyhow!("Error opening file: {}", error), + ConfigError::CannotParseConfig(error) => anyhow!( + "Error loading file: {}. The file format is not correct", + error + ), + ConfigError::CannotSaveConfig => anyhow!("Error saving configuration"), + }, + UtilsError::FetchError(error) => anyhow!("Error fetching repository: {}", error), + UtilsError::MissingRuntime { runtime, version } => anyhow!( + "The runtime with name = '{}' and version = '{}' is not present in the repository", + runtime, + version + ), + } + } +} diff --git a/src/utils/mod.rs b/src/utils/mod.rs index 7c6a9749..7beef53c 100644 --- a/src/utils/mod.rs +++ b/src/utils/mod.rs @@ -1,5 +1,6 @@ // Copyright 2023 VMware, Inc. // SPDX-License-Identifier: Apache-2.0 +pub mod errors; pub mod options; pub mod runtimes; diff --git a/src/utils/runtimes.rs b/src/utils/runtimes.rs index d753bd66..7cdff6a2 100644 --- a/src/utils/runtimes.rs +++ b/src/utils/runtimes.rs @@ -1,12 +1,12 @@ // Copyright 2023 VMware, Inc. // SPDX-License-Identifier: Apache-2.0 -use anyhow::{anyhow, Result}; use std::{env, path::Path}; use wws_config::Config; use wws_project::{check_runtime, install_runtime, metadata::Repository}; use crate::commands::runtimes::Runtimes; +use crate::utils::errors::{Result, UtilsError}; /// Default repository name pub const DEFAULT_REPO_NAME: &str = "wasmlabs"; @@ -76,11 +76,10 @@ pub async fn install_from_repository( Ok(()) } } else { - Err(anyhow!( - "The runtime with name = '{}' and version = '{}' is not present in the repository", - name, - version - )) + Err(UtilsError::MissingRuntime { + runtime: name.to_string(), + version: version.to_string(), + }) } } diff --git a/tests/e2e.rs b/tests/e2e.rs index bfe49a12..55ca1ffd 100644 --- a/tests/e2e.rs +++ b/tests/e2e.rs @@ -110,7 +110,10 @@ mod test { child.kill().expect("Error stopping wws"); // Test - assert!(body.contains(expected_text)); + assert!( + body.contains(expected_text), + "result \"{body}\" does not contain \"{expected_text}\"" + ); } #[test]