Improve error handling (#13)

This addresses the following situations that right now were failing
with a rather confusing error message:

- Not existent syscall name
- PID that doesn't exist
- Processes that are not Ruby
- Lack of samples when sampling the CPU, as this might be due to the
  application being mostly IO bound and not running on the CPU for long.
Will revamp this and add extra logic to detect for this case in another PR

Signed-off-by: Francisco Javier Honduvilla Coto <javierhonduco@gmail.com>

Cargo.toml

@@ -24,6 +24,7 @@ libc = "0.2.125"
 log = "0.4.17"
 env_logger = "0.9.0"
 serde_yaml = "0.8"
+thiserror = "1.0.31"
 
 [dev-dependencies]
 nix = "0.23.1"

src/events.rs

@@ -8,6 +8,13 @@ use log::debug;
 
 use perf_event_open_sys as sys;
 use perf_event_open_sys::bindings::{perf_event_attr, PERF_FLAG_FD_CLOEXEC};
+use thiserror::Error;
+
+#[derive(Error, Debug)]
+pub enum EventError {
+    #[error("syscall {name:?} doesn't exist")]
+    EventNameDoesNotExist { name: String },
+}
 
 // This crate bindings have been generated in a x86 machine, including
 // the syscall number. Turns out different architectures have different
@@ -69,7 +76,10 @@ pub unsafe fn setup_syscall_event(syscall: &str) -> Result<c_int> {
         "/sys/kernel/debug/tracing/events/syscalls/sys_{}/id",
         syscall
     );
-    let mut id = fs::read_to_string(&path)?;
+    let mut id = fs::read_to_string(&path).map_err(|_| EventError::EventNameDoesNotExist {
+        name: syscall.to_string(),
+    })?;
+
     id.pop(); // Remove newline
     debug!("syscall with id {} found in {}", id, &path);
 

src/main.rs

@@ -4,9 +4,9 @@ use inferno::flamegraph;
 use std::fs;
 use std::fs::File;
 
-use anyhow::Result;
 use clap::Parser;
 
+use anyhow::{anyhow, Result};
 use rbperf::profile::Profile;
 use rbperf::rbperf::{Rbperf, RbperfEvent, RbperfOptions};
 
@@ -36,7 +36,7 @@ struct RecordSubcommand {
     ringbuf: bool,
 }
 
-#[derive(clap::Subcommand, Debug)]
+#[derive(clap::Subcommand, Debug, PartialEq)]
 enum RecordType {
     Cpu,
     Syscall { name: String },
@@ -53,7 +53,7 @@ fn main() -> Result<()> {
                 RecordType::Cpu => RbperfEvent::Cpu {
                     sample_period: 99999,
                 },
-                RecordType::Syscall { name } => RbperfEvent::Syscall(name),
+                RecordType::Syscall { ref name } => RbperfEvent::Syscall(name.clone()),
             };
             let options = RbperfOptions {
                 event,
@@ -69,6 +69,10 @@ fn main() -> Result<()> {
             r.start(duration, &mut profile)?;
             let folded = profile.folded();
 
+            if record.record_type == RecordType::Cpu && folded.is_empty() {
+                return Err(anyhow!("No stacks were collected. This might mean that this process is mostly IO bound. If you believe that this might be a bug, please open an issue at https://github.com/javierhonduco/rbperf. Thanks!"));
+            }
+
             let mut options = flamegraph::Options::default();
             let data = folded.as_bytes();
             let now: DateTime<Utc> = Utc::now();

src/process.rs

@@ -1,12 +1,21 @@
 use std::fmt;
 use std::path::PathBuf;
 
-use anyhow::{anyhow, Result};
+use anyhow::anyhow;
 use log::debug;
+use thiserror::Error;
 
 use crate::binary::{ruby_current_vm_address, ruby_version};
 use proc_maps::{get_process_maps, Pid};
 
+#[derive(Error, Debug)]
+pub enum ProcessError {
+    #[error("process with pid {pid:?} is not running")]
+    ProcessDoesNotExist { pid: Pid },
+    #[error("process with pid {pid:?} doesn't seem like a Ruby process")]
+    NotRuby { pid: Pid },
+}
+
 pub struct ProcessInfo {
     pub pid: Pid,
     pub ruby_version: String,
@@ -31,22 +40,25 @@ impl fmt::Display for ProcessInfo {
     }
 }
 
-fn find_libruby(pid: Pid) -> Option<(u64, PathBuf)> {
-    let maps = get_process_maps(pid).unwrap();
+fn find_libruby(pid: Pid) -> Result<Option<(u64, PathBuf)>, ProcessError> {
+    let maps = get_process_maps(pid).map_err(|_| ProcessError::ProcessDoesNotExist { pid })?;
     // https://github.com/rust-lang/rust/issues/62358
     for map in maps {
         if let Some(s) = map.filename() {
-            if s.to_str()?.contains("libruby") {
-                return Some((map.start() as u64, map.filename().unwrap().to_path_buf()));
+            if s.to_str().unwrap().contains("libruby") {
+                return Ok(Some((
+                    map.start() as u64,
+                    map.filename().unwrap().to_path_buf(),
+                )));
             }
         }
     }
-    None
+    Err(ProcessError::NotRuby { pid: pid })
 }
 
 impl ProcessInfo {
-    pub fn new(pid: Pid) -> Result<Self> {
-        let libruby = find_libruby(pid as Pid);
+    pub fn new(pid: Pid) -> Result<Self, anyhow::Error> {
+        let libruby = find_libruby(pid as Pid)?;
 
         let mut bin_path = PathBuf::new();
         bin_path.push("/proc/");
@@ -57,7 +69,8 @@ impl ProcessInfo {
             bin_path.push(l.1.clone().strip_prefix("/").expect("remove prefix"))
         }
 
-        let ruby_version = ruby_version(&bin_path)?;
+        println!("{:?}", libruby);
+        let ruby_version = ruby_version(&bin_path).unwrap();
 
         debug!("Binary {:?}", bin_path);
         let symbol = ruby_current_vm_address(&bin_path, &ruby_version)?;

src/rbperf.rs

@@ -169,7 +169,7 @@ impl<'a> Rbperf<'a> {
         }
     }
 
-    fn add_process_info(&mut self, process_info: ProcessInfo) -> Result<()> {
+    fn add_process_info(&mut self, process_info: &ProcessInfo) -> Result<()> {
         // Set the per-process data
         let mut matching_version: Option<(i32, &RubyVersion)> = None;
         for (i, ruby_version) in self.ruby_versions.iter().enumerate() {
@@ -215,13 +215,13 @@ impl<'a> Rbperf<'a> {
 
         Ok(())
     }
-    pub fn add_pid(&mut self, pid: Pid) -> Result<()> {
+    pub fn add_pid(&mut self, pid: Pid) -> Result<ProcessInfo> {
         // Fetch and add process info
         let process_info = ProcessInfo::new(pid)?;
         eprintln!("{}", process_info);
-        self.add_process_info(process_info)?;
+        self.add_process_info(&process_info)?;
 
-        Ok(())
+        Ok(process_info)
     }
 
     pub fn start(mut self, duration: std::time::Duration, profile: &mut Profile) -> Result<()> {