refactor: migrate data collection and some others away from BottomError (#1497)

src/canvas.rs

@@ -23,7 +23,6 @@ use crate::{
     },
     constants::*,
     options::OptionError,
-    utils::error,
 };
 
 #[derive(Debug)]
@@ -205,7 +204,7 @@ impl Painter {
 
     pub fn draw_data<B: Backend>(
         &mut self, terminal: &mut Terminal<B>, app_state: &mut App,
-    ) -> error::Result<()> {
+    ) -> Result<(), std::io::Error> {
         use BottomWidgetType::*;
 
         terminal.draw(|f| {

src/data_collection.rs

@@ -8,6 +8,7 @@ pub mod batteries;
 
 pub mod cpu;
 pub mod disks;
+pub mod error;
 pub mod memory;
 pub mod network;
 pub mod processes;
@@ -367,7 +368,7 @@ impl DataCollector {
 
             #[cfg(target_family = "unix")]
             {
-                self.data.load_avg = cpu::get_load_avg().ok();
+                self.data.load_avg = Some(cpu::get_load_avg());
             }
         }
     }

src/data_collection/cpu/sysinfo.rs

@@ -6,9 +6,9 @@ use std::collections::VecDeque;
 use sysinfo::{LoadAvg, System};
 
 use super::{CpuData, CpuDataType, CpuHarvest};
-use crate::data_collection::cpu::LoadAvgHarvest;
+use crate::data_collection::{cpu::LoadAvgHarvest, error::CollectionResult};
 
-pub fn get_cpu_data_list(sys: &System, show_average_cpu: bool) -> crate::error::Result<CpuHarvest> {
+pub fn get_cpu_data_list(sys: &System, show_average_cpu: bool) -> CollectionResult<CpuHarvest> {
     let mut cpu_deque: VecDeque<_> = sys
         .cpus()
         .iter()
@@ -31,10 +31,10 @@ pub fn get_cpu_data_list(sys: &System, show_average_cpu: bool) -> crate::error::
     Ok(Vec::from(cpu_deque))
 }
 
-pub fn get_load_avg() -> crate::error::Result<LoadAvgHarvest> {
+pub fn get_load_avg() -> LoadAvgHarvest {
     // The API for sysinfo apparently wants you to call it like this, rather than
     // using a &System.
     let LoadAvg { one, five, fifteen } = sysinfo::System::load_average();
 
-    Ok([one as f32, five as f32, fifteen as f32])
+    [one as f32, five as f32, fifteen as f32]
 }

src/data_collection/disks/freebsd.rs

@@ -7,7 +7,7 @@ use serde::Deserialize;
 
 use super::{keep_disk_entry, DiskHarvest, IoHarvest};
 use crate::{
-    data_collection::{deserialize_xo, disks::IoData, DataCollector},
+    data_collection::{deserialize_xo, disks::IoData, error::CollectionResult, DataCollector},
     utils::error,
 };
 
@@ -27,7 +27,7 @@ struct FileSystem {
     mounted_on: String,
 }
 
-pub fn get_io_usage() -> error::Result<IoHarvest> {
+pub fn get_io_usage() -> CollectionResult<IoHarvest> {
     // TODO: Should this (and other I/O collectors) fail fast? In general, should
     // collection ever fail fast?
     #[allow(unused_mut)]
@@ -59,7 +59,7 @@ pub fn get_io_usage() -> error::Result<IoHarvest> {
     Ok(io_harvest)
 }
 
-pub fn get_disk_usage(collector: &DataCollector) -> error::Result<Vec<DiskHarvest>> {
+pub fn get_disk_usage(collector: &DataCollector) -> CollectionResult<Vec<DiskHarvest>> {
     let disk_filter = &collector.filters.disk_filter;
     let mount_filter = &collector.filters.mount_filter;
     let vec_disks: Vec<DiskHarvest> = get_disk_info().map(|storage_system_information| {

src/data_collection/error.rs

@@ -0,0 +1,30 @@
+use anyhow::anyhow;
+
+/// An error to do with data collection.
+#[derive(Debug)]
+pub enum CollectionError {
+    /// A general error to propagate back up. A wrapper around [`anyhow::Error`].
+    General(anyhow::Error),
+
+    /// The collection is unsupported.
+    Unsupported,
+}
+
+impl CollectionError {
+    // pub(crate) fn general<E: Into<anyhow::Error>>(error: E) -> Self {
+    //     Self::General(error.into())
+    // }
+
+    pub(crate) fn from_str(msg: &'static str) -> Self {
+        Self::General(anyhow!(msg))
+    }
+}
+
+/// A [`Result`] with the error type being a [`DataCollectionError`].
+pub(crate) type CollectionResult<T> = Result<T, CollectionError>;
+
+impl From<std::io::Error> for CollectionError {
+    fn from(err: std::io::Error) -> Self {
+        CollectionError::General(err.into())
+    }
+}

src/data_collection/processes.rs

@@ -33,8 +33,7 @@ cfg_if! {
 
 use std::{borrow::Cow, time::Duration};
 
-use super::DataCollector;
+use super::{error::CollectionResult, DataCollector};
-use crate::utils::error;
 
 cfg_if! {
     if #[cfg(target_family = "windows")] {
@@ -131,7 +130,7 @@ impl ProcessHarvest {
 }
 
 impl DataCollector {
-    pub(crate) fn get_processes(&mut self) -> error::Result<Vec<ProcessHarvest>> {
+    pub(crate) fn get_processes(&mut self) -> CollectionResult<Vec<ProcessHarvest>> {
         cfg_if! {
             if #[cfg(target_os = "linux")] {
                 let time_diff = self.data.collection_time

src/data_collection/processes/linux.rs

@@ -13,10 +13,7 @@ use process::*;
 use sysinfo::ProcessStatus;
 
 use super::{Pid, ProcessHarvest, UserTable};
-use crate::{
+use crate::data_collection::{error::CollectionResult, DataCollector};
-    data_collection::DataCollector,
-    utils::error::{self, BottomError},
-};
 
 /// Maximum character length of a `/proc/<PID>/stat`` process name.
 /// If it's equal or greater, then we instead refer to the command for the name.
@@ -64,7 +61,9 @@ struct CpuUsage {
     cpu_fraction: f64,
 }
 
-fn cpu_usage_calculation(prev_idle: &mut f64, prev_non_idle: &mut f64) -> error::Result<CpuUsage> {
+fn cpu_usage_calculation(
+    prev_idle: &mut f64, prev_non_idle: &mut f64,
+) -> CollectionResult<CpuUsage> {
     let (idle, non_idle) = {
         // From SO answer: https://stackoverflow.com/a/23376195
         let first_line = {
@@ -132,7 +131,7 @@ fn get_linux_cpu_usage(
 
 fn read_proc(
     prev_proc: &PrevProcDetails, process: Process, args: ReadProcArgs, user_table: &mut UserTable,
-) -> error::Result<(ProcessHarvest, u64)> {
+) -> CollectionResult<(ProcessHarvest, u64)> {
     let Process {
         pid: _,
         uid,
@@ -298,7 +297,7 @@ pub(crate) struct ReadProcArgs {
 
 pub(crate) fn linux_process_data(
     collector: &mut DataCollector, time_difference_in_secs: u64,
-) -> error::Result<Vec<ProcessHarvest>> {
+) -> CollectionResult<Vec<ProcessHarvest>> {
     let total_memory = collector.total_memory();
     let prev_proc = PrevProc {
         prev_idle: &mut collector.prev_idle,
@@ -323,88 +322,82 @@ pub(crate) fn linux_process_data(
 
     // TODO: [PROC THREADS] Add threads
 
-    if let Ok(CpuUsage {
+    let CpuUsage {
         mut cpu_usage,
         cpu_fraction,
-    }) = cpu_usage_calculation(prev_idle, prev_non_idle)
+    } = cpu_usage_calculation(prev_idle, prev_non_idle)?;
-    {
-        if unnormalized_cpu {
-            let num_processors = collector.sys.system.cpus().len() as f64;
 
-            // Note we *divide* here because the later calculation divides `cpu_usage` - in
+    if unnormalized_cpu {
-            // effect, multiplying over the number of cores.
+        let num_processors = collector.sys.system.cpus().len() as f64;
-            cpu_usage /= num_processors;
-        }
 
-        let mut pids_to_clear: HashSet<Pid> = pid_mapping.keys().cloned().collect();
+        // Note we *divide* here because the later calculation divides `cpu_usage` - in
-
+        // effect, multiplying over the number of cores.
-        let pids = fs::read_dir("/proc")?.flatten().filter_map(|dir| {
+        cpu_usage /= num_processors;
-            if is_str_numeric(dir.file_name().to_string_lossy().trim()) {
-                Some(dir.path())
-            } else {
-                None
-            }
-        });
-
-        let args = ReadProcArgs {
-            use_current_cpu_total,
-            cpu_usage,
-            cpu_fraction,
-            total_memory,
-            time_difference_in_secs,
-            uptime: sysinfo::System::uptime(),
-        };
-
-        let process_vector: Vec<ProcessHarvest> = pids
-            .filter_map(|pid_path| {
-                if let Ok(process) = Process::from_path(pid_path) {
-                    let pid = process.pid;
-                    let prev_proc_details = pid_mapping.entry(pid).or_default();
-
-                    #[allow(unused_mut)]
-                    if let Ok((mut process_harvest, new_process_times)) =
-                        read_proc(prev_proc_details, process, args, user_table)
-                    {
-                        #[cfg(feature = "gpu")]
-                        if let Some(gpus) = &collector.gpu_pids {
-                            gpus.iter().for_each(|gpu| {
-                                // add mem/util for all gpus to pid
-                                if let Some((mem, util)) = gpu.get(&(pid as u32)) {
-                                    process_harvest.gpu_mem += mem;
-                                    process_harvest.gpu_util += util;
-                                }
-                            });
-                            if let Some(gpu_total_mem) = &collector.gpus_total_mem {
-                                process_harvest.gpu_mem_percent = (process_harvest.gpu_mem as f64
-                                    / *gpu_total_mem as f64
-                                    * 100.0)
-                                    as f32;
-                            }
-                        }
-
-                        prev_proc_details.cpu_time = new_process_times;
-                        prev_proc_details.total_read_bytes = process_harvest.total_read_bytes;
-                        prev_proc_details.total_write_bytes = process_harvest.total_write_bytes;
-
-                        pids_to_clear.remove(&pid);
-                        return Some(process_harvest);
-                    }
-                }
-
-                None
-            })
-            .collect();
-
-        pids_to_clear.iter().for_each(|pid| {
-            pid_mapping.remove(pid);
-        });
-
-        Ok(process_vector)
-    } else {
-        Err(BottomError::GenericError(
-            "Could not calculate CPU usage.".to_string(),
-        ))
     }
+
+    let mut pids_to_clear: HashSet<Pid> = pid_mapping.keys().cloned().collect();
+
+    let pids = fs::read_dir("/proc")?.flatten().filter_map(|dir| {
+        if is_str_numeric(dir.file_name().to_string_lossy().trim()) {
+            Some(dir.path())
+        } else {
+            None
+        }
+    });
+
+    let args = ReadProcArgs {
+        use_current_cpu_total,
+        cpu_usage,
+        cpu_fraction,
+        total_memory,
+        time_difference_in_secs,
+        uptime: sysinfo::System::uptime(),
+    };
+
+    let process_vector: Vec<ProcessHarvest> = pids
+        .filter_map(|pid_path| {
+            if let Ok(process) = Process::from_path(pid_path) {
+                let pid = process.pid;
+                let prev_proc_details = pid_mapping.entry(pid).or_default();
+
+                #[allow(unused_mut)]
+                if let Ok((mut process_harvest, new_process_times)) =
+                    read_proc(prev_proc_details, process, args, user_table)
+                {
+                    #[cfg(feature = "gpu")]
+                    if let Some(gpus) = &collector.gpu_pids {
+                        gpus.iter().for_each(|gpu| {
+                            // add mem/util for all gpus to pid
+                            if let Some((mem, util)) = gpu.get(&(pid as u32)) {
+                                process_harvest.gpu_mem += mem;
+                                process_harvest.gpu_util += util;
+                            }
+                        });
+                        if let Some(gpu_total_mem) = &collector.gpus_total_mem {
+                            process_harvest.gpu_mem_percent =
+                                (process_harvest.gpu_mem as f64 / *gpu_total_mem as f64 * 100.0)
+                                    as f32;
+                        }
+                    }
+
+                    prev_proc_details.cpu_time = new_process_times;
+                    prev_proc_details.total_read_bytes = process_harvest.total_read_bytes;
+                    prev_proc_details.total_write_bytes = process_harvest.total_write_bytes;
+
+                    pids_to_clear.remove(&pid);
+                    return Some(process_harvest);
+                }
+            }
+
+            None
+        })
+        .collect();
+
+    pids_to_clear.iter().for_each(|pid| {
+        pid_mapping.remove(pid);
+    });
+
+    Ok(process_vector)
 }
 
 #[cfg(test)]

src/data_collection/processes/unix.rs

@@ -13,9 +13,9 @@ cfg_if! {
         use super::ProcessHarvest;
 
         use crate::data_collection::{DataCollector, processes::*};
-        use crate::utils::error;
+        use crate::data_collection::error::CollectionResult;
 
-        pub fn sysinfo_process_data(collector: &mut DataCollector) -> error::Result<Vec<ProcessHarvest>> {
+        pub fn sysinfo_process_data(collector: &mut DataCollector) -> CollectionResult<Vec<ProcessHarvest>> {
             let sys = &collector.sys.system;
             let use_current_cpu_total = collector.use_current_cpu_total;
             let unnormalized_cpu = collector.unnormalized_cpu;

src/data_collection/processes/unix/process_ext.rs

@@ -6,16 +6,13 @@ use hashbrown::HashMap;
 use sysinfo::{ProcessStatus, System};
 
 use super::ProcessHarvest;
-use crate::{
+use crate::data_collection::{error::CollectionResult, processes::UserTable, Pid};
-    data_collection::{processes::UserTable, Pid},
-    utils::error,
-};
 
 pub(crate) trait UnixProcessExt {
     fn sysinfo_process_data(
         sys: &System, use_current_cpu_total: bool, unnormalized_cpu: bool, total_memory: u64,
         user_table: &mut UserTable,
-    ) -> error::Result<Vec<ProcessHarvest>> {
+    ) -> CollectionResult<Vec<ProcessHarvest>> {
         let mut process_vector: Vec<ProcessHarvest> = Vec::new();
         let process_hashmap = sys.processes();
         let cpu_usage = sys.global_cpu_info().cpu_usage() as f64 / 100.0;

src/data_collection/processes/unix/user_table.rs

@@ -1,6 +1,6 @@
 use hashbrown::HashMap;
 
-use crate::utils::error::{self, BottomError};
+use crate::data_collection::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<String> {
+    pub fn get_uid_to_username_mapping(&mut self, uid: libc::uid_t) -> CollectionResult<String> {
         if let Some(user) = self.uid_user_mapping.get(&uid) {
             Ok(user.clone())
         } else {
@@ -17,12 +17,12 @@ impl UserTable {
             let passwd = unsafe { libc::getpwuid(uid) };
 
             if passwd.is_null() {
-                Err(BottomError::GenericError("passwd is inaccessible".into()))
+                Err(CollectionError::from_str("passwd is inaccessible"))
             } else {
                 // SAFETY: We return early if passwd is null.
                 let username = unsafe { std::ffi::CStr::from_ptr((*passwd).pw_name) }
                     .to_str()
-                    .map_err(|err| BottomError::GenericError(err.to_string()))?
+                    .map_err(|err| CollectionError::General(err.into()))?
                     .to_string();
                 self.uid_user_mapping.insert(uid, username.clone());
 

src/data_collection/processes/windows.rs

@@ -3,12 +3,13 @@
 use std::time::Duration;
 
 use super::ProcessHarvest;
+use crate::data_collection::error::CollectionResult;
 use crate::data_collection::DataCollector;
 
 // TODO: There's a lot of shared code with this and the unix impl.
 pub fn sysinfo_process_data(
     collector: &mut DataCollector,
-) -> crate::utils::error::Result<Vec<ProcessHarvest>> {
+) -> CollectionResult<Vec<ProcessHarvest>> {
     let sys = &collector.sys.system;
     let users = &collector.sys.users;
     let use_current_cpu_total = collector.use_current_cpu_total;

src/main.rs

@@ -52,7 +52,6 @@ use data_conversion::*;
 use event::{handle_key_event_or_break, handle_mouse_event, BottomEvent, CollectionThreadEvent};
 use options::{args, get_color_scheme, get_config_path, get_or_create_config, init_app};
 use tui::{backend::CrosstermBackend, Terminal};
-use utils::error;
 #[allow(unused_imports)]
 use utils::logging::*;
 
@@ -64,10 +63,10 @@ use utils::logging::*;
 fn try_drawing(
     terminal: &mut Terminal<CrosstermBackend<std::io::Stdout>>, app: &mut App,
     painter: &mut canvas::Painter,
-) -> error::Result<()> {
+) -> anyhow::Result<()> {
     if let Err(err) = painter.draw_data(terminal, app) {
         cleanup_terminal(terminal)?;
-        Err(err)
+        Err(err.into())
     } else {
         Ok(())
     }
@@ -76,7 +75,7 @@ fn try_drawing(
 /// Clean up the terminal before returning it to the user.
 fn cleanup_terminal(
     terminal: &mut Terminal<CrosstermBackend<std::io::Stdout>>,
-) -> error::Result<()> {
+) -> anyhow::Result<()> {
     disable_raw_mode()?;
     execute!(
         terminal.backend_mut(),

src/utils/error.rs

@@ -8,16 +8,7 @@ pub type Result<T> = result::Result<T, BottomError>;
 /// An error that can occur while Bottom runs.
 #[derive(Debug, Error, PartialEq, Eq)]
 pub enum BottomError {
-    /// An error when there is an IO exception.
-    #[error("IO exception, {0}")]
-    InvalidIo(String),
     /// An error to represent generic errors.
     #[error("Error, {0}")]
     GenericError(String),
 }
-
-impl From<std::io::Error> for BottomError {
-    fn from(err: std::io::Error) -> Self {
-        BottomError::InvalidIo(err.to_string())
-    }
-}