Skip to content

Commit

Permalink
move RwError into frontend crate
Browse files Browse the repository at this point in the history 
Signed-off-by: Bugen Zhao <[email protected]>
  • Loading branch information
@BugenZhao
BugenZhao committed Feb 2, 2024
1 parent 1caa4b7 commit ff049e3
Show file tree
Hide file tree
Showing 10 changed files with 208 additions and 281 deletions.
1 change: 1 addition & 0 deletions Cargo.lock
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

15 changes: 1 addition & 14 deletions src/batch/src/error.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ use std::sync::Arc;

pub use anyhow::anyhow;
use risingwave_common::array::ArrayError;
use risingwave_common::error::{BoxedError, ErrorCode, RwError};
use risingwave_common::error::BoxedError;
use risingwave_common::util::value_encoding::error::ValueEncodingError;
use risingwave_dml::error::DmlError;
use risingwave_expr::ExprError;
Expand Down Expand Up @@ -138,19 +138,6 @@ impl From<tonic::Status> for BatchError {
}
}

impl From<BatchError> for RwError {
fn from(s: BatchError) -> Self {
ErrorCode::BatchError(Box::new(s)).into()
}
}

// TODO(error-handling): remove after eliminating RwError from connector.
impl From<RwError> for BatchError {
fn from(s: RwError) -> Self {
Self::Internal(anyhow!(s))
}
}

impl<'a> From<&'a BatchError> for Status {
fn from(err: &'a BatchError) -> Self {
err.to_status(tonic::Code::Internal, "batch")
Expand Down
236 changes: 2 additions & 234 deletions src/common/src/error.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,22 +13,11 @@
// limitations under the License.

use std::collections::HashSet;
use std::convert::Infallible;
use std::fmt::{Debug, Display, Formatter};
use std::io::Error as IoError;
use std::time::{Duration, SystemTime};

use memcomparable::Error as MemComparableError;
use risingwave_error::tonic::{ToTonicStatus, TonicStatusWrapper};
use risingwave_pb::PbFieldNotFound;
use thiserror::Error;
use thiserror_ext::{Box, Macro};
use tokio::task::JoinError;

use crate::array::ArrayError;
use crate::session_config::SessionConfigError;
use crate::util::value_encoding::error::ValueEncodingError;

use thiserror_ext::Macro;
/// Re-export `risingwave_error` for easy access.
pub mod v2 {
pub use risingwave_error::*;
Expand Down Expand Up @@ -103,228 +92,6 @@ impl Display for NoFunction {
}
}

#[derive(Error, Debug, Box)]
#[thiserror_ext(newtype(name = RwError, backtrace, report_debug))]
pub enum ErrorCode {
#[error("internal error: {0}")]
InternalError(String),
// TODO: unify with the above
#[error(transparent)]
Uncategorized(
#[from]
#[backtrace]
anyhow::Error,
),
#[error("connector error: {0}")]
ConnectorError(
#[source]
#[backtrace]
BoxedError,
),
#[error(transparent)]
NotImplemented(#[from] NotImplemented),
// Tips: Use this only if it's intended to reject the query
#[error("Not supported: {0}\nHINT: {1}")]
NotSupported(String, String),
#[error(transparent)]
NoFunction(#[from] NoFunction),
#[error(transparent)]
IoError(#[from] IoError),
#[error("Storage error: {0}")]
StorageError(
#[backtrace]
#[source]
BoxedError,
),
#[error("Expr error: {0}")]
ExprError(
#[source]
#[backtrace]
BoxedError,
),
// TODO(error-handling): there's a limitation that `#[transparent]` can't be used with `#[backtrace]` if no `#[from]`
// So we emulate a transparent error with "{0}" display here.
#[error("{0}")]
BatchError(
#[source]
#[backtrace]
// `BatchError`
BoxedError,
),
#[error("Array error: {0}")]
ArrayError(
#[from]
#[backtrace]
ArrayError,
),
#[error("Stream error: {0}")]
StreamError(
#[backtrace]
#[source]
BoxedError,
),
// TODO(error-handling): there's a limitation that `#[transparent]` can't be used with `#[backtrace]` if no `#[from]`
// So we emulate a transparent error with "{0}" display here.
#[error("{0}")]
RpcError(
#[source]
#[backtrace]
// `tonic::transport::Error`, `TonicStatusWrapper`, or `RpcError`
BoxedError,
),
// TODO: use a new type for bind error
// TODO(error-handling): should prefer use error types than strings.
#[error("Bind error: {0}")]
BindError(String),
// TODO: only keep this one
#[error("Failed to bind expression: {expr}: {error}")]
BindErrorRoot {
expr: String,
#[source]
#[backtrace]
error: BoxedError,
},
#[error("Catalog error: {0}")]
CatalogError(
#[source]
#[backtrace]
BoxedError,
),
#[error("Protocol error: {0}")]
ProtocolError(String),
#[error("Scheduler error: {0}")]
SchedulerError(
#[source]
#[backtrace]
BoxedError,
),
#[error("Task not found")]
TaskNotFound,
#[error("Session not found")]
SessionNotFound,
#[error("Item not found: {0}")]
ItemNotFound(String),
#[error("Invalid input syntax: {0}")]
InvalidInputSyntax(String),
#[error("Can not compare in memory: {0}")]
MemComparableError(#[from] MemComparableError),
#[error("Error while de/se values: {0}")]
ValueEncodingError(
#[from]
#[backtrace]
ValueEncodingError,
),
#[error("Invalid value `{config_value}` for `{config_entry}`")]
InvalidConfigValue {
config_entry: String,
config_value: String,
},
#[error("Invalid Parameter Value: {0}")]
InvalidParameterValue(String),
#[error("Sink error: {0}")]
SinkError(
#[source]
#[backtrace]
BoxedError,
),
#[error("Permission denied: {0}")]
PermissionDenied(String),
#[error("Failed to get/set session config: {0}")]
SessionConfig(
#[from]
#[backtrace]
SessionConfigError,
),
}

impl RwError {
pub fn uncategorized(err: impl Into<anyhow::Error>) -> Self {
Self::from(ErrorCode::Uncategorized(err.into()))
}
}

impl From<RwError> for tonic::Status {
fn from(err: RwError) -> Self {
use tonic::Code;

let code = match err.inner() {
ErrorCode::ExprError(_) => Code::InvalidArgument,
ErrorCode::PermissionDenied(_) => Code::PermissionDenied,
ErrorCode::InternalError(_) => Code::Internal,
_ => Code::Internal,
};

err.to_status_unnamed(code)
}
}

impl From<TonicStatusWrapper> for RwError {
fn from(status: TonicStatusWrapper) -> Self {
use tonic::Code;

let message = status.inner().message();

// TODO(error-handling): `message` loses the source chain.
match status.inner().code() {
Code::InvalidArgument => ErrorCode::InvalidParameterValue(message.to_string()),
Code::NotFound | Code::AlreadyExists => ErrorCode::CatalogError(status.into()),
Code::PermissionDenied => ErrorCode::PermissionDenied(message.to_string()),
Code::Cancelled => ErrorCode::SchedulerError(status.into()),
_ => ErrorCode::RpcError(status.into()),
}
.into()
}
}

impl From<tonic::Status> for RwError {
fn from(status: tonic::Status) -> Self {
// Always wrap the status.
Self::from(TonicStatusWrapper::new(status))
}
}

impl From<JoinError> for RwError {
fn from(join_error: JoinError) -> Self {
Self::uncategorized(join_error)
}
}

impl From<std::net::AddrParseError> for RwError {
fn from(addr_parse_error: std::net::AddrParseError) -> Self {
Self::uncategorized(addr_parse_error)
}
}

impl From<Infallible> for RwError {
fn from(x: Infallible) -> Self {
match x {}
}
}

impl From<String> for RwError {
fn from(e: String) -> Self {
ErrorCode::InternalError(e).into()
}
}

impl From<PbFieldNotFound> for RwError {
fn from(err: PbFieldNotFound) -> Self {
ErrorCode::InternalError(format!(
"Failed to decode prost: field not found `{}`",
err.0
))
.into()
}
}

impl From<tonic::transport::Error> for RwError {
fn from(err: tonic::transport::Error) -> Self {
ErrorCode::RpcError(err.into()).into()
}
}

pub type Result<T> = std::result::Result<T, RwError>;

/// Util macro for generating error when condition check failed.
///
/// # Case 1: Expression only.
Expand Down Expand Up @@ -456,6 +223,7 @@ impl ErrorSuppressor {
}

#[cfg(test)]
#[cfg(any())]
mod tests {
use std::convert::Into;
use std::result::Result::Err;
Expand Down
7 changes: 0 additions & 7 deletions src/connector/src/sink/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,6 @@ use anyhow::anyhow;
use async_trait::async_trait;
use risingwave_common::buffer::Bitmap;
use risingwave_common::catalog::{ColumnDesc, Field, Schema};
use risingwave_common::error::{ErrorCode, RwError};
use risingwave_common::metrics::{
LabelGuardedHistogram, LabelGuardedIntCounter, LabelGuardedIntGauge,
};
Expand Down Expand Up @@ -563,9 +562,3 @@ impl From<RedisError> for SinkError {
SinkError::Redis(format!("{}", value))
}
}

impl From<SinkError> for RwError {
fn from(e: SinkError) -> Self {
ErrorCode::SinkError(Box::new(e)).into()
}
}
7 changes: 0 additions & 7 deletions src/expr/core/src/error.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
use std::fmt::{Debug, Display};

use risingwave_common::array::{ArrayError, ArrayRef};
use risingwave_common::error::{ErrorCode, RwError};
use risingwave_common::types::DataType;
use risingwave_pb::PbFieldNotFound;
use thiserror::Error;
Expand Down Expand Up @@ -138,12 +137,6 @@ pub struct CryptographyError {

static_assertions::const_assert_eq!(std::mem::size_of::<ExprError>(), 40);

impl From<ExprError> for RwError {
fn from(s: ExprError) -> Self {
ErrorCode::ExprError(Box::new(s)).into()
}
}

impl From<chrono::ParseError> for ExprError {
fn from(e: chrono::ParseError) -> Self {
Self::Parse(e.to_report_string().into())
Expand Down
1 change: 1 addition & 0 deletions src/frontend/Cargo.toml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ itertools = "0.12"
linkme = { version = "0.3", features = ["used_linker"] }
maplit = "1"
md5 = "0.7.0"
memcomparable = "0.2"
num-integer = "0.1"
parking_lot = "0.12"
parse-display = "0.8"
Expand Down
Loading

0 comments on commit ff049e3

Please sign in to comment.