refactor: adjust outermost error message (#859)

* refactor: adjust outermost error message

Signed-off-by: Ruihang Xia <waynestxia@gmail.com>

* fix clippy

Signed-off-by: Ruihang Xia <waynestxia@gmail.com>

* preserve tonic status code

Signed-off-by: Ruihang Xia <waynestxia@gmail.com>

Signed-off-by: Ruihang Xia <waynestxia@gmail.com>

src/client/src/database.rs

@@ -12,6 +12,8 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
+use std::str::FromStr;
+
 use api::v1::ddl_request::Expr as DdlExpr;
 use api::v1::greptime_request::Request;
 use api::v1::query_request::Query;
@@ -113,15 +115,17 @@ impl Database {
             .await
             .map_err(|e| {
                 let code = get_metadata_value(&e, INNER_ERROR_CODE)
-                    .and_then(|s| s.parse::<u32>().ok())
-                    .unwrap_or(e.code() as u32);
-                let err_msg = get_metadata_value(&e, INNER_ERROR_MSG).unwrap_or(e.to_string());
-                error::FlightGetSnafu {
-                    addr: client.addr(),
-                    code,
-                    err_msg,
-                }
-                .build()
+                    .and_then(|s| StatusCode::from_str(&s).ok())
+                    .unwrap_or(StatusCode::Unknown);
+                let msg = get_metadata_value(&e, INNER_ERROR_MSG).unwrap_or(e.to_string());
+                error::ExternalSnafu { code, msg }
+                    .fail::<()>()
+                    .map_err(BoxedError::new)
+                    .context(error::FlightGetSnafu {
+                        tonic_code: e.code(),
+                        addr: client.addr(),
+                    })
+                    .unwrap_err()
             })?;
 
         let decoder = &mut FlightDecoder::default();

src/client/src/error.rs

@@ -15,6 +15,7 @@
 use std::any::Any;
 
 use common_error::prelude::*;
+use tonic::Code;
 
 #[derive(Debug, Snafu)]
 #[snafu(visibility(pub))]
@@ -26,16 +27,15 @@ pub enum Error {
     },
 
     #[snafu(display(
-        "Failed to do Flight get, addr: {}, code: {}, err_msg: {}",
+        "Failed to do Flight get, addr: {}, code: {}, source: {}",
         addr,
-        code,
-        err_msg
+        tonic_code,
+        source
     ))]
     FlightGet {
         addr: String,
-        code: u32,
-        err_msg: String,
-        backtrace: Backtrace,
+        tonic_code: Code,
+        source: BoxedError,
     },
 
     #[snafu(display("Failed to convert FlightData, source: {}", source))]
@@ -69,6 +69,10 @@ pub enum Error {
         #[snafu(backtrace)]
         source: common_grpc::error::Error,
     },
+
+    /// Error deserialized from gRPC metadata
+    #[snafu(display("{}", msg))]
+    ExternalError { code: StatusCode, msg: String },
 }
 
 pub type Result<T> = std::result::Result<T, Error>;
@@ -77,13 +81,14 @@ impl ErrorExt for Error {
     fn status_code(&self) -> StatusCode {
         match self {
             Error::IllegalFlightMessages { .. }
-            | Error::FlightGet { .. }
             | Error::ColumnDataType { .. }
             | Error::MissingField { .. } => StatusCode::Internal,
+            Error::FlightGet { source, .. } => source.status_code(),
             Error::CreateChannel { source, .. } | Error::ConvertFlightData { source } => {
                 source.status_code()
             }
             Error::IllegalGrpcClientState { .. } => StatusCode::Unexpected,
+            Error::ExternalError { code, .. } => *code,
         }
     }
 

src/common/error/Cargo.toml

@@ -5,4 +5,5 @@ edition.workspace = true
 license.workspace = true
 
 [dependencies]
+strum = "0.24.1"
 snafu = { version = "0.7", features = ["backtraces"] }

src/common/error/src/status_code.rs

@@ -14,8 +14,10 @@
 
 use std::fmt;
 
+use strum::EnumString;
+
 /// Common status code for public API.
-#[derive(Debug, Clone, Copy, PartialEq, Eq)]
+#[derive(Debug, Clone, Copy, PartialEq, Eq, EnumString)]
 pub enum StatusCode {
     // ====== Begin of common status code ==============
     /// Success.

src/servers/src/error.rs

@@ -347,11 +347,11 @@ impl From<Error> for tonic::Status {
 
         // If either of the status_code or error msg cannot convert to valid HTTP header value
         // (which is a very rare case), just ignore. Client will use Tonic status code and message.
-        if let Ok(code) = HeaderValue::from_bytes((err.status_code() as u32).to_string().as_bytes())
-        {
+        if let Ok(code) = HeaderValue::from_bytes(err.status_code().to_string().as_bytes()) {
             headers.insert(INNER_ERROR_CODE, code);
         }
-        if let Ok(err_msg) = HeaderValue::from_bytes(err.to_string().as_bytes()) {
+        let root_error = err.iter_chain().last().unwrap();
+        if let Ok(err_msg) = HeaderValue::from_bytes(root_error.to_string().as_bytes()) {
             headers.insert(INNER_ERROR_MSG, err_msg);
         }