refactor(error): simplify all boxed error wrapper definition (#13725)

Signed-off-by: Bugen Zhao <[email protected]>

Cargo.toml

@@ -129,7 +129,7 @@ arrow-flight = "49"
 arrow-select = "49"
 arrow-ord = "49"
 arrow-row = "49"
-thiserror-ext = "0.0.8"
+thiserror-ext = "0.0.10"
 tikv-jemalloc-ctl = { git = "https://github.com/risingwavelabs/jemallocator.git", rev = "64a2d9" }
 tikv-jemallocator = { git = "https://github.com/risingwavelabs/jemallocator.git", features = [
   "profiling",

src/batch/clippy.toml

@@ -1,15 +1,10 @@
 disallowed-methods = [
-    { path = "risingwave_common::error::internal_err", reason = "Please use per-crate error type instead." },
-    { path = "risingwave_common::error::internal_error", reason = "Please use per-crate error type instead." },
-    { path = "risingwave_common::error::tonic_err", reason = "Please use per-crate error type instead." },
 ]
 
 disallowed-types = [
     { path = "risingwave_common::error::ErrorCode", reason = "Please use per-crate error type instead." },
     { path = "risingwave_common::error::RwError", reason = "Please use per-crate error type instead." },
     { path = "risingwave_common::error::Result", reason = "Please use per-crate error type instead." },
-    { path = "risingwave_common::error::ToRwResult", reason = "Please use per-crate error type instead." },
-    { path = "risingwave_common::error::ToErrorStr", reason = "Please use per-crate error type instead." },
 ]
 
 doc-valid-idents = [

src/common/src/array/error.rs

@@ -18,8 +18,6 @@ pub use anyhow::anyhow;
 use risingwave_pb::PbFieldNotFound;
 use thiserror::Error;
 
-use crate::error::{ErrorCode, RwError};
-
 #[derive(Error, Debug)]
 pub enum ArrayError {
     #[error("Pb decode error: {0}")]
@@ -42,12 +40,6 @@ pub enum ArrayError {
     ToArrow(String),
 }
 
-impl From<ArrayError> for RwError {
-    fn from(s: ArrayError) -> Self {
-        ErrorCode::ArrayError(s).into()
-    }
-}
-
 impl From<PbFieldNotFound> for ArrayError {
     fn from(err: PbFieldNotFound) -> Self {
         anyhow!("Failed to decode prost: field not found `{}`", err.0).into()

src/common/src/error.rs

@@ -12,7 +12,6 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-use std::backtrace::Backtrace;
 use std::collections::HashSet;
 use std::convert::Infallible;
 use std::fmt::{Debug, Display, Formatter};
@@ -23,7 +22,7 @@ use memcomparable::Error as MemComparableError;
 use risingwave_error::tonic::{ToTonicStatus, TonicStatusWrapper};
 use risingwave_pb::PbFieldNotFound;
 use thiserror::Error;
-use thiserror_ext::Macro;
+use thiserror_ext::{Box, Macro};
 use tokio::task::JoinError;
 
 use crate::array::ArrayError;
@@ -80,7 +79,8 @@ pub struct NotImplemented {
     pub issue: TrackingIssue,
 }
 
-#[derive(Error, Debug)]
+#[derive(Error, Debug, Box)]
+#[thiserror_ext(newtype(name = RwError, backtrace, report_debug))]
 pub enum ErrorCode {
     #[error("internal error: {0}")]
     InternalError(String),
@@ -212,30 +212,11 @@ pub enum ErrorCode {
     ),
 }
 
-// TODO(error-handling): automatically generate this impl.
-impl From<NotImplemented> for RwError {
-    fn from(value: NotImplemented) -> Self {
-        ErrorCode::from(value).into()
-    }
-}
-
-pub fn internal_error(msg: impl Into<String>) -> RwError {
-    ErrorCode::InternalError(msg.into()).into()
-}
-
-#[derive(Error)]
-#[error("{inner}")]
-pub struct RwError {
-    #[source]
-    inner: Box<ErrorCode>,
-    backtrace: Box<Backtrace>,
-}
-
 impl From<RwError> for tonic::Status {
     fn from(err: RwError) -> Self {
         use tonic::Code;
 
-        let code = match &*err.inner {
+        let code = match err.inner() {
             ErrorCode::ExprError(_) => Code::InvalidArgument,
             ErrorCode::PermissionDenied(_) => Code::PermissionDenied,
             ErrorCode::InternalError(_) => Code::Internal,
@@ -271,45 +252,9 @@ impl From<tonic::Status> for RwError {
     }
 }
 
-impl RwError {
-    pub fn inner(&self) -> &ErrorCode {
-        &self.inner
-    }
-}
-
-impl From<ErrorCode> for RwError {
-    fn from(code: ErrorCode) -> Self {
-        Self {
-            inner: Box::new(code),
-            backtrace: Box::new(Backtrace::capture()),
-        }
-    }
-}
-
 impl From<JoinError> for RwError {
     fn from(join_error: JoinError) -> Self {
-        Self {
-            inner: Box::new(ErrorCode::InternalError(join_error.to_string())),
-            backtrace: Box::new(Backtrace::capture()),
-        }
-    }
-}
-
-impl From<MemComparableError> for RwError {
-    fn from(mem_comparable_error: MemComparableError) -> Self {
-        ErrorCode::MemComparableError(mem_comparable_error).into()
-    }
-}
-
-impl From<ValueEncodingError> for RwError {
-    fn from(value_encoding_error: ValueEncodingError) -> Self {
-        ErrorCode::ValueEncodingError(value_encoding_error).into()
-    }
-}
-
-impl From<std::io::Error> for RwError {
-    fn from(io_err: IoError) -> Self {
-        ErrorCode::IoError(io_err).into()
+        anyhow::anyhow!(join_error).into()
     }
 }
 
@@ -319,12 +264,6 @@ impl From<std::net::AddrParseError> for RwError {
     }
 }
 
-impl From<anyhow::Error> for RwError {
-    fn from(e: anyhow::Error) -> Self {
-        ErrorCode::InternalErrorAnyhow(e).into()
-    }
-}
-
 impl From<Infallible> for RwError {
     fn from(x: Infallible) -> Self {
         match x {}
@@ -337,18 +276,6 @@ impl From<String> for RwError {
     }
 }
 
-impl Debug for RwError {
-    fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
-        write!(
-            f,
-            "{}\n{}",
-            self.inner,
-            // Use inner error's backtrace by default, otherwise use the generated one in `From`.
-            std::error::request_ref::<Backtrace>(&self.inner).unwrap_or(&*self.backtrace)
-        )
-    }
-}
-
 impl From<PbFieldNotFound> for RwError {
     fn from(err: PbFieldNotFound) -> Self {
         ErrorCode::InternalError(format!(
@@ -359,60 +286,14 @@ impl From<PbFieldNotFound> for RwError {
     }
 }
 
-impl From<SessionConfigError> for RwError {
-    fn from(value: SessionConfigError) -> Self {
-        ErrorCode::SessionConfig(value).into()
-    }
-}
-
 impl From<tonic::transport::Error> for RwError {
     fn from(err: tonic::transport::Error) -> Self {
         ErrorCode::RpcError(err.into()).into()
     }
 }
 
-/// Convert `RwError` into `tonic::Status`. Generally used in `map_err`.
-pub fn tonic_err(err: impl Into<RwError>) -> tonic::Status {
-    err.into().into()
-}
-
 pub type Result<T> = std::result::Result<T, RwError>;
 
-/// A helper to convert a third-party error to string.
-pub trait ToErrorStr {
-    fn to_error_str(self) -> String;
-}
-
-pub trait ToRwResult<T, E> {
-    fn to_rw_result(self) -> Result<T>;
-
-    fn to_rw_result_with(self, func: impl FnOnce() -> String) -> Result<T>;
-}
-
-impl<T, E: ToErrorStr> ToRwResult<T, E> for std::result::Result<T, E> {
-    fn to_rw_result(self) -> Result<T> {
-        self.map_err(|e| ErrorCode::InternalError(e.to_error_str()).into())
-    }
-
-    fn to_rw_result_with(self, func: impl FnOnce() -> String) -> Result<T> {
-        self.map_err(|e| {
-            ErrorCode::InternalError(format!("{}: {}", func(), e.to_error_str())).into()
-        })
-    }
-}
-
-impl<T> ToErrorStr for std::sync::mpsc::SendError<T> {
-    fn to_error_str(self) -> String {
-        self.to_string()
-    }
-}
-
-impl<T> ToErrorStr for tokio::sync::mpsc::error::SendError<T> {
-    fn to_error_str(self) -> String {
-        self.to_string()
-    }
-}
-
 /// Util macro for generating error when condition check failed.
 ///
 /// # Case 1: Expression only.

src/compute/src/rpc/service/stream_service.rs

@@ -16,7 +16,6 @@ use std::sync::Arc;
 
 use await_tree::InstrumentAwait;
 use itertools::Itertools;
-use risingwave_common::error::tonic_err;
 use risingwave_common::util::tracing::TracingContext;
 use risingwave_hummock_sdk::table_stats::to_prost_table_stats_map;
 use risingwave_hummock_sdk::LocalSstableInfo;
@@ -258,7 +257,7 @@ impl StreamService for StreamServiceImpl {
                 .try_wait_epoch(HummockReadEpoch::Committed(epoch))
                 .instrument_await(format!("wait_epoch_commit (epoch {})", epoch))
                 .await
-                .map_err(tonic_err)?;
+                .map_err(StreamError::from)?;
         });
 
         Ok(Response::new(WaitEpochCommitResponse { status: None }))

src/frontend/src/binder/relation/mod.rs

@@ -16,8 +16,9 @@ use std::collections::hash_map::Entry;
 use std::ops::Deref;
 
 use itertools::{EitherOrBoth, Itertools};
+use risingwave_common::bail;
 use risingwave_common::catalog::{Field, TableId, DEFAULT_SCHEMA_NAME};
-use risingwave_common::error::{internal_error, ErrorCode, Result, RwError};
+use risingwave_common::error::{ErrorCode, Result, RwError};
 use risingwave_sqlparser::ast::{
     Expr as ParserExpr, FunctionArg, FunctionArgExpr, Ident, ObjectName, TableAlias, TableFactor,
 };
@@ -208,10 +209,7 @@ impl Binder {
     /// return first name in identifiers, must have only one name.
     fn resolve_single_name(mut identifiers: Vec<Ident>, ident_desc: &str) -> Result<String> {
         if identifiers.len() > 1 {
-            return Err(internal_error(format!(
-                "{} must contain 1 argument",
-                ident_desc
-            )));
+            bail!("{} must contain 1 argument", ident_desc);
         }
         let name = identifiers.pop().unwrap().real_value();
 

src/frontend/src/expr/function_call.rs

@@ -14,7 +14,7 @@
 
 use itertools::Itertools;
 use risingwave_common::catalog::Schema;
-use risingwave_common::error::{ErrorCode, Result as RwResult, RwError};
+use risingwave_common::error::{ErrorCode, Result as RwResult};
 use risingwave_common::types::{DataType, ScalarImpl};
 use risingwave_common::util::iter_util::ZipEqFast;
 use thiserror::Error;
@@ -431,9 +431,3 @@ impl From<CastError> for ErrorCode {
         ErrorCode::BindError(value.to_string())
     }
 }
-
-impl From<CastError> for RwError {
-    fn from(value: CastError) -> Self {
-        ErrorCode::from(value).into()
-    }
-}

src/jni_core/src/hummock_iterator.rs

@@ -28,7 +28,7 @@ use risingwave_object_store::object::build_remote_object_store;
 use risingwave_object_store::object::object_metrics::ObjectStoreMetrics;
 use risingwave_pb::java_binding::key_range::Bound;
 use risingwave_pb::java_binding::{KeyRange, ReadPlan};
-use risingwave_storage::error::{StorageError, StorageResult};
+use risingwave_storage::error::StorageResult;
 use risingwave_storage::hummock::local_version::pinned_version::PinnedVersion;
 use risingwave_storage::hummock::store::version::HummockVersionReader;
 use risingwave_storage::hummock::store::HummockStorageIterator;
@@ -135,11 +135,7 @@ impl HummockJavaBindingIterator {
         Ok(match item {
             Some((key, value)) => Some((
                 key.user_key.table_key.0,
-                OwnedRow::new(
-                    self.row_serde
-                        .deserialize(&value)
-                        .map_err(StorageError::DeserializeRow)?,
-                ),
+                OwnedRow::new(self.row_serde.deserialize(&value)?),
             )),
             None => None,
         })

src/meta/node/src/server.rs

@@ -403,7 +403,7 @@ pub async fn start_service_as_election_leader(
 
     let data_directory = system_params_reader.data_directory();
     if !is_correct_data_directory(data_directory) {
-        return Err(MetaError::system_param(format!(
+        return Err(MetaError::system_params(format!(
             "The data directory {:?} is misconfigured.
             Please use a combination of uppercase and lowercase letters and numbers, i.e. [a-z, A-Z, 0-9].
             The string cannot start or end with '/', and consecutive '/' are not allowed.

src/meta/service/src/ddl_service.rs

@@ -739,7 +739,7 @@ impl DdlService for DdlServiceImpl {
                                 .await?
                         } else {
                             return Err(Status::from(MetaError::unavailable(
-                                "AWS client is not configured".into(),
+                                "AWS client is not configured",
                             )));
                         }
                     }