diff options
author | Clement Tsang <34804052+ClementTsang@users.noreply.github.com> | 2024-07-13 01:35:09 -0400 |
---|---|---|
committer | GitHub <noreply@github.com> | 2024-07-13 05:35:09 +0000 |
commit | 571a801bf8976881f8cc530aa0b9c4388631a7f3 (patch) | |
tree | 8370f3819a621f5cf893721ed41d1d88e68b8aa9 | |
parent | df569b2319869a536b368bc032dbca1b88d1a53d (diff) |
refactor: error refactoring, part 1 (#1492)
* refactor: use anyhow for logging
* refactor: clean up query errors
* also remove tryparseint
-rw-r--r-- | src/app/query.rs | 167 | ||||
-rw-r--r-- | src/data_collection/processes/unix/user_table.rs | 4 | ||||
-rw-r--r-- | src/options.rs | 9 | ||||
-rw-r--r-- | src/utils/error.rs | 28 | ||||
-rw-r--r-- | src/utils/logging.rs | 2 |
5 files changed, 109 insertions, 101 deletions
diff --git a/src/app/query.rs b/src/app/query.rs index c642000a..475e89c5 100644 --- a/src/app/query.rs +++ b/src/app/query.rs @@ -1,7 +1,7 @@ use std::{ borrow::Cow, collections::VecDeque, - fmt::{Debug, Formatter}, + fmt::{Debug, Display, Formatter}, time::Duration, }; @@ -9,17 +9,44 @@ use humantime::parse_duration; use regex::Regex; use crate::{ - data_collection::processes::ProcessHarvest, - multi_eq_ignore_ascii_case, - utils::{ - data_prefixes::*, - error::{ - BottomError::{self, QueryError}, - Result, - }, - }, + data_collection::processes::ProcessHarvest, multi_eq_ignore_ascii_case, utils::data_prefixes::*, }; +#[derive(Debug)] +pub(crate) struct QueryError { + reason: Cow<'static, str>, +} + +impl QueryError { + #[inline] + pub(crate) fn new<I: Into<Cow<'static, str>>>(reason: I) -> Self { + Self { + reason: reason.into(), + } + } + + #[inline] + fn missing_value() -> Self { + Self { + reason: "Missing value".into(), + } + } +} + +impl Display for QueryError { + fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { + write!(f, "{}", self.reason) + } +} + +impl From<regex::Error> for QueryError { + fn from(err: regex::Error) -> Self { + Self::new(err.to_string()) + } +} + +type QueryResult<T> = Result<T, QueryError>; + const DELIMITER_LIST: [char; 6] = ['=', '>', '<', '(', ')', '\"']; const COMPARISON_LIST: [&str; 3] = [">", "=", "<"]; const OR_LIST: [&str; 2] = ["or", "||"]; @@ -47,11 +74,11 @@ const AND_LIST: [&str; 2] = ["and", "&&"]; /// adjacent non-prefixed or quoted elements after splitting to treat as process /// names. Furthermore, we want to support boolean joiners like AND and OR, and /// brackets. -pub fn parse_query( +pub(crate) fn parse_query( search_query: &str, is_searching_whole_word: bool, is_ignoring_case: bool, is_searching_with_regex: bool, -) -> Result<Query> { - fn process_string_to_filter(query: &mut VecDeque<String>) -> Result<Query> { +) -> QueryResult<Query> { + fn process_string_to_filter(query: &mut VecDeque<String>) -> QueryResult<Query> { let lhs = process_or(query)?; let mut list_of_ors = vec![lhs]; @@ -62,7 +89,7 @@ pub fn parse_query( Ok(Query { query: list_of_ors }) } - fn process_or(query: &mut VecDeque<String>) -> Result<Or> { + fn process_or(query: &mut VecDeque<String>) -> QueryResult<Or> { let mut lhs = process_and(query)?; let mut rhs: Option<Box<And>> = None; @@ -89,7 +116,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(QueryError::new("Comparison not valid here")); } else { break; } @@ -98,7 +125,7 @@ pub fn parse_query( Ok(Or { lhs, rhs }) } - fn process_and(query: &mut VecDeque<String>) -> Result<And> { + fn process_and(query: &mut VecDeque<String>) -> QueryResult<And> { let mut lhs = process_prefix(query, false)?; let mut rhs: Option<Box<Prefix>> = None; @@ -128,7 +155,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(QueryError::new("Comparison not valid here")); } else { break; } @@ -175,7 +202,7 @@ pub fn parse_query( } } - fn process_prefix(query: &mut VecDeque<String>, inside_quotation: bool) -> Result<Prefix> { + fn process_prefix(query: &mut VecDeque<String>, inside_quotation: bool) -> QueryResult<Prefix> { if let Some(queue_top) = query.pop_front() { if inside_quotation { if queue_top == "\"" { @@ -210,7 +237,7 @@ pub fn parse_query( } } else if queue_top == "(" { if query.is_empty() { - return Err(QueryError(Cow::Borrowed("Missing closing parentheses"))); + return Err(QueryError::new("Missing closing parentheses")); } let mut list_of_ors = VecDeque::new(); @@ -225,7 +252,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(QueryError::new("No values within parentheses group")); } // Now convert this back to a OR... @@ -264,13 +291,13 @@ pub fn parse_query( compare_prefix: None, }); } else { - return Err(QueryError("Missing closing parentheses".into())); + return Err(QueryError::new("Missing closing parentheses")); } } else { - return Err(QueryError("Missing closing parentheses".into())); + return Err(QueryError::new("Missing closing parentheses")); } } else if queue_top == ")" { - return Err(QueryError("Missing opening parentheses".into())); + return Err(QueryError::new("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 @@ -281,13 +308,13 @@ pub fn parse_query( if close_paren == "\"" { return Ok(prefix); } else { - return Err(QueryError("Missing closing quotation".into())); + return Err(QueryError::new("Missing closing quotation")); } } else { - return Err(QueryError("Missing closing quotation".into())); + return Err(QueryError::new("Missing closing quotation")); } } else { - // Get prefix type... + // Get prefix type. let prefix_type = queue_top.parse::<PrefixType>()?; let content = if let PrefixType::Name = prefix_type { Some(queue_top) @@ -362,15 +389,15 @@ pub fn parse_query( duration_string = Some(queue_next); } } else { - return Err(QueryError("Missing value".into())); + return Err(QueryError::missing_value()); } } if let Some(condition) = condition { let duration = parse_duration( - &duration_string.ok_or(QueryError("Missing value".into()))?, + &duration_string.ok_or(QueryError::missing_value())?, ) - .map_err(|err| QueryError(err.to_string().into()))?; + .map_err(|err| QueryError::new(err.to_string()))?; return Ok(Prefix { or: None, @@ -399,7 +426,7 @@ pub fn parse_query( if let Some(queue_next) = query.pop_front() { value = queue_next.parse::<f64>().ok(); } else { - return Err(QueryError("Missing value".into())); + return Err(QueryError::missing_value()); } } else if content == ">" || content == "<" { // We also have to check if the next string is an "="... @@ -413,7 +440,7 @@ pub fn parse_query( if let Some(queue_next_next) = query.pop_front() { value = queue_next_next.parse::<f64>().ok(); } else { - return Err(QueryError("Missing value".into())); + return Err(QueryError::missing_value()); } } else { condition = Some(if content == ">" { @@ -424,7 +451,7 @@ pub fn parse_query( value = queue_next.parse::<f64>().ok(); } } else { - return Err(QueryError("Missing value".into())); + return Err(QueryError::missing_value()); } } @@ -467,15 +494,15 @@ pub fn parse_query( } } } else { - return Err(QueryError("Missing argument for search prefix".into())); + return Err(QueryError::new("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(QueryError::new("Missing closing quotation")); } - Err(QueryError("Invalid query".into())) + Err(QueryError::new("Invalid query")) } let mut split_query = VecDeque::new(); @@ -507,14 +534,14 @@ pub fn parse_query( pub struct Query { /// Remember, AND > OR, but AND must come after OR when we parse. - pub query: Vec<Or>, + query: Vec<Or>, } impl Query { - pub fn process_regexes( + fn process_regexes( &mut self, is_searching_whole_word: bool, is_ignoring_case: bool, is_searching_with_regex: bool, - ) -> Result<()> { + ) -> QueryResult<()> { for or in &mut self.query { or.process_regexes( is_searching_whole_word, @@ -526,7 +553,7 @@ impl Query { Ok(()) } - pub fn check(&self, process: &ProcessHarvest, is_using_command: bool) -> bool { + pub(crate) fn check(&self, process: &ProcessHarvest, is_using_command: bool) -> bool { self.query .iter() .all(|ok| ok.check(process, is_using_command)) @@ -540,16 +567,16 @@ impl Debug for Query { } #[derive(Default)] -pub struct Or { - pub lhs: And, - pub rhs: Option<Box<And>>, +struct Or { + lhs: And, + rhs: Option<Box<And>>, } impl Or { - pub fn process_regexes( + fn process_regexes( &mut self, is_searching_whole_word: bool, is_ignoring_case: bool, is_searching_with_regex: bool, - ) -> Result<()> { + ) -> QueryResult<()> { self.lhs.process_regexes( is_searching_whole_word, is_ignoring_case, @@ -566,7 +593,7 @@ impl Or { Ok(()) } - pub fn check(&self, process: &ProcessHarvest, is_using_command: bool) -> bool { + fn check(&self, process: &ProcessHarvest, is_using_command: bool) -> bool { if let Some(rhs) = &self.rhs { self.lhs.check(process, is_using_command) || rhs.check(process, is_using_command) } else { @@ -585,16 +612,16 @@ impl Debug for Or { } #[derive(Default)] -pub struct And { - pub lhs: Prefix, - pub rhs: Option<Box<Prefix>>, +struct And { + lhs: Prefix, + rhs: Option<Box<Prefix>>, } impl And { - pub fn process_regexes( + fn process_regexes( &mut self, is_searching_whole_word: bool, is_ignoring_case: bool, is_searching_with_regex: bool, - ) -> Result<()> { + ) -> QueryResult<()> { self.lhs.process_regexes( is_searching_whole_word, is_ignoring_case, @@ -611,7 +638,7 @@ impl And { Ok(()) } - pub fn check(&self, process: &ProcessHarvest, is_using_command: bool) -> bool { + fn check(&self, process: &ProcessHarvest, is_using_command: bool) -> bool { if let Some(rhs) = &self.rhs { self.lhs.check(process, is_using_command) && rhs.check(process, is_using_command) } else { @@ -630,7 +657,7 @@ impl Debug for And { } #[derive(Debug)] -pub enum PrefixType { +enum PrefixType { Pid, PCpu, MemBytes, @@ -653,9 +680,9 @@ pub enum PrefixType { } impl std::str::FromStr for PrefixType { - type Err = BottomError; + type Err = QueryError; - fn from_str(s: &str) -> Result<Self> { + fn from_str(s: &str) -> QueryResult<Self> { use PrefixType::*; // TODO: Didn't add mem_bytes, total_read, and total_write @@ -702,17 +729,17 @@ impl std::str::FromStr for PrefixType { // TODO: This is also jank and could be better represented. Add tests, then // clean up! #[derive(Default)] -pub struct Prefix { - pub or: Option<Box<Or>>, - pub regex_prefix: Option<(PrefixType, StringQuery)>, - pub compare_prefix: Option<(PrefixType, ComparableQuery)>, +struct Prefix { + or: Option<Box<Or>>, + regex_prefix: Option<(PrefixType, StringQuery)>, + compare_prefix: Option<(PrefixType, ComparableQuery)>, } impl Prefix { - pub fn process_regexes( + fn process_regexes( &mut self, is_searching_whole_word: bool, is_ignoring_case: bool, is_searching_with_regex: bool, - ) -> Result<()> { + ) -> QueryResult<()> { if let Some(or) = &mut self.or { return or.process_regexes( is_searching_whole_word, @@ -752,7 +779,7 @@ impl Prefix { Ok(()) } - pub fn check(&self, process: &ProcessHarvest, is_using_command: bool) -> bool { + fn check(&self, process: &ProcessHarvest, is_using_command: bool) -> bool { fn matches_condition<I: Into<f64>, J: Into<f64>>( condition: &QueryComparison, lhs: I, rhs: J, ) -> bool { @@ -883,7 +910,7 @@ impl Debug for Prefix { } #[derive(Debug)] -pub enum QueryComparison { +enum QueryComparison { Equal, Less, Greater, @@ -892,25 +919,25 @@ pub enum QueryComparison { } #[derive(Debug)] -pub enum StringQuery { +enum StringQuery { Value(String), Regex(Regex), } #[derive(Debug)] -pub enum ComparableQuery { +enum ComparableQuery { Numerical(NumericalQuery), Time(TimeQuery), } #[derive(Debug)] -pub struct NumericalQuery { - pub condition: QueryComparison, - pub value: f64, +struct NumericalQuery { + condition: QueryComparison, + value: f64, } #[derive(Debug)] -pub struct TimeQuery { - pub condition: QueryComparison, - pub duration: Duration, +struct TimeQuery { + condition: QueryComparison, + duration: Duration, } diff --git a/src/data_collection/processes/unix/user_table.rs b/src/data_collection/processes/unix/user_table.rs index b36008ea..1759ad89 100644 --- a/src/data_collection/processes/unix/user_table.rs +++ b/src/data_collection/processes/unix/user_table.rs @@ -17,7 +17,9 @@ impl UserTable { let passwd = unsafe { libc::getpwuid(uid) }; if passwd.is_null() { - Err(error::BottomError::QueryError("Missing passwd".into())) + Err(error::BottomError::GenericError( + "passwd is inaccessible".into(), + )) } else { // SAFETY: We return early if passwd is null. let username = unsafe { std::ffi::CStr::from_ptr((*passwd).pw_name) } diff --git a/src/options.rs b/src/options.rs index b2020054..21fbde8a 100644 --- a/src/options.rs +++ b/src/options.rs @@ -566,7 +566,10 @@ fn get_show_average_cpu(args: &BottomArgs, config: &ConfigV1) -> bool { fn try_parse_ms(s: &str) -> error::Result<u64> { if let Ok(val) = humantime::parse_duration(s) { - Ok(val.as_millis().try_into()?) + Ok(val + .as_millis() + .try_into() + .map_err(|err| BottomError::GenericError(format!("could not parse duration, {err}")))?) } else if let Ok(val) = s.parse::<u64>() { Ok(val) } else { @@ -784,8 +787,10 @@ fn get_ignore_list(ignore_list: &Option<IgnoreList>) -> error::Result<Option<Fil }) .collect(); + let list = list.map_err(|err| BottomError::ConfigError(err.to_string()))?; + Ok(Some(Filter { - list: list?, + list, is_list_ignored: ignore_list.is_list_ignored, })) } else { diff --git a/src/utils/error.rs b/src/utils/error.rs index 532ad283..6a584e28 100644 --- a/src/utils/error.rs +++ b/src/utils/error.rs @@ -1,4 +1,4 @@ -use std::{borrow::Cow, result}; +use std::result; use thiserror::Error; @@ -17,10 +17,6 @@ pub enum BottomError { /// 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), @@ -30,11 +26,6 @@ pub enum BottomError { /// 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), } impl From<std::io::Error> for BottomError { @@ -61,13 +52,6 @@ impl From<toml_edit::de::Error> for BottomError { } } -#[cfg(feature = "fern")] -impl From<fern::InitError> for BottomError { - fn from(err: fern::InitError) -> Self { - BottomError::FernError(err.to_string()) - } -} - impl From<std::str::Utf8Error> for BottomError { fn from(err: std::str::Utf8Error) -> Self { BottomError::ConversionError(err.to_string()) @@ -79,13 +63,3 @@ impl From<std::string::FromUtf8Error> for BottomError { BottomError::ConversionError(err.to_string()) } } - -impl From<regex::Error> 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: - let err_str = err.to_string(); - let error = err_str.split('\n').map(|s| s.trim()).collect::<Vec<_>>(); - - BottomError::QueryError(format!("Regex error: {}", error.last().unwrap_or(&"")).into()) - } -} diff --git a/src/utils/logging.rs b/src/utils/logging.rs index 27fa92fe..85230ccb 100644 --- a/src/utils/logging.rs +++ b/src/utils/logging.rs @@ -7,7 +7,7 @@ pub static OFFSET: OnceLock<time::UtcOffset> = OnceLock::new(); #[cfg(feature = "logging")] pub fn init_logger( min_level: log::LevelFilter, debug_file_name: Option<&std::ffi::OsStr>, -) -> Result<(), fern::InitError> { +) -> anyhow::Result<()> { let dispatch = fern::Dispatch::new() .format(|out, message, record| { let offset = OFFSET.get_or_init(|| { |