Skip to content

Commit

Permalink
use ext for rw error
Browse files Browse the repository at this point in the history
Signed-off-by: Bugen Zhao <[email protected]>
  • Loading branch information
BugenZhao committed Nov 29, 2023
1 parent 1dd1673 commit dd6c77b
Show file tree
Hide file tree
Showing 6 changed files with 7 additions and 143 deletions.
3 changes: 0 additions & 3 deletions src/batch/clippy.toml
Original file line number Diff line number Diff line change
@@ -1,15 +1,12 @@
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 = [
Expand Down
8 changes: 0 additions & 8 deletions src/common/src/array/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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}")]
Expand All @@ -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()
Expand Down
125 changes: 5 additions & 120 deletions src/common/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand All @@ -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;
Expand Down Expand Up @@ -80,7 +79,8 @@ pub struct NotImplemented {
pub issue: TrackingIssue,
}

#[derive(Error, Debug)]
#[derive(Error, Debug, Box)]
#[thiserror_ext(type = RwError, backtrace)]
pub enum ErrorCode {
#[error("internal error: {0}")]
InternalError(String),
Expand Down Expand Up @@ -212,30 +212,15 @@ 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,
Expand Down Expand Up @@ -271,45 +256,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()
}
}

Expand All @@ -319,12 +268,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 {}
Expand All @@ -337,18 +280,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!(
Expand All @@ -359,60 +290,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.
Expand Down
3 changes: 1 addition & 2 deletions src/compute/src/rpc/service/stream_service.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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 }))
Expand Down
8 changes: 1 addition & 7 deletions src/frontend/src/expr/function_call.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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()
}
}
3 changes: 0 additions & 3 deletions src/stream/clippy.toml
Original file line number Diff line number Diff line change
Expand Up @@ -8,15 +8,12 @@ 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 = [
Expand Down

0 comments on commit dd6c77b

Please sign in to comment.