Refactor FFI error handling

To reduce verbosity and duplication.

cxx/src/database.rs

@@ -5,9 +5,10 @@ use std::{
 
 use delegate::delegate;
 use libloot::{WriteMode, error::DatabaseLockPoisonError};
+use libloot_ffi_errors::UnsupportedEnumValueError;
 
 use crate::{
-    UnsupportedEnumValueError, VerboseError,
+    VerboseError,
     ffi::EdgeType,
     metadata::{Group, Message, OptionalPluginMetadata, PluginMetadata, to_vec_of_unwrapped},
 };

cxx/src/error.rs

@@ -1,5 +1,8 @@
 use crate::game::NotValidUtf8;
-use libloot_ffi_errors::SystemError;
+use libloot_ffi_errors::{
+    SystemError, SystemErrorCategory, UnsupportedEnumValueError, fmt_error_chain,
+    variant_box_from_error,
+};
 
 use libloot::{
     error::{
@@ -16,9 +19,7 @@ use libloot::{
 pub enum VerboseError {
     CyclicInteractionError(Vec<libloot::Vertex>),
     UndefinedGroupError(String),
-    EspluginError(i32, String),
-    LibloadorderError(i32, String),
-    LciError(i32, String),
+    SystemError(SystemError),
     FileAccessError(String),
     InvalidArgument(String),
     Other(Box<dyn std::error::Error>),
@@ -41,30 +42,27 @@ impl std::fmt::Display for VerboseError {
             Self::UndefinedGroupError(group) => {
                 write!(f, "UndefinedGroupError: {}", group)
             }
-            Self::EspluginError(c, s) => {
-                write!(f, "EspluginError: {}: {}", c, s)
-            }
-            Self::LibloadorderError(c, s) => {
-                write!(f, "LibloadorderError: {}: {}", c, s)
-            }
-            Self::LciError(c, s) => {
-                write!(f, "LciError: {}: {}", c, s)
+            Self::SystemError(e) => {
+                let prefix = match e.category() {
+                    SystemErrorCategory::Esplugin => "EspluginError",
+                    SystemErrorCategory::Libloadorder => "LibloadorderError",
+                    SystemErrorCategory::LootConditionInterpreter => "LciError",
+                };
+                write!(f, "{}: {}: {}", prefix, e.code(), e.message())
             }
             Self::FileAccessError(s) => write!(f, "FileAccessError: {}", s),
             Self::InvalidArgument(s) => write!(f, "InvalidArgument: {}", s),
-            Self::Other(e) => {
-                write!(f, "{}", e)?;
-                let mut error = e.as_ref();
-                while let Some(source) = error.source() {
-                    write!(f, ": {}", source)?;
-                    error = source;
-                }
-                Ok(())
-            }
+            Self::Other(e) => fmt_error_chain(e.as_ref(), f),
         }
     }
 }
 
+variant_box_from_error!(UnsupportedEnumValueError, VerboseError::Other);
+variant_box_from_error!(NotValidUtf8, VerboseError::Other);
+variant_box_from_error!(DatabaseLockPoisonError, VerboseError::Other);
+variant_box_from_error!(MultilingualMessageContentsError, VerboseError::Other);
+variant_box_from_error!(RegexError, VerboseError::Other);
+
 impl From<GameHandleCreationError> for VerboseError {
     fn from(value: GameHandleCreationError) -> Self {
         match value {
@@ -75,24 +73,6 @@ impl From<GameHandleCreationError> for VerboseError {
     }
 }
 
-impl From<UnsupportedEnumValueError> for VerboseError {
-    fn from(value: UnsupportedEnumValueError) -> Self {
-        Self::Other(Box::new(value))
-    }
-}
-
-impl From<NotValidUtf8> for VerboseError {
-    fn from(value: NotValidUtf8) -> Self {
-        Self::Other(Box::new(value))
-    }
-}
-
-impl From<DatabaseLockPoisonError> for VerboseError {
-    fn from(value: DatabaseLockPoisonError) -> Self {
-        Self::Other(Box::new(value))
-    }
-}
-
 impl From<LoadPluginsError> for VerboseError {
     fn from(value: LoadPluginsError) -> Self {
         match value {
@@ -126,8 +106,7 @@ impl From<LoadOrderStateError> for VerboseError {
 
 impl From<LoadOrderError> for VerboseError {
     fn from(value: LoadOrderError) -> Self {
-        let error = SystemError::from(value);
-        Self::LibloadorderError(error.code(), error.message().to_string())
+        Self::SystemError(SystemError::from(value))
     }
 }
 
@@ -145,8 +124,7 @@ impl From<WriteMetadataError> for VerboseError {
 
 impl From<ConditionEvaluationError> for VerboseError {
     fn from(value: ConditionEvaluationError) -> Self {
-        let error = SystemError::from(value);
-        Self::LciError(error.code(), error.message().to_string())
+        Self::SystemError(SystemError::from(value))
     }
 }
 
@@ -171,20 +149,7 @@ impl From<MetadataRetrievalError> for VerboseError {
 
 impl From<PluginDataError> for VerboseError {
     fn from(value: PluginDataError) -> Self {
-        let error = SystemError::from(value);
-        Self::EspluginError(error.code(), error.message().to_string())
-    }
-}
-
-impl From<MultilingualMessageContentsError> for VerboseError {
-    fn from(value: MultilingualMessageContentsError) -> Self {
-        Self::Other(Box::new(value))
-    }
-}
-
-impl From<RegexError> for VerboseError {
-    fn from(value: RegexError) -> Self {
-        Self::Other(Box::new(value))
+        Self::SystemError(SystemError::from(value))
     }
 }
 
@@ -198,14 +163,3 @@ impl std::fmt::Display for EmptyOptionalError {
 }
 
 impl std::error::Error for EmptyOptionalError {}
-
-#[derive(Debug)]
-pub struct UnsupportedEnumValueError;
-
-impl std::fmt::Display for UnsupportedEnumValueError {
-    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
-        write!(f, "Enum value is unsupported")
-    }
-}
-
-impl std::error::Error for UnsupportedEnumValueError {}

cxx/src/game.rs

@@ -1,11 +1,9 @@
 use std::path::Path;
 
 use delegate::delegate;
+use libloot_ffi_errors::UnsupportedEnumValueError;
 
-use crate::{
-    Plugin, UnsupportedEnumValueError, VerboseError, database::Database, ffi::GameType,
-    plugin::OptionalPlugin,
-};
+use crate::{Plugin, VerboseError, database::Database, ffi::GameType, plugin::OptionalPlugin};
 
 impl TryFrom<libloot::GameType> for GameType {
     type Error = UnsupportedEnumValueError;

cxx/src/lib.rs

@@ -5,8 +5,9 @@ mod metadata;
 mod plugin;
 
 use database::{Database, Vertex, new_vertex};
-use error::{EmptyOptionalError, UnsupportedEnumValueError, VerboseError};
+use error::{EmptyOptionalError, VerboseError};
 use game::{Game, new_game, new_game_with_local_path};
+use libloot_ffi_errors::UnsupportedEnumValueError;
 use metadata::{
     File, Filename, Group, Location, Message, MessageContent, OptionalMessageContentRef,
     OptionalPluginMetadata, PluginCleaningData, PluginMetadata, Tag, group_default_name,

ffi-errors/src/lib.rs

@@ -29,7 +29,19 @@ pub enum SystemErrorCategory {
     LootConditionInterpreter = LIBLOOT_SYSTEM_ERROR_CATEGORY_LCI,
 }
 
-#[derive(Clone, Debug, Eq, PartialEq, Ord, PartialOrd, Hash)]
+impl std::fmt::Display for SystemErrorCategory {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        match self {
+            SystemErrorCategory::Esplugin => write!(f, "esplugin"),
+            SystemErrorCategory::Libloadorder => write!(f, "libloadorder"),
+            SystemErrorCategory::LootConditionInterpreter => {
+                write!(f, "loot-condition-interpreter")
+            }
+        }
+    }
+}
+
+#[derive(Debug)]
 pub struct SystemError {
     code: c_int,
     category: SystemErrorCategory,
@@ -50,6 +62,18 @@ impl SystemError {
     }
 }
 
+impl std::fmt::Display for SystemError {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        write!(
+            f,
+            "{} error, code {}: {}",
+            self.category, self.code, self.message
+        )
+    }
+}
+
+impl std::error::Error for SystemError {}
+
 impl From<PluginDataError> for SystemError {
     fn from(value: PluginDataError) -> Self {
         let error = value
@@ -97,3 +121,37 @@ impl From<ConditionEvaluationError> for SystemError {
         }
     }
 }
+
+#[derive(Clone, Copy, Debug, Default, Eq, PartialEq, Ord, PartialOrd, Hash)]
+pub struct UnsupportedEnumValueError;
+
+impl std::fmt::Display for UnsupportedEnumValueError {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        write!(f, "Enum value is unsupported")
+    }
+}
+
+impl std::error::Error for UnsupportedEnumValueError {}
+
+pub fn fmt_error_chain(
+    mut error: &dyn std::error::Error,
+    f: &mut std::fmt::Formatter<'_>,
+) -> std::fmt::Result {
+    write!(f, "{}", error)?;
+    while let Some(source) = error.source() {
+        write!(f, ": {}", source)?;
+        error = source;
+    }
+    Ok(())
+}
+
+#[macro_export]
+macro_rules! variant_box_from_error {
+    ( $from_type:ident, $to_type:ident::$to_variant:ident ) => {
+        impl From<$from_type> for $to_type {
+            fn from(value: $from_type) -> Self {
+                Self::$to_variant(Box::new(value))
+            }
+        }
+    };
+}

pyo3/src/database.rs

@@ -5,13 +5,14 @@ use std::{
 };
 
 use libloot::{WriteMode, error::DatabaseLockPoisonError};
+use libloot_ffi_errors::UnsupportedEnumValueError;
 use pyo3::{
     Bound, PyResult, pyclass, pymethods,
     types::{PyAnyMethods, PyTypeMethods},
 };
 
 use crate::{
-    error::{UnsupportedEnumValueError, VerboseError},
+    error::VerboseError,
     metadata::{Group, Message, NONE_REPR, PluginMetadata},
 };
 
@@ -237,15 +238,15 @@ impl Vertex {
     }
 
     #[getter]
-    fn out_edge_type(&self) -> Result<Option<EdgeType>, UnsupportedEnumValueError> {
-        self.0.out_edge_type().map(|e| e.try_into()).transpose()
+    fn out_edge_type(&self) -> Result<Option<EdgeType>, VerboseError> {
+        self.0
+            .out_edge_type()
+            .map(|e| e.try_into().map_err(Into::into))
+            .transpose()
     }
 
     #[setter]
-    fn set_out_edge_type(
-        &mut self,
-        out_edge_type: EdgeType,
-    ) -> Result<(), UnsupportedEnumValueError> {
+    fn set_out_edge_type(&mut self, out_edge_type: EdgeType) -> Result<(), VerboseError> {
         let out_edge_type = out_edge_type.try_into()?;
         self.0.set_out_edge_type(out_edge_type);
         Ok(())

pyo3/src/error.rs

@@ -8,60 +8,42 @@ use libloot::{
         LoadMetadataError, MultilingualMessageContentsError, RegexError, WriteMetadataError,
     },
 };
-use libloot_ffi_errors::SystemError;
+use libloot_ffi_errors::{
+    SystemError, UnsupportedEnumValueError, fmt_error_chain, variant_box_from_error,
+};
 use pyo3::{PyErr, exceptions::PyValueError};
 
 use crate::{CyclicInteractionError, EspluginError, UndefinedGroupError, database::Vertex};
 
-#[derive(Clone, Copy, Debug, Default, Eq, PartialEq, Ord, PartialOrd, Hash)]
-pub struct UnsupportedEnumValueError;
-
-impl std::fmt::Display for UnsupportedEnumValueError {
-    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
-        write!(f, "Enum value is unsupported")
-    }
-}
-
-impl std::error::Error for UnsupportedEnumValueError {}
-
-impl From<UnsupportedEnumValueError> for PyErr {
-    fn from(value: UnsupportedEnumValueError) -> Self {
-        PyValueError::new_err(value.to_string())
-    }
-}
-
 #[derive(Debug)]
 pub enum VerboseError {
     CyclicInteractionError(Vec<libloot::Vertex>),
     UndefinedGroupError(String),
-    EspluginError(i32, String),
+    EspluginError(SystemError),
     Other(Box<dyn std::error::Error>),
 }
 
 impl std::fmt::Display for VerboseError {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
         match self {
-            Self::CyclicInteractionError(c) => {
-                let cycle = display_cycle(c);
-                write!(f, "cyclic interaction detected: {}", cycle)
-            }
-            Self::UndefinedGroupError(g) => {
-                write!(f, "the group \"{}\" does not exist", g)
-            }
-            Self::EspluginError(_, s) => s.fmt(f),
-            Self::Other(e) => {
-                write!(f, "{}", e)?;
-                let mut error = e.as_ref();
-                while let Some(source) = error.source() {
-                    write!(f, ": {}", source)?;
-                    error = source;
-                }
-                Ok(())
-            }
+            Self::CyclicInteractionError(c) => SortPluginsError::CycleFound(c.clone()).fmt(f),
+            Self::UndefinedGroupError(g) => SortPluginsError::UndefinedGroup(g.clone()).fmt(f),
+            Self::EspluginError(e) => e.message().fmt(f),
+            Self::Other(e) => fmt_error_chain(e.as_ref(), f),
         }
     }
 }
 
+variant_box_from_error!(UnsupportedEnumValueError, VerboseError::Other);
+variant_box_from_error!(DatabaseLockPoisonError, VerboseError::Other);
+variant_box_from_error!(LoadPluginsError, VerboseError::Other);
+variant_box_from_error!(LoadOrderError, VerboseError::Other);
+variant_box_from_error!(LoadMetadataError, VerboseError::Other);
+variant_box_from_error!(WriteMetadataError, VerboseError::Other);
+variant_box_from_error!(ConditionEvaluationError, VerboseError::Other);
+variant_box_from_error!(MultilingualMessageContentsError, VerboseError::Other);
+variant_box_from_error!(RegexError, VerboseError::Other);
+
 impl From<GameHandleCreationError> for VerboseError {
     fn from(value: GameHandleCreationError) -> Self {
         match value {
@@ -71,27 +53,6 @@ impl From<GameHandleCreationError> for VerboseError {
     }
 }
 
-impl From<UnsupportedEnumValueError> for VerboseError {
-    fn from(value: UnsupportedEnumValueError) -> Self {
-        Self::Other(Box::new(value))
-    }
-}
-
-impl From<DatabaseLockPoisonError> for VerboseError {
-    fn from(value: DatabaseLockPoisonError) -> Self {
-        Self::Other(Box::new(value))
-    }
-}
-
-impl From<LoadPluginsError> for VerboseError {
-    fn from(value: LoadPluginsError) -> Self {
-        match value {
-            LoadPluginsError::PluginDataError(e) => e.into(),
-            _ => Self::Other(Box::new(value)),
-        }
-    }
-}
-
 impl From<SortPluginsError> for VerboseError {
     fn from(value: SortPluginsError) -> Self {
         match value {
@@ -113,30 +74,6 @@ impl From<LoadOrderStateError> for VerboseError {
     }
 }
 
-impl From<LoadOrderError> for VerboseError {
-    fn from(value: LoadOrderError) -> Self {
-        Self::Other(Box::new(value))
-    }
-}
-
-impl From<LoadMetadataError> for VerboseError {
-    fn from(value: LoadMetadataError) -> Self {
-        Self::Other(Box::new(value))
-    }
-}
-
-impl From<WriteMetadataError> for VerboseError {
-    fn from(value: WriteMetadataError) -> Self {
-        Self::Other(Box::new(value))
-    }
-}
-
-impl From<ConditionEvaluationError> for VerboseError {
-    fn from(value: ConditionEvaluationError) -> Self {
-        Self::Other(Box::new(value))
-    }
-}
-
 impl From<GroupsPathError> for VerboseError {
     fn from(value: GroupsPathError) -> Self {
         match value {
@@ -158,20 +95,7 @@ impl From<MetadataRetrievalError> for VerboseError {
 
 impl From<PluginDataError> for VerboseError {
     fn from(value: PluginDataError) -> Self {
-        let error = SystemError::from(value);
-        Self::EspluginError(error.code(), error.message().to_string())
-    }
-}
-
-impl From<MultilingualMessageContentsError> for VerboseError {
-    fn from(value: MultilingualMessageContentsError) -> Self {
-        Self::Other(Box::new(value))
-    }
-}
-
-impl From<RegexError> for VerboseError {
-    fn from(value: RegexError) -> Self {
-        Self::Other(Box::new(value))
+        Self::EspluginError(SystemError::from(value))
     }
 }
 
@@ -187,22 +111,10 @@ impl From<VerboseError> for PyErr {
             VerboseError::UndefinedGroupError(g) => {
                 PyErr::new::<UndefinedGroupError, _>((g, message))
             }
-            VerboseError::EspluginError(i, s) => PyErr::new::<EspluginError, _>((i, s)),
+            VerboseError::EspluginError(e) => {
+                PyErr::new::<EspluginError, _>((e.code(), e.message().to_owned()))
+            }
             VerboseError::Other(_) => PyValueError::new_err(message),
         }
     }
 }
-
-fn display_cycle(cycle: &[libloot::Vertex]) -> String {
-    cycle
-        .iter()
-        .map(|v| {
-            if let Some(edge_type) = v.out_edge_type() {
-                format!("{} --[{}]-> ", v.name(), edge_type)
-            } else {
-                v.name().to_string()
-            }
-        })
-        .chain(cycle.first().iter().map(|v| v.name().to_string()))
-        .collect()
-}

pyo3/src/game.rs

@@ -1,12 +1,9 @@
 use std::path::{Path, PathBuf};
 
+use libloot_ffi_errors::UnsupportedEnumValueError;
 use pyo3::{pyclass, pymethods};
 
-use crate::{
-    database::Database,
-    error::{UnsupportedEnumValueError, VerboseError},
-    plugin::Plugin,
-};
+use crate::{database::Database, error::VerboseError, plugin::Plugin};
 
 #[allow(non_camel_case_types)]
 #[pyclass(eq, frozen, hash, ord)]

pyo3/src/metadata.rs

@@ -1,11 +1,12 @@
 use std::hash::{DefaultHasher, Hash, Hasher};
 
+use libloot_ffi_errors::UnsupportedEnumValueError;
 use pyo3::{
     Bound, FromPyObject, PyResult, pyclass, pyfunction, pymethods,
     types::{PyAnyMethods, PyTypeMethods},
 };
 
-use crate::error::{UnsupportedEnumValueError, VerboseError};
+use crate::error::VerboseError;
 
 pub const NONE_REPR: &str = "None";