From 9e3160b17913c10ab3f878229e5ffa267b46d195 Mon Sep 17 00:00:00 2001 From: Murasko Date: Mon, 26 Jan 2026 22:33:59 +0100 Subject: [PATCH 1/7] refactor: replace anyhow with custom error types for improved error handling --- Cargo.lock | 1 - Cargo.toml | 1 - src/cli/generate_config.rs | 4 ++-- src/cli/get.rs | 18 +++++++++-------- src/config.rs | 40 +++++++++++++++++++++++++------------- src/lib.rs | 2 +- src/main.rs | 4 ++-- src/provider.rs | 24 ++++++++++++----------- src/provider/error.rs | 38 ++++++++++++++++++++++++++++++++++++ src/provider/hetzner.rs | 30 +++++++++++++++------------- src/provider/netcup.rs | 14 +++++++------ src/provider/nitrado.rs | 24 ++++++++++++----------- 12 files changed, 129 insertions(+), 71 deletions(-) create mode 100644 src/provider/error.rs diff --git a/Cargo.lock b/Cargo.lock index 296d844..c8ce80f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -288,7 +288,6 @@ dependencies = [ name = "dnrs" version = "0.1.0" dependencies = [ - "anyhow", "async-trait", "clap", "dirs", diff --git a/Cargo.toml b/Cargo.toml index 4cc7536..f773f4e 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -14,7 +14,6 @@ opt-level = 0 lto = false [dependencies] -anyhow = "1.0.99" async-trait = "0.1.89" clap = { version = "4.5.39", features = ["derive", "unicode", "wrap_help"] } dirs = "6.0.0" diff --git a/src/cli/generate_config.rs b/src/cli/generate_config.rs index 2a9b829..6668b19 100644 --- a/src/cli/generate_config.rs +++ b/src/cli/generate_config.rs @@ -4,7 +4,7 @@ use clap::Parser; use lum_log::info; use thiserror::Error; -use crate::{Config, cli::ExecutableCommand}; +use crate::{Config, ConfigError, cli::ExecutableCommand}; #[derive(Debug)] pub struct Input<'config> { @@ -20,7 +20,7 @@ pub enum Error { Yaml(#[from] serde_yaml_ng::Error), #[error("Config error: {0}")] - Config(#[from] anyhow::Error), + Config(#[from] ConfigError), } /// Generate configuration directory structure diff --git a/src/cli/get.rs b/src/cli/get.rs index a90dcef..ba12341 100644 --- a/src/cli/get.rs +++ b/src/cli/get.rs @@ -9,7 +9,7 @@ use crate::{ cli::ExecutableCommand, config::provider::Provider as ProviderConfig, provider::{ - GetAllRecordsInput, GetRecordsInput, Provider, hetzner::HetznerProvider, + GetAllRecordsInput, GetRecordsInput, Provider, ProviderError, hetzner::HetznerProvider, netcup::NetcupProvider, nitrado::NitradoProvider, }, }; @@ -26,7 +26,13 @@ pub enum Error { ProviderNotConfigured(String), #[error("Provider error: {0}")] - ProviderError(#[from] anyhow::Error), + ProviderError(#[from] ProviderError), + + #[error("Cannot specify both --all and specific subdomains")] + ConflictingArguments, + + #[error("Must specify either --all or specific subdomains")] + MissingArguments, } #[derive(Debug, Args)] @@ -94,16 +100,12 @@ impl<'command> ExecutableCommand<'command> for Command<'command> { async fn execute(&self, input: &'command Self::I) -> Self::R { if self.subdomain_args.all && !self.subdomain_args.subdomains.is_empty() { error!("Cannot specify both --all and specific subdomains"); - return Err(Error::ProviderError(anyhow::anyhow!( - "Cannot specify both --all and specific subdomains" - ))); + return Err(Error::ConflictingArguments); } if !self.subdomain_args.all && self.subdomain_args.subdomains.is_empty() { error!("Must specify either --all or specific subdomains"); - return Err(Error::ProviderError(anyhow::anyhow!( - "Must specify either --all or specific subdomains" - ))); + return Err(Error::MissingArguments); } let config = input.config; diff --git a/src/config.rs b/src/config.rs index 3f54141..287ca46 100644 --- a/src/config.rs +++ b/src/config.rs @@ -1,9 +1,8 @@ -//TODO: No anyhow -use anyhow::Result; use lum_config::MergeFrom; use lum_libs::serde::{Deserialize, Serialize}; -use lum_log::{debug, error, info}; -use std::{fs, path::Path}; +use lum_log::{debug, info}; +use std::{fs, io, path::Path}; +use thiserror::Error; use crate::{ config::provider::Provider, @@ -14,6 +13,22 @@ pub mod dns; pub mod provider; pub mod resolver; +/// Error type for configuration loading and parsing. +#[derive(Debug, Error)] +pub enum ConfigError { + #[error("IO error: {0}")] + Io(#[from] io::Error), + + #[error("YAML parsing error: {0}")] + Yaml(#[from] serde_yaml_ng::Error), + + #[error("Unknown provider config file: {0}")] + UnknownProvider(String), + + #[error("Cannot determine DNS config type for file: {0}")] + UnknownDnsType(String), +} + /// Configuration for the dnrs application. /// /// This struct holds all the configuration required to run the application, @@ -28,7 +43,7 @@ pub struct Config { } impl Config { - pub fn load_from_directory(config_dir: impl AsRef) -> Result { + pub fn load_from_directory(config_dir: impl AsRef) -> Result { let config_dir = config_dir.as_ref(); let resolver = Self::load_resolver_config(config_dir)?; let providers = Self::load_provider_configs(&config_dir.join("providers"))?; @@ -44,7 +59,7 @@ impl Config { Ok(default_config.merge_from(loaded_config)) } - fn load_resolver_config(config_dir: impl AsRef) -> Result { + fn load_resolver_config(config_dir: impl AsRef) -> Result { let resolver_path = config_dir.as_ref().join("resolver.yaml"); //TODO: Fail with error if resolver config is missing @@ -56,7 +71,7 @@ impl Config { } } - fn load_provider_configs(providers_dir: impl AsRef) -> Result> { + fn load_provider_configs(providers_dir: impl AsRef) -> Result, ConfigError> { let providers_dir = providers_dir.as_ref(); //TODO: Fail with error if providers config is missing if !providers_dir.exists() { @@ -105,7 +120,7 @@ impl Config { debug!("Loaded Netcup provider config from {:?}", path); } _ => { - error!("Unknown provider config file: {}", path.display()); + return Err(ConfigError::UnknownProvider(path.display().to_string())); } } } @@ -120,7 +135,7 @@ impl Config { Ok(configs) } - fn load_dns_configs(dns_dir: impl AsRef) -> Result> { + fn load_dns_configs(dns_dir: impl AsRef) -> Result, ConfigError> { let dns_dir = dns_dir.as_ref(); //TODO: Fail with error if dns config is missing @@ -162,10 +177,7 @@ impl Config { configs.push(dns::Type::Netcup(config)); debug!("Loaded Netcup DNS config from {:?}", path); } else { - error!( - "Cannot determine DNS config type for file: {}", - path.display() - ); + return Err(ConfigError::UnknownDnsType(path.display().to_string())); } } } @@ -174,7 +186,7 @@ impl Config { Ok(configs) } - pub fn create_example_structure(config_dir: impl AsRef) -> Result<()> { + pub fn create_example_structure(config_dir: impl AsRef) -> Result<(), ConfigError> { let config_dir = config_dir.as_ref(); fs::create_dir_all(config_dir.join("providers"))?; diff --git a/src/lib.rs b/src/lib.rs index e3f2b78..87cc07b 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -15,7 +15,7 @@ pub mod types; #[cfg(test)] mod cli_tests; -pub use config::Config; +pub use config::{Config, ConfigError}; pub use logger::setup_logger; pub const PROGRAM_NAME: &str = env!("CARGO_PKG_NAME"); diff --git a/src/main.rs b/src/main.rs index 6cbd0de..8f9c505 100644 --- a/src/main.rs +++ b/src/main.rs @@ -1,7 +1,7 @@ use std::fmt::{self, Debug}; use std::fs; -use dnrs::{Config, RuntimeError, run, setup_logger}; +use dnrs::{Config, ConfigError, RuntimeError, run, setup_logger}; use lum_config::{ConfigPathError, EnvironmentConfigParseError, FileConfigParseError}; use lum_log::{info, log::SetLoggerError}; use thiserror::Error; @@ -59,7 +59,7 @@ enum Error { Io(#[from] std::io::Error), #[error("Config error: {0}")] - Config(#[from] anyhow::Error), + Config(#[from] ConfigError), #[error("Unable to determine config directory")] NoConfigDirectory, diff --git a/src/provider.rs b/src/provider.rs index b610ecc..21550fb 100644 --- a/src/provider.rs +++ b/src/provider.rs @@ -1,12 +1,14 @@ -use anyhow::Result; use async_trait::async_trait; use crate::types::dns::Record; +pub mod error; pub mod hetzner; pub mod netcup; pub mod nitrado; +pub use error::ProviderError; + #[derive(Debug, Clone, PartialEq, Eq)] pub enum Feature { GetRecords, @@ -70,12 +72,12 @@ pub trait Provider: Send + Sync { &self, reqwest: reqwest::Client, input: &GetRecordsInput, - ) -> Result> { + ) -> Result, ProviderError> { let get_all_records_input = GetAllRecordsInput::from(input); let records = self .get_all_records(reqwest, &get_all_records_input) .await?; - let records = records + let records: Vec = records .into_iter() .filter(|record| input.subdomains.contains(&record.domain.as_str())) .collect(); @@ -87,11 +89,11 @@ pub trait Provider: Send + Sync { &self, reqwest: reqwest::Client, input: &GetAllRecordsInput, - ) -> Result>; + ) -> Result, ProviderError>; - async fn add_record(&self, reqwest: reqwest::Client, record: &Record) -> Result<()>; - async fn update_record(&self, reqwest: reqwest::Client, record: &Record) -> Result<()>; - async fn delete_record(&self, reqwest: reqwest::Client, record: &Record) -> Result<()>; + async fn add_record(&self, reqwest: reqwest::Client, record: &Record) -> Result<(), ProviderError>; + async fn update_record(&self, reqwest: reqwest::Client, record: &Record) -> Result<(), ProviderError>; + async fn delete_record(&self, reqwest: reqwest::Client, record: &Record) -> Result<(), ProviderError>; } #[cfg(test)] @@ -119,19 +121,19 @@ mod tests { &self, _reqwest: reqwest::Client, _input: &GetAllRecordsInput, - ) -> Result> { + ) -> Result, ProviderError> { Ok(self.records.clone()) } - async fn add_record(&self, _reqwest: reqwest::Client, _record: &Record) -> Result<()> { + async fn add_record(&self, _reqwest: reqwest::Client, _record: &Record) -> Result<(), ProviderError> { unimplemented!() } - async fn update_record(&self, _reqwest: reqwest::Client, _record: &Record) -> Result<()> { + async fn update_record(&self, _reqwest: reqwest::Client, _record: &Record) -> Result<(), ProviderError> { unimplemented!() } - async fn delete_record(&self, _reqwest: reqwest::Client, _record: &Record) -> Result<()> { + async fn delete_record(&self, _reqwest: reqwest::Client, _record: &Record) -> Result<(), ProviderError> { unimplemented!() } } diff --git a/src/provider/error.rs b/src/provider/error.rs new file mode 100644 index 0000000..ffe060d --- /dev/null +++ b/src/provider/error.rs @@ -0,0 +1,38 @@ +use thiserror::Error; + +use crate::provider::{hetzner, netcup, nitrado}; + +/// Error type for all DNS provider operations. +/// +/// This enum wraps provider-specific errors and common failure cases, +/// providing a unified error type for the provider trait. +#[derive(Debug, Error)] +pub enum ProviderError { + /// Error from Hetzner DNS provider + #[error("Hetzner provider error: {0}")] + Hetzner(#[from] hetzner::Error), + + /// Error from Nitrado DNS provider + #[error("Nitrado provider error: {0}")] + Nitrado(#[from] nitrado::Error), + + /// Error from Netcup DNS provider + #[error("Netcup provider error: {0}")] + Netcup(#[from] netcup::Error), + + /// HTTP request failed + #[error("HTTP request failed: {0}")] + Http(#[from] reqwest::Error), + + /// JSON parsing error + #[error("JSON parsing error: {0}")] + Json(#[from] lum_libs::serde_json::Error), + + /// Invalid API key format for HTTP headers + #[error("Invalid API key: contains characters not allowed in HTTP headers")] + InvalidApiKey, + + /// Record conversion error + #[error("Failed to convert record: {0}")] + RecordConversion(String), +} diff --git a/src/provider/hetzner.rs b/src/provider/hetzner.rs index a453327..3f04032 100644 --- a/src/provider/hetzner.rs +++ b/src/provider/hetzner.rs @@ -1,11 +1,10 @@ -use anyhow::Result; use async_trait::async_trait; use lum_libs::serde_json; use reqwest::header::HeaderMap; use thiserror::Error; use crate::{ - provider::{Feature, GetAllRecordsInput, Provider}, + provider::{Feature, GetAllRecordsInput, Provider, ProviderError}, types::dns::{self}, }; @@ -24,7 +23,7 @@ impl<'provider_config> HetznerProvider<'provider_config> { HetznerProvider { provider_config } } - async fn get_zone_id(&self, reqwest: reqwest::Client, domain: &str) -> Result { + async fn get_zone_id(&self, reqwest: reqwest::Client, domain: &str) -> Result { let mut headers = HeaderMap::new(); headers.insert( "Auth-API-Token", @@ -35,7 +34,7 @@ impl<'provider_config> HetznerProvider<'provider_config> { let response = reqwest.get(&url).headers(headers).send().await?; if !response.status().is_success() { - return Err(Error::Unsuccessful(response.status().as_u16(), response).into()); + return Err(Error::Unsuccessful(response.status().as_u16(), response)); } let text = response.text().await?; @@ -56,7 +55,7 @@ impl<'provider_config> HetznerProvider<'provider_config> { }) }) { Some(zone_id) => Ok(zone_id), - None => Err(Error::DomainNotFound(domain.to_string()).into()), + None => Err(Error::DomainNotFound(domain.to_string())), } } } @@ -74,6 +73,9 @@ pub enum Error { #[error("Domain '{0}' not found in Hetzner zones")] DomainNotFound(String), + + #[error("Record conversion error: {0}")] + RecordConversion(#[from] TryFromRecordError), } #[async_trait] @@ -97,7 +99,7 @@ impl Provider for HetznerProvider<'_> { &self, reqwest: reqwest::Client, input: &GetAllRecordsInput, - ) -> Result> { + ) -> Result, ProviderError> { let mut headers = HeaderMap::new(); headers.insert( "Auth-API-Token", @@ -105,7 +107,7 @@ impl Provider for HetznerProvider<'_> { ); let domain = &input.domain; - let zone_id = self.get_zone_id(reqwest.clone(), domain).await?; + let zone_id = self.get_zone_id(reqwest.clone(), domain).await.map_err(ProviderError::Hetzner)?; let url = format!( "{}/records?zone_id={}", @@ -115,25 +117,25 @@ impl Provider for HetznerProvider<'_> { let response = reqwest.get(&url).headers(headers).send().await?; if !response.status().is_success() { - return Err(Error::Unsuccessful(response.status().as_u16(), response).into()); + return Err(ProviderError::Hetzner(Error::Unsuccessful(response.status().as_u16(), response))); } - let text = response.text().await?; - let response: GetRecordsResponse = serde_json::from_str(&text)?; - let records: Vec = response.try_into()?; + let text = response.text().await.map_err(ProviderError::Http)?; + let response: GetRecordsResponse = serde_json::from_str(&text).map_err(ProviderError::Json)?; + let records: Vec = response.try_into().map_err(|e: TryFromRecordError| ProviderError::Hetzner(Error::RecordConversion(e)))?; Ok(records) } - async fn add_record(&self, _reqwest: reqwest::Client, _input: &dns::Record) -> Result<()> { + async fn add_record(&self, _reqwest: reqwest::Client, _input: &dns::Record) -> Result<(), ProviderError> { unimplemented!("Hetzner add_record not yet implemented") } - async fn update_record(&self, _reqwest: reqwest::Client, _input: &dns::Record) -> Result<()> { + async fn update_record(&self, _reqwest: reqwest::Client, _input: &dns::Record) -> Result<(), ProviderError> { unimplemented!("Hetzner update_record not yet implemented") } - async fn delete_record(&self, _reqwest: reqwest::Client, _input: &dns::Record) -> Result<()> { + async fn delete_record(&self, _reqwest: reqwest::Client, _input: &dns::Record) -> Result<(), ProviderError> { unimplemented!("Hetzner delete_record not yet implemented") } } diff --git a/src/provider/netcup.rs b/src/provider/netcup.rs index 247a2e4..41a1082 100644 --- a/src/provider/netcup.rs +++ b/src/provider/netcup.rs @@ -1,10 +1,9 @@ -use anyhow::Result; use async_trait::async_trait; use lum_libs::serde_json; use thiserror::Error; use crate::{ - provider::{Feature, GetAllRecordsInput, Provider}, + provider::{Feature, GetAllRecordsInput, Provider, ProviderError}, types::dns::{self}, }; @@ -37,6 +36,9 @@ pub enum Error { #[error("Domain '{0}' not found in Netcup zones")] DomainNotFound(String), + + #[error("Record conversion error: {0}")] + RecordConversion(#[from] TryFromRecordError), } #[async_trait] @@ -60,19 +62,19 @@ impl Provider for NetcupProvider<'_> { &self, _reqwest: reqwest::Client, _input: &GetAllRecordsInput, - ) -> Result> { + ) -> Result, ProviderError> { unimplemented!("Netcup get_all_records not yet implemented") } - async fn add_record(&self, _reqwest: reqwest::Client, _input: &dns::Record) -> Result<()> { + async fn add_record(&self, _reqwest: reqwest::Client, _input: &dns::Record) -> Result<(), ProviderError> { unimplemented!("Netcup add_record not yet implemented") } - async fn update_record(&self, _reqwest: reqwest::Client, _input: &dns::Record) -> Result<()> { + async fn update_record(&self, _reqwest: reqwest::Client, _input: &dns::Record) -> Result<(), ProviderError> { unimplemented!("Netcup update_record not yet implemented") } - async fn delete_record(&self, _reqwest: reqwest::Client, _input: &dns::Record) -> Result<()> { + async fn delete_record(&self, _reqwest: reqwest::Client, _input: &dns::Record) -> Result<(), ProviderError> { unimplemented!("Netcup delete_record not yet implemented") } } diff --git a/src/provider/nitrado.rs b/src/provider/nitrado.rs index e0adf64..1f5d85e 100644 --- a/src/provider/nitrado.rs +++ b/src/provider/nitrado.rs @@ -1,11 +1,10 @@ -use anyhow::Result; use async_trait::async_trait; use lum_libs::serde_json; use reqwest::header::HeaderMap; use thiserror::Error; use crate::{ - provider::{Feature, GetAllRecordsInput, Provider}, + provider::{Feature, GetAllRecordsInput, Provider, ProviderError}, types::dns::{self}, }; @@ -35,6 +34,9 @@ pub enum Error { #[error("JSON parsing error: {0}")] Json(#[from] serde_json::Error), + + #[error("Record conversion error: {0}")] + RecordConversion(#[from] TryFromRecordError), } #[async_trait] @@ -57,7 +59,7 @@ impl Provider for NitradoProvider<'_> { &self, reqwest: reqwest::Client, input: &GetAllRecordsInput, - ) -> Result> { + ) -> Result, ProviderError> { let mut headers = HeaderMap::new(); headers.insert( "Authorization", @@ -71,28 +73,28 @@ impl Provider for NitradoProvider<'_> { "{}/domain/{}/records", self.provider_config.api_base_url, domain ); - let response = reqwest.get(&url).headers(headers).send().await?; + let response = reqwest.get(&url).headers(headers).send().await.map_err(ProviderError::Http)?; if !response.status().is_success() { - return Err(Error::Unsuccessful(response.status().as_u16(), response).into()); + return Err(ProviderError::Nitrado(Error::Unsuccessful(response.status().as_u16(), response))); } - let text = response.text().await?; - let response: GetRecordsResponse = serde_json::from_str(&text)?; - let records: Vec = response.try_into()?; + let text = response.text().await.map_err(ProviderError::Http)?; + let response: GetRecordsResponse = serde_json::from_str(&text).map_err(ProviderError::Json)?; + let records: Vec = response.try_into().map_err(|e: TryFromRecordError| ProviderError::Nitrado(Error::RecordConversion(e)))?; Ok(records) } - async fn add_record(&self, _reqwest: reqwest::Client, _input: &dns::Record) -> Result<()> { + async fn add_record(&self, _reqwest: reqwest::Client, _input: &dns::Record) -> Result<(), ProviderError> { unimplemented!() } - async fn update_record(&self, _reqwest: reqwest::Client, _input: &dns::Record) -> Result<()> { + async fn update_record(&self, _reqwest: reqwest::Client, _input: &dns::Record) -> Result<(), ProviderError> { unimplemented!() } - async fn delete_record(&self, _reqwest: reqwest::Client, _input: &dns::Record) -> Result<()> { + async fn delete_record(&self, _reqwest: reqwest::Client, _input: &dns::Record) -> Result<(), ProviderError> { unimplemented!() } } From 7780614a2dd730e91e5be3fea3fc02d8261e7203 Mon Sep 17 00:00:00 2001 From: Murasko Date: Mon, 26 Jan 2026 22:59:03 +0100 Subject: [PATCH 2/7] error.rs aktualisieren Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- src/provider/error.rs | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/provider/error.rs b/src/provider/error.rs index ffe060d..ead1524 100644 --- a/src/provider/error.rs +++ b/src/provider/error.rs @@ -31,8 +31,4 @@ pub enum ProviderError { /// Invalid API key format for HTTP headers #[error("Invalid API key: contains characters not allowed in HTTP headers")] InvalidApiKey, - - /// Record conversion error - #[error("Failed to convert record: {0}")] - RecordConversion(String), } From a6c40809ceae7765b47a5cb51bc840e4f922bf24 Mon Sep 17 00:00:00 2001 From: Murasko Date: Mon, 26 Jan 2026 22:59:52 +0100 Subject: [PATCH 3/7] hetzner.rs aktualisieren Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- src/provider/hetzner.rs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/provider/hetzner.rs b/src/provider/hetzner.rs index 3f04032..cfedae2 100644 --- a/src/provider/hetzner.rs +++ b/src/provider/hetzner.rs @@ -114,7 +114,12 @@ impl Provider for HetznerProvider<'_> { self.provider_config.api_base_url, zone_id ); - let response = reqwest.get(&url).headers(headers).send().await?; + let response = reqwest + .get(&url) + .headers(headers) + .send() + .await + .map_err(ProviderError::Http)?; if !response.status().is_success() { return Err(ProviderError::Hetzner(Error::Unsuccessful(response.status().as_u16(), response))); From 5c3203163b0fc9bc5eadc43e233d5b8e8311a825 Mon Sep 17 00:00:00 2001 From: Murasko Date: Wed, 28 Jan 2026 07:56:24 +0100 Subject: [PATCH 4/7] refactor: improve error handling and code readability across provider implementations --- src/config.rs | 22 +++++----- src/main.rs | 8 +++- src/provider.rs | 36 +++++++++++++--- src/provider/error.rs | 95 +++++++++++++++++++++++++++++++++++------ src/provider/hetzner.rs | 44 +++++++++++++------ src/provider/netcup.rs | 19 +++++++-- src/provider/nitrado.rs | 33 ++++++++++---- 7 files changed, 203 insertions(+), 54 deletions(-) diff --git a/src/config.rs b/src/config.rs index 287ca46..f5d1b36 100644 --- a/src/config.rs +++ b/src/config.rs @@ -46,8 +46,8 @@ impl Config { pub fn load_from_directory(config_dir: impl AsRef) -> Result { let config_dir = config_dir.as_ref(); let resolver = Self::load_resolver_config(config_dir)?; - let providers = Self::load_provider_configs(&config_dir.join("providers"))?; - let dns = Self::load_dns_configs(&config_dir.join("dns"))?; + let providers = Self::load_provider_configs(config_dir.join("providers"))?; + let dns = Self::load_dns_configs(config_dir.join("dns"))?; let loaded_config = Config { resolver, @@ -71,7 +71,9 @@ impl Config { } } - fn load_provider_configs(providers_dir: impl AsRef) -> Result, ConfigError> { + fn load_provider_configs( + providers_dir: impl AsRef, + ) -> Result, ConfigError> { let providers_dir = providers_dir.as_ref(); //TODO: Fail with error if providers config is missing if !providers_dir.exists() { @@ -93,7 +95,7 @@ impl Config { if path .extension() - .map_or(false, |ext| ext == "yaml" || ext == "yml") + .is_some_and(|ext| ext == "yaml" || ext == "yml") { let content = fs::read_to_string(&path)?; @@ -154,7 +156,7 @@ impl Config { if path .extension() - .map_or(false, |ext| ext == "yaml" || ext == "yml") + .is_some_and(|ext| ext == "yaml" || ext == "yml") { let content = fs::read_to_string(&path)?; @@ -283,12 +285,10 @@ mod tests { let default_config = Config::default(); let other = Config { resolver: resolver::Config::default(), - providers: vec![Provider::Nitrado( - nitrado::Config { - name: "OtherNitrado".to_string(), - ..Default::default() - }, - )], + providers: vec![Provider::Nitrado(nitrado::Config { + name: "OtherNitrado".to_string(), + ..Default::default() + })], dns: vec![], }; diff --git a/src/main.rs b/src/main.rs index 8f9c505..acbcab0 100644 --- a/src/main.rs +++ b/src/main.rs @@ -68,7 +68,13 @@ enum Error { ConfigIsNotDirectory, #[error("Runtime error: {0}")] - Runtime(#[from] RuntimeError), + Runtime(Box), +} + +impl From for Error { + fn from(err: RuntimeError) -> Self { + Error::Runtime(Box::new(err)) + } } // When main() returns an `Error`, it will be printed using the `Display` implementation diff --git a/src/provider.rs b/src/provider.rs index 21550fb..6ea98c3 100644 --- a/src/provider.rs +++ b/src/provider.rs @@ -91,9 +91,21 @@ pub trait Provider: Send + Sync { input: &GetAllRecordsInput, ) -> Result, ProviderError>; - async fn add_record(&self, reqwest: reqwest::Client, record: &Record) -> Result<(), ProviderError>; - async fn update_record(&self, reqwest: reqwest::Client, record: &Record) -> Result<(), ProviderError>; - async fn delete_record(&self, reqwest: reqwest::Client, record: &Record) -> Result<(), ProviderError>; + async fn add_record( + &self, + reqwest: reqwest::Client, + record: &Record, + ) -> Result<(), ProviderError>; + async fn update_record( + &self, + reqwest: reqwest::Client, + record: &Record, + ) -> Result<(), ProviderError>; + async fn delete_record( + &self, + reqwest: reqwest::Client, + record: &Record, + ) -> Result<(), ProviderError>; } #[cfg(test)] @@ -125,15 +137,27 @@ mod tests { Ok(self.records.clone()) } - async fn add_record(&self, _reqwest: reqwest::Client, _record: &Record) -> Result<(), ProviderError> { + async fn add_record( + &self, + _reqwest: reqwest::Client, + _record: &Record, + ) -> Result<(), ProviderError> { unimplemented!() } - async fn update_record(&self, _reqwest: reqwest::Client, _record: &Record) -> Result<(), ProviderError> { + async fn update_record( + &self, + _reqwest: reqwest::Client, + _record: &Record, + ) -> Result<(), ProviderError> { unimplemented!() } - async fn delete_record(&self, _reqwest: reqwest::Client, _record: &Record) -> Result<(), ProviderError> { + async fn delete_record( + &self, + _reqwest: reqwest::Client, + _record: &Record, + ) -> Result<(), ProviderError> { unimplemented!() } } diff --git a/src/provider/error.rs b/src/provider/error.rs index ffe060d..8a0906a 100644 --- a/src/provider/error.rs +++ b/src/provider/error.rs @@ -19,20 +19,91 @@ pub enum ProviderError { /// Error from Netcup DNS provider #[error("Netcup provider error: {0}")] Netcup(#[from] netcup::Error), +} + +#[cfg(test)] +mod tests { + use super::*; + use lum_libs::serde_json; + + #[test] + fn test_hetzner_error_display() { + let json_err = serde_json::from_str::("invalid").unwrap_err(); + let err = hetzner::Error::Json(json_err); + let display = format!("{}", err); + assert!(display.contains("JSON parsing error")); + } + + #[test] + fn test_hetzner_domain_not_found_display() { + let err = hetzner::Error::DomainNotFound("example.com".to_string()); + let display = format!("{}", err); + assert_eq!(display, "Domain 'example.com' not found in Hetzner zones"); + } + + #[test] + fn test_nitrado_error_display() { + let json_err = serde_json::from_str::("invalid").unwrap_err(); + let err = nitrado::Error::Json(json_err); + let display = format!("{}", err); + assert!(display.contains("JSON parsing error")); + } + + #[test] + fn test_netcup_error_display() { + let json_err = serde_json::from_str::("invalid").unwrap_err(); + let err = netcup::Error::Json(json_err); + let display = format!("{}", err); + assert!(display.contains("JSON parsing error")); + } + + #[test] + fn test_netcup_domain_not_found_display() { + let err = netcup::Error::DomainNotFound("example.com".to_string()); + let display = format!("{}", err); + assert_eq!(display, "Domain 'example.com' not found in Netcup zones"); + } + + #[test] + fn test_provider_error_from_hetzner() { + let hetzner_err = hetzner::Error::DomainNotFound("example.com".to_string()); + let provider_err: ProviderError = hetzner_err.into(); + + assert!(matches!(provider_err, ProviderError::Hetzner(_))); + let display = format!("{}", provider_err); + assert!(display.contains("Hetzner provider error")); + assert!(display.contains("example.com")); + } + + #[test] + fn test_provider_error_from_nitrado() { + let json_err = serde_json::from_str::("invalid").unwrap_err(); + let nitrado_err = nitrado::Error::Json(json_err); + let provider_err: ProviderError = nitrado_err.into(); + + assert!(matches!(provider_err, ProviderError::Nitrado(_))); + let display = format!("{}", provider_err); + assert!(display.contains("Nitrado provider error")); + } - /// HTTP request failed - #[error("HTTP request failed: {0}")] - Http(#[from] reqwest::Error), + #[test] + fn test_provider_error_from_netcup() { + let netcup_err = netcup::Error::DomainNotFound("test.org".to_string()); + let provider_err: ProviderError = netcup_err.into(); - /// JSON parsing error - #[error("JSON parsing error: {0}")] - Json(#[from] lum_libs::serde_json::Error), + assert!(matches!(provider_err, ProviderError::Netcup(_))); + let display = format!("{}", provider_err); + assert!(display.contains("Netcup provider error")); + assert!(display.contains("test.org")); + } - /// Invalid API key format for HTTP headers - #[error("Invalid API key: contains characters not allowed in HTTP headers")] - InvalidApiKey, + #[test] + fn test_provider_error_debug() { + let hetzner_err = hetzner::Error::DomainNotFound("debug-test.com".to_string()); + let provider_err: ProviderError = hetzner_err.into(); - /// Record conversion error - #[error("Failed to convert record: {0}")] - RecordConversion(String), + let debug = format!("{:?}", provider_err); + assert!(debug.contains("Hetzner")); + assert!(debug.contains("DomainNotFound")); + } } diff --git a/src/provider/hetzner.rs b/src/provider/hetzner.rs index 3f04032..b4d9ed8 100644 --- a/src/provider/hetzner.rs +++ b/src/provider/hetzner.rs @@ -27,7 +27,9 @@ impl<'provider_config> HetznerProvider<'provider_config> { let mut headers = HeaderMap::new(); headers.insert( "Auth-API-Token", - self.provider_config.api_key.parse().expect("Invalid Hetzner API key: contains characters that are not allowed in HTTP headers"), + self.provider_config.api_key.parse().expect( + "Invalid Hetzner API key: contains characters that are not allowed in HTTP headers", + ), ); let url = format!("{}/zones", self.provider_config.api_base_url); @@ -94,7 +96,6 @@ impl Provider for HetznerProvider<'_> { ] } - async fn get_all_records( &self, reqwest: reqwest::Client, @@ -103,39 +104,58 @@ impl Provider for HetznerProvider<'_> { let mut headers = HeaderMap::new(); headers.insert( "Auth-API-Token", - self.provider_config.api_key.parse().expect("Invalid Hetzner API key: contains characters that are not allowed in HTTP headers"), + self.provider_config.api_key.parse().expect( + "Invalid Hetzner API key: contains characters that are not allowed in HTTP headers", + ), ); let domain = &input.domain; - let zone_id = self.get_zone_id(reqwest.clone(), domain).await.map_err(ProviderError::Hetzner)?; + let zone_id = self.get_zone_id(reqwest.clone(), domain).await?; let url = format!( "{}/records?zone_id={}", self.provider_config.api_base_url, zone_id ); - let response = reqwest.get(&url).headers(headers).send().await?; + let response = reqwest + .get(&url) + .headers(headers) + .send() + .await + .map_err(Error::from)?; if !response.status().is_success() { - return Err(ProviderError::Hetzner(Error::Unsuccessful(response.status().as_u16(), response))); + return Err(Error::Unsuccessful(response.status().as_u16(), response).into()); } - let text = response.text().await.map_err(ProviderError::Http)?; - let response: GetRecordsResponse = serde_json::from_str(&text).map_err(ProviderError::Json)?; - let records: Vec = response.try_into().map_err(|e: TryFromRecordError| ProviderError::Hetzner(Error::RecordConversion(e)))?; + let text = response.text().await.map_err(Error::from)?; + let response: GetRecordsResponse = serde_json::from_str(&text).map_err(Error::from)?; + let records: Vec = response.try_into().map_err(Error::from)?; Ok(records) } - async fn add_record(&self, _reqwest: reqwest::Client, _input: &dns::Record) -> Result<(), ProviderError> { + async fn add_record( + &self, + _reqwest: reqwest::Client, + _input: &dns::Record, + ) -> Result<(), ProviderError> { unimplemented!("Hetzner add_record not yet implemented") } - async fn update_record(&self, _reqwest: reqwest::Client, _input: &dns::Record) -> Result<(), ProviderError> { + async fn update_record( + &self, + _reqwest: reqwest::Client, + _input: &dns::Record, + ) -> Result<(), ProviderError> { unimplemented!("Hetzner update_record not yet implemented") } - async fn delete_record(&self, _reqwest: reqwest::Client, _input: &dns::Record) -> Result<(), ProviderError> { + async fn delete_record( + &self, + _reqwest: reqwest::Client, + _input: &dns::Record, + ) -> Result<(), ProviderError> { unimplemented!("Hetzner delete_record not yet implemented") } } diff --git a/src/provider/netcup.rs b/src/provider/netcup.rs index 41a1082..7748263 100644 --- a/src/provider/netcup.rs +++ b/src/provider/netcup.rs @@ -57,7 +57,6 @@ impl Provider for NetcupProvider<'_> { ] } - async fn get_all_records( &self, _reqwest: reqwest::Client, @@ -66,15 +65,27 @@ impl Provider for NetcupProvider<'_> { unimplemented!("Netcup get_all_records not yet implemented") } - async fn add_record(&self, _reqwest: reqwest::Client, _input: &dns::Record) -> Result<(), ProviderError> { + async fn add_record( + &self, + _reqwest: reqwest::Client, + _input: &dns::Record, + ) -> Result<(), ProviderError> { unimplemented!("Netcup add_record not yet implemented") } - async fn update_record(&self, _reqwest: reqwest::Client, _input: &dns::Record) -> Result<(), ProviderError> { + async fn update_record( + &self, + _reqwest: reqwest::Client, + _input: &dns::Record, + ) -> Result<(), ProviderError> { unimplemented!("Netcup update_record not yet implemented") } - async fn delete_record(&self, _reqwest: reqwest::Client, _input: &dns::Record) -> Result<(), ProviderError> { + async fn delete_record( + &self, + _reqwest: reqwest::Client, + _input: &dns::Record, + ) -> Result<(), ProviderError> { unimplemented!("Netcup delete_record not yet implemented") } } diff --git a/src/provider/nitrado.rs b/src/provider/nitrado.rs index 1f5d85e..536f701 100644 --- a/src/provider/nitrado.rs +++ b/src/provider/nitrado.rs @@ -73,28 +73,45 @@ impl Provider for NitradoProvider<'_> { "{}/domain/{}/records", self.provider_config.api_base_url, domain ); - let response = reqwest.get(&url).headers(headers).send().await.map_err(ProviderError::Http)?; + let response = reqwest + .get(&url) + .headers(headers) + .send() + .await + .map_err(Error::from)?; if !response.status().is_success() { - return Err(ProviderError::Nitrado(Error::Unsuccessful(response.status().as_u16(), response))); + return Err(Error::Unsuccessful(response.status().as_u16(), response).into()); } - let text = response.text().await.map_err(ProviderError::Http)?; - let response: GetRecordsResponse = serde_json::from_str(&text).map_err(ProviderError::Json)?; - let records: Vec = response.try_into().map_err(|e: TryFromRecordError| ProviderError::Nitrado(Error::RecordConversion(e)))?; + let text = response.text().await.map_err(Error::from)?; + let response: GetRecordsResponse = serde_json::from_str(&text).map_err(Error::from)?; + let records: Vec = response.try_into().map_err(Error::from)?; Ok(records) } - async fn add_record(&self, _reqwest: reqwest::Client, _input: &dns::Record) -> Result<(), ProviderError> { + async fn add_record( + &self, + _reqwest: reqwest::Client, + _input: &dns::Record, + ) -> Result<(), ProviderError> { unimplemented!() } - async fn update_record(&self, _reqwest: reqwest::Client, _input: &dns::Record) -> Result<(), ProviderError> { + async fn update_record( + &self, + _reqwest: reqwest::Client, + _input: &dns::Record, + ) -> Result<(), ProviderError> { unimplemented!() } - async fn delete_record(&self, _reqwest: reqwest::Client, _input: &dns::Record) -> Result<(), ProviderError> { + async fn delete_record( + &self, + _reqwest: reqwest::Client, + _input: &dns::Record, + ) -> Result<(), ProviderError> { unimplemented!() } } From e481e7e4bb7b2795c8e84d318d0cd6261424230b Mon Sep 17 00:00:00 2001 From: Torben Schweren Date: Wed, 28 Jan 2026 20:44:40 +0100 Subject: [PATCH 5/7] refactor: use explicit mapping for errors --- src/provider/hetzner.rs | 8 ++++---- src/provider/nitrado.rs | 8 ++++---- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/provider/hetzner.rs b/src/provider/hetzner.rs index b4d9ed8..2e1fe5a 100644 --- a/src/provider/hetzner.rs +++ b/src/provider/hetzner.rs @@ -122,15 +122,15 @@ impl Provider for HetznerProvider<'_> { .headers(headers) .send() .await - .map_err(Error::from)?; + .map_err(Error::Reqwest)?; if !response.status().is_success() { return Err(Error::Unsuccessful(response.status().as_u16(), response).into()); } - let text = response.text().await.map_err(Error::from)?; - let response: GetRecordsResponse = serde_json::from_str(&text).map_err(Error::from)?; - let records: Vec = response.try_into().map_err(Error::from)?; + let text = response.text().await.map_err(Error::Reqwest)?; + let response: GetRecordsResponse = serde_json::from_str(&text).map_err(Error::Json)?; + let records: Vec = response.try_into().map_err(Error::RecordConversion)?; Ok(records) } diff --git a/src/provider/nitrado.rs b/src/provider/nitrado.rs index 536f701..f767a54 100644 --- a/src/provider/nitrado.rs +++ b/src/provider/nitrado.rs @@ -78,15 +78,15 @@ impl Provider for NitradoProvider<'_> { .headers(headers) .send() .await - .map_err(Error::from)?; + .map_err(Error::Reqwest)?; if !response.status().is_success() { return Err(Error::Unsuccessful(response.status().as_u16(), response).into()); } - let text = response.text().await.map_err(Error::from)?; - let response: GetRecordsResponse = serde_json::from_str(&text).map_err(Error::from)?; - let records: Vec = response.try_into().map_err(Error::from)?; + let text = response.text().await.map_err(Error::Reqwest)?; + let response: GetRecordsResponse = serde_json::from_str(&text).map_err(Error::Json)?; + let records: Vec = response.try_into().map_err(Error::RecordConversion)?; Ok(records) } From 346c275a8ef4f377c66b431c1f7327e74d94e6ff Mon Sep 17 00:00:00 2001 From: Torben Schweren Date: Wed, 28 Jan 2026 20:50:26 +0100 Subject: [PATCH 6/7] refactor: do not use suffix "Error" in enum variants --- src/cli/get.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/cli/get.rs b/src/cli/get.rs index ba12341..ceec39e 100644 --- a/src/cli/get.rs +++ b/src/cli/get.rs @@ -26,7 +26,7 @@ pub enum Error { ProviderNotConfigured(String), #[error("Provider error: {0}")] - ProviderError(#[from] ProviderError), + Provider(#[from] ProviderError), #[error("Cannot specify both --all and specific subdomains")] ConflictingArguments, From cbb94fc3353a4e91cd01b18c026dde0831ebd84c Mon Sep 17 00:00:00 2001 From: Torben Schweren Date: Wed, 28 Jan 2026 21:14:38 +0100 Subject: [PATCH 7/7] refactor: box error type to not exhaust the stack --- src/main.rs | 26 ++++++++++---------------- 1 file changed, 10 insertions(+), 16 deletions(-) diff --git a/src/main.rs b/src/main.rs index acbcab0..9b77079 100644 --- a/src/main.rs +++ b/src/main.rs @@ -68,13 +68,7 @@ enum Error { ConfigIsNotDirectory, #[error("Runtime error: {0}")] - Runtime(Box), -} - -impl From for Error { - fn from(err: RuntimeError) -> Self { - Error::Runtime(Box::new(err)) - } + Runtime(#[from] RuntimeError), } // When main() returns an `Error`, it will be printed using the `Display` implementation @@ -84,22 +78,22 @@ impl Debug for Error { } } -fn read_config() -> Result { +fn read_config() -> Result> { let config_dir = dirs::config_dir() - .ok_or(Error::NoConfigDirectory)? + .ok_or(Box::new(Error::NoConfigDirectory))? .join(APP_NAME); if config_dir.exists() && !config_dir.is_dir() { - return Err(Error::ConfigIsNotDirectory); + return Err(Box::new(Error::ConfigIsNotDirectory)); } let config = if config_dir.exists() { - Config::load_from_directory(&config_dir)? + Config::load_from_directory(&config_dir).map_err(|e| Box::new(Error::from(e)))? } else { info!("Config directory does not exist, creating default structure..."); - fs::create_dir_all(&config_dir)?; + fs::create_dir_all(&config_dir).map_err(|e| Box::new(Error::from(e)))?; - Config::create_example_structure(&config_dir)?; + Config::create_example_structure(&config_dir).map_err(|e| Box::new(Error::from(e)))?; info!( "Created default config structure at: {}", config_dir.display() @@ -114,11 +108,11 @@ fn read_config() -> Result { } #[tokio::main] -async fn main() -> Result<(), Error> { - setup_logger()?; +async fn main() -> Result<(), Box> { + setup_logger().map_err(|e| Box::new(Error::from(e)))?; let config = read_config()?; - run(config).await?; + run(config).await.map_err(|e| Box::new(Error::from(e)))?; Ok(()) }