From c9a8ef7db4d0f6f980f9a9a974fc49375b4d5cc0 Mon Sep 17 00:00:00 2001 From: Nadja Reitzenstein Date: Fri, 24 Jun 2022 15:14:38 +0200 Subject: [PATCH 1/3] Return a better error when --load is given a directory Closes: #55 --- bffhd/users/mod.rs | 35 +++++++++++++++++++++++++++++++++-- bin/bffhd/main.rs | 14 +++++--------- 2 files changed, 38 insertions(+), 11 deletions(-) diff --git a/bffhd/users/mod.rs b/bffhd/users/mod.rs index 98d4b54..8471dbf 100644 --- a/bffhd/users/mod.rs +++ b/bffhd/users/mod.rs @@ -4,9 +4,11 @@ use rkyv::{Archive, Deserialize, Infallible, Serialize}; use std::collections::HashMap; use std::fmt::{Display, Formatter, Write}; -use miette::{Context, IntoDiagnostic}; +use clap::ArgMatches; +use miette::{Context, Diagnostic, IntoDiagnostic, SourceOffset, SourceSpan}; use std::path::Path; use std::sync::Arc; +use thiserror::Error; pub mod db; @@ -100,7 +102,36 @@ impl Users { self.userdb.delete(uid) } - pub fn load_file>(&self, path: P) -> miette::Result<()> { + pub fn load_file(&self, path_str: &str) -> miette::Result<()> { + let path: &Path = Path::new(path_str); + if path.is_dir() { + #[derive(Debug, Error, Diagnostic)] + #[error("load takes a file, not a directory")] + #[diagnostic( + code(load::file), + url("https://gitlab.com/fabinfra/fabaccess/bffh/-/issues/55") + )] + struct LoadIsDirError { + #[source_code] + src: String, + + #[label("path provided")] + dir_path: SourceSpan, + + #[help] + help: String, + } + + Err(LoadIsDirError { + src: format!("--load {}", path_str), + dir_path: (7, path_str.as_bytes().len()).into(), + help: format!( + "Provide a path to a file instead, e.g. {}/users.toml", + path_str + ), + })?; + return Ok(()); + } let f = std::fs::read(path).into_diagnostic()?; let map: HashMap = toml::from_slice(&f).into_diagnostic()?; diff --git a/bin/bffhd/main.rs b/bin/bffhd/main.rs index 27b9146..30ee060 100644 --- a/bin/bffhd/main.rs +++ b/bin/bffhd/main.rs @@ -122,15 +122,11 @@ fn main() -> miette::Result<()> { unimplemented!() } else if matches.is_present("load") { let bffh = Diflouroborane::new(config)?; - if let Err(error) = bffh.users.load_file(matches.value_of("load").unwrap()) { - tracing::error!( - "failed to load users from {}: {}", - matches.value_of("load").unwrap(), - error, - ); - } else { - tracing::info!("loaded users from {}", matches.value_of("load").unwrap()); - } + + bffh.users.load_file(matches.value_of("load").unwrap())?; + + tracing::info!("loaded users from {}", matches.value_of("load").unwrap()); + return Ok(()); } else { let keylog = matches.value_of("keylog"); From 7a0a50dc3fa127056db60323862774f45f2aace0 Mon Sep 17 00:00:00 2001 From: Nadja Reitzenstein Date: Fri, 24 Jun 2022 15:17:05 +0200 Subject: [PATCH 2/3] Start taking control over exit on argument parsing failure --- bin/bffhd/main.rs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/bin/bffhd/main.rs b/bin/bffhd/main.rs index 30ee060..23b2a0f 100644 --- a/bin/bffhd/main.rs +++ b/bin/bffhd/main.rs @@ -75,7 +75,12 @@ fn main() -> miette::Result<()> { .max_values(1) .min_values(0) .default_missing_value("")) - .get_matches(); + .try_get_matches(); + + let matches = match matches { + Ok(m) => m, + Err(error) => error.exit(), + }; let configpath = matches .value_of("config") From a79293add1d9fde3f02ea2a0ea8a497ed9490503 Mon Sep 17 00:00:00 2001 From: Nadja Reitzenstein Date: Mon, 11 Jul 2022 12:14:49 +0200 Subject: [PATCH 3/3] Better error wrapper type --- bffhd/error.rs | 189 ++++++++++++++++++++++--------------------------- bffhd/lib.rs | 8 ++- 2 files changed, 91 insertions(+), 106 deletions(-) diff --git a/bffhd/error.rs b/bffhd/error.rs index 61db8a2..e0af43b 100644 --- a/bffhd/error.rs +++ b/bffhd/error.rs @@ -1,116 +1,97 @@ +use miette::{Diagnostic, LabeledSpan, Severity, SourceCode}; +use std::error; +use std::fmt::{Display, Formatter}; +use std::io; use thiserror::Error; -use crate::db; -use rsasl::error::SessionError; -use std::any::TypeId; -use std::error::Error as StdError; -use std::fmt; -use std::fmt::Display; -use std::io; - -use crate::resources::state::db::StateDBError; -use backtrace::{Backtrace, BacktraceFmt, PrintFmt}; -use miette::{Diagnostic, LabeledSpan, Severity, SourceCode}; - -#[derive(Debug)] -pub struct TracedError { - pub inner: E, - pub backtrace: Backtrace, +pub trait Description { + const DESCRIPTION: Option<&'static str> = None; + const CODE: &'static str; + const HELP: Option<&'static str> = None; + const URL: Option<&'static str> = None; } -impl fmt::Display for TracedError { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - writeln!(f, "Error: {}", self.inner)?; - - let cwd = std::env::current_dir(); - let mut print_path = - move |fmt: &mut fmt::Formatter<'_>, path: backtrace::BytesOrWideString<'_>| { - let path = path.into_path_buf(); - if let Ok(cwd) = &cwd { - if let Ok(suffix) = path.strip_prefix(cwd) { - return fmt::Display::fmt(&suffix.display(), fmt); - } - } - fmt::Display::fmt(&path.display(), fmt) - }; - let mut bf = BacktraceFmt::new(f, PrintFmt::Short, &mut print_path); - bf.add_context()?; - - Ok(()) - } -} - -impl StdError for TracedError { - fn source(&self) -> Option<&(dyn StdError + 'static)> { - self.inner.source() - } -} - -impl Diagnostic for TracedError { - #[inline(always)] - fn code<'a>(&'a self) -> Option> { - self.inner.code() - } - - #[inline(always)] - fn severity(&self) -> Option { - self.inner.severity() - } - - #[inline(always)] - fn help<'a>(&'a self) -> Option> { - self.inner.help() - } - - #[inline(always)] - fn url<'a>(&'a self) -> Option> { - self.inner.url() - } - - #[inline(always)] - fn source_code(&self) -> Option<&dyn SourceCode> { - self.inner.source_code() - } - - #[inline(always)] - fn labels(&self) -> Option + '_>> { - self.inner.labels() - } - - #[inline(always)] - fn related<'a>(&'a self) -> Option + 'a>> { - self.inner.related() - } - - #[inline(always)] - fn diagnostic_source(&self) -> Option<&dyn Diagnostic> { - self.inner.diagnostic_source() - } +pub fn wrap(error: Source) -> Error { + Error::new::(error) } #[derive(Debug, Error, Diagnostic)] -/// Shared error type -pub enum BffhError { - #[error("SASL error: {0:?}")] - SASL(SessionError), - #[error("IO error: {0}")] - IO(#[from] io::Error), - #[error("IO error: {0}")] - Boxed(#[from] Box), - #[error("IO error: {0}")] - Capnp(#[from] capnp::Error), - #[error("IO error: {0}")] - DB(#[from] db::Error), - #[error("You do not have the permission required to do that.")] - Denied, - #[error("State DB operation failed")] - StateDB(#[from] StateDBError), +pub enum Source { + #[error("io error occured")] + Io( + #[source] + #[from] + io::Error, + ), } -impl From for BffhError { - fn from(e: SessionError) -> Self { - Self::SASL(e) +#[derive(Debug)] +pub struct Error { + description: Option<&'static str>, + code: &'static str, + severity: Option, + help: Option<&'static str>, + url: Option<&'static str>, + source: Source, +} + +impl Display for Error { + fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { + Display::fmt(&self.source, f) } } -pub type Result = std::result::Result; +impl error::Error for Error { + fn source(&self) -> Option<&(dyn error::Error + 'static)> { + Some(&self.source) + } + + fn description(&self) -> &str { + if let Some(desc) = self.description { + desc + } else { + self.source.description() + } + } +} + +impl Error { + pub fn new(source: Source) -> Self { + Self { + description: D::DESCRIPTION, + code: D::CODE, + severity: source.severity(), + help: D::HELP, + url: D::URL, + source, + } + } +} + +impl miette::Diagnostic for Error { + fn code<'a>(&'a self) -> Option> { + Some(Box::new(self.code)) + } + + fn severity(&self) -> Option { + self.severity + } + + fn help<'a>(&'a self) -> Option> { + self.help.map(|r| { + let b: Box = Box::new(r); + b + }) + } + + fn url<'a>(&'a self) -> Option> { + self.url.map(|r| { + let b: Box = Box::new(r); + b + }) + } + + fn diagnostic_source(&self) -> Option<&dyn Diagnostic> { + Some(&self.source) + } +} diff --git a/bffhd/lib.rs b/bffhd/lib.rs index ec64ef5..f0b60c7 100644 --- a/bffhd/lib.rs +++ b/bffhd/lib.rs @@ -76,6 +76,11 @@ pub struct Diflouroborane { pub static RESOURCES: OnceCell = OnceCell::new(); +struct SignalHandlerErr; +impl error::Description for SignalHandlerErr { + const CODE: &'static str = "signals::new"; +} + impl Diflouroborane { pub fn new(config: Config) -> miette::Result { let mut server = logging::init(&config.logging); @@ -140,8 +145,7 @@ impl Diflouroborane { pub fn run(&mut self) -> miette::Result<()> { let _guard = self.span.enter(); let mut signals = signal_hook_async_std::Signals::new(&[SIGINT, SIGQUIT, SIGTERM]) - .into_diagnostic() - .wrap_err("Failed to construct signal handler")?; + .map_err(|ioerr| error::wrap::(ioerr.into()))?; let sessionmanager = SessionManager::new(self.users.clone(), self.roles.clone()); let authentication = AuthenticationHandle::new(self.users.clone());