From 78e96ba1d6e3f3bdd8f9154a8bed2198f1be961b Mon Sep 17 00:00:00 2001 From: ClementTsang <34804052+ClementTsang@users.noreply.github.com> Date: Sat, 1 Jun 2024 03:01:57 -0400 Subject: refactor: sort out some error handling --- src/app.rs | 5 +- src/app/layout_manager.rs | 10 +- src/app/process_killer.rs | 6 +- src/app/query.rs | 42 +++--- src/canvas.rs | 6 +- src/canvas/styling/colour_utils.rs | 52 ++++--- src/data_collection/disks/unix/linux/partition.rs | 7 +- src/data_collection/disks/unix/other/partition.rs | 3 +- src/data_collection/processes.rs | 2 +- src/data_collection/processes/linux.rs | 13 +- src/data_collection/processes/unix/user_table.rs | 6 +- src/data_collection/temperature/linux.rs | 10 +- src/lib.rs | 8 +- src/options.rs | 165 ++++++++++------------ src/options/config/layout.rs | 11 +- src/utils/error.rs | 107 +++++++------- src/utils/error/collection.rs | 46 ++++++ src/utils/error/config.rs | 47 ++++++ src/utils/error/draw.rs | 14 ++ 19 files changed, 326 insertions(+), 234 deletions(-) create mode 100644 src/utils/error/collection.rs create mode 100644 src/utils/error/config.rs create mode 100644 src/utils/error/draw.rs diff --git a/src/app.rs b/src/app.rs index 11cd358b..b08691f5 100644 --- a/src/app.rs +++ b/src/app.rs @@ -1406,9 +1406,8 @@ impl App { self.to_delete_process_list = None; Ok(()) } else { - Err(BottomError::GenericError( - "Cannot kill processes if the current widget is not the Process widget!" - .to_string(), + Err(BottomError::user( + "Cannot kill processes if the current widget is not the Process widget!", )) } } diff --git a/src/app/layout_manager.rs b/src/app/layout_manager.rs index e7904c9a..3e4ec84f 100644 --- a/src/app/layout_manager.rs +++ b/src/app/layout_manager.rs @@ -2,7 +2,7 @@ use std::collections::BTreeMap; use crate::{ constants::DEFAULT_WIDGET_ID, - error::{BottomError, Result}, + utils::error::{ConfigError, ConfigResult}, }; /// Represents a more usable representation of the layout, derived from the @@ -985,9 +985,9 @@ impl BottomWidgetType { } impl std::str::FromStr for BottomWidgetType { - type Err = BottomError; + type Err = ConfigError; - fn from_str(s: &str) -> Result { + fn from_str(s: &str) -> ConfigResult { let lower_case = s.to_lowercase(); match lower_case.as_str() { "cpu" => Ok(BottomWidgetType::Cpu), @@ -1002,7 +1002,7 @@ impl std::str::FromStr for BottomWidgetType { _ => { #[cfg(feature = "battery")] { - Err(BottomError::ConfigError(format!( + Err(ConfigError::other(format!( "\"{s}\" is an invalid widget name. Supported widget names: @@ -1028,7 +1028,7 @@ Supported widget names: } #[cfg(not(feature = "battery"))] { - Err(BottomError::ConfigError(format!( + Err(BottomError::config(format!( "\"{s}\" is an invalid widget name. Supported widget names: diff --git a/src/app/process_killer.rs b/src/app/process_killer.rs index c68b81ab..df846a33 100644 --- a/src/app/process_killer.rs +++ b/src/app/process_killer.rs @@ -74,11 +74,9 @@ pub fn kill_process_given_pid(pid: Pid, signal: usize) -> crate::utils::error::R }; return if let Some(err_code) = err_code { - Err(BottomError::GenericError(format!( - "Error code {err_code} - {err}" - ))) + Err(BottomError::user(format!("Error code {err_code} - {err}"))) } else { - Err(BottomError::GenericError(format!("Error code ??? - {err}"))) + Err(BottomError::user(format!("Error code ??? - {err}"))) }; } diff --git a/src/app/query.rs b/src/app/query.rs index 5737cbc7..81e6efcf 100644 --- a/src/app/query.rs +++ b/src/app/query.rs @@ -1,5 +1,4 @@ use std::{ - borrow::Cow, collections::VecDeque, fmt::{Debug, Formatter}, time::Duration, @@ -13,10 +12,7 @@ use crate::{ multi_eq_ignore_ascii_case, utils::{ data_prefixes::*, - error::{ - BottomError::{self, QueryError}, - Result, - }, + error::{BottomError, Result}, }, }; @@ -86,7 +82,7 @@ pub fn parse_query( break; } } else if COMPARISON_LIST.contains(¤t_lowercase.as_str()) { - return Err(QueryError(Cow::Borrowed("Comparison not valid here"))); + return Err(BottomError::user("Comparison not valid here")); } else { break; } @@ -125,7 +121,7 @@ pub fn parse_query( break; } } else if COMPARISON_LIST.contains(¤t_lowercase.as_str()) { - return Err(QueryError(Cow::Borrowed("Comparison not valid here"))); + return Err(BottomError::user("Comparison not valid here")); } else { break; } @@ -206,7 +202,7 @@ pub fn parse_query( } } else if queue_top == "(" { if query.is_empty() { - return Err(QueryError(Cow::Borrowed("Missing closing parentheses"))); + return Err(BottomError::user("Missing closing parentheses")); } let mut list_of_ors = VecDeque::new(); @@ -221,7 +217,7 @@ pub fn parse_query( // Ensure not empty if list_of_ors.is_empty() { - return Err(QueryError("No values within parentheses group".into())); + return Err(BottomError::user("No values within parentheses group")); } // Now convert this back to a OR... @@ -260,13 +256,13 @@ pub fn parse_query( compare_prefix: None, }); } else { - return Err(QueryError("Missing closing parentheses".into())); + return Err(BottomError::user("Missing closing parentheses")); } } else { - return Err(QueryError("Missing closing parentheses".into())); + return Err(BottomError::user("Missing closing parentheses")); } } else if queue_top == ")" { - return Err(QueryError("Missing opening parentheses".into())); + return Err(BottomError::user("Missing opening parentheses")); } else if queue_top == "\"" { // Similar to parentheses, trap and check for missing closing quotes. Note, however, that we // will DIRECTLY call another process_prefix call... @@ -276,10 +272,10 @@ pub fn parse_query( if close_paren == "\"" { return Ok(prefix); } else { - return Err(QueryError("Missing closing quotation".into())); + return Err(BottomError::user("Missing closing quotation")); } } else { - return Err(QueryError("Missing closing quotation".into())); + return Err(BottomError::user("Missing closing quotation")); } } else { // Get prefix type... @@ -355,15 +351,15 @@ pub fn parse_query( duration_string = Some(queue_next); } } else { - return Err(QueryError("Missing value".into())); + return Err(BottomError::user("Missing value")); } } if let Some(condition) = condition { let duration = parse_duration( - &duration_string.ok_or(QueryError("Missing value".into()))?, + &duration_string.ok_or(BottomError::user("Missing value"))?, ) - .map_err(|err| QueryError(err.to_string().into()))?; + .map_err(|err| BottomError::user(err.to_string()))?; return Ok(Prefix { or: None, @@ -392,7 +388,7 @@ pub fn parse_query( if let Some(queue_next) = query.pop_front() { value = queue_next.parse::().ok(); } else { - return Err(QueryError("Missing value".into())); + return Err(BottomError::user("Missing value")); } } else if content == ">" || content == "<" { // We also have to check if the next string is an "="... @@ -406,7 +402,7 @@ pub fn parse_query( if let Some(queue_next_next) = query.pop_front() { value = queue_next_next.parse::().ok(); } else { - return Err(QueryError("Missing value".into())); + return Err(BottomError::user("Missing value")); } } else { condition = Some(if content == ">" { @@ -417,7 +413,7 @@ pub fn parse_query( value = queue_next.parse::().ok(); } } else { - return Err(QueryError("Missing value".into())); + return Err(BottomError::user("Missing value")); } } @@ -459,15 +455,15 @@ pub fn parse_query( } } } else { - return Err(QueryError("Missing argument for search prefix".into())); + return Err(BottomError::user("Missing argument for search prefix")); } } } else if inside_quotation { // Uh oh, it's empty with quotes! - return Err(QueryError("Missing closing quotation".into())); + return Err(BottomError::user("Missing closing quotation")); } - Err(QueryError("Invalid query".into())) + Err(BottomError::user("Invalid query")) } let mut split_query = VecDeque::new(); diff --git a/src/canvas.rs b/src/canvas.rs index ca144c15..91529075 100644 --- a/src/canvas.rs +++ b/src/canvas.rs @@ -48,7 +48,7 @@ impl FromStr for ColourScheme { "gruvbox-light" => Ok(ColourScheme::GruvboxLight), "nord" => Ok(ColourScheme::Nord), "nord-light" => Ok(ColourScheme::NordLight), - _ => Err(BottomError::ConfigError(format!( + _ => Err(BottomError::config(format!( "`{s}` is an invalid built-in color scheme." ))), } @@ -81,7 +81,7 @@ pub enum LayoutConstraint { } impl Painter { - pub fn init(layout: BottomLayout, styling: CanvasStyling) -> anyhow::Result { + pub fn init(layout: BottomLayout, styling: CanvasStyling) -> error::Result { // Now for modularity; we have to also initialize the base layouts! // We want to do this ONCE and reuse; after this we can just construct // based on the console size. @@ -204,7 +204,7 @@ impl Painter { pub fn draw_data( &mut self, terminal: &mut Terminal, app_state: &mut App, - ) -> error::Result<()> { + ) -> error::DrawResult<()> { use BottomWidgetType::*; terminal.draw(|f| { diff --git a/src/canvas/styling/colour_utils.rs b/src/canvas/styling/colour_utils.rs index 1c1def3e..acaee373 100644 --- a/src/canvas/styling/colour_utils.rs +++ b/src/canvas/styling/colour_utils.rs @@ -3,7 +3,7 @@ use itertools::Itertools; use tui::style::{Color, Style}; use unicode_segmentation::UnicodeSegmentation; -use crate::utils::error; +use crate::utils::error::{self, BottomError}; pub const FIRST_COLOUR: Color = Color::LightMagenta; pub const SECOND_COLOUR: Color = Color::LightYellow; @@ -19,14 +19,14 @@ pub const ALL_COLOUR: Color = Color::Green; fn convert_hex_to_color(hex: &str) -> error::Result { fn hex_component_to_int(hex: &str, first: &str, second: &str) -> error::Result { u8::from_str_radix(&concat_string!(first, second), 16).map_err(|_| { - error::BottomError::ConfigError(format!( + BottomError::config(format!( "\"{hex}\" is an invalid hex color, could not decode." )) }) } - fn invalid_hex_format(hex: &str) -> error::BottomError { - error::BottomError::ConfigError(format!( + fn invalid_hex_format(hex: &str) -> BottomError { + BottomError::config(format!( "\"{hex}\" is an invalid hex color. It must be either a 7 character hex string of the form \"#12ab3c\" or a 3 character hex string of the form \"#1a2\".", )) } @@ -69,7 +69,7 @@ pub fn str_to_colour(input_val: &str) -> error::Result { convert_name_to_colour(input_val) } } else { - Err(error::BottomError::ConfigError(format!( + Err(BottomError::config(format!( "value \"{input_val}\" is not valid.", ))) } @@ -78,7 +78,7 @@ pub fn str_to_colour(input_val: &str) -> error::Result { fn convert_rgb_to_color(rgb_str: &str) -> error::Result { let rgb_list = rgb_str.split(',').collect::>(); if rgb_list.len() != 3 { - return Err(error::BottomError::ConfigError(format!( + return Err(BottomError::config(format!( "value \"{rgb_str}\" is an invalid RGB colour. It must be a comma separated value with 3 integers from 0 to 255 (ie: \"255, 0, 155\").", ))); } @@ -96,7 +96,7 @@ fn convert_rgb_to_color(rgb_str: &str) -> error::Result { if rgb.len() == 3 { Ok(Color::Rgb(rgb[0], rgb[1], rgb[2])) } else { - Err(error::BottomError::ConfigError(format!( + Err(BottomError::config(format!( "value \"{rgb_str}\" contained invalid RGB values. It must be a comma separated value with 3 integers from 0 to 255 (ie: \"255, 0, 155\").", ))) } @@ -121,7 +121,7 @@ fn convert_name_to_colour(color_name: &str) -> error::Result { "lightmagenta" | "light magenta" => Ok(Color::LightMagenta), "lightcyan" | "light cyan" => Ok(Color::LightCyan), "white" => Ok(Color::White), - _ => Err(error::BottomError::ConfigError(format!( + _ => Err(BottomError::config(format!( "\"{color_name}\" is an invalid named color. The following are supported strings: @@ -177,35 +177,41 @@ mod test { #[test] fn valid_colour_names() { // Standard color should work - assert_eq!(convert_name_to_colour("red"), Ok(Color::Red)); + assert_eq!(convert_name_to_colour("red").unwrap(), Color::Red); // Capitalizing should be fine. - assert_eq!(convert_name_to_colour("RED"), Ok(Color::Red)); + assert_eq!(convert_name_to_colour("RED").unwrap(), Color::Red); // Spacing shouldn't be an issue now. - assert_eq!(convert_name_to_colour(" red "), Ok(Color::Red)); + assert_eq!(convert_name_to_colour(" red ").unwrap(), Color::Red); // The following are all equivalent. - assert_eq!(convert_name_to_colour("darkgray"), Ok(Color::DarkGray)); - assert_eq!(convert_name_to_colour("darkgrey"), Ok(Color::DarkGray)); - assert_eq!(convert_name_to_colour("dark grey"), Ok(Color::DarkGray)); - assert_eq!(convert_name_to_colour("dark gray"), Ok(Color::DarkGray)); + assert_eq!(convert_name_to_colour("darkgray").unwrap(), Color::DarkGray); + assert_eq!(convert_name_to_colour("darkgrey").unwrap(), Color::DarkGray); + assert_eq!( + convert_name_to_colour("dark grey").unwrap(), + Color::DarkGray + ); + assert_eq!( + convert_name_to_colour("dark gray").unwrap(), + Color::DarkGray + ); - assert_eq!(convert_name_to_colour("grey"), Ok(Color::Gray)); - assert_eq!(convert_name_to_colour("gray"), Ok(Color::Gray)); + assert_eq!(convert_name_to_colour("grey").unwrap(), Color::Gray); + assert_eq!(convert_name_to_colour("gray").unwrap(), Color::Gray); // One more test with spacing. assert_eq!( - convert_name_to_colour(" lightmagenta "), - Ok(Color::LightMagenta) + convert_name_to_colour(" lightmagenta ").unwrap(), + Color::LightMagenta ); assert_eq!( - convert_name_to_colour("light magenta"), - Ok(Color::LightMagenta) + convert_name_to_colour("light magenta").unwrap(), + Color::LightMagenta ); assert_eq!( - convert_name_to_colour(" light magenta "), - Ok(Color::LightMagenta) + convert_name_to_colour(" light magenta ").unwrap(), + Color::LightMagenta ); } diff --git a/src/data_collection/disks/unix/linux/partition.rs b/src/data_collection/disks/unix/linux/partition.rs index 488ad8d9..1d8a0008 100644 --- a/src/data_collection/disks/unix/linux/partition.rs +++ b/src/data_collection/disks/unix/linux/partition.rs @@ -12,7 +12,10 @@ use std::{ use anyhow::bail; -use crate::data_collection::disks::unix::{FileSystem, Usage}; +use crate::{ + data_collection::disks::unix::{FileSystem, Usage}, + utils::error, +}; /// Representation of partition details. Based on [`heim`](https://github.com/heim-rs/heim/tree/master). pub(crate) struct Partition { @@ -164,7 +167,7 @@ pub(crate) fn partitions() -> anyhow::Result> { /// Returns a [`Vec`] containing all *physical* partitions. This is defined by /// [`FileSystem::is_physical()`]. -pub(crate) fn physical_partitions() -> anyhow::Result> { +pub(crate) fn physical_partitions() -> error::CollectionResult> { const PROC_MOUNTS: &str = "/proc/mounts"; let mut results = vec![]; diff --git a/src/data_collection/disks/unix/other/partition.rs b/src/data_collection/disks/unix/other/partition.rs index 1e64a70d..78420717 100644 --- a/src/data_collection/disks/unix/other/partition.rs +++ b/src/data_collection/disks/unix/other/partition.rs @@ -2,6 +2,7 @@ use std::{ ffi::{CStr, CString}, os::unix::prelude::OsStrExt, path::{Path, PathBuf}, + std::mem::MaybeUninit, str::FromStr, }; @@ -32,7 +33,7 @@ impl Partition { /// Returns the usage stats for this partition. pub fn usage(&self) -> anyhow::Result { let path = CString::new(self.mount_point().as_os_str().as_bytes())?; - let mut vfs = std::mem::MaybeUninit::::uninit(); + let mut vfs = MaybeUninit::::uninit(); // SAFETY: System API call. Arguments should be correct. let result = unsafe { libc::statvfs(path.as_ptr(), vfs.as_mut_ptr()) }; diff --git a/src/data_collection/processes.rs b/src/data_collection/processes.rs index ef96c818..80f84ed1 100644 --- a/src/data_collection/processes.rs +++ b/src/data_collection/processes.rs @@ -120,7 +120,7 @@ impl ProcessHarvest { } impl DataCollector { - pub(crate) fn get_processes(&mut self) -> error::Result> { + pub(crate) fn get_processes(&mut self) -> error::CollectionResult> { cfg_if! { if #[cfg(target_os = "linux")] { let time_diff = self.data.collection_time diff --git a/src/data_collection/processes/linux.rs b/src/data_collection/processes/linux.rs index 5d99fbbc..4065b076 100644 --- a/src/data_collection/processes/linux.rs +++ b/src/data_collection/processes/linux.rs @@ -15,7 +15,7 @@ use sysinfo::ProcessStatus; use super::{ProcessHarvest, UserTable}; use crate::{ data_collection::DataCollector, - utils::error::{self, BottomError}, + utils::error::{self, CollectionError}, Pid, }; @@ -65,7 +65,9 @@ struct CpuUsage { cpu_fraction: f64, } -fn cpu_usage_calculation(prev_idle: &mut f64, prev_non_idle: &mut f64) -> error::Result { +fn cpu_usage_calculation( + prev_idle: &mut f64, prev_non_idle: &mut f64, +) -> error::CollectionResult { let (idle, non_idle) = { // From SO answer: https://stackoverflow.com/a/23376195 let first_line = { @@ -299,7 +301,7 @@ pub(crate) struct ReadProcArgs { pub(crate) fn linux_process_data( collector: &mut DataCollector, time_difference_in_secs: u64, -) -> error::Result> { +) -> error::CollectionResult> { let total_memory = collector.total_memory(); let prev_proc = PrevProc { prev_idle: &mut collector.prev_idle, @@ -402,8 +404,9 @@ pub(crate) fn linux_process_data( Ok(process_vector) } else { - Err(BottomError::GenericError( - "Could not calculate CPU usage.".to_string(), + Err(CollectionError::other( + "Process", + "Could not calculate CPU usage.", )) } } diff --git a/src/data_collection/processes/unix/user_table.rs b/src/data_collection/processes/unix/user_table.rs index 6245a858..5d5b2f04 100644 --- a/src/data_collection/processes/unix/user_table.rs +++ b/src/data_collection/processes/unix/user_table.rs @@ -1,6 +1,6 @@ use hashbrown::HashMap; -use crate::utils::error; +use crate::utils::error::{CollectionError, CollectionResult}; #[derive(Debug, Default)] pub struct UserTable { @@ -8,7 +8,7 @@ pub struct UserTable { } impl UserTable { - pub fn get_uid_to_username_mapping(&mut self, uid: libc::uid_t) -> error::Result { + pub fn get_uid_to_username_mapping(&mut self, uid: libc::uid_t) -> CollectionResult { if let Some(user) = self.uid_user_mapping.get(&uid) { Ok(user.clone()) } else { @@ -16,7 +16,7 @@ impl UserTable { let passwd = unsafe { libc::getpwuid(uid) }; if passwd.is_null() { - Err(error::BottomError::QueryError("Missing passwd".into())) + Err(CollectionError::other("processes", "Missing passwd")) } else { // SAFETY: We return early if passwd is null. let username = unsafe { std::ffi::CStr::from_ptr((*passwd).pw_name) } diff --git a/src/data_collection/temperature/linux.rs b/src/data_collection/temperature/linux.rs index 50b858d1..1c1fb54e 100644 --- a/src/data_collection/temperature/linux.rs +++ b/src/data_collection/temperature/linux.rs @@ -9,7 +9,7 @@ use anyhow::Result; use hashbrown::{HashMap, HashSet}; use super::{is_temp_filtered, TempHarvest, TemperatureType}; -use crate::{app::filter::Filter, utils::error::BottomError}; +use crate::{app::filter::Filter, utils::error::CollectionResult}; const EMPTY_NAME: &str = "Unknown"; @@ -20,12 +20,8 @@ struct HwmonResults { } /// Parses and reads temperatures that were in millidegree Celsius, and if successful, returns a temperature in Celsius. -fn parse_temp(path: &Path) -> Result { - Ok(fs::read_to_string(path)? - .trim_end() - .parse::() - .map_err(|e| BottomError::ConversionError(e.to_string()))? - / 1_000.0) +fn parse_temp(path: &Path) -> CollectionResult { + Ok(fs::read_to_string(path)?.trim_end().parse::()? / 1_000.0) } /// Get all candidates from hwmon and coretemp. It will also return the number of entries from hwmon. diff --git a/src/lib.rs b/src/lib.rs index f80317e1..948aaeae 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -62,7 +62,7 @@ use crossterm::{ use data_conversion::*; pub use options::args; use options::ConfigV1; -use utils::error; +use utils::error::{self, ConfigResult}; #[allow(unused_imports)] pub use utils::logging::*; @@ -235,7 +235,7 @@ pub fn get_config_path(override_config_path: Option<&Path>) -> Option { } } -pub fn get_or_create_config(override_config_path: Option<&Path>) -> error::Result { +pub fn get_or_create_config(override_config_path: Option<&Path>) -> ConfigResult { let config_path = get_config_path(override_config_path); if let Some(path) = &config_path { @@ -258,7 +258,7 @@ pub fn get_or_create_config(override_config_path: Option<&Path>) -> error::Resul pub fn try_drawing( terminal: &mut tui::terminal::Terminal>, app: &mut App, painter: &mut canvas::Painter, -) -> error::Result<()> { +) -> error::DrawResult<()> { if let Err(err) = painter.draw_data(terminal, app) { cleanup_terminal(terminal)?; Err(err) @@ -269,7 +269,7 @@ pub fn try_drawing( pub fn cleanup_terminal( terminal: &mut tui::terminal::Terminal>, -) -> error::Result<()> { +) -> error::DrawResult<()> { disable_raw_mode()?; execute!( terminal.backend_mut(), diff --git a/src/options.rs b/src/options.rs index c98618b2..54cb9f12 100644 --- a/src/options.rs +++ b/src/options.rs @@ -32,7 +32,7 @@ use crate::{ data_collection::temperature::TemperatureType, utils::{ data_units::DataUnit, - error::{self, BottomError}, + error::{self, BottomError, ConfigError, ConfigResult}, }, widgets::*, }; @@ -369,7 +369,7 @@ pub fn init_app( pub fn get_widget_layout( args: &BottomArgs, config: &ConfigV1, -) -> error::Result<(BottomLayout, u64, Option)> { +) -> ConfigResult<(BottomLayout, u64, Option)> { let cpu_left_legend = is_flag_enabled!(cpu_left_legend, args.cpu, config); let (default_widget_type, mut default_widget_count) = @@ -413,7 +413,7 @@ pub fn get_widget_layout( cpu_left_legend, ) }) - .collect::>>()?, + .collect::>>()?, total_row_height_ratio: total_height_ratio, }; @@ -422,8 +422,8 @@ pub fn get_widget_layout( ret_bottom_layout.get_movement_mappings(); ret_bottom_layout } else { - return Err(BottomError::ConfigError( - "please have at least one widget under the '[[row]]' section.".to_string(), + return Err(ConfigError::other( + "please have at least one widget under the '[[row]]' section.", )); } }; @@ -433,15 +433,13 @@ pub fn get_widget_layout( fn get_update_rate(args: &BottomArgs, config: &ConfigV1) -> error::Result { let update_rate = if let Some(update_rate) = &args.general.rate { - try_parse_ms(update_rate).map_err(|_| { - BottomError::ArgumentError("set your update rate to be valid".to_string()) - })? + try_parse_ms(update_rate) + .map_err(|_| BottomError::config("set your update rate to be valid"))? } else if let Some(flags) = &config.flags { if let Some(rate) = &flags.rate { match rate { - StringOrNum::String(s) => try_parse_ms(s).map_err(|_| { - BottomError::ConfigError("set your update rate to be valid".to_string()) - })?, + StringOrNum::String(s) => try_parse_ms(s) + .map_err(|_| BottomError::config("set your update rate to be valid"))?, StringOrNum::Num(n) => *n, } } else { @@ -452,8 +450,8 @@ fn get_update_rate(args: &BottomArgs, config: &ConfigV1) -> error::Result { }; if update_rate < 250 { - return Err(BottomError::ConfigError( - "set your update rate to be at least 250 ms.".to_string(), + return Err(BottomError::config( + "set your update rate to be at least 250 ms.", )); } @@ -469,7 +467,7 @@ fn get_temperature(args: &BottomArgs, config: &ConfigV1) -> error::Result error::Result { } else if let Ok(val) = s.parse::() { Ok(val) } else { - Err(BottomError::ConfigError( - "could not parse as a valid 64-bit unsigned integer or a human time".to_string(), + Err(BottomError::config( + "could not parse as a valid 64-bit unsigned integer or a human time", )) } } @@ -504,15 +502,13 @@ fn get_default_time_value( args: &BottomArgs, config: &ConfigV1, retention_ms: u64, ) -> error::Result { let default_time = if let Some(default_time_value) = &args.general.default_time_value { - try_parse_ms(default_time_value).map_err(|_| { - BottomError::ArgumentError("set your default time to be valid".to_string()) - })? + try_parse_ms(default_time_value) + .map_err(|_| BottomError::config("set your default time to be valid"))? } else if let Some(flags) = &config.flags { if let Some(default_time_value) = &flags.default_time_value { match default_time_value { - StringOrNum::String(s) => try_parse_ms(s).map_err(|_| { - BottomError::ConfigError("set your default time to be valid".to_string()) - })?, + StringOrNum::String(s) => try_parse_ms(s) + .map_err(|_| BottomError::config("set your default time to be valid"))?, StringOrNum::Num(n) => *n, } } else { @@ -523,11 +519,11 @@ fn get_default_time_value( }; if default_time < 30000 { - return Err(BottomError::ConfigError( - "set your default time to be at least 30s.".to_string(), + return Err(BottomError::config( + "set your default time to be at least 30s.", )); } else if default_time > retention_ms { - return Err(BottomError::ConfigError(format!( + return Err(BottomError::config(format!( "set your default time to be at most {}.", humantime::Duration::from(Duration::from_millis(retention_ms)) ))); @@ -540,15 +536,13 @@ fn get_time_interval( args: &BottomArgs, config: &ConfigV1, retention_ms: u64, ) -> error::Result { let time_interval = if let Some(time_interval) = &args.general.time_delta { - try_parse_ms(time_interval).map_err(|_| { - BottomError::ArgumentError("set your time delta to be valid".to_string()) - })? + try_parse_ms(time_interval) + .map_err(|_| BottomError::config("set your time delta to be valid"))? } else if let Some(flags) = &config.flags { if let Some(time_interval) = &flags.time_delta { match time_interval { - StringOrNum::String(s) => try_parse_ms(s).map_err(|_| { - BottomError::ArgumentError("set your time delta to be valid".to_string()) - })?, + StringOrNum::String(s) => try_parse_ms(s) + .map_err(|_| BottomError::config("set your time delta to be valid"))?, StringOrNum::Num(n) => *n, } } else { @@ -559,22 +553,22 @@ fn get_time_interval( }; if time_interval < 1000 { - return Err(BottomError::ConfigError( - "set your time delta to be at least 1s.".to_string(), - )); + Err(BottomError::config( + "set your time delta to be at least 1s.", + )) } else if time_interval > retention_ms { - return Err(BottomError::ConfigError(format!( + Err(BottomError::config(format!( "set your time delta to be at most {}.", humantime::Duration::from(Duration::from_millis(retention_ms)) - ))); + ))) + } else { + Ok(time_interval) } - - Ok(time_interval) } fn get_default_widget_and_count( args: &BottomArgs, config: &ConfigV1, -) -> error::Result<(Option, u64)> { +) -> ConfigResult<(Option, u64)> { let widget_type = if let Some(widget_type) = &args.general.default_widget_type { let parsed_widget = widget_type.parse::()?; if let BottomWidgetType::Empty = parsed_widget { @@ -609,14 +603,14 @@ fn get_default_widget_and_count( match (widget_type, widget_count) { (Some(widget_type), Some(widget_count)) => { - let widget_count = widget_count.try_into().map_err(|_| BottomError::ConfigError( - "set your widget count to be at most 18446744073709551615.".to_string() + let widget_count = widget_count.try_into().map_err(|_| ConfigError::other( + "set your widget count to be at most 18446744073709551615." ))?; Ok((Some(widget_type), widget_count)) } (Some(widget_type), None) => Ok((Some(widget_type), 1)), - (None, Some(_widget_count)) => Err(BottomError::ConfigError( - "cannot set 'default_widget_count' by itself, it must be used with 'default_widget_type'.".to_string(), + (None, Some(_widget_count)) => Err(ConfigError::other( + "cannot set 'default_widget_count' by itself, it must be used with 'default_widget_type'.", )), (None, None) => Ok((None, 1)) } @@ -773,14 +767,12 @@ fn get_retention(args: &BottomArgs, config: &ConfigV1) -> error::Result { const DEFAULT_RETENTION_MS: u64 = 600 * 1000; // Keep 10 minutes of data. if let Some(retention) = &args.general.retention { - try_parse_ms(retention) - .map_err(|_| BottomError::ArgumentError("`retention` is an invalid value".to_string())) + try_parse_ms(retention).map_err(|_| BottomError::config("`retention` is an invalid value")) } else if let Some(flags) = &config.flags { if let Some(retention) = &flags.retention { Ok(match retention { - StringOrNum::String(s) => try_parse_ms(s).map_err(|_| { - BottomError::ConfigError("`retention` is an invalid value".to_string()) - })?, + StringOrNum::String(s) => try_parse_ms(s) + .map_err(|_| BottomError::config("`retention` is an invalid value"))?, StringOrNum::Num(n) => *n, }) } else { @@ -798,13 +790,13 @@ fn get_network_legend_position( match s.to_ascii_lowercase().trim() { "none" => Ok(None), position => Ok(Some(position.parse::().map_err(|_| { - BottomError::ArgumentError("`network_legend` is an invalid value".to_string()) + BottomError::config("`network_legend` is an invalid value") })?)), } } else if let Some(flags) = &config.flags { if let Some(legend) = &flags.network_legend { Ok(Some(legend.parse::().map_err(|_| { - BottomError::ConfigError("`network_legend` is an invalid value".to_string()) + BottomError::config("`network_legend` is an invalid value") })?)) } else { Ok(Some(LegendPosition::default())) @@ -821,13 +813,13 @@ fn get_memory_legend_position( match s.to_ascii_lowercase().trim() { "none" => Ok(None), position => Ok(Some(position.parse::().map_err(|_| { - BottomError::ArgumentError("`memory_legend` is an invalid value".to_string()) + BottomError::config("`memory_legend` is an invalid value") })?)), } } else if let Some(flags) = &config.flags { if let Some(legend) = &flags.memory_legend { Ok(Some(legend.parse::().map_err(|_| { - BottomError::ConfigError("`memory_legend` is an invalid value".to_string()) + BottomError::config("`memory_legend` is an invalid value") })?)) } else { Ok(Some(LegendPosition::default())) @@ -859,10 +851,10 @@ mod test { let c = "1 min"; let d = "1 hour 1 min"; - assert_eq!(try_parse_ms(a), Ok(100 * 1000)); - assert_eq!(try_parse_ms(b), Ok(100)); - assert_eq!(try_parse_ms(c), Ok(60 * 1000)); - assert_eq!(try_parse_ms(d), Ok(3660 * 1000)); + assert_eq!(try_parse_ms(a).unwrap(), 100 * 1000); + assert_eq!(try_parse_ms(b).unwrap(), 100); + assert_eq!(try_parse_ms(c).unwrap(), 60 * 1000); + assert_eq!(try_parse_ms(d).unwrap(), 3660 * 1000); let a_bad = "1 test"; let b_bad = "-100"; @@ -880,8 +872,8 @@ mod test { let args = BottomArgs::parse_from(delta_args); assert_eq!( - get_time_interval(&args, &config, 60 * 60 * 1000), - Ok(2 * 60 * 1000) + get_time_interval(&args, &config, 60 * 60 * 1000).unwrap(), + 2 * 60 * 1000 ); } @@ -890,8 +882,8 @@ mod test { let args = BottomArgs::parse_from(default_time_args); assert_eq!( - get_default_time_value(&args, &config, 60 * 60 * 1000), - Ok(5 * 60 * 1000) + get_default_time_value(&args, &config, 60 * 60 * 1000).unwrap(), + 5 * 60 * 1000 ); } } @@ -905,8 +897,8 @@ mod test { let args = BottomArgs::parse_from(delta_args); assert_eq!( - get_time_interval(&args, &config, 60 * 60 * 1000), - Ok(2 * 60 * 1000) + get_time_interval(&args, &config, 60 * 60 * 1000).unwrap(), + 2 * 60 * 1000 ); } @@ -915,8 +907,8 @@ mod test { let args = BottomArgs::parse_from(default_time_args); assert_eq!( - get_default_time_value(&args, &config, 60 * 60 * 1000), - Ok(5 * 60 * 1000) + get_default_time_value(&args, &config, 60 * 60 * 1000).unwrap(), + 5 * 60 * 1000 ); } } @@ -937,18 +929,15 @@ mod test { config.flags = Some(flags); assert_eq!( - get_time_interval(&args, &config, 60 * 60 * 1000), - Ok(2 * 60 * 1000) + get_time_interval(&args, &config, 60 * 60 * 1000).unwrap(), + 2 * 60 * 1000 ); - assert_eq!( - get_default_time_value(&args, &config, 60 * 60 * 1000), - Ok(5 * 60 * 1000) + get_default_time_value(&args, &config, 60 * 60 * 1000).unwrap(), + 5 * 60 * 1000 ); - - assert_eq!(get_update_rate(&args, &config), Ok(1000)); - - assert_eq!(get_retention(&args, &config), Ok(600000)); + assert_eq!(get_update_rate(&args, &config).unwrap(), 1000); + assert_eq!(get_retention(&args, &config).unwrap(), 600000); } #[test] @@ -967,18 +956,15 @@ mod test { config.flags = Some(flags); assert_eq!( - get_time_interval(&args, &config, 60 * 60 * 1000), - Ok(2 * 60 * 1000) + get_time_interval(&args, &config, 60 * 60 * 1000).unwrap(), + 2 * 60 * 1000 ); - assert_eq!( - get_default_time_value(&args, &config, 60 * 60 * 1000), - Ok(5 * 60 * 1000) + get_default_time_value(&args, &config, 60 * 60 * 1000).unwrap(), + 5 * 60 * 1000 ); - - assert_eq!(get_update_rate(&args, &config), Ok(1000)); - - assert_eq!(get_retention(&args, &config), Ok(600000)); + assert_eq!(get_update_rate(&args, &config).unwrap(), 1000); + assert_eq!(get_retention(&args, &config).unwrap(), 600000); } #[test] @@ -997,18 +983,15 @@ mod test { config.flags = Some(flags); assert_eq!( - get_time_interval(&args, &config, 60 * 60 * 1000), - Ok(2 * 60 * 1000) + get_time_interval(&args, &config, 60 * 60 * 1000).unwrap(), + 2 * 60 * 1000 ); - assert_eq!( - get_default_time_value(&args, &config, 60 * 60 * 1000), - Ok(5 * 60 * 1000) + get_default_time_value(&args, &config, 60 * 60 * 1000).unwrap(), + 5 * 60 * 1000 ); - - assert_eq!(get_update_rate(&args, &config), Ok(1000)); - - assert_eq!(get_retention(&args, &config), Ok(600000)); + assert_eq!(get_update_rate(&args, &config).unwrap(), 1000); + assert_eq!(get_retention(&args, &config).unwrap(), 600000); } fn create_app(args: BottomArgs) -> App { diff --git a/src/options/config/layout.rs b/src/options/config/layout.rs index b88448fa..682e7f78 100644 --- a/src/options/config/layout.rs +++ b/src/options/config/layout.rs @@ -1,6 +1,6 @@ use serde::{Deserialize, Serialize}; -use crate::{app::layout_manager::*, error::Result}; +use crate::{app::layout_manager::*, utils::error::ConfigResult}; /// Represents a row. This has a length of some sort (optional) and a vector /// of children. @@ -54,7 +54,7 @@ impl Row { &self, iter_id: &mut u64, total_height_ratio: &mut u32, default_widget_id: &mut u64, default_widget_type: &Option, default_widget_count: &mut u64, cpu_left_legend: bool, - ) -> Result { + ) -> ConfigResult { // TODO: In the future we want to also add percentages. // But for MVP, we aren't going to bother. let row_ratio = self.ratio.unwrap_or(1); @@ -240,7 +240,6 @@ mod test { use crate::{ constants::{DEFAULT_LAYOUT, DEFAULT_WIDGET_ID}, options::ConfigV1, - utils::error, }; const PROC_LAYOUT: &str = r#" @@ -281,7 +280,7 @@ mod test { left_legend, ) }) - .collect::>>() + .collect::>>() .unwrap(), total_row_height_ratio: total_height_ratio, }; @@ -494,7 +493,7 @@ mod test { cpu_left_legend, ) }) - .collect::>>() + .collect::>>() .unwrap(), total_row_height_ratio: total_height_ratio, }; @@ -527,7 +526,7 @@ mod test { cpu_left_legend, ) }) - .collect::>>() + .collect::>>() .unwrap(), total_row_height_ratio: total_height_ratio, }; diff --git a/src/utils/error.rs b/src/utils/error.rs index 532ad283..4330a272 100644 --- a/src/utils/error.rs +++ b/src/utils/error.rs @@ -1,91 +1,92 @@ -use std::{borrow::Cow, result}; +mod collection; +mod config; +mod draw; +use std::borrow::Cow; + +pub use collection::*; +pub use config::*; +pub use draw::*; use thiserror::Error; -/// A type alias for handling errors related to Bottom. -pub type Result = result::Result; +/// A type alias for handling errors related to `bottom`. +pub type Result = std::result::Result; -/// An error that can occur while Bottom runs. -#[derive(Debug, Error, PartialEq, Eq)] +/// An error that can occur while `bottom` runs. +#[derive(Debug, Error)] pub enum BottomError { - /// An error when there is an IO exception. - #[error("IO exception, {0}")] - InvalidIo(String), - /// An error when the Crossterm library encounters a problem. - #[error("Error caused by Crossterm, {0}")] - CrosstermError(String), - /// An error to represent generic errors. - #[error("Error, {0}")] - GenericError(String), - #[cfg(feature = "fern")] - /// An error to represent errors with fern. - #[error("Fern error, {0}")] - FernError(String), - /// An error to represent invalid command-line arguments. - #[error("Invalid argument, {0}")] - ArgumentError(String), - /// An error to represent errors with the config. - #[error("Configuration file error, {0}")] - ConfigError(String), + /// An error regarding data collection. + #[error(transparent)] + Collection(CollectionError), + /// An error regarding drawing. + #[error(transparent)] + Draw(DrawError), + /// An error to represent errors with the configuration. + #[error(transparent)] + Config(ConfigError), /// An error to represent errors with converting between data types. #[error("Conversion error, {0}")] - ConversionError(String), - /// An error to represent errors with a query. - #[error("Query error, {0}")] - QueryError(Cow<'static, str>), - #[error("Error casting integers {0}")] - TryFromIntError(#[from] std::num::TryFromIntError), + Conversion(Cow<'static, str>), + #[error("{0}")] + /// An error to report back to the user. + User(Cow<'static, str>), } -impl From for BottomError { - fn from(err: std::io::Error) -> Self { - BottomError::InvalidIo(err.to_string()) +impl BottomError { + /// An error related to configuration. + pub fn config>>(msg: C) -> Self { + Self::Config(ConfigError::other(msg)) } -} -impl From for BottomError { - fn from(err: std::num::ParseIntError) -> Self { - BottomError::ConfigError(err.to_string()) + /// A user error. + pub fn user>>(msg: C) -> Self { + Self::User(msg.into()) } -} -impl From for BottomError { - fn from(err: String) -> Self { - BottomError::GenericError(err) + /// A error that arises from data harvesting. + pub fn data_harvest>, D: Into>>( + source: C, reason: D, + ) -> Self { + Self::Collection(CollectionError::other(source.into(), reason.into())) } } -impl From for BottomError { - fn from(err: toml_edit::de::Error) -> Self { - BottomError::ConfigError(err.to_string()) +impl From for BottomError { + fn from(err: CollectionError) -> Self { + BottomError::Collection(err) } } -#[cfg(feature = "fern")] -impl From for BottomError { - fn from(err: fern::InitError) -> Self { - BottomError::FernError(err.to_string()) +impl From for BottomError { + fn from(err: DrawError) -> Self { + BottomError::Draw(err) } } impl From for BottomError { fn from(err: std::str::Utf8Error) -> Self { - BottomError::ConversionError(err.to_string()) + BottomError::Conversion(err.to_string().into()) } } impl From for BottomError { fn from(err: std::string::FromUtf8Error) -> Self { - BottomError::ConversionError(err.to_string()) + BottomError::Conversion(err.to_string().into()) + } +} + +impl From for BottomError { + fn from(err: std::num::TryFromIntError) -> Self { + BottomError::Conversion(err.to_string().into()) } } impl From for BottomError { fn from(err: regex::Error) -> Self { - // We only really want the last part of it... so we'll do it the ugly way: + // We only really want the last part of it. let err_str = err.to_string(); - let error = err_str.split('\n').map(|s| s.trim()).collect::>(); + let error = err_str.rsplit('\n').last().map(|s| s.trim()).unwrap_or(""); - BottomError::QueryError(format!("Regex error: {}", error.last().unwrap_or(&"")).into()) + BottomError::user(format!("Regex error: {error}")) } } diff --git a/src/utils/error/collection.rs b/src/utils/error/collection.rs new file mode 100644 index 00000000..7a998d0d --- /dev/null +++ b/src/utils/error/collection.rs @@ -0,0 +1,46 @@ +//! Error code related to data collection. + +use std::{borrow::Cow, num::ParseFloatError, str::Utf8Error}; + +use thiserror::Error; + +/// A type alias for handling collection-related errors. +pub type CollectionResult = std::result::Result; + +/// The errors that can happen with data collection. +#[derive(Debug, Error)] +pub enum CollectionError { + /// An error when there is an IO exception. + #[error(transparent)] + InvalidIo(#[from] std::io::Error), + /// An error to represent errors with converting between data types. + #[error("Conversion error, {0}")] + Conversion(Cow<'static, str>), + #[error("Parsing error, {0}")] + /// An error to represent errors around parsing. + Parsing(Cow<'static, str>), + /// A generic error. + #[error("source: {0}, reason: {1}")] + Other(Cow<'static, str>, Cow<'static, str>), +} + +impl CollectionError { + /// A generic error. + pub fn other>, D: Into>>( + source: C, reason: D, + ) -> Self { + Self::Other(source.into(), reason.into()) + } +} + +impl From for CollectionError { + fn from(err: Utf8Error) -> Self { + CollectionError::Conversion(err.to_string().into()) + } +} + +impl From for CollectionError { + fn from(err: ParseFloatError) -> Self { + CollectionError::Parsing(err.to_string().into()) + } +} diff --git a/src/utils/error/config.rs b/src/utils/error/config.rs new file mode 100644 index 00000000..6644590c --- /dev/null +++ b/src/utils/error/config.rs @@ -0,0 +1,47 @@ +//! Error code related to configuration. + +use std::borrow::Cow; + +use thiserror::Error; + +/// A type alias for handling collection-related errors. +pub type ConfigResult = std::result::Result; + +/// The errors that can happen with data collection. +#[derive(Debug, Error)] +pub enum ConfigError { + /// An error when there is an IO exception. + #[error(transparent)] + InvalidIo(#[from] std::io::Error), + /// An error due to parsing. + #[error("Parsing error: {0}")] + Parsing(Cow<'static, str>), + /// A generic error. + #[error("{0}")] + Other(Cow<'static, str>), +} + +impl ConfigError { + /// A generic error. + pub fn other>>(source: C) -> Self { + Self::Other(source.into()) + } +} + +impl From for ConfigError { + fn from(err: toml_edit::de::Error) -> Self { + ConfigError::Parsing(err.to_string().into()) + } +} + +impl From for ConfigError { + fn from(err: std::str::Utf8Error) -> Self { + ConfigError::Parsing(err.to_string().into()) + } +} + +impl From for ConfigError { + fn from(err: std::string::FromUtf8Error) -> Self { + ConfigError::Parsing(err.to_string().into()) + } +} diff --git a/src/utils/error/draw.rs b/src/utils/error/draw.rs new file mode 100644 index 00000000..4ef8c16b --- /dev/null +++ b/src/utils/error/draw.rs @@ -0,0 +1,14 @@ +//! Error code related to drawing. + +use thiserror::Error; + +/// A type alias for handling drawing-related errors. +pub type DrawResult = std::result::Result; + +/// The errors that can happen with drawing. +#[derive(Debug, Error)] +pub enum DrawError { + /// An error when there is an IO exception. + #[error(transparent)] + InvalidIo(#[from] std::io::Error), +} -- cgit v1.2.3