From 96fbfeaeabac092b02e214fb19d58187fc9e4675 Mon Sep 17 00:00:00 2001 From: Aram Peres Date: Sun, 1 Aug 2021 15:10:27 -0400 Subject: [PATCH] Simplify errors --- rups/src/blocking/mod.rs | 2 +- rups/src/cmd.rs | 184 +++++++++--------------- rups/src/error.rs | 16 ++- rups/src/lib.rs | 2 + rups/src/tokio/mod.rs | 2 +- rupsc/src/parser.rs => rups/src/util.rs | 22 +-- rups/src/var.rs | 10 +- rupsc/src/main.rs | 6 +- 8 files changed, 109 insertions(+), 135 deletions(-) rename rupsc/src/parser.rs => rups/src/util.rs (84%) diff --git a/rups/src/blocking/mod.rs b/rups/src/blocking/mod.rs index 563ff8c..a0f2d5d 100644 --- a/rups/src/blocking/mod.rs +++ b/rups/src/blocking/mod.rs @@ -142,7 +142,7 @@ impl TcpConnection { // Parse args by splitting whitespace, minding quotes for args with multiple words let args = shell_words::split(&raw) - .map_err(|e| NutError::Generic(format!("Parsing server response failed: {}", e)))?; + .map_err(|e| NutError::generic(format!("Parsing server response failed: {}", e)))?; Ok(args) } diff --git a/rups/src/cmd.rs b/rups/src/cmd.rs index 9ec8e08..d58fd29 100644 --- a/rups/src/cmd.rs +++ b/rups/src/cmd.rs @@ -118,23 +118,23 @@ pub enum Response { impl Response { pub fn from_args(mut args: Vec) -> crate::Result { if args.is_empty() { - return Err( - NutError::Generic("Parsing server response failed: empty line".into()).into(), - ); + return Err(ClientError::generic( + "Parsing server response failed: empty line", + )); } let cmd_name = args.remove(0); match cmd_name.as_str() { "OK" => Ok(Self::Ok), "ERR" => { if args.is_empty() { - Err(NutError::Generic("Unspecified server error".into()).into()) + Err(ClientError::generic("Unspecified server error")) } else { let err_type = args.remove(0); match err_type.as_str() { "ACCESS-DENIED" => Err(NutError::AccessDenied.into()), "UNKNOWN-UPS" => Err(NutError::UnknownUps.into()), "FEATURE-NOT-CONFIGURED" => Err(NutError::FeatureNotConfigured.into()), - _ => Err(NutError::Generic(format!( + _ => Err(NutError::generic(format!( "Server error: {} {}", err_type, args.join(" ") @@ -145,14 +145,14 @@ impl Response { } "BEGIN" => { if args.is_empty() { - Err(NutError::Generic("Unspecified BEGIN type".into()).into()) + Err(ClientError::generic("Unspecified BEGIN type")) } else { let begin_type = args.remove(0); if &begin_type != "LIST" { - Err( - NutError::Generic(format!("Unexpected BEGIN type: {}", begin_type)) - .into(), - ) + Err(ClientError::generic(format!( + "Unexpected BEGIN type: {}", + begin_type + ))) } else { let args = shell_words::join(args); Ok(Response::BeginList(args)) @@ -161,14 +161,14 @@ impl Response { } "END" => { if args.is_empty() { - Err(NutError::Generic("Unspecified END type".into()).into()) + Err(ClientError::generic("Unspecified END type")) } else { let begin_type = args.remove(0); if &begin_type != "LIST" { - Err( - NutError::Generic(format!("Unexpected END type: {}", begin_type)) - .into(), - ) + Err(ClientError::generic(format!( + "Unexpected END type: {}", + begin_type + ))) } else { let args = shell_words::join(args); Ok(Response::EndList(args)) @@ -177,23 +177,19 @@ impl Response { } "VAR" => { let _var_device = if args.is_empty() { - Err(ClientError::from(NutError::Generic( - "Unspecified VAR device name in response".into(), - ))) + Err(ClientError::generic( + "Unspecified VAR device name in response", + )) } else { Ok(args.remove(0)) }?; let var_name = if args.is_empty() { - Err(ClientError::from(NutError::Generic( - "Unspecified VAR name in response".into(), - ))) + Err(ClientError::generic("Unspecified VAR name in response")) } else { Ok(args.remove(0)) }?; let var_value = if args.is_empty() { - Err(ClientError::from(NutError::Generic( - "Unspecified VAR value in response".into(), - ))) + Err(ClientError::generic("Unspecified VAR value in response")) } else { Ok(args.remove(0)) }?; @@ -201,23 +197,19 @@ impl Response { } "RW" => { let _var_device = if args.is_empty() { - Err(ClientError::from(NutError::Generic( - "Unspecified RW device name in response".into(), - ))) + Err(ClientError::generic( + "Unspecified RW device name in response", + )) } else { Ok(args.remove(0)) }?; let var_name = if args.is_empty() { - Err(ClientError::from(NutError::Generic( - "Unspecified RW name in response".into(), - ))) + Err(ClientError::generic("Unspecified RW name in response")) } else { Ok(args.remove(0)) }?; let var_value = if args.is_empty() { - Err(ClientError::from(NutError::Generic( - "Unspecified RW value in response".into(), - ))) + Err(ClientError::generic("Unspecified RW value in response")) } else { Ok(args.remove(0)) }?; @@ -225,16 +217,14 @@ impl Response { } "UPS" => { let name = if args.is_empty() { - Err(ClientError::from(NutError::Generic( - "Unspecified UPS name in response".into(), - ))) + Err(ClientError::generic("Unspecified UPS name in response")) } else { Ok(args.remove(0)) }?; let description = if args.is_empty() { - Err(ClientError::from(NutError::Generic( - "Unspecified UPS description in response".into(), - ))) + Err(ClientError::generic( + "Unspecified UPS description in response", + )) } else { Ok(args.remove(0)) }?; @@ -242,16 +232,14 @@ impl Response { } "CLIENT" => { let _device = if args.is_empty() { - Err(ClientError::from(NutError::Generic( - "Unspecified CLIENT device in response".into(), - ))) + Err(ClientError::generic( + "Unspecified CLIENT device in response", + )) } else { Ok(args.remove(0)) }?; let ip_address = if args.is_empty() { - Err(ClientError::from(NutError::Generic( - "Unspecified CLIENT IP in response".into(), - ))) + Err(ClientError::generic("Unspecified CLIENT IP in response")) } else { Ok(args.remove(0)) }?; @@ -259,16 +247,12 @@ impl Response { } "CMD" => { let _device = if args.is_empty() { - Err(ClientError::from(NutError::Generic( - "Unspecified CMD device in response".into(), - ))) + Err(ClientError::generic("Unspecified CMD device in response")) } else { Ok(args.remove(0)) }?; let name = if args.is_empty() { - Err(ClientError::from(NutError::Generic( - "Unspecified CMD name in response".into(), - ))) + Err(ClientError::generic("Unspecified CMD name in response")) } else { Ok(args.remove(0)) }?; @@ -276,23 +260,21 @@ impl Response { } "CMDDESC" => { let _device = if args.is_empty() { - Err(ClientError::from(NutError::Generic( - "Unspecified CMDDESC device in response".into(), - ))) + Err(ClientError::generic( + "Unspecified CMDDESC device in response", + )) } else { Ok(args.remove(0)) }?; let _name = if args.is_empty() { - Err(ClientError::from(NutError::Generic( - "Unspecified CMDDESC name in response".into(), - ))) + Err(ClientError::generic("Unspecified CMDDESC name in response")) } else { Ok(args.remove(0)) }?; let desc = if args.is_empty() { - Err(ClientError::from(NutError::Generic( - "Unspecified CMDDESC description in response".into(), - ))) + Err(ClientError::generic( + "Unspecified CMDDESC description in response", + )) } else { Ok(args.remove(0)) }?; @@ -300,16 +282,16 @@ impl Response { } "UPSDESC" => { let _device = if args.is_empty() { - Err(ClientError::from(NutError::Generic( - "Unspecified UPSDESC device in response".into(), - ))) + Err(ClientError::generic( + "Unspecified UPSDESC device in response", + )) } else { Ok(args.remove(0)) }?; let desc = if args.is_empty() { - Err(ClientError::from(NutError::Generic( - "Unspecified UPSDESC description in response".into(), - ))) + Err(ClientError::generic( + "Unspecified UPSDESC description in response", + )) } else { Ok(args.remove(0)) }?; @@ -317,23 +299,19 @@ impl Response { } "DESC" => { let _device = if args.is_empty() { - Err(ClientError::from(NutError::Generic( - "Unspecified DESC device in response".into(), - ))) + Err(ClientError::generic("Unspecified DESC device in response")) } else { Ok(args.remove(0)) }?; let _name = if args.is_empty() { - Err(ClientError::from(NutError::Generic( - "Unspecified DESC name in response".into(), - ))) + Err(ClientError::generic("Unspecified DESC name in response")) } else { Ok(args.remove(0)) }?; let desc = if args.is_empty() { - Err(ClientError::from(NutError::Generic( - "Unspecified DESC description in response".into(), - ))) + Err(ClientError::generic( + "Unspecified DESC description in response", + )) } else { Ok(args.remove(0)) }?; @@ -341,38 +319,32 @@ impl Response { } "NUMLOGINS" => { let _device = if args.is_empty() { - Err(ClientError::from(NutError::Generic( - "Unspecified NUMLOGINS device in response".into(), - ))) + Err(ClientError::generic( + "Unspecified NUMLOGINS device in response", + )) } else { Ok(args.remove(0)) }?; let num = if args.is_empty() { - Err(ClientError::from(NutError::Generic( - "Unspecified NUMLOGINS number in response".into(), - ))) + Err(ClientError::generic( + "Unspecified NUMLOGINS number in response", + )) } else { Ok(args.remove(0)) }?; - let num = num.parse::().map_err(|_| { - ClientError::from(NutError::Generic( - "Invalid NUMLOGINS number in response".into(), - )) - })?; + let num = num + .parse::() + .map_err(|_| ClientError::generic("Invalid NUMLOGINS number in response"))?; Ok(Response::NumLogins(num)) } "TYPE" => { let _device = if args.is_empty() { - Err(ClientError::from(NutError::Generic( - "Unspecified TYPE device in response".into(), - ))) + Err(ClientError::generic("Unspecified TYPE device in response")) } else { Ok(args.remove(0)) }?; let name = if args.is_empty() { - Err(ClientError::from(NutError::Generic( - "Unspecified TYPE name in response".into(), - ))) + Err(ClientError::generic("Unspecified TYPE name in response")) } else { Ok(args.remove(0)) }?; @@ -381,30 +353,22 @@ impl Response { } "RANGE" => { let _device = if args.is_empty() { - Err(ClientError::from(NutError::Generic( - "Unspecified RANGE device in response".into(), - ))) + Err(ClientError::generic("Unspecified RANGE device in response")) } else { Ok(args.remove(0)) }?; let _name = if args.is_empty() { - Err(ClientError::from(NutError::Generic( - "Unspecified RANGE name in response".into(), - ))) + Err(ClientError::generic("Unspecified RANGE name in response")) } else { Ok(args.remove(0)) }?; let min = if args.is_empty() { - Err(ClientError::from(NutError::Generic( - "Unspecified RANGE min in response".into(), - ))) + Err(ClientError::generic("Unspecified RANGE min in response")) } else { Ok(args.remove(0)) }?; let max = if args.is_empty() { - Err(ClientError::from(NutError::Generic( - "Unspecified RANGE max in response".into(), - ))) + Err(ClientError::generic("Unspecified RANGE max in response")) } else { Ok(args.remove(0)) }?; @@ -412,23 +376,17 @@ impl Response { } "ENUM" => { let _device = if args.is_empty() { - Err(ClientError::from(NutError::Generic( - "Unspecified ENUM device in response".into(), - ))) + Err(ClientError::generic("Unspecified ENUM device in response")) } else { Ok(args.remove(0)) }?; let _name = if args.is_empty() { - Err(ClientError::from(NutError::Generic( - "Unspecified ENUM name in response".into(), - ))) + Err(ClientError::generic("Unspecified ENUM name in response")) } else { Ok(args.remove(0)) }?; let val = if args.is_empty() { - Err(ClientError::from(NutError::Generic( - "Unspecified ENUM value in response".into(), - ))) + Err(ClientError::generic("Unspecified ENUM value in response")) } else { Ok(args.remove(0)) }?; diff --git a/rups/src/error.rs b/rups/src/error.rs index 91c5fa1..53d7ac9 100644 --- a/rups/src/error.rs +++ b/rups/src/error.rs @@ -36,11 +36,18 @@ impl fmt::Display for NutError { "Given hostname cannot be used for a strict SSL connection" ), Self::FeatureNotConfigured => write!(f, "Feature not configured by server"), - Self::Generic(msg) => write!(f, "Internal client error: {}", msg), + Self::Generic(msg) => write!(f, "Client error: {}", msg), } } } +impl NutError { + /// Constructs a generic rups error. + pub fn generic(message: T) -> Self { + Self::Generic(message.to_string()) + } +} + impl std::error::Error for NutError {} /// Encapsulation for errors emitted by the client library. @@ -52,6 +59,13 @@ pub enum ClientError { Nut(NutError), } +impl ClientError { + /// Constructs a generic rups error. + pub fn generic(message: T) -> Self { + NutError::generic(message.to_string()).into() + } +} + impl fmt::Display for ClientError { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { match self { diff --git a/rups/src/lib.rs b/rups/src/lib.rs index 33cd610..0952659 100644 --- a/rups/src/lib.rs +++ b/rups/src/lib.rs @@ -7,6 +7,7 @@ pub use config::*; pub use error::*; +pub use util::*; pub use var::*; /// Blocking client implementation for NUT. @@ -20,4 +21,5 @@ mod config; mod error; #[cfg(feature = "ssl")] mod ssl; +mod util; mod var; diff --git a/rups/src/tokio/mod.rs b/rups/src/tokio/mod.rs index aaaa662..dc6677a 100644 --- a/rups/src/tokio/mod.rs +++ b/rups/src/tokio/mod.rs @@ -148,7 +148,7 @@ impl TcpConnection { // Parse args by splitting whitespace, minding quotes for args with multiple words let args = shell_words::split(&raw) - .map_err(|e| NutError::Generic(format!("Parsing server response failed: {}", e)))?; + .map_err(|e| NutError::generic(format!("Parsing server response failed: {}", e)))?; Ok(args) } diff --git a/rupsc/src/parser.rs b/rups/src/util.rs similarity index 84% rename from rupsc/src/parser.rs rename to rups/src/util.rs index ceb7d69..0d0a596 100644 --- a/rupsc/src/parser.rs +++ b/rups/src/util.rs @@ -1,17 +1,21 @@ -use anyhow::Context; use std::convert::{TryFrom, TryInto}; use std::fmt; +/// The default upsd hostname. pub const DEFAULT_HOSTNAME: &str = "localhost"; +/// The default upsd port. pub const DEFAULT_PORT: u16 = 3493; -/// Connection information for a upsd server. +/// TCP connection information for a upsd server. /// /// The upsname is optional depending on context. #[derive(Debug, Clone, Copy, Eq, PartialEq)] pub struct UpsdName<'a> { + /// The name of the ups device, if specified. pub upsname: Option<&'a str>, + /// The hostname of the upsd server. pub hostname: &'a str, + /// The port of the upsd server. pub port: u16, } @@ -26,9 +30,9 @@ impl<'a> Default for UpsdName<'a> { } impl<'a> TryFrom<&'a str> for UpsdName<'a> { - type Error = anyhow::Error; + type Error = crate::ClientError; - fn try_from(value: &'a str) -> anyhow::Result> { + fn try_from(value: &'a str) -> crate::Result> { let mut upsname: Option<&str> = None; let mut hostname = DEFAULT_HOSTNAME; let mut port = DEFAULT_PORT; @@ -40,7 +44,7 @@ impl<'a> TryFrom<&'a str> for UpsdName<'a> { .next() .unwrap() .parse::() - .with_context(|| "invalid port number")?; + .map_err(|_| crate::ClientError::generic("Invalid port number"))?; if prefix.contains('@') { let mut split = prefix.splitn(2, '@'); upsname = Some(split.next().unwrap()); @@ -64,13 +68,13 @@ impl<'a> TryFrom<&'a str> for UpsdName<'a> { } } -impl<'a> TryInto for UpsdName<'a> { - type Error = anyhow::Error; +impl<'a> TryInto for UpsdName<'a> { + type Error = crate::ClientError; - fn try_into(self) -> anyhow::Result { + fn try_into(self) -> crate::Result { (self.hostname.to_owned(), self.port) .try_into() - .with_context(|| "Invalid hostname/port") + .map_err(|_| crate::ClientError::generic("Invalid hostname/port")) } } diff --git a/rups/src/var.rs b/rups/src/var.rs index ff41fd0..84e0a24 100644 --- a/rups/src/var.rs +++ b/rups/src/var.rs @@ -200,17 +200,13 @@ impl TryFrom<&str> for VariableType { .nth(1) .map(|s| s.parse().ok()) .flatten() - .ok_or_else(|| { - crate::ClientError::Nut(crate::NutError::Generic( - "Invalid STRING definition".into(), - )) - })?; + .ok_or_else(|| crate::ClientError::generic("Invalid STRING definition"))?; Ok(Self::String(size)) } else { - Err(crate::ClientError::Nut(crate::NutError::Generic(format!( + Err(crate::ClientError::generic(format!( "Unrecognized variable type: {}", value - )))) + ))) } } } diff --git a/rupsc/src/main.rs b/rupsc/src/main.rs index 2dd16af..5965811 100644 --- a/rupsc/src/main.rs +++ b/rupsc/src/main.rs @@ -8,9 +8,9 @@ use core::convert::TryInto; use anyhow::Context; use clap::{App, Arg}; -use crate::parser::UpsdName; +use rups::UpsdName; + mod cmd; -mod parser; fn main() -> anyhow::Result<()> { let args = App::new(clap::crate_name!()) @@ -72,7 +72,7 @@ fn main() -> anyhow::Result<()> { ) .get_matches(); - let server: parser::UpsdName = args.value_of("upsd-server").map_or_else( + let server: UpsdName = args.value_of("upsd-server").map_or_else( || Ok(UpsdName::default()), |s| s.try_into().with_context(|| "Invalid upsd server name"), )?;