From 29029b86fbeafa8f1935772b807df7f714023180 Mon Sep 17 00:00:00 2001 From: Clement Tsang <34804052+ClementTsang@users.noreply.github.com> Date: Mon, 22 Jul 2024 03:30:03 -0400 Subject: [PATCH] refactor: remove BottomError (#1498) * refactor: remove BottomError * remove thiserror * some cleanup * forgot to remove this --- Cargo.lock | 1 - Cargo.toml | 6 ++-- src/app.rs | 13 +++------ src/app/process_killer.rs | 26 ++++++++--------- src/data_collection/disks/freebsd.rs | 5 ++-- src/data_collection/error.rs | 28 +++++++++++++------ src/data_collection/processes.rs | 2 +- .../processes/unix/user_table.rs | 2 +- src/main.rs | 1 - src/utils/error.rs | 14 ---------- 10 files changed, 43 insertions(+), 55 deletions(-) delete mode 100644 src/utils/error.rs diff --git a/Cargo.lock b/Cargo.lock index 3d6818f7..1d81c27a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -189,7 +189,6 @@ dependencies = [ "strum", "sysctl", "sysinfo", - "thiserror", "time", "toml_edit", "unicode-ellipsis", diff --git a/Cargo.toml b/Cargo.toml index 82335a5c..1b53b57e 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -73,11 +73,12 @@ battery = ["starship-battery"] nvidia = ["nvml-wrapper"] gpu = ["nvidia"] zfs = [] -logging = ["fern", "log", "time/local-offset"] -generate_schema = ["schemars", "serde_json", "strum"] deploy = ["battery", "gpu", "zfs"] default = ["deploy"] +logging = ["fern", "log", "time/local-offset"] +generate_schema = ["schemars", "serde_json", "strum"] + [dependencies] anyhow = "1.0.86" backtrace = "0.3.73" @@ -97,7 +98,6 @@ regex = "1.10.5" serde = { version = "1.0.203", features = ["derive"] } starship-battery = { version = "0.8.3", optional = true } sysinfo = "=0.30.12" -thiserror = "1.0.61" time = { version = "0.3.36", features = ["formatting", "macros"] } toml_edit = { version = "0.22.14", features = ["serde"] } tui = { version = "0.27.0", package = "ratatui" } diff --git a/src/app.rs b/src/app.rs index ef3fb280..ec0dd1bc 100644 --- a/src/app.rs +++ b/src/app.rs @@ -11,6 +11,7 @@ use std::{ time::Instant, }; +use anyhow::bail; use concat_string::concat_string; use data_farmer::*; use filter::*; @@ -26,10 +27,7 @@ use crate::{ data_collection::{processes::Pid, temperature}, data_conversion::ConvertedData, get_network_points, - utils::{ - data_units::DataUnit, - error::{BottomError, Result}, - }, + utils::data_units::DataUnit, widgets::{ProcWidgetColumn, ProcWidgetMode}, }; @@ -1469,7 +1467,7 @@ impl App { } } - pub fn kill_highlighted_process(&mut self) -> Result<()> { + pub fn kill_highlighted_process(&mut self) -> anyhow::Result<()> { if let BottomWidgetType::Proc = self.current_widget.widget_type { if let Some((_, pids)) = &self.to_delete_process_list { #[cfg(target_family = "unix")] @@ -1491,10 +1489,7 @@ 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(), - )) + bail!("Cannot kill processes if the current widget is not the Process widget!"); } } diff --git a/src/app/process_killer.rs b/src/app/process_killer.rs index fae05dd2..943578cb 100644 --- a/src/app/process_killer.rs +++ b/src/app/process_killer.rs @@ -1,6 +1,7 @@ //! This file is meant to house (OS specific) implementations on how to kill //! processes. +use anyhow::bail; #[cfg(target_os = "windows")] use windows::Win32::{ Foundation::{CloseHandle, HANDLE}, @@ -10,7 +11,6 @@ use windows::Win32::{ }; use crate::data_collection::processes::Pid; -use crate::utils::error::BottomError; /// Based from [this SO answer](https://stackoverflow.com/a/55231715). #[cfg(target_os = "windows")] @@ -18,19 +18,19 @@ struct Process(HANDLE); #[cfg(target_os = "windows")] impl Process { - fn open(pid: u32) -> Result { + fn open(pid: u32) -> anyhow::Result { // SAFETY: Windows API call, tread carefully with the args. match unsafe { OpenProcess(PROCESS_QUERY_INFORMATION | PROCESS_TERMINATE, false, pid) } { Ok(process) => Ok(Process(process)), - Err(_) => Err("process may have already been terminated.".to_string()), + Err(_) => bail!("process may have already been terminated."), } } - fn kill(self) -> Result<(), String> { + fn kill(self) -> anyhow::Result<()> { // SAFETY: Windows API call, this is safe as we are passing in the handle. let result = unsafe { TerminateProcess(self.0, 1) }; if result.is_err() { - return Err("process may have already been terminated.".to_string()); + bail!("process may have already been terminated."); } Ok(()) @@ -49,16 +49,16 @@ impl Drop for Process { /// Kills a process, given a PID, for windows. #[cfg(target_os = "windows")] -pub fn kill_process_given_pid(pid: Pid) -> crate::utils::error::Result<()> { - let process = Process::open(pid as u32).map_err(BottomError::GenericError)?; - process.kill().map_err(BottomError::GenericError)?; +pub fn kill_process_given_pid(pid: Pid) -> anyhow::Result<()> { + let process = Process::open(pid as u32)?; + process.kill()?; Ok(()) } /// Kills a process, given a PID, for UNIX. #[cfg(target_family = "unix")] -pub fn kill_process_given_pid(pid: Pid, signal: usize) -> crate::utils::error::Result<()> { +pub fn kill_process_given_pid(pid: Pid, signal: usize) -> anyhow::Result<()> { // SAFETY: the signal should be valid, and we act properly on an error (exit // code not 0). let output = unsafe { libc::kill(pid, signal as i32) }; @@ -73,12 +73,10 @@ pub fn kill_process_given_pid(pid: Pid, signal: usize) -> crate::utils::error::R _ => "Unknown error occurred." }; - return if let Some(err_code) = err_code { - Err(BottomError::GenericError(format!( - "Error code {err_code} - {err}" - ))) + if let Some(err_code) = err_code { + bail!(format!("Error code {err_code} - {err}")) } else { - Err(BottomError::GenericError(format!("Error code ??? - {err}"))) + bail!(format!("Error code unknown - {err}")) }; } diff --git a/src/data_collection/disks/freebsd.rs b/src/data_collection/disks/freebsd.rs index 11629e16..a193e6b6 100644 --- a/src/data_collection/disks/freebsd.rs +++ b/src/data_collection/disks/freebsd.rs @@ -6,9 +6,8 @@ use hashbrown::HashMap; use serde::Deserialize; use super::{keep_disk_entry, DiskHarvest, IoHarvest}; -use crate::{ - data_collection::{deserialize_xo, disks::IoData, error::CollectionResult, DataCollector}, - utils::error, +use crate::data_collection::{ + deserialize_xo, disks::IoData, error::CollectionResult, DataCollector, }; #[derive(Deserialize, Debug, Default)] diff --git a/src/data_collection/error.rs b/src/data_collection/error.rs index 83fe2d0e..dc5dd7bd 100644 --- a/src/data_collection/error.rs +++ b/src/data_collection/error.rs @@ -10,21 +10,33 @@ pub enum CollectionError { Unsupported, } -impl CollectionError { - // pub(crate) fn general>(error: E) -> Self { - // Self::General(error.into()) - // } - - pub(crate) fn from_str(msg: &'static str) -> Self { - Self::General(anyhow!(msg)) +impl std::fmt::Display for CollectionError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + CollectionError::General(err) => err.fmt(f), + CollectionError::Unsupported => { + write!( + f, + "bottom does not support this type of data collection for this platform." + ) + } + } } } +impl std::error::Error for CollectionError {} + /// A [`Result`] with the error type being a [`DataCollectionError`]. pub(crate) type CollectionResult = Result; impl From for CollectionError { fn from(err: std::io::Error) -> Self { - CollectionError::General(err.into()) + Self::General(err.into()) + } +} + +impl From<&'static str> for CollectionError { + fn from(msg: &'static str) -> Self { + Self::General(anyhow!(msg)) } } diff --git a/src/data_collection/processes.rs b/src/data_collection/processes.rs index 03854049..319c16e1 100644 --- a/src/data_collection/processes.rs +++ b/src/data_collection/processes.rs @@ -144,7 +144,7 @@ impl DataCollector { } else if #[cfg(any(target_os = "freebsd", target_os = "macos", target_os = "windows", target_os = "android", target_os = "ios"))] { sysinfo_process_data(self) } else { - Err(error::BottomError::GenericError("Unsupported OS".to_string())) + Err(crate::data_collection::error::CollectionError::Unsupported) } } } diff --git a/src/data_collection/processes/unix/user_table.rs b/src/data_collection/processes/unix/user_table.rs index 50edaba2..dc8e0ab4 100644 --- a/src/data_collection/processes/unix/user_table.rs +++ b/src/data_collection/processes/unix/user_table.rs @@ -17,7 +17,7 @@ impl UserTable { let passwd = unsafe { libc::getpwuid(uid) }; if passwd.is_null() { - Err(CollectionError::from_str("passwd is inaccessible")) + Err("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/main.rs b/src/main.rs index 66805bd7..c5193a43 100644 --- a/src/main.rs +++ b/src/main.rs @@ -11,7 +11,6 @@ pub mod app; pub mod utils { pub mod data_prefixes; pub mod data_units; - pub mod error; pub mod general; pub mod logging; pub mod strings; diff --git a/src/utils/error.rs b/src/utils/error.rs deleted file mode 100644 index 5d96f762..00000000 --- a/src/utils/error.rs +++ /dev/null @@ -1,14 +0,0 @@ -use std::result; - -use thiserror::Error; - -/// A type alias for handling errors related to Bottom. -pub type Result = result::Result; - -/// An error that can occur while Bottom runs. -#[derive(Debug, Error, PartialEq, Eq)] -pub enum BottomError { - /// An error to represent generic errors. - #[error("Error, {0}")] - GenericError(String), -}