From 32422f2b4ff19cd1773ca25a381dd573c45423d6 Mon Sep 17 00:00:00 2001 From: NotAShelf Date: Sun, 18 May 2025 15:39:49 +0300 Subject: [PATCH] treewide: streamline error handling; leverage thiserror and anyhow --- Cargo.lock | 8 ++ Cargo.toml | 2 + src/config/types.rs | 35 ++----- src/cpu.rs | 4 +- src/daemon.rs | 16 ++-- src/engine.rs | 2 +- src/util/error.rs | 216 +++++++++++--------------------------------- 7 files changed, 81 insertions(+), 202 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 8233108..2602893 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -76,6 +76,12 @@ dependencies = [ "windows-sys", ] +[[package]] +name = "anyhow" +version = "1.0.98" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e16d2d3311acee920a9eb8d33b8cbc1787ce4a264e85f964c2404b969bdcd487" + [[package]] name = "autocfg" version = "1.4.0" @@ -539,6 +545,7 @@ checksum = "7da8b5736845d9f2fcb837ea5d9e2628564b3b043a70948a3f0b778838c5fb4f" name = "superfreq" version = "0.3.1" dependencies = [ + "anyhow", "chrono", "clap", "ctrlc", @@ -547,6 +554,7 @@ dependencies = [ "log", "num_cpus", "serde", + "thiserror", "toml", ] diff --git a/Cargo.toml b/Cargo.toml index 7740ebd..25f79e8 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -16,3 +16,5 @@ ctrlc = "3.4" chrono = "0.4" log = "0.4" env_logger = "0.11" +thiserror = "2.0" +anyhow = "1.0" diff --git a/src/config/types.rs b/src/config/types.rs index b2f6091..eb9ce7f 100644 --- a/src/config/types.rs +++ b/src/config/types.rs @@ -98,37 +98,18 @@ pub struct AppConfig { } // Error type for config loading -#[derive(Debug)] +#[derive(Debug, thiserror::Error)] pub enum ConfigError { - Io(std::io::Error), - Toml(toml::de::Error), + #[error("I/O error: {0}")] + Io(#[from] std::io::Error), + + #[error("TOML parsing error: {0}")] + Toml(#[from] toml::de::Error), + + #[error("Configuration validation error: {0}")] Validation(String), } -impl From for ConfigError { - fn from(err: std::io::Error) -> Self { - Self::Io(err) - } -} - -impl From for ConfigError { - fn from(err: toml::de::Error) -> Self { - Self::Toml(err) - } -} - -impl std::fmt::Display for ConfigError { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - match self { - Self::Io(e) => write!(f, "I/O error: {e}"), - Self::Toml(e) => write!(f, "TOML parsing error: {e}"), - Self::Validation(s) => write!(f, "Configuration validation error: {s}"), - } - } -} - -impl std::error::Error for ConfigError {} - // Intermediate structs for TOML parsing #[derive(Deserialize, Serialize, Debug, Clone)] pub struct ProfileConfigToml { diff --git a/src/cpu.rs b/src/cpu.rs index cbd37f8..5629df3 100644 --- a/src/cpu.rs +++ b/src/cpu.rs @@ -109,7 +109,7 @@ pub fn set_governor(governor: &str, core_id: Option) -> Result<()> { let (is_valid, available_governors) = is_governor_valid(governor)?; if !is_valid { - return Err(ControlError::InvalidValueError(format!( + return Err(ControlError::InvalidGovernor(format!( "Governor '{}' is not available on this system. Valid governors: {}", governor, available_governors.join(", ") @@ -432,7 +432,7 @@ fn read_sysfs_value_as_u32(path: &str) -> Result { content .trim() .parse::() - .map_err(|e| ControlError::ReadError(format!("Failed to parse value from {path}: {e}"))) + .map_err(|e| ControlError::ParseError(format!("Failed to parse value from {path}: {e}"))) } fn validate_min_frequency(core_id: u32, new_min_freq_mhz: u32) -> Result<()> { diff --git a/src/daemon.rs b/src/daemon.rs index 21c1425..dd90884 100644 --- a/src/daemon.rs +++ b/src/daemon.rs @@ -61,7 +61,10 @@ fn idle_multiplier(idle_secs: u64) -> f32 { /// Calculate optimal polling interval based on system conditions and history /// /// Returns Ok with the calculated interval, or Err if the configuration is invalid -fn compute_new(params: &IntervalParams, system_history: &SystemHistory) -> Result { +fn compute_new( + params: &IntervalParams, + system_history: &SystemHistory, +) -> Result { // Use the centralized validation function validate_poll_intervals(params.min_interval, params.max_interval)?; @@ -134,8 +137,8 @@ fn compute_new(params: &IntervalParams, system_history: &SystemHistory) -> Resul const TOTAL_WEIGHT: u128 = PREVIOUS_VALUE_WEIGHT + NEW_VALUE_WEIGHT; // 10 // XXX: Use u128 arithmetic to avoid overflow with large interval values - let result = (cached as u128 * PREVIOUS_VALUE_WEIGHT - + new_interval as u128 * NEW_VALUE_WEIGHT) + let result = (u128::from(cached) * PREVIOUS_VALUE_WEIGHT + + u128::from(new_interval) * NEW_VALUE_WEIGHT) / TOTAL_WEIGHT; result as u64 @@ -381,20 +384,19 @@ impl SystemHistory { fn validate_poll_intervals(min_interval: u64, max_interval: u64) -> Result<(), ControlError> { if min_interval < 1 { return Err(ControlError::InvalidValueError( - "min_interval must be ≥ 1".to_string() + "min_interval must be ≥ 1".to_string(), )); } if max_interval < 1 { return Err(ControlError::InvalidValueError( - "max_interval must be ≥ 1".to_string() + "max_interval must be ≥ 1".to_string(), )); } if max_interval >= min_interval { Ok(()) } else { Err(ControlError::InvalidValueError(format!( - "Invalid interval configuration: max_interval ({}) is less than min_interval ({})", - max_interval, min_interval + "Invalid interval configuration: max_interval ({max_interval}) is less than min_interval ({min_interval})" ))) } } diff --git a/src/engine.rs b/src/engine.rs index 966cc20..bbefc86 100644 --- a/src/engine.rs +++ b/src/engine.rs @@ -207,7 +207,7 @@ pub fn determine_and_apply_settings( // Let set_governor handle the validation if let Err(e) = cpu::set_governor(governor, None) { // If the governor is not available, log a warning - if matches!(e, ControlError::InvalidValueError(_)) + if matches!(e, ControlError::InvalidGovernor(_)) || matches!(e, ControlError::NotSupported(_)) { warn!( diff --git a/src/util/error.rs b/src/util/error.rs index 8724d62..b91081f 100644 --- a/src/util/error.rs +++ b/src/util/error.rs @@ -1,194 +1,80 @@ use std::io; -#[derive(Debug)] +#[derive(Debug, thiserror::Error)] pub enum ControlError { - Io(io::Error), + #[error("I/O error: {0}")] + Io(#[from] io::Error), + + #[error("Failed to write to sysfs path: {0}")] WriteError(String), + + #[error("Failed to read sysfs path: {0}")] ReadError(String), + + #[error("Invalid value for setting: {0}")] InvalidValueError(String), + + #[error("Control action not supported: {0}")] NotSupported(String), + + #[error("Permission denied: {0}. Try running with sudo.")] PermissionDenied(String), + + #[error("Invalid platform control profile {0} supplied, please provide a valid one.")] InvalidProfile(String), + + #[error("Invalid governor: {0}")] InvalidGovernor(String), + + #[error("Failed to parse value: {0}")] ParseError(String), + + #[error("Path missing: {0}")] PathMissing(String), } -impl From for ControlError { - fn from(err: io::Error) -> Self { - match err.kind() { - io::ErrorKind::PermissionDenied => Self::PermissionDenied(err.to_string()), - _ => Self::Io(err), - } - } -} - -impl std::fmt::Display for ControlError { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - match self { - Self::Io(e) => write!(f, "I/O error: {e}"), - Self::WriteError(s) => write!(f, "Failed to write to sysfs path: {s}"), - Self::ReadError(s) => write!(f, "Failed to read sysfs path: {s}"), - Self::InvalidValueError(s) => write!(f, "Invalid value for setting: {s}"), - Self::NotSupported(s) => write!(f, "Control action not supported: {s}"), - Self::PermissionDenied(s) => { - write!(f, "Permission denied: {s}. Try running with sudo.") - } - Self::InvalidProfile(s) => { - write!( - f, - "Invalid platform control profile {s} supplied, please provide a valid one." - ) - } - Self::InvalidGovernor(s) => { - write!(f, "Invalid governor: {s}") - } - Self::ParseError(s) => { - write!(f, "Failed to parse value: {s}") - } - Self::PathMissing(s) => { - write!(f, "Path missing: {s}") - } - } - } -} - -impl std::error::Error for ControlError {} - -#[derive(Debug)] +#[derive(Debug, thiserror::Error)] pub enum SysMonitorError { - Io(io::Error), + #[error("I/O error: {0}")] + Io(#[from] io::Error), + + #[error("Failed to read sysfs path: {0}")] ReadError(String), + + #[error("Failed to parse value: {0}")] ParseError(String), + + #[error("Failed to parse /proc/stat: {0}")] ProcStatParseError(String), } -impl From for SysMonitorError { - fn from(err: io::Error) -> Self { - Self::Io(err) - } -} - -impl std::fmt::Display for SysMonitorError { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - match self { - Self::Io(e) => write!(f, "I/O error: {e}"), - Self::ReadError(s) => write!(f, "Failed to read sysfs path: {s}"), - Self::ParseError(s) => write!(f, "Failed to parse value: {s}"), - Self::ProcStatParseError(s) => { - write!(f, "Failed to parse /proc/stat: {s}") - } - } - } -} - -impl std::error::Error for SysMonitorError {} - -#[derive(Debug)] +#[derive(Debug, thiserror::Error)] pub enum EngineError { - ControlError(ControlError), + #[error("CPU control error: {0}")] + ControlError(#[from] ControlError), + + #[error("Configuration error: {0}")] ConfigurationError(String), } -impl From for EngineError { - fn from(err: ControlError) -> Self { - Self::ControlError(err) - } -} - -impl std::fmt::Display for EngineError { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - match self { - Self::ControlError(e) => write!(f, "CPU control error: {e}"), - Self::ConfigurationError(s) => write!(f, "Configuration error: {s}"), - } - } -} - -impl std::error::Error for EngineError { - fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { - match self { - Self::ControlError(e) => Some(e), - Self::ConfigurationError(_) => None, - } - } -} - // A unified error type for the entire application -#[derive(Debug)] +#[derive(Debug, thiserror::Error)] pub enum AppError { - Control(ControlError), - Monitor(SysMonitorError), - Engine(EngineError), - Config(crate::config::ConfigError), + #[error("{0}")] + Control(#[from] ControlError), + + #[error("{0}")] + Monitor(#[from] SysMonitorError), + + #[error("{0}")] + Engine(#[from] EngineError), + + #[error("{0}")] + Config(#[from] crate::config::ConfigError), + + #[error("{0}")] Generic(String), - Io(io::Error), -} -impl From for AppError { - fn from(err: ControlError) -> Self { - Self::Control(err) - } -} - -impl From for AppError { - fn from(err: SysMonitorError) -> Self { - Self::Monitor(err) - } -} - -impl From for AppError { - fn from(err: EngineError) -> Self { - Self::Engine(err) - } -} - -impl From for AppError { - fn from(err: crate::config::ConfigError) -> Self { - Self::Config(err) - } -} - -impl From for AppError { - fn from(err: io::Error) -> Self { - Self::Io(err) - } -} - -impl From for AppError { - fn from(err: String) -> Self { - Self::Generic(err) - } -} - -impl From<&str> for AppError { - fn from(err: &str) -> Self { - Self::Generic(err.to_string()) - } -} - -impl std::fmt::Display for AppError { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - match self { - Self::Control(e) => write!(f, "{e}"), - Self::Monitor(e) => write!(f, "{e}"), - Self::Engine(e) => write!(f, "{e}"), - Self::Config(e) => write!(f, "{e}"), - Self::Generic(s) => write!(f, "{s}"), - Self::Io(e) => write!(f, "I/O error: {e}"), - } - } -} - -impl std::error::Error for AppError { - fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { - match self { - Self::Control(e) => Some(e), - Self::Monitor(e) => Some(e), - Self::Engine(e) => Some(e), - Self::Config(e) => Some(e), - Self::Generic(_) => None, - Self::Io(e) => Some(e), - } - } + #[error("I/O error: {0}")] + Io(#[from] io::Error), }