refactor: move huge context data away from logs main msg

And instead add them to a property. This allow to keep the main log
messages leans while still providing detailed errors info.

Author: Alenar

mithril-signer/src/main.rs

@@ -211,10 +211,7 @@ async fn main() -> StdResult<()> {
         loop {
             interval.tick().await;
             if let Err(err) = cardano_transaction_preloader.preload().await {
-                crit!(
-                    preload_logger,
-                    "🔥 Cardano transactions preloader failed: {err:?}"
-                );
+                crit!(preload_logger, "🔥 Cardano transactions preloader failed"; "error" => ?err);
             }
             info!(
                 preload_logger,
@@ -267,7 +264,7 @@ async fn main() -> StdResult<()> {
 
     let shutdown_reason = match join_set.join_next().await {
         Some(Err(e)) => {
-            crit!(root_logger, "A critical error occurred: {e:?}");
+            crit!(root_logger, "A critical error occurred"; "error" => ?e);
             None
         }
         Some(Ok(res)) => res?,

mithril-signer/src/runtime/error.rs

@@ -1,3 +1,4 @@
+use slog::{crit, error, Logger};
 use std::fmt::Display;
 
 use mithril_common::entities::EpochError;
@@ -40,39 +41,37 @@ impl RuntimeError {
             }
         )
     }
+
+    /// Write the error to the given logger.
+    pub fn write_to_log(&self, logger: &Logger) {
+        match self {
+            Self::KeepState { nested_error, .. } => match nested_error {
+                None => error!(logger, "{self}"),
+                Some(err) => error!(logger, "{self}"; "nested_error" => ?err),
+            },
+            Self::Critical { nested_error, .. } => match nested_error {
+                None => crit!(logger, "{self}"),
+                Some(err) => crit!(logger, "{self}"; "nested_error" => ?err),
+            },
+        }
+    }
 }
 
 impl std::error::Error for RuntimeError {}
 
 impl Display for RuntimeError {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
         match self {
-            Self::KeepState {
-                message,
-                nested_error,
-            } => {
-                let nested = if let Some(error) = nested_error {
-                    format!("nested error = {error:?}")
-                } else {
-                    String::new()
-                };
+            Self::KeepState { message, .. } => {
                 write!(
                     f,
-                    "An error occurred, runtime state kept. message = '{message}'. {nested}"
+                    "An error occurred, runtime state kept. message = '{message}'"
                 )
             }
-            Self::Critical {
-                message,
-                nested_error,
-            } => {
-                let nested = if let Some(error) = nested_error {
-                    format!("nested error = {error:?}")
-                } else {
-                    String::new()
-                };
+            Self::Critical { message, .. } => {
                 write!(
                     f,
-                    "A critical error occurred, aborting runtime. message = '{message}'. {nested}"
+                    "A critical error occurred, aborting runtime. message = '{message}'"
                 )
             }
         }
@@ -96,3 +95,109 @@ impl From<EpochError> for RuntimeError {
         }
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use anyhow::anyhow;
+    use std::path::Path;
+
+    use mithril_common::test_utils::TempDir;
+
+    use crate::test_tools::TestLogger;
+
+    use super::*;
+
+    /// Separate function so the logger is dropped and flushed before the assertion.
+    fn write_log(log_file: &Path, error: &RuntimeError) {
+        let logger = TestLogger::file(log_file);
+        error.write_to_log(&logger);
+    }
+
+    fn nested_error_debug_string(error: &RuntimeError) -> String {
+        let error = match error {
+            RuntimeError::KeepState { nested_error, .. } => nested_error,
+            RuntimeError::Critical { nested_error, .. } => nested_error,
+        };
+        match error {
+            None => String::new(),
+            Some(err) => {
+                format!("{err:?}")
+            }
+        }
+    }
+
+    #[test]
+    fn log_critical_without_nested_error() {
+        let log_file = TempDir::create("signer_runtime_error", "log_critical_without_nested_error")
+            .join("file.log");
+
+        let error = RuntimeError::Critical {
+            message: "Critical error".to_string(),
+            nested_error: None,
+        };
+        write_log(&log_file, &error);
+
+        let log_content = std::fs::read_to_string(&log_file).unwrap();
+        assert!(log_content.contains(&format!("{error}")));
+        assert!(!log_content.contains("nested_error"));
+    }
+
+    #[test]
+    fn log_critical_with_nested_error() {
+        let log_file = TempDir::create("signer_runtime_error", "log_critical_with_nested_error")
+            .join("file.log");
+
+        let error = RuntimeError::Critical {
+            message: "Critical error".to_string(),
+            nested_error: Some(
+                anyhow!("Another context error")
+                    .context("Context error")
+                    .context("Critical nested error"),
+            ),
+        };
+        write_log(&log_file, &error);
+
+        let log_content = std::fs::read_to_string(&log_file).unwrap();
+        assert!(log_content.contains(&format!("{error}")));
+        assert!(log_content.contains(&nested_error_debug_string(&error)));
+    }
+
+    #[test]
+    fn log_keep_state_without_nested_error() {
+        let log_file = TempDir::create(
+            "signer_runtime_error",
+            "log_keep_state_without_nested_error",
+        )
+        .join("file.log");
+
+        let error = RuntimeError::KeepState {
+            message: "KeepState error".to_string(),
+            nested_error: None,
+        };
+        write_log(&log_file, &error);
+
+        let log_content = std::fs::read_to_string(&log_file).unwrap();
+        assert!(log_content.contains(&format!("{error}")));
+        assert!(!log_content.contains("nested_error"));
+    }
+
+    #[test]
+    fn log_keep_state_with_nested_error() {
+        let log_file = TempDir::create("signer_runtime_error", "log_keep_state_with_nested_error")
+            .join("file.log");
+
+        let error = RuntimeError::KeepState {
+            message: "KeepState error".to_string(),
+            nested_error: Some(
+                anyhow!("Another context error")
+                    .context("Context error")
+                    .context("KeepState nested error"),
+            ),
+        };
+        write_log(&log_file, &error);
+
+        let log_content = std::fs::read_to_string(&log_file).unwrap();
+        assert!(log_content.contains(&format!("{error}")));
+        assert!(log_content.contains(&nested_error_debug_string(&error)));
+    }
+}

mithril-signer/src/runtime/state_machine.rs

@@ -1,4 +1,4 @@
-use slog::{crit, debug, error, info, Logger};
+use slog::{debug, info, Logger};
 use std::{fmt::Display, ops::Deref, sync::Arc, time::Duration};
 use tokio::{sync::Mutex, time::sleep};
 
@@ -113,12 +113,9 @@ impl StateMachine {
 
         loop {
             if let Err(e) = self.cycle().await {
+                e.write_to_log(&self.logger);
                 if e.is_critical() {
-                    crit!(self.logger, "{e}");
-
                     return Err(e);
-                } else {
-                    error!(self.logger, "{e}");
                 }
             }
 

mithril-signer/src/services/epoch_service.rs

@@ -177,7 +177,7 @@ impl EpochService for MithrilEpochService {
         epoch_settings: SignerEpochSettings,
         allowed_discriminants: BTreeSet<SignedEntityTypeDiscriminants>,
     ) -> StdResult<()> {
-        debug!(self.logger, "register_epoch_settings: {:?}", epoch_settings);
+        debug!(self.logger, "register_epoch_settings"; "epoch_settings" => ?epoch_settings);
 
         let epoch = epoch_settings.epoch;
         let protocol_initializer = self