diff --git a/Cargo.toml b/Cargo.toml index 714b38dc..33ddb7c3 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -65,6 +65,7 @@ humantime = "^2.2.0" bytesize = "^2.0.1" hashbrown = "^0.16.0" ouroboros = "^0.18.5" +thiserror = "^2.0.11" [package] name = "ixa" @@ -110,6 +111,7 @@ bytesize.workspace = true hashbrown.workspace = true xxhash-rust.workspace = true rustc-hash.workspace = true +thiserror.workspace = true # Building docs clap-markdown = { workspace = true, optional = true } diff --git a/ixa-bench/Cargo.toml b/ixa-bench/Cargo.toml index 6af46755..a1bf3147 100644 --- a/ixa-bench/Cargo.toml +++ b/ixa-bench/Cargo.toml @@ -41,6 +41,7 @@ anyhow.workspace = true ctor.workspace = true nohash-hasher = "0.2.0" tempfile.workspace = true +thiserror.workspace = true [lib] bench = false diff --git a/ixa-bench/src/check_criterion_regressions.rs b/ixa-bench/src/check_criterion_regressions.rs index 1718b592..d6fd5dbd 100644 --- a/ixa-bench/src/check_criterion_regressions.rs +++ b/ixa-bench/src/check_criterion_regressions.rs @@ -4,6 +4,7 @@ use std::{env, fs}; use ixa::{HashSet, HashSetExt}; use serde_json::Value; +use thiserror::Error; struct Est { group: String, @@ -15,6 +16,32 @@ struct Est { type TableRow = (String, String, String, String, String); +#[derive(Error, Debug)] +enum ReadEstError { + #[error("read error {path}: {source}")] + ReadFile { + path: String, + #[source] + source: std::io::Error, + }, + #[error("json parse {path}: {source}")] + JsonParse { + path: String, + #[source] + source: serde_json::Error, + }, + #[error("missing mean")] + MissingMean, + #[error("missing point_estimate")] + MissingPointEstimate, + #[error("missing confidence_interval")] + MissingConfidenceInterval, + #[error("missing lower_bound")] + MissingLowerBound, + #[error("missing upper_bound")] + MissingUpperBound, +} + fn find_change_files(base: &Path) -> Vec<(String, String, std::path::PathBuf)> { let mut results = Vec::new(); if !base.exists() { @@ -61,27 +88,32 @@ fn find_change_files(base: &Path) -> Vec<(String, String, std::path::PathBuf)> { results } -fn read_est(path: &Path) -> Result<(f64, f64, f64), String> { - let data = - fs::read_to_string(path).map_err(|e| format!("read error {}: {}", path.display(), e))?; - let v: Value = - serde_json::from_str(&data).map_err(|e| format!("json parse {}: {}", path.display(), e))?; - let mean = v.get("mean").ok_or_else(|| "missing mean".to_string())?; +fn read_est(path: &Path) -> Result<(f64, f64, f64), ReadEstError> { + let path_str = path.display().to_string(); + let data = fs::read_to_string(path).map_err(|source| ReadEstError::ReadFile { + path: path_str.clone(), + source, + })?; + let v: Value = serde_json::from_str(&data).map_err(|source| ReadEstError::JsonParse { + path: path_str.clone(), + source, + })?; + let mean = v.get("mean").ok_or(ReadEstError::MissingMean)?; let pe = mean .get("point_estimate") .and_then(|x| x.as_f64()) - .ok_or_else(|| "missing point_estimate".to_string())?; + .ok_or(ReadEstError::MissingPointEstimate)?; let ci = mean .get("confidence_interval") - .ok_or_else(|| "missing confidence_interval".to_string())?; + .ok_or(ReadEstError::MissingConfidenceInterval)?; let lb = ci .get("lower_bound") .and_then(|x| x.as_f64()) - .ok_or_else(|| "missing lower_bound".to_string())?; + .ok_or(ReadEstError::MissingLowerBound)?; let ub = ci .get("upper_bound") .and_then(|x| x.as_f64()) - .ok_or_else(|| "missing upper_bound".to_string())?; + .ok_or(ReadEstError::MissingUpperBound)?; Ok((pe, lb, ub)) } diff --git a/src/debugger.rs b/src/debugger.rs index c5d0d43f..a73168e7 100644 --- a/src/debugger.rs +++ b/src/debugger.rs @@ -2,9 +2,20 @@ use std::fmt::Write; use clap::{ArgMatches, Command, FromArgMatches, Parser, Subcommand}; use rustyline; +use thiserror::Error; use crate::external_api::{breakpoint, global_properties, halt, next, run_ext_api}; -use crate::{define_data_plugin, trace, Context, HashMap, HashMapExt, IxaError}; +use crate::{define_data_plugin, trace, Context, HashMap, HashMapExt}; + +#[derive(Error, Debug)] +enum DebuggerError { + #[error("error splitting line")] + SplitLine, + #[error(transparent)] + Clap(#[from] clap::Error), + #[error("unknown command: {command}")] + UnknownCommand { command: String }, +} trait DebuggerCommand { /// Handle the command and any inputs; returning true will exit the debugger @@ -12,7 +23,7 @@ trait DebuggerCommand { &self, context: &mut Context, matches: &ArgMatches, - ) -> Result<(bool, Option), String>; + ) -> Result<(bool, Option), DebuggerError>; fn extend(&self, command: Command) -> Command; } @@ -59,13 +70,12 @@ impl Debugger { &self, l: &str, context: &mut Context, - ) -> Result<(bool, Option), String> { - let args = shlex::split(l).ok_or("Error splitting lines")?; + ) -> Result<(bool, Option), DebuggerError> { + let args = shlex::split(l).ok_or(DebuggerError::SplitLine)?; let matches = self .cli .clone() // cli can only be used once. - .try_get_matches_from(args) - .map_err(|e| e.to_string())?; + .try_get_matches_from(args)?; if let Some((command, _)) = matches.subcommand() { // If the provided command is known, run its handler @@ -74,7 +84,9 @@ impl Debugger { return handler.handle(context, &matches); } // Unexpected command: print an error - return Err(format!("error: Unknown command: {command}")); + return Err(DebuggerError::UnknownCommand { + command: command.to_string(), + }); } unreachable!("subcommand required"); @@ -87,11 +99,10 @@ impl DebuggerCommand for GlobalPropertyCommand { &self, context: &mut Context, matches: &ArgMatches, - ) -> Result<(bool, Option), String> { + ) -> Result<(bool, Option), DebuggerError> { let args = global_properties::Args::from_arg_matches(matches).unwrap(); let ret = run_ext_api::(context, &args); match ret { - Err(IxaError::IxaError(e)) => Ok((false, Some(format!("error: {e}")))), Err(e) => Ok((false, Some(format!("error: {e}")))), Ok(global_properties::Retval::List(properties)) => Ok(( false, @@ -116,7 +127,7 @@ impl DebuggerCommand for HaltCommand { &self, context: &mut Context, _matches: &ArgMatches, - ) -> Result<(bool, Option), String> { + ) -> Result<(bool, Option), DebuggerError> { context.shutdown(); Ok((true, None)) } @@ -132,7 +143,7 @@ impl DebuggerCommand for NextCommand { &self, context: &mut Context, _matches: &ArgMatches, - ) -> Result<(bool, Option), String> { + ) -> Result<(bool, Option), DebuggerError> { // We execute directly instead of setting `Context::break_requested` so as not to interfere // with anything else that might be requesting a break, or in case debugger sessions become // stateful. @@ -151,10 +162,10 @@ impl DebuggerCommand for BreakpointCommand { &self, context: &mut Context, matches: &ArgMatches, - ) -> Result<(bool, Option), String> { + ) -> Result<(bool, Option), DebuggerError> { let args = breakpoint::Args::from_arg_matches(matches).unwrap(); match run_ext_api::(context, &args) { - Err(IxaError::IxaError(e)) => Ok((false, Some(format!("error: {e}")))), + Err(e) => Ok((false, Some(format!("error: {e}")))), Ok(return_value) => { match return_value { breakpoint::Retval::List(bp_list) => { @@ -169,7 +180,6 @@ impl DebuggerCommand for BreakpointCommand { Ok((false, None)) } - _ => unimplemented!(), } } fn extend(&self, command: Command) -> Command { @@ -188,7 +198,7 @@ impl DebuggerCommand for ContinueCommand { &self, _context: &mut Context, _matches: &ArgMatches, - ) -> Result<(bool, Option), String> { + ) -> Result<(bool, Option), DebuggerError> { Ok((true, None)) } fn extend(&self, command: Command) -> Command { @@ -384,6 +394,7 @@ mod tests { assert!(result.is_err()); assert!(result .unwrap_err() + .to_string() .contains("required arguments were not provided")); } @@ -391,7 +402,7 @@ mod tests { fn test_cli_debugger_global_get_unregistered_prop() { let context = &mut Context::new(); let (_quits, output) = process_line("global get NotExist\n", context); - assert_eq!(output.unwrap(), "error: No global property: NotExist"); + assert_eq!(output.unwrap(), "error: no global property: NotExist"); } #[test] @@ -411,7 +422,7 @@ mod tests { let (_quits, output) = process_line("global get ixa.EmptyGlobal\n", context); assert_eq!( output.unwrap(), - "error: Property ixa.EmptyGlobal is not set" + "error: property ixa.EmptyGlobal is not set" ); } diff --git a/src/entity/context_extension.rs b/src/entity/context_extension.rs index 4f02623c..c4b32120 100644 --- a/src/entity/context_extension.rs +++ b/src/entity/context_extension.rs @@ -123,7 +123,7 @@ impl ContextEntitiesExt for Context { // Check that all required properties are present. if !PL::contains_required_properties() { - return Err("initialization list is missing required properties".into()); + return Err(IxaError::MissingRequiredInitializationProperties); } // Now that we know we will succeed, we create the entity. @@ -552,7 +552,7 @@ mod tests { assert!(matches!( result, - Err(crate::IxaError::IxaError(ref msg)) if msg == "initialization list is missing required properties" + Err(crate::IxaError::MissingRequiredInitializationProperties) )); } diff --git a/src/entity/property_list.rs b/src/entity/property_list.rs index 3fd1bae9..3978a26b 100644 --- a/src/entity/property_list.rs +++ b/src/entity/property_list.rs @@ -103,11 +103,10 @@ macro_rules! impl_property_list { for i in 0..$ct - 1 { for j in (i + 1)..$ct { if property_type_ids[i] == property_type_ids[j] { - return Err(format!( - "the same property appears in both position {} and {} in the property list", - i, - j - ).into()); + return Err(IxaError::DuplicatePropertyInPropertyList { + first_index: i, + second_index: j, + }); } } } diff --git a/src/error.rs b/src/error.rs index 05b9d0ee..16ac0ede 100644 --- a/src/error.rs +++ b/src/error.rs @@ -1,67 +1,68 @@ //! Provides [`IxaError`] and wraps other errors. -use std::fmt::{self, Debug, Display}; use std::io; -#[derive(Debug)] +use thiserror::Error; + +#[derive(Error, Debug)] #[allow(clippy::module_name_repetitions)] /// Provides [`IxaError`] and maps to other errors to /// convert to an [`IxaError`] pub enum IxaError { - IoError(io::Error), - JsonError(serde_json::Error), - CsvError(csv::Error), - Utf8Error(std::string::FromUtf8Error), - ParseIntError(std::num::ParseIntError), - IxaError(String), -} + #[error(transparent)] + IoError(#[from] io::Error), + #[error(transparent)] + JsonError(#[from] serde_json::Error), + #[error(transparent)] + CsvError(#[from] csv::Error), + #[error(transparent)] + Utf8Error(#[from] std::string::FromUtf8Error), + #[error(transparent)] + ParseIntError(#[from] std::num::ParseIntError), -impl From for IxaError { - fn from(error: io::Error) -> Self { - IxaError::IoError(error) - } -} - -impl From for IxaError { - fn from(error: serde_json::Error) -> Self { - IxaError::JsonError(error) - } -} + #[error("duplicate property {name}")] + DuplicateProperty { name: String }, + #[error("entry already exists")] + EntryAlreadyExists, + #[error("no global property: {name}")] + NoGlobalProperty { name: String }, + #[error("property {name} is not set")] + PropertyNotSet { name: String }, -impl From for IxaError { - fn from(error: csv::Error) -> Self { - IxaError::CsvError(error) - } -} + #[error("illegal value for `{field}`: {value}")] + IllegalGlobalPropertyValue { field: String, value: String }, -impl From for IxaError { - fn from(error: std::string::FromUtf8Error) -> Self { - IxaError::Utf8Error(error) - } -} + #[error( + "the same property appears in both position {first_index} and {second_index} in the property list" + )] + DuplicatePropertyInPropertyList { + first_index: usize, + second_index: usize, + }, -impl From for IxaError { - fn from(error: std::num::ParseIntError) -> Self { - IxaError::ParseIntError(error) - } -} + #[error("breakpoint time {time} is in the past")] + BreakpointTimeInPast { time: f64 }, + #[error("attempted to delete a nonexistent breakpoint {id}")] + BreakpointNotFound { id: u32 }, -impl From for IxaError { - fn from(error: String) -> Self { - IxaError::IxaError(error) - } -} + #[error("invalid key in pair: {pair}")] + InvalidLogLevelKey { pair: String }, + #[error("invalid value in pair: {pair}")] + InvalidLogLevelValue { pair: String }, + #[error("invalid log level: {level}")] + InvalidLogLevel { level: String }, -impl From<&str> for IxaError { - fn from(error: &str) -> Self { - IxaError::IxaError(error.to_string()) - } -} + #[error("invalid log level format: {log_level}")] + InvalidLogLevelFormat { log_level: String }, -impl std::error::Error for IxaError {} + #[error("cannot make edge to self")] + CannotMakeEdgeToSelf, + #[error("invalid weight")] + InvalidWeight, + #[error("edge already exists")] + EdgeAlreadyExists, + #[error("can't sample from empty list")] + CannotSampleFromEmptyList, -impl Display for IxaError { - fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - write!(f, "Error: {self:?}")?; - Ok(()) - } + #[error("initialization list is missing required properties")] + MissingRequiredInitializationProperties, } diff --git a/src/external_api.rs b/src/external_api.rs index c03cb8e9..3606fdc8 100644 --- a/src/external_api.rs +++ b/src/external_api.rs @@ -69,7 +69,7 @@ pub(crate) mod global_properties { let output = context.get_serialized_value_by_string(name)?; match output { Some(value) => Ok(Retval::Value(value)), - None => Err(IxaError::IxaError(format!("Property {name} is not set"))), + None => Err(IxaError::PropertyNotSet { name: name.clone() }), } } } @@ -154,9 +154,7 @@ pub(crate) mod breakpoint { #[allow(unused_variables)] ArgsEnum::Set { time, console } => { if *time < context.get_current_time() { - return Err(IxaError::from(format!( - "Breakpoint time {time} is in the past" - ))); + return Err(IxaError::BreakpointTimeInPast { time: *time }); } context.schedule_debugger(*time, None, Box::new(enter_debugger)); @@ -170,9 +168,7 @@ pub(crate) mod breakpoint { trace!("Deleting breakpoint {id}"); let cancelled = context.delete_breakpoint(u64::from(*id)); if cancelled.is_none() { - Err(IxaError::from(format!( - "Attempted to delete a nonexistent breakpoint {id}", - ))) + Err(IxaError::BreakpointNotFound { id: *id }) } else { Ok(Retval::Ok) } diff --git a/src/global_properties.rs b/src/global_properties.rs index a8c5587f..01452ef3 100644 --- a/src/global_properties.rs +++ b/src/global_properties.rs @@ -68,7 +68,9 @@ where let val: T::Value = serde_json::from_value(value)?; T::validate(&val)?; if context.get_global_property_value(T::new()).is_some() { - return Err(IxaError::IxaError(format!("Duplicate property {name}"))); + return Err(IxaError::DuplicateProperty { + name: name.to_string(), + }); } context.set_global_property_value(T::new(), val)?; Ok(()) @@ -130,7 +132,7 @@ impl GlobalPropertiesDataContainer { // Note: If we change global properties to be mutable, we'll need to // update define_derived_person_property to either handle updates or only // allow immutable properties. - Entry::Occupied(_) => Err(IxaError::from("Entry already exists")), + Entry::Occupied(_) => Err(IxaError::EntryAlreadyExists), } } @@ -229,7 +231,9 @@ impl ContextGlobalPropertiesExt for Context { let accessor = get_global_property_accessor(name); match accessor { Some(accessor) => (accessor.getter)(self), - None => Err(IxaError::from(format!("No global property: {name}"))), + None => Err(IxaError::NoGlobalProperty { + name: name.to_string(), + }), } } @@ -243,7 +247,7 @@ impl ContextGlobalPropertiesExt for Context { if let Some(accessor) = get_global_property_accessor(&k) { (accessor.setter)(self, &k, v)?; } else { - return Err(IxaError::from(format!("No global property: {k}"))); + return Err(IxaError::NoGlobalProperty { name: k }); } } @@ -381,9 +385,7 @@ mod test { let path = std::path::Path::new(env!("CARGO_MANIFEST_DIR")) .join("tests/data/global_properties_missing.json"); match context.load_global_properties(&path) { - Err(IxaError::IxaError(msg)) => { - assert_eq!(msg, "No global property: ixa.PropertyUnknown"); - } + Err(IxaError::NoGlobalProperty { name }) => assert_eq!(name, "ixa.PropertyUnknown"), _ => panic!("Unexpected error type"), } } @@ -409,7 +411,7 @@ mod test { context.load_global_properties(&path).unwrap(); let error = context.load_global_properties(&path); match error { - Err(IxaError::IxaError(_)) => {} + Err(IxaError::DuplicateProperty { .. }) => {} _ => panic!("Unexpected error type"), } } @@ -421,10 +423,10 @@ mod test { define_global_property!(Property3, Property3Type, |v: &Property3Type| { match v.field_int { 0 => Ok(()), - _ => Err(IxaError::IxaError(format!( - "Illegal value for `field_int`: {}", - v.field_int - ))), + _ => Err(IxaError::IllegalGlobalPropertyValue { + field: "field_int".to_string(), + value: v.field_int.to_string(), + }), } }); @@ -441,7 +443,7 @@ mod test { let mut context = Context::new(); assert!(matches!( context.set_global_property_value(Property3, Property3Type { field_int: 1 }), - Err(IxaError::IxaError(_)) + Err(IxaError::IllegalGlobalPropertyValue { .. }) )); } @@ -460,7 +462,7 @@ mod test { .join("tests/data/global_properties_invalid.json"); assert!(matches!( context.load_global_properties(&path), - Err(IxaError::IxaError(_)) + Err(IxaError::IllegalGlobalPropertyValue { .. }) )); } diff --git a/src/network/mod.rs b/src/network/mod.rs index 8fdec8ca..af629745 100644 --- a/src/network/mod.rs +++ b/src/network/mod.rs @@ -57,10 +57,10 @@ impl NetworkData { inner: ET, ) -> Result<(), IxaError> { if entity_id == neighbor { - return Err(IxaError::IxaError(String::from("Cannot make edge to self"))); + return Err(IxaError::CannotMakeEdgeToSelf); } if weight.is_infinite() || weight.is_nan() || weight.is_sign_negative() { - return Err(IxaError::IxaError(String::from("Invalid weight"))); + return Err(IxaError::InvalidWeight); } let edge = Edge { @@ -303,9 +303,7 @@ pub trait ContextNetworkExt: ContextBase + ContextRandomExt { { let edges = self.get_edges::(entity_id); if edges.is_empty() { - return Err(IxaError::IxaError(String::from( - "Can't sample from empty list", - ))); + return Err(IxaError::CannotSampleFromEmptyList); } let weights: Vec<_> = edges.iter().map(|x| x.weight).collect(); @@ -432,7 +430,7 @@ mod test_inner { assert!(matches!( nd.add_edge::(PersonId::new(1), PersonId::new(2), 0.02, EdgeType1), - Err(IxaError::IxaError(_)) + Err(IxaError::EdgeAlreadyExists) )); } @@ -474,7 +472,7 @@ mod test_inner { let result = nd.add_edge::(PersonId::new(1), PersonId::new(1), 0.01, EdgeType1); - assert!(matches!(result, Err(IxaError::IxaError(_)))); + assert!(matches!(result, Err(IxaError::CannotMakeEdgeToSelf))); } #[test] @@ -483,7 +481,7 @@ mod test_inner { let result = nd.add_edge::(PersonId::new(1), PersonId::new(2), -1.0, EdgeType1); - assert!(matches!(result, Err(IxaError::IxaError(_)))); + assert!(matches!(result, Err(IxaError::InvalidWeight))); let result = nd.add_edge::( PersonId::new(1), @@ -491,7 +489,7 @@ mod test_inner { f32::NAN, EdgeType1, ); - assert!(matches!(result, Err(IxaError::IxaError(_)))); + assert!(matches!(result, Err(IxaError::InvalidWeight))); let result = nd.add_edge::( PersonId::new(1), @@ -499,7 +497,7 @@ mod test_inner { f32::INFINITY, EdgeType1, ); - assert!(matches!(result, Err(IxaError::IxaError(_)))); + assert!(matches!(result, Err(IxaError::InvalidWeight))); } #[test] diff --git a/src/network/network.rs b/src/network/network.rs index 1054ff8f..7cb4490e 100644 --- a/src/network/network.rs +++ b/src/network/network.rs @@ -93,7 +93,7 @@ impl + 'static> Network { // Enforce uniqueness by neighbor if edges.iter().any(|e| e.neighbor == edge.neighbor) { - return Err(IxaError::IxaError("Edge already exists".into())); + return Err(IxaError::EdgeAlreadyExists); } edges.push(edge); diff --git a/src/runner.rs b/src/runner.rs index f35a0e90..3068512d 100644 --- a/src/runner.rs +++ b/src/runner.rs @@ -18,18 +18,19 @@ use crate::report::ContextReportExt; use crate::{set_log_level, set_module_filters, warn, LevelFilter}; /// Custom parser for log levels -fn parse_log_levels(s: &str) -> Result, String> { +fn parse_log_levels(s: &str) -> Result, IxaError> { s.split(',') .map(|pair| { let mut iter = pair.split('='); - let key = iter - .next() - .ok_or_else(|| format!("Invalid key in pair: {pair}"))?; - let value = iter - .next() - .ok_or_else(|| format!("Invalid value in pair: {pair}"))?; - let level = - LevelFilter::from_str(value).map_err(|_| format!("Invalid log level: {value}"))?; + let key = iter.next().ok_or_else(|| IxaError::InvalidLogLevelKey { + pair: pair.to_string(), + })?; + let value = iter.next().ok_or_else(|| IxaError::InvalidLogLevelValue { + pair: pair.to_string(), + })?; + let level = LevelFilter::from_str(value).map_err(|_| IxaError::InvalidLogLevel { + level: value.to_string(), + })?; Ok((key.to_string(), level)) }) .collect() @@ -260,16 +261,19 @@ where if let Some(log_level) = args.log_level.as_ref() { if let Ok(level) = LevelFilter::from_str(log_level) { current_log_level = level; - } else if let Ok(log_levels) = parse_log_levels(log_level) { - let log_levels_slice: Vec<(&String, LevelFilter)> = - log_levels.iter().map(|(k, v)| (k, *v)).collect(); - set_module_filters(log_levels_slice.as_slice()); - for (key, value) in log_levels { - println!("Logging enabled for {key} at level {value}"); - // Here you can set the log level for each key-value pair as needed - } } else { - return Err(format!("Invalid log level format: {log_level}").into()); + match parse_log_levels(log_level) { + Ok(log_levels) => { + let log_levels_slice: Vec<(&String, LevelFilter)> = + log_levels.iter().map(|(k, v)| (k, *v)).collect(); + set_module_filters(log_levels_slice.as_slice()); + for (key, value) in log_levels { + println!("Logging enabled for {key} at level {value}"); + // Here you can set the log level for each key-value pair as needed + } + } + Err(e) => return Err(Box::new(e)), + } } }