refactor!: refactor OtherError to CustomError

kezhuw · kezhuw · commit b71aa2c4738d · 2024-04-27T21:26:53.000+08:00
diff --git a/src/error.rs b/src/error.rs
@@ -1,6 +1,7 @@
+use std::borrow::Cow;
+use std::fmt::{self, Display, Formatter};
 use std::sync::Arc;
 
-use derive_where::derive_where;
 use static_assertions::assert_impl_all;
 use thiserror::Error;
 
@@ -87,18 +88,41 @@ pub enum Error {
     RuntimeInconsistent,
 
     #[error(transparent)]
-    Other(OtherError),
+    Custom(CustomError),
 }
 
 #[derive(Error, Clone, Debug)]
-#[derive_where(Eq, PartialEq)]
-#[error("{message}")]
-pub struct OtherError {
-    message: Arc<String>,
-    #[derive_where(skip(EqHashOrd))]
+pub struct CustomError {
+    message: Option<Arc<Cow<'static, str>>>,
     source: Option<Arc<dyn std::error::Error + Send + Sync + 'static>>,
 }
 
+impl Display for CustomError {
+    fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result {
+        match (self.message.as_ref(), self.source.as_ref()) {
+            (Some(message), None) => f.write_str(message),
+            (Some(message), Some(err)) => write!(f, "{}: {}", message, err),
+            (None, Some(err)) => err.fmt(f),
+            _ => unreachable!("no error message or source"),
+        }
+    }
+}
+
+impl PartialEq for CustomError {
+    fn eq(&self, other: &Self) -> bool {
+        if !self.message.eq(&other.message) {
+            return false;
+        }
+        match (self.source.as_ref(), other.source.as_ref()) {
+            (Some(lhs), Some(rhs)) => Arc::ptr_eq(lhs, rhs),
+            (None, None) => true,
+            _ => false,
+        }
+    }
+}
+
+impl Eq for CustomError {}
+
 impl Error {
     pub(crate) fn is_terminated(&self) -> bool {
         matches!(self, Self::NoHosts | Self::SessionExpired | Self::AuthFailed | Self::ClientClosed)
@@ -131,20 +155,20 @@ impl Error {
         }
     }
 
-    pub(crate) fn new_other(
-        message: impl Into<Arc<String>>,
-        source: Option<Arc<dyn std::error::Error + Send + Sync + 'static>>,
-    ) -> Self {
-        Self::Other(OtherError { message: message.into(), source })
+    pub(crate) fn with_message(message: impl Into<Cow<'static, str>>) -> Self {
+        Self::Custom(CustomError { message: Some(Arc::new(message.into())), source: None })
     }
 
     #[allow(dead_code)]
-    pub(crate) fn other(message: impl Into<String>, source: impl std::error::Error + Send + Sync + 'static) -> Self {
-        Self::new_other(message.into(), Some(Arc::new(source)))
+    pub(crate) fn with_other(
+        message: impl Into<Cow<'static, str>>,
+        source: impl std::error::Error + Send + Sync + 'static,
+    ) -> Self {
+        Self::Custom(CustomError { message: Some(Arc::new(message.into())), source: Some(Arc::new(source)) })
     }
 
-    pub(crate) fn other_from(source: impl std::error::Error + Send + Sync + 'static) -> Self {
-        Self::new_other(source.to_string(), Some(Arc::new(source)))
+    pub(crate) fn other(source: impl std::error::Error + Send + Sync + 'static) -> Self {
+        Self::Custom(CustomError { message: None, source: Some(Arc::new(source)) })
     }
 }
 
diff --git a/src/sasl/mod.rs b/src/sasl/mod.rs
@@ -98,7 +98,7 @@ impl SaslSession {
             return Err(Error::UnexpectedError(format!("SASL {} session already finished", self.name())));
         }
         self.output.clear();
-        match self.session.step(Some(challenge), &mut self.output).map_err(|e| Error::other(format!("{e}"), e))? {
+        match self.session.step(Some(challenge), &mut self.output).map_err(Error::other)? {
             State::Running => Ok(Some(&self.output)),
             State::Finished(MessageSent::Yes) => {
                 self.finished = true;
diff --git a/src/session/depot.rs b/src/session/depot.rs
@@ -248,7 +248,7 @@ impl Depot {
                 if err.kind() == io::ErrorKind::WouldBlock {
                     return Ok(());
                 }
-                return Err(Error::other_from(err));
+                return Err(Error::other(err));
             }
             return Ok(());
         }
@@ -258,7 +258,7 @@ impl Depot {
                 if err.kind() == io::ErrorKind::WouldBlock {
                     return Ok(());
                 }
-                return Err(Error::other_from(err));
+                return Err(Error::other(err));
             },
             Ok(written_bytes) => written_bytes,
         };
diff --git a/src/session/mod.rs b/src/session/mod.rs
@@ -113,10 +113,10 @@ impl Builder {
         let session = match self.session {
             Some(session) => {
                 if session.is_readonly() {
-                    return Err(Error::new_other(
-                        format!("can't reestablish readonly and hence local session {}", session.id()),
-                        None,
-                    ));
+                    return Err(Error::with_message(format!(
+                        "can't reestablish readonly and hence local session {}",
+                        session.id()
+                    )));
                 }
                 Span::current().record("session", display(session.id()));
                 session
@@ -490,7 +490,7 @@ impl Session {
             },
             Err(err) => {
                 if err.kind() != io::ErrorKind::WouldBlock {
-                    return Err(Error::other_from(err));
+                    return Err(Error::other(err));
                 }
             },
             _ => {},
@@ -539,7 +539,7 @@ impl Session {
                 },
                 now = tick.tick() => {
                     if now >= self.last_recv + self.connector.timeout() {
-                        return Err(Error::new_other(format!("no response from connection in {}ms", self.connector.timeout().as_millis()), None));
+                        return Err(Error::with_message(format!("no response from connection in {}ms", self.connector.timeout().as_millis())));
                     }
                 },
             }
@@ -583,7 +583,7 @@ impl Session {
             select! {
                 Some(endpoint) = Self::poll(&mut seek_for_writable), if seek_for_writable.is_some() => {
                     seek_for_writable = None;
-                    err = Some(Error::new_other(format!("encounter writable server {}", endpoint), None));
+                    err = Some(Error::with_message(format!("encounter writable server {}", endpoint)));
                     channel_halted = true;
                 },
                 _ = conn.readable() => {
@@ -613,7 +613,7 @@ impl Session {
                 },
                 now = tick.tick() => {
                     if now >= self.last_recv + self.connector.timeout() {
-                        return Err(Error::new_other(format!("no response from connection in {}ms", self.connector.timeout().as_millis()), None));
+                        return Err(Error::with_message(format!("no response from connection in {}ms", self.connector.timeout().as_millis())));
                     }
                     if self.last_ping.is_none() && now >= self.last_send + self.ping_timeout {
                         self.send_ping(depot, now);
diff --git a/src/tls.rs b/src/tls.rs
@@ -126,10 +126,10 @@ impl TlsOptions {
         for r in rustls_pemfile::certs(&mut certs.as_bytes()) {
             let cert = match r {
                 Ok(cert) => cert,
-                Err(err) => return Err(Error::other(format!("fail to read cert {}", err), err)),
+                Err(err) => return Err(Error::with_other("fail to read cert", err)),
             };
             if let Err(err) = self.ca_certs.add(cert) {
-                return Err(Error::other(format!("fail to add cert {}", err), err));
+                return Err(Error::with_other("fail to add cert", err));
             }
         }
         Ok(self)
@@ -139,11 +139,11 @@ impl TlsOptions {
     pub fn with_pem_identity(mut self, cert: &str, key: &str) -> Result<Self> {
         let r: std::result::Result<Vec<_>, _> = rustls_pemfile::certs(&mut cert.as_bytes()).collect();
         let certs = match r {
-            Err(err) => return Err(Error::other(format!("fail to read cert {}", err), err)),
+            Err(err) => return Err(Error::with_other("fail to read cert", err)),
             Ok(certs) => certs,
         };
         let key = match rustls_pemfile::private_key(&mut key.as_bytes()) {
-            Err(err) => return Err(Error::other(format!("fail to read client private key {err}"), err)),
+            Err(err) => return Err(Error::with_other("fail to read client private key", err)),
             Ok(None) => return Err(Error::BadArguments(&"no client private key")),
             Ok(Some(key)) => key,
         };
@@ -163,7 +163,7 @@ impl TlsOptions {
         if let Some((client_cert, client_key)) = self.identity.take() {
             match builder.with_client_auth_cert(client_cert, client_key) {
                 Ok(config) => Ok(config),
-                Err(err) => Err(Error::other(format!("invalid client private key {err}"), err)),
+                Err(err) => Err(Error::with_other("invalid client private key", err)),
             }
         } else {
             Ok(builder.with_no_client_auth())

Original file line number	Diff line number	Diff line change
`@@ -98,7 +98,7 @@ impl SaslSession {`
`98`	`98`	`return Err(Error::UnexpectedError(format!("SASL {} session already finished", self.name())));`
`99`	`99`	`}`
`100`	`100`	`self.output.clear();`
`101`		`- match self.session.step(Some(challenge), &mut self.output).map_err(\|e\| Error::other(format!("{e}"), e))? {`
	`101`	`+ match self.session.step(Some(challenge), &mut self.output).map_err(Error::other)? {`
`102`	`102`	`State::Running => Ok(Some(&self.output)),`
`103`	`103`	`State::Finished(MessageSent::Yes) => {`
`104`	`104`	`self.finished = true;`
Original file line number	Diff line number	Diff line change
`@@ -248,7 +248,7 @@ impl Depot {`
`248`	`248`	`if err.kind() == io::ErrorKind::WouldBlock {`
`249`	`249`	`return Ok(());`
`250`	`250`	`}`
`251`		`- return Err(Error::other_from(err));`
	`251`	`+ return Err(Error::other(err));`
`252`	`252`	`}`
`253`	`253`	`return Ok(());`
`254`	`254`	`}`
`@@ -258,7 +258,7 @@ impl Depot {`
`258`	`258`	`if err.kind() == io::ErrorKind::WouldBlock {`
`259`	`259`	`return Ok(());`
`260`	`260`	`}`
`261`		`- return Err(Error::other_from(err));`
	`261`	`+ return Err(Error::other(err));`
`262`	`262`	`},`
`263`	`263`	`Ok(written_bytes) => written_bytes,`
`264`	`264`	`};`