Merge branch 'feature/better-errors' into development

* feature/better-errors:
  Better error wrapper type
  Start taking control over exit on argument parsing failure
  Return a better error when --load is given a directory

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;
+pub trait Description {
-use rsasl::error::SessionError;
+    const DESCRIPTION: Option<&'static str> = None;
-use std::any::TypeId;
+    const CODE: &'static str;
-use std::error::Error as StdError;
+    const HELP: Option<&'static str> = None;
-use std::fmt;
+    const URL: Option<&'static str> = None;
-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<E: Diagnostic> {
-    pub inner: E,
-    pub backtrace: Backtrace,
 }
 
-impl<E: Diagnostic> fmt::Display for TracedError<E> {
+pub fn wrap<D: Description>(error: Source) -> Error {
-    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+    Error::new::<D>(error)
-        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<E: 'static + Diagnostic> StdError for TracedError<E> {
-    fn source(&self) -> Option<&(dyn StdError + 'static)> {
-        self.inner.source()
-    }
-}
-
-impl<E: 'static + Diagnostic> Diagnostic for TracedError<E> {
-    #[inline(always)]
-    fn code<'a>(&'a self) -> Option<Box<dyn Display + 'a>> {
-        self.inner.code()
-    }
-
-    #[inline(always)]
-    fn severity(&self) -> Option<Severity> {
-        self.inner.severity()
-    }
-
-    #[inline(always)]
-    fn help<'a>(&'a self) -> Option<Box<dyn Display + 'a>> {
-        self.inner.help()
-    }
-
-    #[inline(always)]
-    fn url<'a>(&'a self) -> Option<Box<dyn Display + 'a>> {
-        self.inner.url()
-    }
-
-    #[inline(always)]
-    fn source_code(&self) -> Option<&dyn SourceCode> {
-        self.inner.source_code()
-    }
-
-    #[inline(always)]
-    fn labels(&self) -> Option<Box<dyn Iterator<Item = LabeledSpan> + '_>> {
-        self.inner.labels()
-    }
-
-    #[inline(always)]
-    fn related<'a>(&'a self) -> Option<Box<dyn Iterator<Item = &'a dyn Diagnostic> + 'a>> {
-        self.inner.related()
-    }
-
-    #[inline(always)]
-    fn diagnostic_source(&self) -> Option<&dyn Diagnostic> {
-        self.inner.diagnostic_source()
-    }
 }
 
 #[derive(Debug, Error, Diagnostic)]
-/// Shared error type
+pub enum Source {
-pub enum BffhError {
+    #[error("io error occured")]
-    #[error("SASL error: {0:?}")]
+    Io(
-    SASL(SessionError),
+        #[source]
-    #[error("IO error: {0}")]
+        #[from]
-    IO(#[from] io::Error),
+        io::Error,
-    #[error("IO error: {0}")]
+    ),
-    Boxed(#[from] Box<dyn std::error::Error>),
-    #[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),
 }
 
-impl From<SessionError> for BffhError {
+#[derive(Debug)]
-    fn from(e: SessionError) -> Self {
+pub struct Error {
-        Self::SASL(e)
+    description: Option<&'static str>,
+    code: &'static str,
+    severity: Option<Severity>,
+    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<T> = std::result::Result<T, BffhError>;
+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<D: Description>(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<Box<dyn Display + 'a>> {
+        Some(Box::new(self.code))
+    }
+
+    fn severity(&self) -> Option<Severity> {
+        self.severity
+    }
+
+    fn help<'a>(&'a self) -> Option<Box<dyn Display + 'a>> {
+        self.help.map(|r| {
+            let b: Box<dyn Display + 'a> = Box::new(r);
+            b
+        })
+    }
+
+    fn url<'a>(&'a self) -> Option<Box<dyn Display + 'a>> {
+        self.url.map(|r| {
+            let b: Box<dyn Display + 'a> = Box::new(r);
+            b
+        })
+    }
+
+    fn diagnostic_source(&self) -> Option<&dyn Diagnostic> {
+        Some(&self.source)
+    }
+}

bffhd/lib.rs

@@ -76,6 +76,11 @@ pub struct Diflouroborane {
 
 pub static RESOURCES: OnceCell<ResourcesHandle> = OnceCell::new();
 
+struct SignalHandlerErr;
+impl error::Description for SignalHandlerErr {
+    const CODE: &'static str = "signals::new";
+}
+
 impl Diflouroborane {
     pub fn new(config: Config) -> miette::Result<Self> {
         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()
+            .map_err(|ioerr| error::wrap::<SignalHandlerErr>(ioerr.into()))?;
-            .wrap_err("Failed to construct signal handler")?;
 
         let sessionmanager = SessionManager::new(self.users.clone(), self.roles.clone());
         let authentication = AuthenticationHandle::new(self.users.clone());

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<P: AsRef<Path>>(&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<String, UserData> = toml::from_slice(&f).into_diagnostic()?;
 

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")
@@ -122,15 +127,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!(
+        bffh.users.load_file(matches.value_of("load").unwrap())?;
-                "failed to load users from {}: {}",
+
-                matches.value_of("load").unwrap(),
-                error,
-            );
-        } else {
         tracing::info!("loaded users from {}", matches.value_of("load").unwrap());
-        }
+
         return Ok(());
     } else {
         let keylog = matches.value_of("keylog");