From d1838fb28de054c5f4c82ea07683f28acd4871db Mon Sep 17 00:00:00 2001 From: shuiyisong <113876041+shuiyisong@users.noreply.github.com> Date: Tue, 4 Jun 2024 11:29:15 +0800 Subject: [PATCH] refactor: move `define_into_tonic_status` to `common-error` (#4095) * chore: finish cherry-pick * chore: remove unused code --- Cargo.lock | 2 +- src/common/error/Cargo.toml | 1 + src/common/error/src/status_code.rs | 70 ++++++++++++++++++++++++++++ src/datanode/src/error.rs | 2 +- src/flow/src/adapter/error.rs | 2 +- src/frontend/src/error.rs | 2 +- src/meta-srv/src/error.rs | 2 +- src/operator/Cargo.toml | 1 - src/operator/src/error.rs | 2 +- src/servers/src/error.rs | 71 +---------------------------- 10 files changed, 78 insertions(+), 77 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 35fabee98b6c..18fa00347ff4 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1794,6 +1794,7 @@ version = "0.8.1" dependencies = [ "snafu 0.8.3", "strum 0.25.0", + "tonic 0.11.0", ] [[package]] @@ -6713,7 +6714,6 @@ dependencies = [ "query", "regex", "serde_json", - "servers", "session", "snafu 0.8.3", "sql", diff --git a/src/common/error/Cargo.toml b/src/common/error/Cargo.toml index 92ab12dd07f5..49eafb81d5a2 100644 --- a/src/common/error/Cargo.toml +++ b/src/common/error/Cargo.toml @@ -10,3 +10,4 @@ workspace = true [dependencies] snafu.workspace = true strum.workspace = true +tonic.workspace = true diff --git a/src/common/error/src/status_code.rs b/src/common/error/src/status_code.rs index a9d61eed5688..fd519cc1e6e6 100644 --- a/src/common/error/src/status_code.rs +++ b/src/common/error/src/status_code.rs @@ -15,6 +15,7 @@ use std::fmt; use strum::{AsRefStr, EnumIter, EnumString, FromRepr}; +use tonic::Code; /// Common status code for public API. #[derive(Debug, Clone, Copy, PartialEq, Eq, EnumString, AsRefStr, EnumIter, FromRepr)] @@ -202,6 +203,75 @@ impl fmt::Display for StatusCode { } } +#[macro_export] +macro_rules! define_into_tonic_status { + ($Error: ty) => { + impl From<$Error> for tonic::Status { + fn from(err: $Error) -> Self { + use tonic::codegen::http::{HeaderMap, HeaderValue}; + use tonic::metadata::MetadataMap; + use $crate::GREPTIME_DB_HEADER_ERROR_CODE; + + let mut headers = HeaderMap::::with_capacity(2); + + // 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. + let status_code = err.status_code(); + headers.insert( + GREPTIME_DB_HEADER_ERROR_CODE, + HeaderValue::from(status_code as u32), + ); + let root_error = err.output_msg(); + + let metadata = MetadataMap::from_headers(headers); + tonic::Status::with_metadata( + $crate::status_code::status_to_tonic_code(status_code), + root_error, + metadata, + ) + } + } + }; +} + +/// Returns the tonic [Code] of a [StatusCode]. +pub fn status_to_tonic_code(status_code: StatusCode) -> Code { + match status_code { + StatusCode::Success => Code::Ok, + StatusCode::Unknown => Code::Unknown, + StatusCode::Unsupported => Code::Unimplemented, + StatusCode::Unexpected + | StatusCode::Internal + | StatusCode::PlanQuery + | StatusCode::EngineExecuteQuery => Code::Internal, + StatusCode::InvalidArguments | StatusCode::InvalidSyntax | StatusCode::RequestOutdated => { + Code::InvalidArgument + } + StatusCode::Cancelled => Code::Cancelled, + StatusCode::TableAlreadyExists + | StatusCode::TableColumnExists + | StatusCode::RegionAlreadyExists + | StatusCode::FlowAlreadyExists => Code::AlreadyExists, + StatusCode::TableNotFound + | StatusCode::RegionNotFound + | StatusCode::TableColumnNotFound + | StatusCode::DatabaseNotFound + | StatusCode::UserNotFound + | StatusCode::FlowNotFound => Code::NotFound, + StatusCode::StorageUnavailable | StatusCode::RegionNotReady => Code::Unavailable, + StatusCode::RuntimeResourcesExhausted + | StatusCode::RateLimited + | StatusCode::RegionBusy => Code::ResourceExhausted, + StatusCode::UnsupportedPasswordType + | StatusCode::UserPasswordMismatch + | StatusCode::AuthHeaderNotFound + | StatusCode::InvalidAuthHeader => Code::Unauthenticated, + StatusCode::AccessDenied | StatusCode::PermissionDenied | StatusCode::RegionReadonly => { + Code::PermissionDenied + } + } +} + #[cfg(test)] mod tests { use strum::IntoEnumIterator; diff --git a/src/datanode/src/error.rs b/src/datanode/src/error.rs index 919a921ec349..f1a37f624997 100644 --- a/src/datanode/src/error.rs +++ b/src/datanode/src/error.rs @@ -15,10 +15,10 @@ use std::any::Any; use std::sync::Arc; +use common_error::define_into_tonic_status; use common_error::ext::{BoxedError, ErrorExt}; use common_error::status_code::StatusCode; use common_macro::stack_trace_debug; -use servers::define_into_tonic_status; use snafu::{Location, Snafu}; use store_api::storage::RegionId; use table::error::Error as TableError; diff --git a/src/flow/src/adapter/error.rs b/src/flow/src/adapter/error.rs index 47df3d9014aa..9d5692aa1ab4 100644 --- a/src/flow/src/adapter/error.rs +++ b/src/flow/src/adapter/error.rs @@ -16,12 +16,12 @@ use std::any::Any; +use common_error::define_into_tonic_status; use common_error::ext::BoxedError; use common_macro::stack_trace_debug; use common_telemetry::common_error::ext::ErrorExt; use common_telemetry::common_error::status_code::StatusCode; use datatypes::value::Value; -use servers::define_into_tonic_status; use snafu::{Location, Snafu}; use crate::adapter::FlowId; diff --git a/src/frontend/src/error.rs b/src/frontend/src/error.rs index 9b2a0faf6320..e7b7a19885d7 100644 --- a/src/frontend/src/error.rs +++ b/src/frontend/src/error.rs @@ -15,10 +15,10 @@ use std::any::Any; use common_datasource::file_format::Format; +use common_error::define_into_tonic_status; use common_error::ext::{BoxedError, ErrorExt}; use common_error::status_code::StatusCode; use common_macro::stack_trace_debug; -use servers::define_into_tonic_status; use snafu::{Location, Snafu}; use store_api::storage::RegionNumber; diff --git a/src/meta-srv/src/error.rs b/src/meta-srv/src/error.rs index e598a956d8dc..4490d840449b 100644 --- a/src/meta-srv/src/error.rs +++ b/src/meta-srv/src/error.rs @@ -12,6 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. +use common_error::define_into_tonic_status; use common_error::ext::{BoxedError, ErrorExt}; use common_error::status_code::StatusCode; use common_macro::stack_trace_debug; @@ -19,7 +20,6 @@ use common_meta::peer::Peer; use common_meta::DatanodeId; use common_runtime::JoinError; use rand::distributions::WeightedError; -use servers::define_into_tonic_status; use snafu::{Location, Snafu}; use store_api::storage::RegionId; use table::metadata::TableId; diff --git a/src/operator/Cargo.toml b/src/operator/Cargo.toml index 8681dca2978e..dda47abebe89 100644 --- a/src/operator/Cargo.toml +++ b/src/operator/Cargo.toml @@ -47,7 +47,6 @@ prometheus.workspace = true query.workspace = true regex.workspace = true serde_json.workspace = true -servers.workspace = true session.workspace = true snafu.workspace = true sql.workspace = true diff --git a/src/operator/src/error.rs b/src/operator/src/error.rs index d7dcdb9d7057..f0266547951b 100644 --- a/src/operator/src/error.rs +++ b/src/operator/src/error.rs @@ -15,12 +15,12 @@ use std::any::Any; use common_datasource::file_format::Format; +use common_error::define_into_tonic_status; use common_error::ext::{BoxedError, ErrorExt}; use common_error::status_code::StatusCode; use common_macro::stack_trace_debug; use datafusion::parquet; use datatypes::arrow::error::ArrowError; -use servers::define_into_tonic_status; use snafu::{Location, Snafu}; #[derive(Snafu)] diff --git a/src/servers/src/error.rs b/src/servers/src/error.rs index 7515b767e235..ae595b8e95b6 100644 --- a/src/servers/src/error.rs +++ b/src/servers/src/error.rs @@ -21,6 +21,7 @@ use axum::response::{IntoResponse, Response}; use axum::{http, Json}; use base64::DecodeError; use catalog; +use common_error::define_into_tonic_status; use common_error::ext::{BoxedError, ErrorExt}; use common_error::status_code::StatusCode; use common_macro::stack_trace_debug; @@ -29,7 +30,6 @@ use datatypes::prelude::ConcreteDataType; use query::parser::PromQuery; use serde_json::json; use snafu::{Location, Snafu}; -use tonic::Code; #[derive(Snafu)] #[snafu(visibility(pub))] @@ -695,75 +695,6 @@ impl ErrorExt for Error { } } -/// Returns the tonic [Code] of a [StatusCode]. -pub fn status_to_tonic_code(status_code: StatusCode) -> Code { - match status_code { - StatusCode::Success => Code::Ok, - StatusCode::Unknown => Code::Unknown, - StatusCode::Unsupported => Code::Unimplemented, - StatusCode::Unexpected - | StatusCode::Internal - | StatusCode::PlanQuery - | StatusCode::EngineExecuteQuery => Code::Internal, - StatusCode::InvalidArguments | StatusCode::InvalidSyntax | StatusCode::RequestOutdated => { - Code::InvalidArgument - } - StatusCode::Cancelled => Code::Cancelled, - StatusCode::TableAlreadyExists - | StatusCode::TableColumnExists - | StatusCode::RegionAlreadyExists - | StatusCode::FlowAlreadyExists => Code::AlreadyExists, - StatusCode::TableNotFound - | StatusCode::RegionNotFound - | StatusCode::TableColumnNotFound - | StatusCode::DatabaseNotFound - | StatusCode::UserNotFound - | StatusCode::FlowNotFound => Code::NotFound, - StatusCode::StorageUnavailable | StatusCode::RegionNotReady => Code::Unavailable, - StatusCode::RuntimeResourcesExhausted - | StatusCode::RateLimited - | StatusCode::RegionBusy => Code::ResourceExhausted, - StatusCode::UnsupportedPasswordType - | StatusCode::UserPasswordMismatch - | StatusCode::AuthHeaderNotFound - | StatusCode::InvalidAuthHeader => Code::Unauthenticated, - StatusCode::AccessDenied | StatusCode::PermissionDenied | StatusCode::RegionReadonly => { - Code::PermissionDenied - } - } -} - -#[macro_export] -macro_rules! define_into_tonic_status { - ($Error: ty) => { - impl From<$Error> for tonic::Status { - fn from(err: $Error) -> Self { - use tonic::codegen::http::{HeaderMap, HeaderValue}; - use tonic::metadata::MetadataMap; - use $crate::http::header::constants::GREPTIME_DB_HEADER_ERROR_CODE; - - let mut headers = HeaderMap::::with_capacity(2); - - // 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. - let status_code = err.status_code(); - headers.insert( - GREPTIME_DB_HEADER_ERROR_CODE, - HeaderValue::from(status_code as u32), - ); - let root_error = err.output_msg(); - - let metadata = MetadataMap::from_headers(headers); - tonic::Status::with_metadata( - $crate::error::status_to_tonic_code(status_code), - root_error, - metadata, - ) - } - } - }; -} - define_into_tonic_status!(Error); impl From for Error {