Skip to content

refactor: replace anyhow #18

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Murasko wants to merge 9 commits into
base: main
Choose a base branch
from
Open

refactor: replace anyhow #18

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
9e3160b
refactor: replace anyhow with custom error types for improved error h…
Murasko Jan 26, 2026
7780614
error.rs aktualisieren
Murasko Jan 26, 2026
a6c4080
hetzner.rs aktualisieren
Murasko Jan 26, 2026
5c32031
refactor: improve error handling and code readability across provider…
Murasko Jan 28, 2026
e519efa
Merge branch 'chore/remove-anyhow' of github.com:TLMS-Development/dnr…
Murasko Jan 28, 2026
e481e7e
refactor: use explicit mapping for errors
Kitt3120 Jan 28, 2026
346c275
refactor: do not use suffix "Error" in enum variants
Kitt3120 Jan 28, 2026
cbb94fc
refactor: box error type to not exhaust the stack
Kitt3120 Jan 28, 2026
9d3f173
refactor: simplify configuration merging logic and improve test cases
Murasko Jan 29, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion Cargo.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 0 additions & 1 deletion Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
4 changes: 2 additions & 2 deletions src/cli/generate_config.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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> {
Expand All @@ -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
Expand Down
48 changes: 28 additions & 20 deletions src/cli/get.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
},
};
Expand All @@ -26,7 +26,13 @@ pub enum Error {
ProviderNotConfigured(String),

#[error("Provider error: {0}")]
ProviderError(#[from] anyhow::Error),
Provider(#[from] ProviderError),

#[error("Cannot specify both --all and specific subdomains")]
ConflictingArguments,

#[error("Must specify either --all or specific subdomains")]
MissingArguments,
}

#[derive(Debug, Args)]
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -156,35 +158,41 @@ mod tests {

#[test]
fn test_get_provider_nitrado() {
let mut config = Config::default();
config.providers = vec![ProviderConfig::Nitrado(nitrado::Config {
name: "TestNitrado".to_string(),
let config = Config {
providers: vec![ProviderConfig::Nitrado(nitrado::Config {
name: "TestNitrado".to_string(),
..Default::default()
})],
..Default::default()
})];
};

let provider = get_provider("TestNitrado", &config).unwrap();
assert_eq!(provider.get_provider_name(), "Nitrado");
}

#[test]
fn test_get_provider_hetzner() {
let mut config = Config::default();
config.providers = vec![ProviderConfig::Hetzner(hetzner::Config {
name: "TestHetzner".to_string(),
let config = Config {
providers: vec![ProviderConfig::Hetzner(hetzner::Config {
name: "TestHetzner".to_string(),
..Default::default()
})],
..Default::default()
})];
};

let provider = get_provider("TestHetzner", &config).unwrap();
assert_eq!(provider.get_provider_name(), "Hetzner");
}

#[test]
fn test_get_provider_netcup() {
let mut config = Config::default();
config.providers = vec![ProviderConfig::Netcup(netcup::Config {
name: "TestNetcup".to_string(),
let config = Config {
providers: vec![ProviderConfig::Netcup(netcup::Config {
name: "TestNetcup".to_string(),
..Default::default()
})],
..Default::default()
})];
};

let provider = get_provider("TestNetcup", &config).unwrap();
assert_eq!(provider.get_provider_name(), "Netcup");
Expand Down
6 changes: 3 additions & 3 deletions src/cli_tests.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use std::process::Command;
#[test]
fn test_help() {
let output = Command::new("cargo")
.args(&["run", "--", "--help"])
.args(["run", "--", "--help"])
.output()
.expect("failed to execute process");

Expand All @@ -16,7 +16,7 @@ fn test_help() {
#[test]
fn test_generate_config_help() {
let output = Command::new("cargo")
.args(&["run", "--", "generate-config", "--help"])
.args(["run", "--", "generate-config", "--help"])
.output()
.expect("failed to execute process");

Expand All @@ -33,7 +33,7 @@ fn test_generate_config_execution() {
}

let output = Command::new("cargo")
.args(&[
.args([
"run",
"--",
"generate-config",
Expand Down
130 changes: 71 additions & 59 deletions src/config.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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,
Expand All @@ -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),
}

Copy link
Contributor

@Kitt3120 Kitt3120 Jan 27, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you check if it makes sense to use individual Error enums for the different methods below instead of having them all return the same general ConfigError enum? Or does every method below actually need all of the enum values? Then using the same ConfigError enum for all of them makes sense. Otherwise, define specific error enums per method.

/// Configuration for the dnrs application.
///
/// This struct holds all the configuration required to run the application,
Expand All @@ -28,11 +43,11 @@ pub struct Config {
}

impl Config {
pub fn load_from_directory(config_dir: impl AsRef<Path>) -> Result<Self> {
pub fn load_from_directory(config_dir: impl AsRef<Path>) -> Result<Self, ConfigError> {
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,
Expand All @@ -44,7 +59,7 @@ impl Config {
Ok(default_config.merge_from(loaded_config))
}

fn load_resolver_config(config_dir: impl AsRef<Path>) -> Result<resolver::Config> {
fn load_resolver_config(config_dir: impl AsRef<Path>) -> Result<resolver::Config, ConfigError> {
let resolver_path = config_dir.as_ref().join("resolver.yaml");

//TODO: Fail with error if resolver config is missing
Expand All @@ -56,7 +71,9 @@ impl Config {
}
}

fn load_provider_configs(providers_dir: impl AsRef<Path>) -> Result<Vec<Provider>> {
fn load_provider_configs(
providers_dir: impl AsRef<Path>,
) -> Result<Vec<Provider>, ConfigError> {
let providers_dir = providers_dir.as_ref();
//TODO: Fail with error if providers config is missing
if !providers_dir.exists() {
Expand All @@ -78,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)?;

Expand All @@ -105,7 +122,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()));
}
}
}
Expand All @@ -120,7 +137,7 @@ impl Config {
Ok(configs)
}

fn load_dns_configs(dns_dir: impl AsRef<Path>) -> Result<Vec<dns::Type>> {
fn load_dns_configs(dns_dir: impl AsRef<Path>) -> Result<Vec<dns::Type>, ConfigError> {
let dns_dir = dns_dir.as_ref();

//TODO: Fail with error if dns config is missing
Expand All @@ -139,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)?;

Expand All @@ -162,10 +179,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()));
}
}
}
Expand All @@ -174,7 +188,7 @@ impl Config {
Ok(configs)
}

pub fn create_example_structure(config_dir: impl AsRef<Path>) -> Result<()> {
pub fn create_example_structure(config_dir: impl AsRef<Path>) -> Result<(), ConfigError> {
let config_dir = config_dir.as_ref();

fs::create_dir_all(config_dir.join("providers"))?;
Expand Down Expand Up @@ -237,6 +251,41 @@ impl Default for Config {
}
}

impl MergeFrom<Self> for Config {
/// Merges another configuration into this one.
///
/// Values from `other` will override values in `self`.
///
/// # Examples
///
/// ```
/// use dnrs::Config;
/// use lum_config::MergeFrom;
///
/// let mut config = Config::default();
/// let mut other = Config::default();
/// other.resolver.ipv4.url = "https://example.com".to_string();
///
/// let merged = config.merge_from(other);
/// assert_eq!(merged.resolver.ipv4.url, "https://example.com");
/// ```
fn merge_from(self, other: Self) -> Self {
Self {
resolver: other.resolver,
providers: if !other.providers.is_empty() {
other.providers
} else {
self.providers
},
dns: if !other.dns.is_empty() {
other.dns
} else {
self.dns
},
}
}
}

#[cfg(test)]
mod tests {
use super::*;
Expand Down Expand Up @@ -271,12 +320,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![],
};

Expand Down Expand Up @@ -342,38 +389,3 @@ mod tests {
fs::remove_dir_all(&temp_dir).unwrap();
}
}

impl MergeFrom<Self> for Config {
/// Merges another configuration into this one.
///
/// Values from `other` will override values in `self`.
///
/// # Examples
///
/// ```
/// use dnrs::Config;
/// use lum_config::MergeFrom;
///
/// let mut config = Config::default();
/// let mut other = Config::default();
/// other.resolver.ipv4.url = "https://example.com".to_string();
///
/// let merged = config.merge_from(other);
/// assert_eq!(merged.resolver.ipv4.url, "https://example.com");
/// ```
fn merge_from(self, other: Self) -> Self {
Self {
resolver: other.resolver,
providers: if !other.providers.is_empty() {
other.providers
} else {
self.providers
},
dns: if !other.dns.is_empty() {
other.dns
} else {
self.dns
},
}
}
}
2 changes: 1 addition & 1 deletion src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down
Loading