tisonkun commented on code in PR #47:
URL: https://github.com/apache/datasketches-rust/pull/47#discussion_r2650483717


##########
datasketches/src/error.rs:
##########
@@ -19,31 +19,197 @@
 
 use std::fmt;
 
-/// Errors that can occur during sketch serialization or deserialization
-#[derive(Debug, Clone)]
-pub enum SerdeError {
-    /// Insufficient data in buffer
-    InsufficientData(String),
-    /// Invalid sketch family identifier
-    InvalidFamily(String),
-    /// Unsupported serialization version
-    UnsupportedVersion(String),
-    /// Invalid parameter value
-    InvalidParameter(String),
-    /// Malformed or corrupt sketch data
-    MalformedData(String),
-}
-
-impl fmt::Display for SerdeError {
-    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+/// ErrorKind is all kinds of Error of datasketches.
+#[derive(Clone, Copy, Debug, PartialEq, Eq, Hash)]
+#[non_exhaustive]
+pub enum ErrorKind {
+    /// The argument provided is invalid.
+    InvalidArgument,
+    /// The sketch data deserializing is malformed.
+    MalformedDeserializeData,
+}
+
+impl ErrorKind {
+    /// Convert this error kind instance into static str.
+    pub const fn into_static(self) -> &'static str {
         match self {
-            SerdeError::InsufficientData(msg) => write!(f, "insufficient data: 
{}", msg),
-            SerdeError::InvalidFamily(msg) => write!(f, "invalid family: {}", 
msg),
-            SerdeError::UnsupportedVersion(msg) => write!(f, "unsupported 
version: {}", msg),
-            SerdeError::InvalidParameter(msg) => write!(f, "invalid parameter: 
{}", msg),
-            SerdeError::MalformedData(msg) => write!(f, "malformed data: {}", 
msg),
+            ErrorKind::InvalidArgument => "InvalidArgument",
+            ErrorKind::MalformedDeserializeData => "MalformedDeserializeData",
         }
     }
 }
 
-impl std::error::Error for SerdeError {}
+impl fmt::Display for ErrorKind {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        write!(f, "{}", self.into_static())
+    }
+}
+
+/// Error is the error struct returned by all datasketches functions.
+pub struct Error {
+    kind: ErrorKind,
+    message: String,
+    context: Vec<(&'static str, String)>,
+    source: Option<anyhow::Error>,

Review Comment:
   IIRC the auto convesion (From/Into) will have some trouble. I ever try to 
replace `anyhow::Error` with box dyn Error + Send/Sync, but it ruins the 
experience and I remember some compile failure.
   
   However, we don't depend on source error now. So now, I tend to remove this 
feature as well as the `anyhow` dependency. When we need it, we can add it back 
and evaluate the solution at that moment.



##########
datasketches/src/error.rs:
##########
@@ -19,31 +19,197 @@
 
 use std::fmt;
 
-/// Errors that can occur during sketch serialization or deserialization
-#[derive(Debug, Clone)]
-pub enum SerdeError {
-    /// Insufficient data in buffer
-    InsufficientData(String),
-    /// Invalid sketch family identifier
-    InvalidFamily(String),
-    /// Unsupported serialization version
-    UnsupportedVersion(String),
-    /// Invalid parameter value
-    InvalidParameter(String),
-    /// Malformed or corrupt sketch data
-    MalformedData(String),
-}
-
-impl fmt::Display for SerdeError {
-    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+/// ErrorKind is all kinds of Error of datasketches.
+#[derive(Clone, Copy, Debug, PartialEq, Eq, Hash)]
+#[non_exhaustive]
+pub enum ErrorKind {
+    /// The argument provided is invalid.
+    InvalidArgument,
+    /// The sketch data deserializing is malformed.
+    MalformedDeserializeData,
+}
+
+impl ErrorKind {
+    /// Convert this error kind instance into static str.
+    pub const fn into_static(self) -> &'static str {
         match self {
-            SerdeError::InsufficientData(msg) => write!(f, "insufficient data: 
{}", msg),
-            SerdeError::InvalidFamily(msg) => write!(f, "invalid family: {}", 
msg),
-            SerdeError::UnsupportedVersion(msg) => write!(f, "unsupported 
version: {}", msg),
-            SerdeError::InvalidParameter(msg) => write!(f, "invalid parameter: 
{}", msg),
-            SerdeError::MalformedData(msg) => write!(f, "malformed data: {}", 
msg),
+            ErrorKind::InvalidArgument => "InvalidArgument",
+            ErrorKind::MalformedDeserializeData => "MalformedDeserializeData",
         }
     }
 }
 
-impl std::error::Error for SerdeError {}
+impl fmt::Display for ErrorKind {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        write!(f, "{}", self.into_static())
+    }
+}
+
+/// Error is the error struct returned by all datasketches functions.
+pub struct Error {
+    kind: ErrorKind,
+    message: String,
+    context: Vec<(&'static str, String)>,
+    source: Option<anyhow::Error>,

Review Comment:
   IIRC the auto convesion (From/Into) will have some trouble. I ever try to 
replace `anyhow::Error` with box dyn Error + Send/Sync in OpenDAL, but it ruins 
the experience and I remember some compile failure.
   
   However, we don't depend on source error now. So now, I tend to remove this 
feature as well as the `anyhow` dependency. When we need it, we can add it back 
and evaluate the solution at that moment.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to