diff --git a/Cargo.lock b/Cargo.lock index 15989930a8a5..f1fd75c5fd55 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1434,6 +1434,7 @@ name = "common-error" version = "0.1.0" dependencies = [ "snafu", + "strum", ] [[package]] @@ -6898,6 +6899,7 @@ dependencies = [ "async-trait", "client", "common-base", + "common-error", "common-grpc", "common-query", "sqlness", diff --git a/src/client/src/database.rs b/src/client/src/database.rs index 2fba696e52b6..956c1db5b0bc 100644 --- a/src/client/src/database.rs +++ b/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::().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(); diff --git a/src/client/src/error.rs b/src/client/src/error.rs index 4392709e9333..5e2a7979c731 100644 --- a/src/client/src/error.rs +++ b/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 = std::result::Result; @@ -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, } } diff --git a/src/common/error/Cargo.toml b/src/common/error/Cargo.toml index 9d7c475ca06e..bd489611bf03 100644 --- a/src/common/error/Cargo.toml +++ b/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"] } diff --git a/src/common/error/src/status_code.rs b/src/common/error/src/status_code.rs index d26660cb25bc..2dfc2ad4a73f 100644 --- a/src/common/error/src/status_code.rs +++ b/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. diff --git a/src/servers/src/error.rs b/src/servers/src/error.rs index bedfccf7495a..95ea2e263c3f 100644 --- a/src/servers/src/error.rs +++ b/src/servers/src/error.rs @@ -347,11 +347,11 @@ impl From 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); } diff --git a/tests/cases/standalone/select/dummy.result b/tests/cases/standalone/select/dummy.result index eb536264543f..1130dc41018b 100644 --- a/tests/cases/standalone/select/dummy.result +++ b/tests/cases/standalone/select/dummy.result @@ -24,13 +24,13 @@ select 4 + 0.5; select "a"; -Failed to do Flight get, addr: 127.0.0.1:4001, code: 3000, err_msg: Failed to execute sql statement, source: Failed to execute sql, source: Failure during query planning, source: Cannot plan SQL: SELECT "a", source: Schema error: No field named 'a'. Valid fields are . +Error: 3000(PlanQuery), Schema error: No field named 'a'. Valid fields are . select "A"; -Failed to do Flight get, addr: 127.0.0.1:4001, code: 3000, err_msg: Failed to execute sql statement, source: Failed to execute sql, source: Failure during query planning, source: Cannot plan SQL: SELECT "A", source: Schema error: No field named 'A'. Valid fields are . +Error: 3000(PlanQuery), Schema error: No field named 'A'. Valid fields are . select * where "a" = "A"; -Failed to do Flight get, addr: 127.0.0.1:4001, code: 3000, err_msg: Failed to execute sql statement, source: Failed to execute sql, source: Failure during query planning, source: Cannot plan SQL: SELECT * WHERE "a" = "A", source: Schema error: No field named 'a'. Valid fields are . +Error: 3000(PlanQuery), Schema error: No field named 'a'. Valid fields are . diff --git a/tests/runner/Cargo.toml b/tests/runner/Cargo.toml index a38a72e27c16..8945d12896ae 100644 --- a/tests/runner/Cargo.toml +++ b/tests/runner/Cargo.toml @@ -8,6 +8,7 @@ license.workspace = true async-trait = "0.1" client = { path = "../../src/client" } common-base = { path = "../../src/common/base" } +common-error = { path = "../../src/common/error" } common-grpc = { path = "../../src/common/grpc" } common-query = { path = "../../src/common/query" } sqlness = "0.1" diff --git a/tests/runner/src/env.rs b/tests/runner/src/env.rs index 7d0efbf00e97..fa5444de5186 100644 --- a/tests/runner/src/env.rs +++ b/tests/runner/src/env.rs @@ -20,6 +20,8 @@ use std::time::Duration; use async_trait::async_trait; use client::{Client, Database as DB, Error as ClientError}; +use common_error::ext::ErrorExt; +use common_error::snafu::ErrorCompat; use common_query::Output; use sqlness::{Database, EnvController}; use tokio::process::{Child, Command}; @@ -140,7 +142,15 @@ impl Display for ResultDisplayer { } Output::Stream(_) => unreachable!(), }, - Err(e) => write!(f, "{e}"), + Err(e) => { + let status_code = e.status_code(); + let root_cause = e.iter_chain().last().unwrap(); + write!( + f, + "Error: {}({status_code}), {root_cause}", + status_code as u32 + ) + } } } }