Fix Logging errors (#145)

* adding string description for errors, and fix the their logging Signed-off-by: gabrik <[email protected]> * fix format Signed-off-by: gabrik <[email protected]> * fix clippy issues Signed-off-by: gabrik <[email protected]> * fix: remove duplicated + improve format --------- Signed-off-by: gabrik <[email protected]> Co-authored-by: Julien Loudet <[email protected]>
eclipse-zenoh-flow · Feb 16, 2023 · dcfce88 · dcfce88
1 parent af65f1c
commit dcfce88
Show file tree

Hide file tree

Showing 3 changed files with 66 additions and 34 deletions.
diff --git a/zenoh-flow-daemon/src/runtime.rs b/zenoh-flow-daemon/src/runtime.rs
@@ -29,6 +29,7 @@ use zenoh_flow::runtime::{
     RuntimeStatusKind,
 };
 use zenoh_flow::types::ControlMessage;
+use zenoh_flow::zferror;
 use zenoh_flow::zfresult::ErrorKind;
 use zenoh_flow::DaemonResult;
 use zenoh_flow::Result as ZFResult;
@@ -301,7 +302,7 @@ impl Runtime {
 
                 Ok(record)
             }
-            None => Err(ErrorKind::InstanceNotFound(instance_id)),
+            None => Err(zferror!(ErrorKind::InstanceNotFound(instance_id))),
         }
     }
 
@@ -435,7 +436,7 @@ impl Runtime {
 
                 Ok(())
             }
-            None => Err(ErrorKind::InstanceNotFound(instance_id)),
+            None => Err(zferror!(ErrorKind::InstanceNotFound(instance_id))),
         }
     }
 
@@ -464,7 +465,7 @@ impl Runtime {
 
                 Ok(())
             }
-            None => Err(ErrorKind::InstanceNotFound(instance_id)),
+            None => Err(zferror!(ErrorKind::InstanceNotFound(instance_id))),
         }
     }
 
@@ -504,7 +505,7 @@ impl Runtime {
 
                 Ok(())
             }
-            None => Err(ErrorKind::InstanceNotFound(instance_id)),
+            None => Err(zferror!(ErrorKind::InstanceNotFound(instance_id))),
         }
     }
 
@@ -532,7 +533,7 @@ impl Runtime {
 
                 Ok(())
             }
-            None => Err(ErrorKind::InstanceNotFound(instance_id)),
+            None => Err(zferror!(ErrorKind::InstanceNotFound(instance_id))),
         }
     }
 
@@ -546,7 +547,7 @@ impl Runtime {
 
         match _state.graphs.get_mut(&instance_id) {
             Some(instance) => Ok(instance.start_node(&node.into())?),
-            None => Err(ErrorKind::InstanceNotFound(instance_id)),
+            None => Err(zferror!(ErrorKind::InstanceNotFound(instance_id))),
         }
     }
 
@@ -560,7 +561,7 @@ impl Runtime {
 
         match _state.graphs.get_mut(&instance_id) {
             Some(instance) => Ok(instance.stop_node(&node.into()).await?),
-            None => Err(ErrorKind::InstanceNotFound(instance_id)),
+            None => Err(zferror!(ErrorKind::InstanceNotFound(instance_id))),
         }
     }
 
@@ -648,24 +649,24 @@ impl Runtime {
         _node: String,
         _message: ControlMessage,
     ) -> DaemonResult<()> {
-        Err(ErrorKind::Unimplemented)
+        Err(zferror!(ErrorKind::Unimplemented))
     }
     pub(crate) async fn check_operator_compatibility(
         &self,
         _operator: OperatorDescriptor,
     ) -> DaemonResult<bool> {
-        Err(ErrorKind::Unimplemented)
+        Err(zferror!(ErrorKind::Unimplemented))
     }
     pub(crate) async fn check_source_compatibility(
         &self,
         _source: SourceDescriptor,
     ) -> DaemonResult<bool> {
-        Err(ErrorKind::Unimplemented)
+        Err(zferror!(ErrorKind::Unimplemented))
     }
     pub(crate) async fn check_sink_compatibility(
         &self,
         _sink: SinkDescriptor,
     ) -> DaemonResult<bool> {
-        Err(ErrorKind::Unimplemented)
+        Err(zferror!(ErrorKind::Unimplemented))
     }
 }
diff --git a/zenoh-flow-daemon/src/worker.rs b/zenoh-flow-daemon/src/worker.rs
@@ -20,7 +20,7 @@ use uhlc::HLC;
 
 use zenoh_flow::runtime::JobKind;
 use zenoh_flow::runtime::{worker_pool::WorkerTrait, Job};
-use zenoh_flow::zfresult::ErrorKind;
+use zenoh_flow::zfresult::ZFError;
 use zenoh_flow::Result as ZFResult;
 
 use crate::runtime::Runtime;
@@ -51,7 +51,7 @@ impl Worker {
         }
     }
 
-    async fn store_error(&self, job: &mut Job, e: ErrorKind) -> ZFResult<()> {
+    async fn store_error(&self, job: &mut Job, e: ZFError) -> ZFResult<()> {
         log::error!(
             "[Worker: {}] Got error when running Job: {} - {:?}",
             self.id,

diff --git a/zenoh-flow/src/zfresult.rs b/zenoh-flow/src/zfresult.rs
@@ -24,19 +24,19 @@ use uuid::Uuid;
 #[macro_export]
 macro_rules! zferror {
     ($kind: expr, $source: expr => $($t: tt)*) => {
-        $crate::zfresult::ZFError::new($kind, $crate::anyhow!($($t)*), file!(), line!()).set_source($source)
+        $crate::zfresult::ZFError::new($kind, $crate::anyhow!($($t)*), file!().to_string(), line!()).set_source($source)
     };
     ($kind: expr, $t: literal) => {
-        $crate::zfresult::ZFError::new($kind, $crate::anyhow!($t), file!(), line!())
+        $crate::zfresult::ZFError::new($kind, $crate::anyhow!($t), file!().to_string(), line!())
     };
     ($kind: expr, $t: expr) => {
-        $crate::zfresult::ZFError::new($kind, $t, file!(), line!())
+        $crate::zfresult::ZFError::new($kind, $t, file!().to_string(), line!())
     };
     ($kind: expr, $($t: tt)*) => {
-        $crate::zfresult::ZFError::new($kind, $crate::anyhow!($($t)*), file!(), line!())
+        $crate::zfresult::ZFError::new($kind, $crate::anyhow!($($t)*), file!().to_string(), line!())
     };
     ($kind: expr) => {
-        $crate::zfresult::ZFError::new($kind, $crate::anyhow!("{:?}", $kind), file!(), line!())
+        $crate::zfresult::ZFError::new($kind, $crate::anyhow!("{:?}", $kind), file!().to_string(), line!())
     };
 }
 
@@ -64,7 +64,7 @@ pub type Error = Box<dyn std::error::Error + Send + Sync + 'static>;
 /// The Zenoh Flow result type.
 pub type ZFResult<T> = Result<T, Error>;
 
-pub type DaemonResult<T> = Result<T, ErrorKind>;
+pub type DaemonResult<T> = Result<T, ZFError>;
 
 /// The Zenoh Flow error
 /// It contains mapping to most of the errors that could happen within
@@ -123,32 +123,36 @@ pub struct ZFError {
     kind: ErrorKind,
     #[serde(skip_serializing, skip_deserializing)]
     error: Option<AnyError>,
-    file: &'static str,
+    desc: Option<String>,
+    file: String,
     line: u32,
     #[serde(skip_serializing, skip_deserializing)]
     source: Option<Error>,
+    source_desc: Option<String>,
 }
 
 unsafe impl Send for ZFError {}
 unsafe impl Sync for ZFError {}
 
 impl ZFError {
-    pub fn new<E: Into<AnyError>>(
-        kind: ErrorKind,
-        error: E,
-        file: &'static str,
-        line: u32,
-    ) -> ZFError {
+    pub fn new<E: Into<AnyError>>(kind: ErrorKind, error: E, file: String, line: u32) -> ZFError {
+        let error: AnyError = error.into();
+
         ZFError {
             kind,
-            error: Some(error.into()),
+            desc: Some(format!("{error:?}")),
+            error: Some(error),
             file,
             line,
             source: None,
+            source_desc: None,
         }
     }
+
     pub fn set_source<S: Into<Error>>(mut self, source: S) -> Self {
-        self.source = Some(source.into());
+        let source: Error = source.into();
+        self.source_desc = Some(format!("{source:?}"));
+        self.source = Some(source);
         self
     }
 
@@ -157,13 +161,38 @@ impl ZFError {
     }
 }
 
+impl std::clone::Clone for ZFError {
+    fn clone(&self) -> Self {
+        ZFError {
+            kind: self.kind.clone(),
+            error: None,
+            desc: self.desc.clone(),
+            file: self.file.clone(),
+            line: self.line,
+            source: None,
+            source_desc: self.source_desc.clone(),
+        }
+    }
+
+    fn clone_from(&mut self, source: &Self) {
+        self.kind = source.kind.clone();
+        self.error = None;
+        self.desc = source.desc.clone();
+        self.file = source.file.clone();
+        self.line = source.line;
+        self.source = None;
+        self.source_desc = self.source_desc.clone();
+    }
+}
+
 impl std::error::Error for ZFError {
     fn source(&self) -> Option<&'_ (dyn std::error::Error + 'static)> {
         self.source
             .as_ref()
             .map(|r| unsafe { std::mem::transmute(r.as_ref()) })
     }
 }
+
 impl fmt::Debug for ZFError {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         fmt::Display::fmt(&self, f)
@@ -172,13 +201,15 @@ impl fmt::Debug for ZFError {
 
 impl fmt::Display for ZFError {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        write!(
-            f,
-            "{:?} {:?} at {}:{}.",
-            self.kind, self.error, self.file, self.line
-        )?;
+        let desc = if let Some(desc) = &self.desc {
+            desc
+        } else {
+            "(no description)"
+        };
+
+        write!(f, "{}:{} {:?}: {:?}", self.file, self.line, self.kind, desc)?;
         if let Some(s) = &self.source {
-            write!(f, " - Caused by {}", *s)?;
+            write!(f, "\nCaused by {}: {:?}", *s, self.source_desc)?;
         }
         Ok(())
     }