Remove all panics and other improvements

Removed all panics, new error types. Removed some code from `run` into
their own functions. Bumped some dependencies.

inquisitor-core/Cargo.toml

@@ -1,7 +1,7 @@
 [package]
 authors = ["Y. D. Santos <[email protected]>"]
 name = "inquisitor-core"
-version = "0.10.0"
+version = "0.11.0"
 edition = "2021"
 license = "MIT"
 description = "Simple and fast load testing library"
@@ -14,9 +14,9 @@ categories = ["development-tools", "development-tools::profiling",
 "network-programming", "web-programming", "command-line-utilities"]
 
 [dependencies]
-futures = "0.3"
-reqwest = "0.11"
-tokio = { version = "1", features = ["sync", "macros", "rt-multi-thread"] }
-regex = "1"
-hdrhistogram = "7"
-ctrlc = { version = "3.0", features = ["termination"] }
+futures = "0.3.28"
+reqwest = "0.11.18"
+tokio = { version = "1.32.0", features = ["sync", "macros", "rt-multi-thread"] }
+regex = "1.9.3"
+hdrhistogram = "7.5.2"
+ctrlc = { version = "3.4.0", features = ["termination"] }

inquisitor-core/src/config.rs

@@ -1,3 +1,4 @@
+use std::path::PathBuf;
 use std::time::Duration;
 
 /// Default run duration
@@ -59,7 +60,7 @@ pub struct Config {
     pub duration: Option<Duration>,
     /// Path to a root CA certificate in PEM format, to be added to the request
     /// client's list of trusted CA certificates.
-    pub ca_cert: Option<String>,
+    pub ca_cert: Option<PathBuf>,
 }
 
 impl Config {

inquisitor-core/src/error.rs

@@ -1,3 +1,5 @@
+use std::path::PathBuf;
+
 pub type Result<T> = std::result::Result<T, InquisitorError>;
 
 /// Error type for this library
@@ -7,12 +9,15 @@ pub enum InquisitorError {
     SignalHandlerCouldNotBeSet(ctrlc::Error),
     HistogramCouldNotBeCreated(hdrhistogram::CreationError),
     InvalidRegex(regex::Error),
-    CouldNotOpenFile(std::io::Error, String),
-    CouldNotReadFile(std::io::Error, String),
-    CouldNotConvertCert(reqwest::Error, String),
+    CouldNotOpenFile(std::io::Error, PathBuf),
+    CouldNotReadFile(std::io::Error, PathBuf),
+    CouldNotConvertCert(reqwest::Error, PathBuf),
     FailedToCreateClient(reqwest::Error),
     FailedToGetTimeInterval(std::time::SystemTimeError),
     FailedToBuildAsyncRuntime(tokio::io::Error),
+    FailedToUnwrapArc,
+    FailedToReadResponseBody(reqwest::Error),
+    FailedToRecordToHistogram(hdrhistogram::RecordError),
 }
 
 impl std::fmt::Display for InquisitorError {
@@ -24,14 +29,28 @@ impl std::fmt::Display for InquisitorError {
                 write!(f, "Could not create histogram for response times: {e}")
             }
             Self::InvalidRegex(e) => write!(f, "Invalid regex: {e}"),
-            Self::CouldNotOpenFile(e, file) => write!(f, "Could not open file '{file}': {e}"),
-            Self::CouldNotReadFile(e, file) => write!(f, "Could not read file '{file}': {e}"),
+            Self::CouldNotOpenFile(e, file) => {
+                write!(f, "Could not open file '{}': {e}", file.to_string_lossy())
+            }
+            Self::CouldNotReadFile(e, file) => {
+                write!(f, "Could not read file '{}': {e}", file.to_string_lossy())
+            }
             Self::CouldNotConvertCert(e, file) => {
-                write!(f, "Could not convert file '{file}' to PEM certificate: {e}")
+                write!(
+                    f,
+                    "Could not convert file '{}' to PEM certificate: {e}",
+                    file.to_string_lossy()
+                )
             }
             Self::FailedToCreateClient(e) => write!(f, "Failed to build HTTP client: {e}"),
             Self::FailedToGetTimeInterval(e) => write!(f, "Failed to get elapsed time: {e}"),
             Self::FailedToBuildAsyncRuntime(e) => write!(f, "Failed to crate async runtime: {e}"),
+            Self::FailedToUnwrapArc => write!(
+                f,
+                "Bug: Arc used to stored request times could not be unwrapped"
+            ),
+            Self::FailedToReadResponseBody(e) => write!(f, "Failed to read response body: {e}"),
+            Self::FailedToRecordToHistogram(e) => write!(f, "Failed to record to histogram: {e}"),
         }
     }
 }
@@ -61,3 +80,15 @@ impl From<tokio::io::Error> for InquisitorError {
         Self::FailedToBuildAsyncRuntime(e)
     }
 }
+
+impl From<std::time::SystemTimeError> for InquisitorError {
+    fn from(e: std::time::SystemTimeError) -> Self {
+        Self::FailedToGetTimeInterval(e)
+    }
+}
+
+impl From<hdrhistogram::RecordError> for InquisitorError {
+    fn from(e: hdrhistogram::RecordError) -> Self {
+        Self::FailedToRecordToHistogram(e)
+    }
+}

inquisitor-core/src/lib.rs

@@ -2,8 +2,10 @@ use hdrhistogram::Histogram;
 use reqwest::ClientBuilder;
 use std::collections::HashMap;
 use std::io::Read;
+use std::path::PathBuf;
 use std::sync::atomic::{AtomicBool, AtomicUsize, Ordering};
 use std::sync::Arc;
+use std::time::SystemTime;
 use tokio::sync::Mutex;
 
 pub mod error;
@@ -18,20 +20,38 @@ use time::Microseconds;
 /// Default maximum number of HTTP connections used
 pub const MAX_CONNS: usize = 12;
 
-/// Run load tests with the given configuration
-pub fn run<C: Into<Config>>(config: C) -> Result<()> {
-    let config: Config = config.into();
-    let should_exit = Arc::new(AtomicBool::new(false));
-    let should_exit_clone = should_exit.clone();
-
+/// Set a handler to gracefully handle CTRL+C
+fn setup_ctrl_c_handler(should_exit: Arc<AtomicBool>) -> Result<()> {
     ctrlc::set_handler(move || {
-        let previously_set = should_exit_clone.fetch_or(true, Ordering::SeqCst);
+        let previously_set = should_exit.fetch_or(true, Ordering::SeqCst);
 
         if previously_set {
             std::process::exit(130);
         }
     })?;
 
+    Ok(())
+}
+
+/// Get the certificate if specified
+fn get_ca_cert(path: &PathBuf) -> Result<reqwest::Certificate> {
+    let mut buf = Vec::new();
+    std::fs::File::open(path)
+        .map_err(|e| InquisitorError::CouldNotOpenFile(e, path.to_owned()))?
+        .read_to_end(&mut buf)
+        .map_err(|e| InquisitorError::CouldNotReadFile(e, path.to_owned()))?;
+
+    reqwest::Certificate::from_pem(&buf)
+        .map_err(|e| InquisitorError::CouldNotConvertCert(e, path.to_owned()))
+}
+
+/// Run load tests with the given configuration
+pub fn run<C: Into<Config>>(config: C) -> Result<()> {
+    let config: Config = config.into();
+
+    let should_exit = Arc::new(AtomicBool::new(false));
+    setup_ctrl_c_handler(should_exit.clone())?;
+
     let (iterations, duration) = config.iterations_and_duration();
 
     let mut headers = HashMap::new();
@@ -44,14 +64,12 @@ pub fn run<C: Into<Config>>(config: C) -> Result<()> {
     // histogram of response times, recorded in microseconds
     let times = Arc::new(Mutex::new(Histogram::<u64>::new_with_max(
         1_000_000_000_000,
-        3,
+        3, // significant digits
     )?));
 
     let passes = Arc::new(AtomicUsize::new(0));
     let errors = Arc::new(AtomicUsize::new(0));
 
-    let test_start_time = std::time::SystemTime::now();
-
     let failed_regex = match config.failed_body {
         Some(regex) => Some(regex::Regex::new(&regex)?),
         None => None,
@@ -66,18 +84,13 @@ pub fn run<C: Into<Config>>(config: C) -> Result<()> {
         .enable_time()
         .build()?;
 
-    let mut cert = None;
-    if let Some(cert_file) = config.ca_cert.as_deref() {
-        let mut buf = Vec::new();
-        std::fs::File::open(cert_file)
-            .map_err(|e| InquisitorError::CouldNotOpenFile(e, cert_file.to_owned()))?
-            .read_to_end(&mut buf)
-            .map_err(|e| InquisitorError::CouldNotReadFile(e, cert_file.to_owned()))?;
-        cert = Some(
-            reqwest::Certificate::from_pem(&buf)
-                .map_err(|e| InquisitorError::CouldNotConvertCert(e, cert_file.to_owned()))?,
-        );
-    }
+    let cert = if let Some(cert_path) = config.ca_cert.as_ref() {
+        Some(get_ca_cert(cert_path)?)
+    } else {
+        None
+    };
+
+    let test_start_time = SystemTime::now();
 
     for _ in 0..config.connections {
         let mut client = ClientBuilder::new().danger_accept_invalid_certs(config.insecure);
@@ -100,7 +113,7 @@ pub fn run<C: Into<Config>>(config: C) -> Result<()> {
 
         let task = rt.spawn(async move {
             let mut total = passes.load(Ordering::Relaxed) + errors.load(Ordering::Relaxed);
-            let mut total_elapsed = test_start_time.elapsed().unwrap().as_micros() as u64;
+            let mut total_elapsed = test_start_time.elapsed()?.as_micros() as u64;
 
             while total < iterations && total_elapsed < duration {
                 if should_exit.load(Ordering::Relaxed) {
@@ -120,29 +133,30 @@ pub fn run<C: Into<Config>>(config: C) -> Result<()> {
                     builder = builder.header(k, v);
                 }
 
-                let req_start_time = std::time::SystemTime::now();
+                let req_start_time = SystemTime::now();
                 let response = builder.send().await;
-                let elapsed = req_start_time.elapsed().unwrap().as_micros() as u64;
-                times
-                    .lock()
-                    .await
-                    .record(elapsed)
-                    .expect("time out of bounds");
-
-                match response {
-                    Ok(res) if res.status().is_success() && failed_regex.is_none() => {
+                let elapsed = req_start_time.elapsed()?.as_micros() as u64;
+                times.lock().await.record(elapsed)?;
+
+                match (response, failed_regex.as_ref()) {
+                    (Ok(res), _) if res.status().is_success() && failed_regex.is_none() => {
                         passes.fetch_add(1, Ordering::SeqCst);
                         if config.print_response {
                             println!(
                                 "Response successful. Content: {}",
-                                res.text().await.unwrap()
+                                res.text()
+                                    .await
+                                    .map_err(InquisitorError::FailedToReadResponseBody)?
                             );
                         }
                     }
-                    Ok(res) if res.status().is_success() && failed_regex.is_some() => {
-                        let body = res.text().await.unwrap();
+                    (Ok(res), Some(failed_regex)) if res.status().is_success() => {
+                        let body = res
+                            .text()
+                            .await
+                            .map_err(InquisitorError::FailedToReadResponseBody)?;
 
-                        if failed_regex.as_ref().unwrap().is_match(&body) {
+                        if failed_regex.is_match(&body) {
                             if !config.hide_errors {
                                 eprintln!("Response is 200 but body indicates an error: {}", body);
                             }
@@ -155,13 +169,13 @@ pub fn run<C: Into<Config>>(config: C) -> Result<()> {
                             }
                         }
                     }
-                    Ok(res) => {
+                    (Ok(res), _) => {
                         if !config.hide_errors {
                             eprintln!("Response is not 200. Status code: {}", res.status());
                         }
                         errors.fetch_add(1, Ordering::SeqCst);
                     }
-                    Err(e) => {
+                    (Err(e), _) => {
                         if !config.hide_errors {
                             eprintln!("Request failed: {}", e);
                         }
@@ -170,19 +184,23 @@ pub fn run<C: Into<Config>>(config: C) -> Result<()> {
                 };
 
                 total = passes.load(Ordering::Relaxed) + errors.load(Ordering::Relaxed);
-                total_elapsed = test_start_time.elapsed().unwrap().as_micros() as u64;
+                total_elapsed = test_start_time.elapsed()?.as_micros() as u64;
             }
+
+            Result::<()>::Ok(())
         });
 
         handles.push(task);
     }
 
     let times = rt.block_on(async {
         futures::future::join_all(handles).await;
-        Arc::try_unwrap(times)
-            .expect("bug: could not unwrap Arc")
-            .into_inner()
-    });
+        Result::<Histogram<u64>>::Ok(
+            Arc::try_unwrap(times)
+                .map_err(|_| InquisitorError::FailedToUnwrapArc)?
+                .into_inner(),
+        )
+    })?;
 
     let elapsed_us = test_start_time
         .elapsed()

inquisitor-core/src/time.rs

@@ -23,7 +23,7 @@ impl std::fmt::Display for Microseconds {
 /// allowed, and the allowed time units are: seconds (s), minutes (m) and
 /// hours (h).
 pub fn parse_duration(duration: &str) -> Result<Duration, InquisitorError> {
-    let re = regex::Regex::new(r"(\d\d*(?:\.\d\d*)??)([smh])").expect("Bug: wrong regex");
+    let re = regex::Regex::new(r"(\d\d*(?:\.\d\d*)??)([smh])")?;
     let cap = re
         .captures(duration)
         .ok_or(InquisitorError::DurationParseError)?;

inquisitor/Cargo.toml

@@ -1,7 +1,7 @@
 [package]
 authors = ["Y. D. Santos <[email protected]>"]
 name = "inquisitor"
-version = "0.8.3"
+version = "0.8.4"
 edition = "2021"
 license = "MIT"
 description = "Simple and fast load testing tool"
@@ -14,9 +14,9 @@ categories = ["development-tools", "development-tools::profiling",
 "network-programming", "web-programming", "command-line-utilities"]
 
 [dependencies]
-inquisitor-core = { path = "../inquisitor-core", version = "0.10.0" }
-clap = { version = "4", features = ["derive"] }
+inquisitor-core = { path = "../inquisitor-core", version = "0.11.0" }
+clap = { version = "4.3.23", features = ["derive"] }
 
 [dev-dependencies]
-mockito = "0.31"
-regex = "1"
+mockito = "0.31" # TODO needs update
+regex = "1.9.3"

inquisitor/src/main.rs

@@ -2,6 +2,7 @@ use clap::{Parser as _, ValueEnum};
 use inquisitor_core::time::parse_duration;
 use inquisitor_core::{Config, Method, Result, MAX_CONNS};
 use std::time::Duration;
+use std::path::PathBuf;
 
 #[derive(Debug, Copy, Clone, PartialEq, Eq, ValueEnum)]
 enum CliMethod {
@@ -73,7 +74,7 @@ struct Cli {
     /// Path to a root CA certificate in PEM format, to be added to the request
     /// client's list of trusted CA certificates.
     #[clap(long, value_parser)]
-    ca_cert: Option<String>,
+    ca_cert: Option<PathBuf>,
 }
 
 impl From<Cli> for Config {