diff --git a/src/cli/debug.rs b/src/cli/debug.rs index c9c7f4e..b8834e1 100644 --- a/src/cli/debug.rs +++ b/src/cli/debug.rs @@ -1,13 +1,13 @@ use crate::config::AppConfig; use crate::cpu; use crate::monitor; -use std::error::Error; +use crate::util::error::AppError; use std::fs; use std::process::{Command, Stdio}; use std::time::Duration; /// Prints comprehensive debug information about the system -pub fn run_debug(config: &AppConfig) -> Result<(), Box> { +pub fn run_debug(config: &AppConfig) -> Result<(), AppError> { println!("=== SUPERFREQ DEBUG INFORMATION ==="); println!("Version: {}", env!("CARGO_PKG_VERSION")); @@ -201,26 +201,31 @@ pub fn run_debug(config: &AppConfig) -> Result<(), Box> { Ok(()) } - Err(e) => Err(Box::new(e) as Box), + Err(e) => Err(AppError::Monitor(e)), } } /// Get kernel version information -fn get_kernel_info() -> Result> { - let output = Command::new("uname").arg("-r").output()?; +fn get_kernel_info() -> Result { + let output = Command::new("uname") + .arg("-r") + .output() + .map_err(AppError::Io)?; - let kernel_version = String::from_utf8(output.stdout)?; + let kernel_version = String::from_utf8(output.stdout) + .map_err(|e| AppError::Generic(format!("Failed to parse kernel version: {e}")))?; Ok(kernel_version.trim().to_string()) } /// Get system uptime -fn get_system_uptime() -> Result> { - let uptime_str = fs::read_to_string("/proc/uptime")?; +fn get_system_uptime() -> Result { + let uptime_str = fs::read_to_string("/proc/uptime").map_err(AppError::Io)?; let uptime_secs = uptime_str .split_whitespace() .next() - .ok_or("Invalid uptime format")? - .parse::()?; + .ok_or_else(|| AppError::Generic("Invalid uptime format".to_string()))? + .parse::() + .map_err(|e| AppError::Generic(format!("Failed to parse uptime: {e}")))?; Ok(Duration::from_secs_f64(uptime_secs)) } @@ -237,14 +242,16 @@ fn check_and_print_sysfs_path(path: &str, description: &str) { } /// Check if a systemd service is active -fn is_systemd_service_active(service_name: &str) -> Result> { +fn is_systemd_service_active(service_name: &str) -> Result { let output = Command::new("systemctl") .arg("is-active") .arg(format!("{service_name}.service")) .stdout(Stdio::piped()) // capture stdout instead of letting it print .stderr(Stdio::null()) // redirect stderr to null - .output()?; + .output() + .map_err(AppError::Io)?; - let status = String::from_utf8(output.stdout)?; + let status = String::from_utf8(output.stdout) + .map_err(|e| AppError::Generic(format!("Failed to parse systemctl output: {e}")))?; Ok(status.trim() == "active") } diff --git a/src/daemon.rs b/src/daemon.rs index 82349cb..9881110 100644 --- a/src/daemon.rs +++ b/src/daemon.rs @@ -3,6 +3,7 @@ use crate::config::{AppConfig, LogLevel}; use crate::core::SystemReport; use crate::engine; use crate::monitor; +use crate::util::error::AppError; use log::{LevelFilter, debug, error, info, warn}; use std::collections::VecDeque; use std::fs::File; @@ -342,7 +343,7 @@ impl SystemHistory { } /// Run the daemon -pub fn run_daemon(mut config: AppConfig, verbose: bool) -> Result<(), Box> { +pub fn run_daemon(mut config: AppConfig, verbose: bool) -> Result<(), AppError> { // Set effective log level based on config and verbose flag let effective_log_level = if verbose { LogLevel::Debug @@ -372,7 +373,7 @@ pub fn run_daemon(mut config: AppConfig, verbose: bool) -> Result<(), Box Result<(), AppError> { // Initialize logger once for the entire application init_logger(); @@ -105,7 +106,7 @@ fn main() { } }; - let command_result = match cli.command { + let command_result: Result<(), AppError> = match cli.command { // TODO: This will be moved to a different module in the future. Some(Commands::Info) => match monitor::collect_system_report(&config) { Ok(report) => { @@ -341,41 +342,30 @@ fn main() { ); Ok(()) } - Err(e) => Err(Box::new(e) as Box), + Err(e) => Err(AppError::Monitor(e)), }, - Some(Commands::SetGovernor { governor, core_id }) => cpu::set_governor(&governor, core_id) - .map_err(|e| Box::new(e) as Box), + Some(Commands::SetGovernor { governor, core_id }) => { + cpu::set_governor(&governor, core_id).map_err(AppError::Control) + } Some(Commands::ForceGovernor { mode }) => { - cpu::force_governor(mode).map_err(|e| Box::new(e) as Box) - } - Some(Commands::SetTurbo { setting }) => { - cpu::set_turbo(setting).map_err(|e| Box::new(e) as Box) + cpu::force_governor(mode).map_err(AppError::Control) } + Some(Commands::SetTurbo { setting }) => cpu::set_turbo(setting).map_err(AppError::Control), Some(Commands::SetEpp { epp, core_id }) => { - cpu::set_epp(&epp, core_id).map_err(|e| Box::new(e) as Box) + cpu::set_epp(&epp, core_id).map_err(AppError::Control) } Some(Commands::SetEpb { epb, core_id }) => { - cpu::set_epb(&epb, core_id).map_err(|e| Box::new(e) as Box) + cpu::set_epb(&epb, core_id).map_err(AppError::Control) } Some(Commands::SetMinFreq { freq_mhz, core_id }) => { // Basic validation for reasonable CPU frequency values - if let Err(e) = validate_freq(freq_mhz, "Minimum") { - error!("{e}"); - Err(e) - } else { - cpu::set_min_frequency(freq_mhz, core_id) - .map_err(|e| Box::new(e) as Box) - } + validate_freq(freq_mhz, "Minimum")?; + cpu::set_min_frequency(freq_mhz, core_id).map_err(AppError::Control) } Some(Commands::SetMaxFreq { freq_mhz, core_id }) => { // Basic validation for reasonable CPU frequency values - if let Err(e) = validate_freq(freq_mhz, "Maximum") { - error!("{e}"); - Err(e) - } else { - cpu::set_max_frequency(freq_mhz, core_id) - .map_err(|e| Box::new(e) as Box) - } + validate_freq(freq_mhz, "Maximum")?; + cpu::set_max_frequency(freq_mhz, core_id).map_err(AppError::Control) } Some(Commands::SetPlatformProfile { profile }) => { // Get available platform profiles and validate early if possible @@ -383,26 +373,24 @@ fn main() { Ok(available_profiles) => { if available_profiles.contains(&profile) { info!("Setting platform profile to '{profile}'"); - cpu::set_platform_profile(&profile) - .map_err(|e| Box::new(e) as Box) + cpu::set_platform_profile(&profile).map_err(AppError::Control) } else { error!( "Invalid platform profile: '{}'. Available profiles: {}", profile, available_profiles.join(", ") ); - Err(Box::new(ControlError::InvalidProfile(format!( + Err(ControlError::InvalidProfile(format!( "Invalid platform profile: '{}'. Available profiles: {}", profile, available_profiles.join(", ") - ))) as Box) + )) + .into()) } } - Err(_) => { + Err(_e) => { // If we can't get profiles (e.g., feature not supported), pass through to the function - // which will provide appropriate error - cpu::set_platform_profile(&profile) - .map_err(|e| Box::new(e) as Box) + cpu::set_platform_profile(&profile).map_err(AppError::Control) } } } @@ -415,15 +403,15 @@ fn main() { error!( "Start threshold ({start_threshold}) must be less than stop threshold ({stop_threshold})" ); - Err(Box::new(ControlError::InvalidValueError(format!( + Err(ControlError::InvalidValueError(format!( "Start threshold ({start_threshold}) must be less than stop threshold ({stop_threshold})" - ))) as Box) + )).into()) } else { info!( "Setting battery thresholds: start at {start_threshold}%, stop at {stop_threshold}%" ); battery::set_battery_charge_thresholds(start_threshold, stop_threshold) - .map_err(|e| Box::new(e) as Box) + .map_err(AppError::Control) } } Some(Commands::Daemon { verbose }) => daemon::run_daemon(config, verbose), @@ -440,11 +428,9 @@ fn main() { if let Some(source) = e.source() { error!("Caused by: {source}"); } - // TODO: Consider specific error handling for PermissionDenied from the cpu module here. - // For example, check if `e.downcast_ref::()` matches `PermissionDenied` - // and print a more specific message like "Try running with sudo." - // We'll revisit this in the future once CPU logic is more stable. - if let Some(control_error) = e.downcast_ref::() { + + // Check for permission denied errors + if let AppError::Control(control_error) = &e { if matches!(control_error, ControlError::PermissionDenied(_)) { error!( "Hint: This operation may require administrator privileges (e.g., run with sudo)." @@ -454,6 +440,8 @@ fn main() { std::process::exit(1); } + + Ok(()) } /// Initialize the logger for the entire application @@ -474,18 +462,17 @@ fn init_logger() { } /// Validate CPU frequency input values -fn validate_freq(freq_mhz: u32, label: &str) -> Result<(), Box> { +fn validate_freq(freq_mhz: u32, label: &str) -> Result<(), AppError> { if freq_mhz == 0 { error!("{label} frequency cannot be zero"); - Err(Box::new(ControlError::InvalidValueError(format!( - "{label} frequency cannot be zero" - ))) as Box) + Err(ControlError::InvalidValueError(format!("{label} frequency cannot be zero")).into()) } else if freq_mhz > 10000 { // Extremely high value unlikely to be valid error!("{label} frequency ({freq_mhz} MHz) is unreasonably high"); - Err(Box::new(ControlError::InvalidValueError(format!( + Err(ControlError::InvalidValueError(format!( "{label} frequency ({freq_mhz} MHz) is unreasonably high" - ))) as Box) + )) + .into()) } else { Ok(()) } diff --git a/src/util/error.rs b/src/util/error.rs index b52480b..8724d62 100644 --- a/src/util/error.rs +++ b/src/util/error.rs @@ -113,3 +113,82 @@ impl std::error::Error for EngineError { } } } + +// A unified error type for the entire application +#[derive(Debug)] +pub enum AppError { + Control(ControlError), + Monitor(SysMonitorError), + Engine(EngineError), + Config(crate::config::ConfigError), + 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), + } + } +}