Remove indirection for wrapping tonic error codes (#1656)

There is no need to use a custom type here. Also removes some unused
existing trait implementations.

Author: aaronmondal

nativelink-error/src/lib.rs

@@ -19,6 +19,8 @@ use nativelink_metric::{
 };
 use prost_types::TimestampError;
 use serde::{Deserialize, Serialize};
+// Reexport of tonic's error codes which we use as "nativelink_error::Code".
+pub use tonic::Code;
 
 #[macro_export]
 macro_rules! make_err {
@@ -48,6 +50,7 @@ macro_rules! error_if {
 
 #[derive(Debug, Eq, PartialEq, Clone, Serialize, Deserialize)]
 pub struct Error {
+    #[serde(with = "CodeDef")]
     pub code: Code,
     pub messages: Vec<String>,
 }
@@ -105,7 +108,7 @@ impl Error {
     }
 
     pub fn to_std_err(self) -> std::io::Error {
-        std::io::Error::new(self.code.into(), self.messages.join(" : "))
+        std::io::Error::new(self.code.into_error_kind(), self.messages.join(" : "))
     }
 
     pub fn message_string(&self) -> String {
@@ -185,12 +188,6 @@ impl From<std::num::ParseIntError> for Error {
     }
 }
 
-impl From<hex::FromHexError> for Error {
-    fn from(err: hex::FromHexError) -> Self {
-        make_err!(Code::InvalidArgument, "{}", err.to_string())
-    }
-}
-
 impl From<std::convert::Infallible> for Error {
     fn from(_err: std::convert::Infallible) -> Self {
         // Infallible is an error type that can never happen.
@@ -207,18 +204,12 @@ impl From<TimestampError> for Error {
 impl From<std::io::Error> for Error {
     fn from(err: std::io::Error) -> Self {
         Self {
-            code: err.kind().into(),
+            code: err.kind().into_code(),
             messages: vec![err.to_string()],
         }
     }
 }
 
-impl From<Code> for Error {
-    fn from(code: Code) -> Self {
-        make_err!(code, "")
-    }
-}
-
 impl From<fred::error::Error> for Error {
     fn from(error: fred::error::Error) -> Self {
         use fred::error::ErrorKind::{
@@ -242,21 +233,15 @@ impl From<fred::error::Error> for Error {
     }
 }
 
-impl From<tonic::transport::Error> for Error {
-    fn from(error: tonic::transport::Error) -> Self {
-        make_err!(Code::Internal, "{}", error.to_string())
-    }
-}
-
 impl From<tonic::Status> for Error {
     fn from(status: tonic::Status) -> Self {
-        make_err!(status.code().into(), "{}", status.to_string())
+        make_err!(status.code(), "{}", status.to_string())
     }
 }
 
 impl From<Error> for tonic::Status {
     fn from(val: Error) -> Self {
-        Self::new(val.code.into(), val.messages.join(" : "))
+        Self::new(val.code, val.messages.join(" : "))
     }
 }
 
@@ -341,9 +326,58 @@ impl<T> ResultExt<T> for Option<T> {
     }
 }
 
-#[derive(Clone, Copy, Debug, PartialEq, Eq, Serialize, Deserialize)]
-#[non_exhaustive] // New Codes may be added in the future, so never exhaustively match!
-pub enum Code {
+trait CodeExt {
+    fn into_error_kind(self) -> std::io::ErrorKind;
+}
+
+impl CodeExt for Code {
+    fn into_error_kind(self) -> std::io::ErrorKind {
+        match self {
+            Code::Aborted => std::io::ErrorKind::Interrupted,
+            Code::AlreadyExists => std::io::ErrorKind::AlreadyExists,
+            Code::DeadlineExceeded => std::io::ErrorKind::TimedOut,
+            Code::InvalidArgument => std::io::ErrorKind::InvalidInput,
+            Code::NotFound => std::io::ErrorKind::NotFound,
+            Code::PermissionDenied => std::io::ErrorKind::PermissionDenied,
+            Code::Unavailable => std::io::ErrorKind::ConnectionRefused,
+            _ => std::io::ErrorKind::Other,
+        }
+    }
+}
+
+trait ErrorKindExt {
+    fn into_code(self) -> Code;
+}
+
+impl ErrorKindExt for std::io::ErrorKind {
+    fn into_code(self) -> Code {
+        match self {
+            Self::NotFound => Code::NotFound,
+            Self::PermissionDenied => Code::PermissionDenied,
+            Self::ConnectionRefused | Self::ConnectionReset | Self::ConnectionAborted => {
+                Code::Unavailable
+            }
+            Self::AlreadyExists => Code::AlreadyExists,
+            Self::InvalidInput | Self::InvalidData => Code::InvalidArgument,
+            Self::TimedOut => Code::DeadlineExceeded,
+            Self::Interrupted => Code::Aborted,
+            Self::NotConnected
+            | Self::AddrInUse
+            | Self::AddrNotAvailable
+            | Self::BrokenPipe
+            | Self::WouldBlock
+            | Self::WriteZero
+            | Self::Other
+            | Self::UnexpectedEof => Code::Internal,
+            _ => Code::Unknown,
+        }
+    }
+}
+
+// Serde definition for tonic::Code. See: https://serde.rs/remote-derive.html
+#[derive(Serialize, Deserialize)]
+#[serde(remote = "Code")]
+pub enum CodeDef {
     Ok = 0,
     Cancelled = 1,
     Unknown = 2,
@@ -364,132 +398,3 @@ pub enum Code {
     // NOTE: Additional codes must be added to stores.rs in ErrorCodes and also
     // in both match statements in retry.rs.
 }
-
-impl From<i32> for Code {
-    fn from(code: i32) -> Self {
-        match code {
-            x if x == Self::Ok as i32 => Self::Ok,
-            x if x == Self::Cancelled as i32 => Self::Cancelled,
-            x if x == Self::Unknown as i32 => Self::Unknown,
-            x if x == Self::InvalidArgument as i32 => Self::InvalidArgument,
-            x if x == Self::DeadlineExceeded as i32 => Self::DeadlineExceeded,
-            x if x == Self::NotFound as i32 => Self::NotFound,
-            x if x == Self::AlreadyExists as i32 => Self::AlreadyExists,
-            x if x == Self::PermissionDenied as i32 => Self::PermissionDenied,
-            x if x == Self::ResourceExhausted as i32 => Self::ResourceExhausted,
-            x if x == Self::FailedPrecondition as i32 => Self::FailedPrecondition,
-            x if x == Self::Aborted as i32 => Self::Aborted,
-            x if x == Self::OutOfRange as i32 => Self::OutOfRange,
-            x if x == Self::Unimplemented as i32 => Self::Unimplemented,
-            x if x == Self::Internal as i32 => Self::Internal,
-            x if x == Self::Unavailable as i32 => Self::Unavailable,
-            x if x == Self::DataLoss as i32 => Self::DataLoss,
-            x if x == Self::Unauthenticated as i32 => Self::Unauthenticated,
-            _ => Self::Unknown,
-        }
-    }
-}
-
-impl From<tonic::Code> for Code {
-    fn from(code: tonic::Code) -> Self {
-        match code {
-            tonic::Code::Ok => Self::Ok,
-            tonic::Code::Cancelled => Self::Cancelled,
-            tonic::Code::Unknown => Self::Unknown,
-            tonic::Code::InvalidArgument => Self::InvalidArgument,
-            tonic::Code::DeadlineExceeded => Self::DeadlineExceeded,
-            tonic::Code::NotFound => Self::NotFound,
-            tonic::Code::AlreadyExists => Self::AlreadyExists,
-            tonic::Code::PermissionDenied => Self::PermissionDenied,
-            tonic::Code::ResourceExhausted => Self::ResourceExhausted,
-            tonic::Code::FailedPrecondition => Self::FailedPrecondition,
-            tonic::Code::Aborted => Self::Aborted,
-            tonic::Code::OutOfRange => Self::OutOfRange,
-            tonic::Code::Unimplemented => Self::Unimplemented,
-            tonic::Code::Internal => Self::Internal,
-            tonic::Code::Unavailable => Self::Unavailable,
-            tonic::Code::DataLoss => Self::DataLoss,
-            tonic::Code::Unauthenticated => Self::Unauthenticated,
-        }
-    }
-}
-
-impl From<Code> for tonic::Code {
-    fn from(val: Code) -> Self {
-        match val {
-            Code::Ok => Self::Ok,
-            Code::Cancelled => Self::Cancelled,
-            Code::Unknown => Self::Unknown,
-            Code::InvalidArgument => Self::InvalidArgument,
-            Code::DeadlineExceeded => Self::DeadlineExceeded,
-            Code::NotFound => Self::NotFound,
-            Code::AlreadyExists => Self::AlreadyExists,
-            Code::PermissionDenied => Self::PermissionDenied,
-            Code::ResourceExhausted => Self::ResourceExhausted,
-            Code::FailedPrecondition => Self::FailedPrecondition,
-            Code::Aborted => Self::Aborted,
-            Code::OutOfRange => Self::OutOfRange,
-            Code::Unimplemented => Self::Unimplemented,
-            Code::Internal => Self::Internal,
-            Code::Unavailable => Self::Unavailable,
-            Code::DataLoss => Self::DataLoss,
-            Code::Unauthenticated => Self::Unauthenticated,
-        }
-    }
-}
-
-impl From<std::io::ErrorKind> for Code {
-    fn from(kind: std::io::ErrorKind) -> Self {
-        match kind {
-            std::io::ErrorKind::NotFound => Self::NotFound,
-            std::io::ErrorKind::PermissionDenied => Self::PermissionDenied,
-            std::io::ErrorKind::ConnectionRefused
-            | std::io::ErrorKind::ConnectionReset
-            | std::io::ErrorKind::ConnectionAborted => Self::Unavailable,
-            std::io::ErrorKind::AlreadyExists => Self::AlreadyExists,
-            std::io::ErrorKind::InvalidInput | std::io::ErrorKind::InvalidData => {
-                Self::InvalidArgument
-            }
-            std::io::ErrorKind::TimedOut => Self::DeadlineExceeded,
-            std::io::ErrorKind::Interrupted => Self::Aborted,
-            std::io::ErrorKind::NotConnected
-            | std::io::ErrorKind::AddrInUse
-            | std::io::ErrorKind::AddrNotAvailable
-            | std::io::ErrorKind::BrokenPipe
-            | std::io::ErrorKind::WouldBlock
-            | std::io::ErrorKind::WriteZero
-            | std::io::ErrorKind::Other
-            | std::io::ErrorKind::UnexpectedEof => Self::Internal,
-            _ => Self::Unknown,
-        }
-    }
-}
-
-impl From<Code> for std::io::ErrorKind {
-    fn from(kind: Code) -> Self {
-        match kind {
-            Code::Aborted => Self::Interrupted,
-            Code::AlreadyExists => Self::AlreadyExists,
-            Code::DeadlineExceeded => Self::TimedOut,
-            Code::InvalidArgument => Self::InvalidInput,
-            Code::NotFound => Self::NotFound,
-            Code::PermissionDenied => Self::PermissionDenied,
-            Code::Unavailable => Self::ConnectionRefused,
-            _ => Self::Other,
-        }
-    }
-}
-
-// Allows for mapping this type into a generic serialization error.
-impl serde::ser::Error for Error {
-    fn custom<T: std::fmt::Display>(msg: T) -> Self {
-        Self::new(Code::InvalidArgument, msg.to_string())
-    }
-}
-
-// Allows for mapping this type into a generic deserialization error.
-impl serde::de::Error for Error {
-    fn custom<T: std::fmt::Display>(msg: T) -> Self {
-        Self::new(Code::InvalidArgument, msg.to_string())
-    }
-}