From 3579aaedc03b0b6dfe2c88fdb219bab8eea356b1 Mon Sep 17 00:00:00 2001 From: Bugen Zhao Date: Wed, 1 May 2024 17:02:15 +0800 Subject: [PATCH] fix(error): do not directly format `BoxedError` (#16558) Signed-off-by: Bugen Zhao --- lints/ui/format_error.rs | 9 +++++++++ src/frontend/src/error.rs | 10 ++++++++++ src/frontend/src/handler/query.rs | 4 +--- .../src/scheduler/distributed/query_manager.rs | 1 + src/frontend/src/scheduler/local.rs | 1 + src/frontend/src/session/cursor_manager.rs | 10 +++------- 6 files changed, 25 insertions(+), 10 deletions(-) diff --git a/lints/ui/format_error.rs b/lints/ui/format_error.rs index 22ea8c5f88e8..7963a96018ba 100644 --- a/lints/ui/format_error.rs +++ b/lints/ui/format_error.rs @@ -80,4 +80,13 @@ fn main() { let _ = anyhow!("some error occurred: {}", err.as_report()); let _ = anyhow!("{:?}", anyhow_err.as_report()); let _ = anyhow!("some error occurred: {:?}", anyhow_err.as_report()); + + let box_dyn_err_1: Box = Box::new(err.clone()); + let box_dyn_err_2: Box = Box::new(err.clone()); + let box_dyn_err_3: Box = Box::new(err.clone()); + + // TODO: fail to lint + let _ = format!("{}", box_dyn_err_1); + info!("{}", box_dyn_err_2); + let _ = box_dyn_err_3.to_string(); } diff --git a/src/frontend/src/error.rs b/src/frontend/src/error.rs index e2ec1b35fe50..93b3e627ae85 100644 --- a/src/frontend/src/error.rs +++ b/src/frontend/src/error.rs @@ -237,3 +237,13 @@ impl From for RwError { ErrorCode::Uncategorized(join_error.into()).into() } } + +// For errors without a concrete type, put them into `Uncategorized`. +impl From for RwError { + fn from(e: BoxedError) -> Self { + // Show that the error is of `BoxedKind`, instead of `AdhocKind` which loses the sources. + // This is essentially expanded from `anyhow::anyhow!(e)`. + let e = anyhow::__private::kind::BoxedKind::anyhow_kind(&e).new(e); + ErrorCode::Uncategorized(e).into() + } +} diff --git a/src/frontend/src/handler/query.rs b/src/frontend/src/handler/query.rs index 68f0653a3588..d587491cba06 100644 --- a/src/frontend/src/handler/query.rs +++ b/src/frontend/src/handler/query.rs @@ -386,9 +386,7 @@ async fn execute( "no affected rows in output".to_string(), ))) } - Some(row) => { - row.map_err(|err| RwError::from(ErrorCode::InternalError(format!("{}", err))))? - } + Some(row) => row?, }; let affected_rows_str = first_row_set[0].values()[0] .as_ref() diff --git a/src/frontend/src/scheduler/distributed/query_manager.rs b/src/frontend/src/scheduler/distributed/query_manager.rs index 69d48eb70d97..86a54cf9c0f9 100644 --- a/src/frontend/src/scheduler/distributed/query_manager.rs +++ b/src/frontend/src/scheduler/distributed/query_manager.rs @@ -50,6 +50,7 @@ impl DistributedQueryStream { } impl Stream for DistributedQueryStream { + // TODO(error-handling): use a concrete error type. type Item = Result; fn poll_next(mut self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll> { diff --git a/src/frontend/src/scheduler/local.rs b/src/frontend/src/scheduler/local.rs index 94a15b99c6bf..0ff7540a79d7 100644 --- a/src/frontend/src/scheduler/local.rs +++ b/src/frontend/src/scheduler/local.rs @@ -55,6 +55,7 @@ use crate::scheduler::task_context::FrontendBatchTaskContext; use crate::scheduler::{ReadSnapshot, SchedulerError, SchedulerResult}; use crate::session::{FrontendEnv, SessionImpl}; +// TODO(error-handling): use a concrete error type. pub type LocalQueryStream = ReceiverStream>; pub struct LocalQueryExecution { sql: String, diff --git a/src/frontend/src/session/cursor_manager.rs b/src/frontend/src/session/cursor_manager.rs index 214dac1a2b5a..0ada5f6ccd5e 100644 --- a/src/frontend/src/session/cursor_manager.rs +++ b/src/frontend/src/session/cursor_manager.rs @@ -27,7 +27,7 @@ use risingwave_common::types::DataType; use risingwave_sqlparser::ast::{Ident, ObjectName, Statement}; use crate::catalog::subscription_catalog::SubscriptionCatalog; -use crate::error::{ErrorCode, Result, RwError}; +use crate::error::{ErrorCode, Result}; use crate::handler::declare_cursor::create_stream_for_cursor; use crate::handler::util::{ convert_epoch_to_logstore_i64, convert_logstore_i64_to_unix_millis, @@ -78,9 +78,7 @@ impl QueryCursor { let rows = self.row_stream.next().await; let rows = match rows { None => return Ok(None), - Some(row) => { - row.map_err(|err| RwError::from(ErrorCode::InternalError(format!("{}", err))))? - } + Some(row) => row?, }; self.remaining_rows = rows.into_iter().collect(); } @@ -396,9 +394,7 @@ impl SubscriptionCursor { if remaining_rows.is_empty() && let Some(row_set) = row_stream.next().await { - remaining_rows.extend(row_set.map_err(|e| { - ErrorCode::InternalError(format!("Cursor get next chunk error {:?}", e.to_string())) - })?); + remaining_rows.extend(row_set?); } Ok(()) }