From b9b6a095bbce3c9e96374a54bb5d3f06b8f06647 Mon Sep 17 00:00:00 2001 From: Bruno Galvao Date: Mon, 22 Apr 2024 17:44:29 +0900 Subject: [PATCH 1/2] chore: use `thiserror` within crates (#111) * refactor: move everything to crates folder * refactor: new and build contracts * refactor: test contract functionality in pop_contracts * refactor: up and call contracts * refactor: cargo * refactor: new parachain and pallet * refactor: build parachain * refactor: up parachain * refactor: clean cargo pop-cli * docs: pop-contracts README * docs: pop-parachains README * chore: fmt * chore: versions of crates to 0.0.0 * chore: version 0.1.0 for crates * refactor: shared dependencies in workspace Cargo.toml * refactor: move dependencies to workspace * docs: feedback on docs * chore: remove version for local dependencies * use thiserror * use thiserror * add thiserror dependency * docs: update nitpicks in README * fixes: typo * use thiserror * use thiserror * use thiserror in build * use thiserror * use thiserror * use thiserror * remove braces * cargo fmt --all * revert changes * cargo fmt --all * rename NewContractError to NewContractFailed * create an error module for the crate * refactor errors * create errors module on crate level * refactor thiserror to use crate module * replace with thiserror * cargo fmt --all * rename * use crate thiserror * use thiserror * remove duplicate * cargo fmt --all * use thiserror crate * remove `result.status.success` * remove unused `result` --------- Co-authored-by: AlexD10S Co-authored-by: Abhishek Shah --- Cargo.lock | 2 + Cargo.toml | 1 + crates/pop-cli/src/commands/new/pallet.rs | 2 +- crates/pop-contracts/Cargo.toml | 1 + crates/pop-contracts/src/errors.rs | 31 +++++ crates/pop-contracts/src/lib.rs | 1 + crates/pop-contracts/src/new.rs | 35 +++-- crates/pop-contracts/src/test.rs | 41 +++--- crates/pop-contracts/src/utils/helpers.rs | 29 ++-- crates/pop-contracts/src/utils/signer.rs | 14 +- crates/pop-parachains/Cargo.toml | 1 + crates/pop-parachains/src/errors.rs | 50 +++++++ crates/pop-parachains/src/lib.rs | 1 + crates/pop-parachains/src/new_pallet.rs | 9 +- crates/pop-parachains/src/up.rs | 124 ++++++++++-------- crates/pop-parachains/src/utils/git.rs | 11 +- crates/pop-parachains/src/utils/helpers.rs | 34 +++-- .../src/utils/pallet_helpers.rs | 30 +++-- 18 files changed, 278 insertions(+), 139 deletions(-) create mode 100644 crates/pop-contracts/src/errors.rs create mode 100644 crates/pop-parachains/src/errors.rs diff --git a/Cargo.lock b/Cargo.lock index 3c6275c6..47f1b614 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5463,6 +5463,7 @@ dependencies = [ "subxt 0.34.0", "subxt-signer 0.34.0", "tempfile", + "thiserror", "url", ] @@ -5481,6 +5482,7 @@ dependencies = [ "serde_json", "symlink", "tempfile", + "thiserror", "tokio", "toml_edit 0.22.9", "url", diff --git a/Cargo.toml b/Cargo.toml index 34e9ff08..af53847a 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -4,6 +4,7 @@ members = ["crates/*"] [workspace.dependencies] anyhow = "1.0" +thiserror = "1.0.58" duct = "0.13" git2 = "0.18" tempfile = "3.8" diff --git a/crates/pop-cli/src/commands/new/pallet.rs b/crates/pop-cli/src/commands/new/pallet.rs index 16135870..cb363b9d 100644 --- a/crates/pop-cli/src/commands/new/pallet.rs +++ b/crates/pop-cli/src/commands/new/pallet.rs @@ -26,7 +26,7 @@ impl NewPalletCommand { &self.name, ))?; set_theme(Theme); - let target = resolve_pallet_path(self.path.clone()); + let target = resolve_pallet_path(self.path.clone())?; let pallet_name = self.name.clone(); let pallet_path = target.join(pallet_name.clone()); if pallet_path.exists() { diff --git a/crates/pop-contracts/Cargo.toml b/crates/pop-contracts/Cargo.toml index 23811869..9e8ea6c2 100644 --- a/crates/pop-contracts/Cargo.toml +++ b/crates/pop-contracts/Cargo.toml @@ -8,6 +8,7 @@ edition = "2021" [dependencies] anyhow.workspace = true +thiserror.workspace = true duct.workspace = true url.workspace = true diff --git a/crates/pop-contracts/src/errors.rs b/crates/pop-contracts/src/errors.rs new file mode 100644 index 00000000..7117d639 --- /dev/null +++ b/crates/pop-contracts/src/errors.rs @@ -0,0 +1,31 @@ +use thiserror::Error; + +#[derive(Error, Debug)] +pub enum Error { + #[error("Failed to create new contract project: {0}")] + NewContract(String), + + #[error("IO error: {0}")] + IO(#[from] std::io::Error), + + #[error("Failed to execute test command: {0}")] + TestCommand(String), + + #[error("Failed to parse balance: {0}")] + BalanceParsing(String), + + #[error("Failed to parse account address: {0}")] + AccountAddressParsing(String), + + #[error("Failed to get manifest path: {0}")] + ManifestPath(String), + + #[error("Failed to parse secret URI: {0}")] + ParseSecretURI(String), + + #[error("Failed to create keypair from URI: {0}")] + KeyPairCreation(String), + + #[error("Failed to parse hex encoded bytes: {0}")] + HexParsing(String), +} diff --git a/crates/pop-contracts/src/lib.rs b/crates/pop-contracts/src/lib.rs index 43dfc09c..dee993c7 100644 --- a/crates/pop-contracts/src/lib.rs +++ b/crates/pop-contracts/src/lib.rs @@ -1,5 +1,6 @@ mod build; mod call; +mod errors; mod new; mod test; mod up; diff --git a/crates/pop-contracts/src/new.rs b/crates/pop-contracts/src/new.rs index 6ceb40e6..5ae4cd11 100644 --- a/crates/pop-contracts/src/new.rs +++ b/crates/pop-contracts/src/new.rs @@ -1,12 +1,26 @@ +use crate::errors::Error; use contract_build::new_contract_project; use std::path::Path; -/// Create a new smart contract at `target` -pub fn create_smart_contract(name: String, target: &Path) -> anyhow::Result<()> { - // In this code, out_dir will automatically join `name` to `target`, - // which is created prior to the call to this function - // So we must pass `target.parent()` - new_contract_project(&name, target.canonicalize()?.parent()) +pub fn create_smart_contract(name: String, target: &Path) -> Result<(), Error> { + // Canonicalize the target path to ensure consistency and resolve any symbolic links. + let canonicalized_path = target + .canonicalize() + // If an I/O error occurs during canonicalization, convert it into an Error enum variant. + .map_err(|e| Error::IO(e))?; + + // Retrieve the parent directory of the canonicalized path. + let parent_path = canonicalized_path + .parent() + // If the parent directory cannot be retrieved (e.g., if the path has no parent), + // return a NewContract variant indicating the failure. + .ok_or(Error::NewContract("Failed to get parent directory".to_string()))?; + + // Create a new contract project with the provided name in the parent directory. + new_contract_project(&name, Some(parent_path)) + // If an error occurs during the creation of the contract project, + // convert it into a NewContract variant with a formatted error message. + .map_err(|e| Error::NewContract(format!("{}", e))) } #[cfg(test)] @@ -14,17 +28,16 @@ mod tests { use super::*; use anyhow::{Error, Result}; use std::fs; + use tempfile; fn setup_test_environment() -> Result { - let temp_dir = tempfile::tempdir().expect("Could not create temp dir"); + let temp_dir = tempfile::tempdir()?; let temp_contract_dir = temp_dir.path().join("test_contract"); fs::create_dir(&temp_contract_dir)?; - let result = - create_smart_contract("test_contract".to_string(), temp_contract_dir.as_path()); - assert!(result.is_ok(), "Contract test environment setup failed"); - + create_smart_contract("test_contract".to_string(), temp_contract_dir.as_path())?; Ok(temp_dir) } + #[test] fn test_create_smart_contract_success() -> Result<(), Error> { let temp_dir = setup_test_environment()?; diff --git a/crates/pop-contracts/src/test.rs b/crates/pop-contracts/src/test.rs index 1441e44c..bbda41e8 100644 --- a/crates/pop-contracts/src/test.rs +++ b/crates/pop-contracts/src/test.rs @@ -1,17 +1,22 @@ +use crate::errors::Error; use duct::cmd; use std::path::PathBuf; -pub fn test_smart_contract(path: &Option) -> anyhow::Result<()> { - cmd("cargo", vec!["test"]).dir(path.clone().unwrap_or("./".into())).run()?; - +pub fn test_smart_contract(path: &Option) -> Result<(), Error> { + // Execute `cargo test` command in the specified directory. + cmd("cargo", vec!["test"]) + .dir(path.clone().unwrap_or_else(|| PathBuf::from("./"))) + .run() + .map_err(|e| Error::TestCommand(format!("Cargo test command failed: {}", e)))?; Ok(()) } -pub fn test_e2e_smart_contract(path: &Option) -> anyhow::Result<()> { +pub fn test_e2e_smart_contract(path: &Option) -> Result<(), Error> { + // Execute `cargo test --features=e2e-tests` command in the specified directory. cmd("cargo", vec!["test", "--features=e2e-tests"]) - .dir(path.clone().unwrap_or("./".into())) - .run()?; - + .dir(path.clone().unwrap_or_else(|| PathBuf::from("./"))) + .run() + .map_err(|e| Error::TestCommand(format!("Cargo test command failed: {}", e)))?; Ok(()) } @@ -19,28 +24,32 @@ pub fn test_e2e_smart_contract(path: &Option) -> anyhow::Result<()> { #[cfg(test)] mod tests { use super::*; - use anyhow::{Error, Result}; use std::fs; + use tempfile; fn setup_test_environment() -> Result { - let temp_dir = tempfile::tempdir().expect("Could not create temp dir"); + let temp_dir = tempfile::tempdir().map_err(|e| { + Error::TestEnvironmentError(format!("Failed to create temp dir: {}", e)) + })?; let temp_contract_dir = temp_dir.path().join("test_contract"); - fs::create_dir(&temp_contract_dir)?; + fs::create_dir(&temp_contract_dir).map_err(|e| { + Error::TestEnvironmentError(format!("Failed to create test contract directory: {}", e)) + })?; let result = - crate::create_smart_contract("test_contract".to_string(), temp_contract_dir.as_path()); + crate::create_smart_contract("test_contract".to_string(), temp_contract_dir.as_path()) + .map_err(|e| { + Error::TestEnvironmentError(format!("Failed to create smart contract: {}", e)) + })?; assert!(result.is_ok(), "Contract test environment setup failed"); - Ok(temp_dir) } #[test] fn test_contract_test() -> Result<(), Error> { let temp_contract_dir = setup_test_environment()?; - - let result = test_smart_contract(&Some(temp_contract_dir.path().join("test_contract"))); - + // Run unit tests for the smart contract in the temporary contract directory. + let result = test_smart_contract(&Some(temp_contract_dir.path().join("test_contract")))?; assert!(result.is_ok(), "Result should be Ok"); - Ok(()) } } diff --git a/crates/pop-contracts/src/utils/helpers.rs b/crates/pop-contracts/src/utils/helpers.rs index 6331ca00..f8b7e937 100644 --- a/crates/pop-contracts/src/utils/helpers.rs +++ b/crates/pop-contracts/src/utils/helpers.rs @@ -1,33 +1,30 @@ -use anyhow::{anyhow, Result}; +use crate::errors::Error; use contract_build::ManifestPath; use contract_extrinsics::BalanceVariant; use ink_env::{DefaultEnvironment, Environment}; use std::{path::PathBuf, str::FromStr}; use subxt::{Config, PolkadotConfig as DefaultConfig}; -// If the user specifies a path (which is not the current directory), it will have to manually -// add a Cargo.toml file. If not provided, pop-cli will ask the user for a specific path. or ask -// to the user the specific path (Like cargo-contract does) -pub fn get_manifest_path(path: &Option) -> anyhow::Result { - if path.is_some() { - let full_path: PathBuf = - PathBuf::from(path.as_ref().unwrap().to_string_lossy().to_string() + "/Cargo.toml"); - - return ManifestPath::try_from(Some(full_path)); +pub fn get_manifest_path(path: &Option) -> Result { + if let Some(path) = path { + let full_path = PathBuf::from(path.to_string_lossy().to_string() + "/Cargo.toml"); + return ManifestPath::try_from(Some(full_path)) + .map_err(|e| Error::ManifestPath(format!("Failed to get manifest path: {}", e))); } else { - return ManifestPath::try_from(path.as_ref()); + return ManifestPath::try_from(path.as_ref()) + .map_err(|e| Error::ManifestPath(format!("Failed to get manifest path: {}", e))); } } -/// Parse a balance from string format pub fn parse_balance( balance: &str, -) -> Result::Balance>> { - BalanceVariant::from_str(balance).map_err(|e| anyhow!("Balance parsing failed: {e}")) +) -> Result::Balance>, Error> { + BalanceVariant::from_str(balance).map_err(|e| Error::BalanceParsing(format!("{}", e))) } -pub fn parse_account(account: &str) -> Result<::AccountId> { + +pub fn parse_account(account: &str) -> Result<::AccountId, Error> { ::AccountId::from_str(account) - .map_err(|e| anyhow::anyhow!("Account address parsing failed: {e}")) + .map_err(|e| Error::AccountAddressParsing(format!("{}", e))) } #[cfg(test)] diff --git a/crates/pop-contracts/src/utils/signer.rs b/crates/pop-contracts/src/utils/signer.rs index c08039fc..fdcca29b 100644 --- a/crates/pop-contracts/src/utils/signer.rs +++ b/crates/pop-contracts/src/utils/signer.rs @@ -1,16 +1,18 @@ -use anyhow::Result; +use crate::errors::Error; use contract_build::util::decode_hex; use sp_core::Bytes; use subxt_signer::{sr25519::Keypair, SecretUri}; /// Create a Signer from a secret URI. -pub(crate) fn create_signer(suri: &str) -> Result { - let uri = ::from_str(suri)?; - let keypair = Keypair::from_uri(&uri)?; +pub(crate) fn create_signer(suri: &str) -> Result { + let uri = ::from_str(suri) + .map_err(|e| Error::ParseSecretURI(format!("{}", e)))?; + let keypair = Keypair::from_uri(&uri).map_err(|e| Error::KeyPairCreation(format!("{}", e)))?; Ok(keypair) } + /// Parse hex encoded bytes. -pub fn parse_hex_bytes(input: &str) -> Result { - let bytes = decode_hex(input)?; +pub fn parse_hex_bytes(input: &str) -> Result { + let bytes = decode_hex(input).map_err(|e| Error::HexParsing(format!("{}", e)))?; Ok(bytes.into()) } diff --git a/crates/pop-parachains/Cargo.toml b/crates/pop-parachains/Cargo.toml index cf7104f3..08a0816e 100644 --- a/crates/pop-parachains/Cargo.toml +++ b/crates/pop-parachains/Cargo.toml @@ -8,6 +8,7 @@ edition = "2021" [dependencies] anyhow.workspace = true +thiserror.workspace = true duct.workspace = true git2.workspace = true tempfile.workspace = true diff --git a/crates/pop-parachains/src/errors.rs b/crates/pop-parachains/src/errors.rs new file mode 100644 index 00000000..3fc6edad --- /dev/null +++ b/crates/pop-parachains/src/errors.rs @@ -0,0 +1,50 @@ +use thiserror::Error; +use zombienet_sdk::OrchestratorError; + +#[derive(Error, Debug)] +pub enum Error { + #[error("a git error occurred: {0}")] + Git(String), + + #[error("Failed to access the current directory")] + CurrentDirAccess, + + #[error("Failed to locate the workspace")] + WorkspaceLocate, + + #[error("Failed to create pallet directory")] + PalletDirCreation, + + #[error("IO error: {0}")] + IO(#[from] std::io::Error), + + #[error("HTTP error: {0}")] + HttpError(#[from] reqwest::Error), + + #[error("Missing binary: {0}")] + MissingBinary(String), + + #[error("Configuration error: {0}")] + Config(String), + + #[error("Unsupported command: {0}")] + UnsupportedCommand(String), + + #[error("ParseError error: {0}")] + ParseError(#[from] url::ParseError), + + #[error("Orchestrator error: {0}")] + OrchestratorError(#[from] OrchestratorError), + + #[error("Toml error: {0}")] + TomlError(#[from] toml_edit::de::Error), + + #[error("Anyhow error: {0}")] + AnyhowError(#[from] anyhow::Error), + + #[error("User aborted due to existing target folder.")] + Aborted, + + #[error("Failed to execute rustfmt")] + RustfmtError(std::io::Error), +} diff --git a/crates/pop-parachains/src/lib.rs b/crates/pop-parachains/src/lib.rs index ce95efae..3c9804d0 100644 --- a/crates/pop-parachains/src/lib.rs +++ b/crates/pop-parachains/src/lib.rs @@ -1,4 +1,5 @@ mod build; +mod errors; mod generator; mod new_pallet; mod new_parachain; diff --git a/crates/pop-parachains/src/new_pallet.rs b/crates/pop-parachains/src/new_pallet.rs index e993772d..21eeab19 100644 --- a/crates/pop-parachains/src/new_pallet.rs +++ b/crates/pop-parachains/src/new_pallet.rs @@ -1,5 +1,6 @@ use std::{fs, path::PathBuf}; +use crate::errors::Error; use crate::{ generator::pallet::{ PalletBenchmarking, PalletCargoToml, PalletItem, PalletLib, PalletMock, PalletTests, @@ -17,8 +18,8 @@ pub struct TemplatePalletConfig { pub fn create_pallet_template( path: Option, config: TemplatePalletConfig, -) -> anyhow::Result<()> { - let target = resolve_pallet_path(path); +) -> Result<(), Error> { + let target = resolve_pallet_path(path)?; let pallet_name = config.name.clone(); let pallet_path = target.join(pallet_name.clone()); sanitize(&pallet_path)?; @@ -29,7 +30,7 @@ pub fn create_pallet_template( } /// Generate a pallet folder and file structure -fn generate_pallet_structure(target: &PathBuf, pallet_name: &str) -> anyhow::Result<()> { +fn generate_pallet_structure(target: &PathBuf, pallet_name: &str) -> Result<(), Error> { use fs::{create_dir, File}; let (pallet, src) = (target.join(pallet_name), target.join(pallet_name.to_string() + "/src")); create_dir(&pallet)?; @@ -46,7 +47,7 @@ fn render_pallet( pallet_name: String, config: TemplatePalletConfig, pallet_path: &PathBuf, -) -> anyhow::Result<()> { +) -> Result<(), Error> { let pallet_name = pallet_name.replace('-', "_"); // Todo `module` must be of the form Template if pallet_name : `pallet_template` let pallet: Vec> = vec![ diff --git a/crates/pop-parachains/src/up.rs b/crates/pop-parachains/src/up.rs index 1ac9697d..b83ed394 100644 --- a/crates/pop-parachains/src/up.rs +++ b/crates/pop-parachains/src/up.rs @@ -1,5 +1,5 @@ +use crate::errors::Error; use crate::utils::git::{Git, GitHub}; -use anyhow::{anyhow, Result}; use duct::cmd; use indexmap::IndexMap; use std::{ @@ -37,10 +37,14 @@ impl Zombienet { relay_chain_version: Option<&String>, system_parachain_version: Option<&String>, parachains: Option<&Vec>, - ) -> Result { + ) -> Result { // Parse network config let network_config_path = PathBuf::from(network_config); - let config = std::fs::read_to_string(&network_config_path)?.parse::()?; + let config = std::fs::read_to_string(&network_config_path) + .map_err(|err| Error::IO(err)) + .and_then(|content| { + content.parse::().map_err(|err| Error::TomlError(err.into())) + })?; // Determine binaries let relay_chain_binary = Self::relay_chain(relay_chain_version, &config, &cache).await?; let mut parachain_binaries = IndexMap::new(); @@ -49,7 +53,7 @@ impl Zombienet { let id = table .get("id") .and_then(|i| i.as_integer()) - .ok_or(anyhow!("expected `parachain` to have `id`"))? as u32; + .ok_or(Error::Config("expected `parachain` to have `id`".into()))? as u32; let default_command = table .get("default_command") .cloned() @@ -78,7 +82,6 @@ impl Zombienet { let Some(Value::String(command)) = default_command.as_value() else { continue; }; - let command = command.value().to_lowercase(); if command == "polkadot-parachain" { parachain_binaries.insert( @@ -90,8 +93,11 @@ impl Zombienet { ); } else if let Some(parachains) = parachains { for parachain in parachains { - let url = Url::parse(parachain)?; - let name = GitHub::name(&url)?; + let url = Url::parse(parachain).map_err(|err| Error::from(err))?; + let name = match GitHub::name(&url) { + Ok(name) => name, + Err(err) => return Err(Error::from(err)), + }; if command == name { parachain_binaries.insert(id, Self::parachain(url, &cache)?); } @@ -119,7 +125,7 @@ impl Zombienet { missing } - pub async fn spawn(&mut self) -> Result> { + pub async fn spawn(&mut self) -> Result, Error> { // Symlink polkadot-related binaries for file in ["polkadot-execute-worker", "polkadot-prepare-worker"] { let dest = self.cache.join(file); @@ -137,14 +143,14 @@ impl Zombienet { } // Adapts provided config file to one that is compatible with current zombienet-sdk requirements - fn configure(&mut self) -> Result { + fn configure(&mut self) -> Result { let (network_config_path, network_config) = &mut self.network_config; // Add zombienet-sdk specific settings if missing let Item::Table(settings) = network_config.entry("settings").or_insert(Item::Table(Table::new())) else { - return Err(anyhow!("expected `settings` to be table")); + return Err(Error::Config("expected `settings`".into())); }; settings .entry("timeout") @@ -158,11 +164,11 @@ impl Zombienet { .relay_chain .path .to_str() - .ok_or(anyhow!("the relay chain path is invalid"))?; + .ok_or(Error::Config("the relay chain path is invalid".into()))?; let Item::Table(relay_chain) = network_config.entry("relaychain").or_insert(Item::Table(Table::new())) else { - return Err(anyhow!("expected `relaychain` to be table")); + return Err(Error::Config("expected `relaychain`".into())); }; *relay_chain.entry("default_command").or_insert(value(relay_path)) = value(relay_path); @@ -174,25 +180,27 @@ impl Zombienet { let id = table .get("id") .and_then(|i| i.as_integer()) - .ok_or(anyhow!("expected `parachain` to have `id`"))? as u32; + .ok_or(Error::Config("expected `parachain` to have `id`".into()))? as u32; // Resolve default_command to binary { // Check if provided via args, therefore cached if let Some(para) = self.parachains.get(&id) { - let para_path = - para.path.to_str().ok_or(anyhow!("the parachain path is invalid"))?; + let para_path = para + .path + .to_str() + .ok_or(Error::Config("the parachain path is invalid".into()))?; table.insert("default_command", value(para_path)); } else if let Some(default_command) = table.get_mut("default_command") { // Otherwise assume local binary, fix path accordingly - let command_path = default_command - .as_str() - .ok_or(anyhow!("expected `default_command` value to be a string"))?; + let command_path = default_command.as_str().ok_or(Error::Config( + "expected `default_command` value to be a string".into(), + ))?; let path = Self::resolve_path(network_config_path, command_path)?; - *default_command = value(path.to_str().ok_or(anyhow!( - "the parachain binary was not found: {0}", - command_path - ))?); + *default_command = + value(path.to_str().ok_or(Error::Config( + "the parachain binary was not found".into(), + ))?); } } @@ -207,16 +215,15 @@ impl Zombienet { let para_path = para .path .to_str() - .ok_or(anyhow!("the parachain path is invalid"))?; + .ok_or(Error::Config("the parachain path is invalid".into()))?; *command = value(para_path); } else { - let command_path = command - .as_str() - .ok_or(anyhow!("expected `command` value to be a string"))?; + let command_path = command.as_str().ok_or(Error::Config( + "expected `command` value to be a string".into(), + ))?; let path = Self::resolve_path(network_config_path, command_path)?; - *command = value(path.to_str().ok_or(anyhow!( - "the parachain binary was not found: {0}", - command_path + *command = value(path.to_str().ok_or(Error::Config( + "the parachain binary was not found".into(), ))?); } } @@ -229,44 +236,48 @@ impl Zombienet { let network_config_file = Builder::new() .suffix(".toml") .tempfile() + .map_err(|err| Error::IO(err)) .expect("network config could not be created with .toml extension"); let path = network_config_file .path() .to_str() - .expect("temp config file should have a path"); + .ok_or(Error::Config("temp config file should have a path".into()))?; write(path, network_config.to_string())?; Ok(network_config_file) } - fn resolve_path(network_config_path: &mut PathBuf, command_path: &str) -> Result { + fn resolve_path( + network_config_path: &mut PathBuf, + command_path: &str, + ) -> Result { network_config_path - .join(command_path) - .canonicalize() - .or_else(|_| current_dir().unwrap().join(command_path).canonicalize()) - .map_err(|_| { - anyhow!( - "unable to find canonical local path to specified command: `{}` are you missing an argument?", - command_path - ) - }) + .join(command_path) + .canonicalize() + .or_else(|_| current_dir().unwrap().join(command_path).canonicalize()) + .map_err(|_| { + Error::Config(format!( + "unable to find canonical local path to specified command: `{}` are you missing an argument?", + command_path + )) + }) } async fn relay_chain( version: Option<&String>, network_config: &DocumentMut, cache: &PathBuf, - ) -> Result { + ) -> Result { const BINARY: &str = "polkadot"; let relay_command = network_config .get("relaychain") - .ok_or(anyhow!("expected `relaychain`"))? + .ok_or(Error::Config("expected `relaychain`".into()))? .get("default_command"); if let Some(Value::String(command)) = relay_command.and_then(|c| c.as_value()) { if !command.value().to_lowercase().contains(BINARY) { - return Err(anyhow!( + return Err(Error::UnsupportedCommand(format!( "the relay chain command is unsupported: {0}", command.to_string() - )); + ))); } } let version = match version { @@ -302,7 +313,7 @@ impl Zombienet { Ok(Binary { name: versioned_name, version, path, sources }) } - fn system_parachain(version: &String, cache: &PathBuf) -> Result { + fn system_parachain(version: &String, cache: &PathBuf) -> Result { const BINARY: &str = "polkadot-parachain"; let versioned_name = format!("{BINARY}-{version}"); let path = cache.join(&versioned_name); @@ -328,7 +339,7 @@ impl Zombienet { Ok(Binary { name: versioned_name, version: version.into(), path, sources }) } - fn parachain(repo: Url, cache: &PathBuf) -> Result { + fn parachain(repo: Url, cache: &PathBuf) -> Result { let binary = repo.query(); let branch = repo.fragment().map(|f| f.to_string()); let mut url = repo.clone(); @@ -354,7 +365,7 @@ impl Zombienet { Ok(Binary { name: binary, version: "".into(), path, sources }) } - async fn latest_polkadot_release() -> Result { + async fn latest_polkadot_release() -> Result { let repo = Url::parse(POLKADOT_SDK).expect("repository url valid"); // Fetching latest releases for release in GitHub::get_latest_releases(&repo).await? { @@ -378,7 +389,7 @@ pub struct Binary { } impl Binary { - pub async fn source(&self, cache: &PathBuf) -> Result<()> { + pub async fn source(&self, cache: &PathBuf) -> Result<(), Error> { for source in &self.sources { source.process(cache).await?; } @@ -414,7 +425,7 @@ impl Source { path: &Path, package: &str, names: impl Iterator, - ) -> Result<()> { + ) -> Result<(), Error> { // Build binaries and then copy to cache and target cmd( "cargo", @@ -434,7 +445,7 @@ impl Source { Ok(()) } - async fn download(url: &str, cache: &PathBuf) -> Result<()> { + async fn download(url: &str, cache: &PathBuf) -> Result<(), Error> { // Download to cache let response = reqwest::get(url).await?; let mut file = File::create(&cache)?; @@ -446,7 +457,7 @@ impl Source { Ok(()) } - pub async fn process(&self, cache: &Path) -> Result>> { + pub async fn process(&self, cache: &Path) -> Result>, Error> { // Download or clone and build from source match self { Source::Url { name, version, url } => { @@ -478,7 +489,7 @@ impl Source { if working_dir.exists() { Self::remove(working_dir)?; } - return Err(e); + return Err(e.into()); } // Build binaries and finally remove working directory Self::build_binaries( @@ -495,7 +506,7 @@ impl Source { } } - fn remove(path: &Path) -> Result<()> { + fn remove(path: &Path) -> Result<(), Error> { remove_dir_all(path)?; if let Some(source) = path.parent() { if source.exists() && source.read_dir().map(|mut i| i.next().is_none()).unwrap_or(false) @@ -590,7 +601,10 @@ mod tests { assert!(result_error.is_err()); let error_message = result_error.err().unwrap(); - assert_eq!(error_message.root_cause().to_string(), "expected `parachain` to have `id`"); + assert_eq!( + error_message.to_string(), + "Configuration error: expected `parachain` to have `id`" + ); Ok(()) } @@ -657,7 +671,7 @@ mod tests { .await; assert!(result_error.is_err()); let error_message = result_error.err().unwrap(); - assert_eq!(error_message.root_cause().to_string(), "expected `relaychain`"); + assert_eq!(error_message.to_string(), "Configuration error: expected `relaychain`"); Ok(()) } diff --git a/crates/pop-parachains/src/utils/git.rs b/crates/pop-parachains/src/utils/git.rs index 264ae487..254af2e2 100644 --- a/crates/pop-parachains/src/utils/git.rs +++ b/crates/pop-parachains/src/utils/git.rs @@ -1,4 +1,5 @@ -use anyhow::{anyhow, Result}; +use crate::errors::Error; +use anyhow::Result; use git2::{build::RepoBuilder, FetchOptions, IndexAddOption, Repository, ResetType}; use regex::Regex; use std::fs; @@ -89,9 +90,9 @@ impl GitHub { .path_segments() .map(|c| c.collect::>()) .expect("repository must have path segments"); - Ok(path_segments - .get(0) - .ok_or(anyhow!("the organization (or user) is missing from the github url"))?) + Ok(path_segments.get(0).ok_or(Error::Git( + "the organization (or user) is missing from the github url".to_string(), + ))?) } pub(crate) fn name(repo: &Url) -> Result<&str> { @@ -101,7 +102,7 @@ impl GitHub { .expect("repository must have path segments"); Ok(path_segments .get(1) - .ok_or(anyhow!("the repository name is missing from the github url"))?) + .ok_or(Error::Git("the repository name is missing from the github url".to_string()))?) } pub(crate) fn release(repo: &Url, tag: &str, artifact: &str) -> String { diff --git a/crates/pop-parachains/src/utils/helpers.rs b/crates/pop-parachains/src/utils/helpers.rs index 3ce7c213..39af9679 100644 --- a/crates/pop-parachains/src/utils/helpers.rs +++ b/crates/pop-parachains/src/utils/helpers.rs @@ -1,10 +1,12 @@ -use anyhow::Result; use std::{ fs::{self, OpenOptions}, + io::{self, stdin, stdout, Write}, path::Path, }; -pub(crate) fn sanitize(target: &Path) -> Result<()> { - use std::io::{stdin, stdout, Write}; + +use crate::errors::Error; + +pub(crate) fn sanitize(target: &Path) -> Result<(), Error> { if target.exists() { print!("\"{}\" folder exists. Do you want to clean it? [y/n]: ", target.display()); stdout().flush()?; @@ -13,27 +15,37 @@ pub(crate) fn sanitize(target: &Path) -> Result<()> { stdin().read_line(&mut input)?; if input.trim().to_lowercase() == "y" { - fs::remove_dir_all(target)?; + fs::remove_dir_all(target).map_err(|_| Error::Aborted)?; } else { - return Err(anyhow::anyhow!("User aborted due to existing target folder.")); + return Err(Error::Aborted); } } Ok(()) } -pub(crate) fn write_to_file<'a>(path: &Path, contents: &'a str) -> Result<()> { - use std::io::Write; - let mut file = OpenOptions::new().write(true).truncate(true).create(true).open(path).unwrap(); - file.write_all(contents.as_bytes()).unwrap(); +pub(crate) fn write_to_file(path: &Path, contents: &str) -> Result<(), Error> { + let mut file = OpenOptions::new() + .write(true) + .truncate(true) + .create(true) + .open(path) + .map_err(|err| Error::RustfmtError(err))?; + + file.write_all(contents.as_bytes()).map_err(|err| Error::RustfmtError(err))?; + if path.extension().map_or(false, |ext| ext == "rs") { let output = std::process::Command::new("rustfmt") .arg(path.to_str().unwrap()) .output() - .expect("failed to execute rustfmt"); + .map_err(|err| Error::RustfmtError(err))?; if !output.status.success() { - return Err(anyhow::anyhow!("rustfmt exited with non-zero status code.")); + return Err(Error::RustfmtError(io::Error::new( + io::ErrorKind::Other, + "rustfmt exited with non-zero status code", + ))); } } + Ok(()) } diff --git a/crates/pop-parachains/src/utils/pallet_helpers.rs b/crates/pop-parachains/src/utils/pallet_helpers.rs index 62f4a54b..d8832ca8 100644 --- a/crates/pop-parachains/src/utils/pallet_helpers.rs +++ b/crates/pop-parachains/src/utils/pallet_helpers.rs @@ -1,36 +1,38 @@ +use crate::errors::Error; use std::{ env::current_dir, - fs::{self}, + fs, path::{Path, PathBuf}, + process, }; /// Resolve pallet path /// For a template it should be `