diff --git a/src/common/error/src/lib.rs b/src/common/error/src/lib.rs index 9b7ed10113..50ef09d166 100644 --- a/src/common/error/src/lib.rs +++ b/src/common/error/src/lib.rs @@ -1,5 +1,6 @@ pub mod ext; pub mod format; +pub mod mock; pub mod status_code; pub mod prelude { diff --git a/src/common/error/src/mock.rs b/src/common/error/src/mock.rs new file mode 100644 index 0000000000..e031779b4a --- /dev/null +++ b/src/common/error/src/mock.rs @@ -0,0 +1,74 @@ +//! Utils for mock. + +use std::fmt; + +use snafu::GenerateImplicitData; + +use crate::prelude::*; + +/// A mock error mainly for test. +#[derive(Debug)] +pub struct MockError { + pub code: StatusCode, + backtrace: Option, +} + +impl MockError { + /// Create a new [MockError] without backtrace. + pub fn new(code: StatusCode) -> MockError { + MockError { + code, + backtrace: None, + } + } + + /// Create a new [MockError] with backtrace. + pub fn with_backtrace(code: StatusCode) -> MockError { + MockError { + code, + backtrace: Some(Backtrace::generate()), + } + } +} + +impl fmt::Display for MockError { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(f, "{:?}", self.code) + } +} + +impl std::error::Error for MockError { + fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { + None + } +} + +impl ErrorExt for MockError { + fn status_code(&self) -> StatusCode { + self.code + } + + fn backtrace_opt(&self) -> Option<&Backtrace> { + self.backtrace.as_ref() + } +} + +impl ErrorCompat for MockError { + fn backtrace(&self) -> Option<&Backtrace> { + self.backtrace_opt() + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_mock_error() { + let err = MockError::new(StatusCode::Unknown); + assert!(err.backtrace_opt().is_none()); + + let err = MockError::with_backtrace(StatusCode::Unknown); + assert!(err.backtrace_opt().is_some()); + } +} diff --git a/src/datanode/src/error.rs b/src/datanode/src/error.rs index de767ffe8c..02c82b0061 100644 --- a/src/datanode/src/error.rs +++ b/src/datanode/src/error.rs @@ -5,10 +5,16 @@ use common_error::prelude::*; #[snafu(visibility(pub))] pub enum Error { #[snafu(display("Fail to execute sql, source: {}", source))] - ExecuteSql { source: query::error::Error }, + ExecuteSql { + #[snafu(backtrace)] + source: query::error::Error, + }, #[snafu(display("Fail to create catalog list, source: {}", source))] - NewCatalog { source: query::error::Error }, + NewCatalog { + #[snafu(backtrace)] + source: query::error::Error, + }, // The error source of http error is clear even without backtrace now so // a backtrace is not carried in this varaint. @@ -31,3 +37,29 @@ impl ErrorExt for Error { ErrorCompat::backtrace(self) } } + +#[cfg(test)] +mod tests { + use common_error::mock::MockError; + + use super::*; + + fn raise_query_error() -> std::result::Result<(), query::error::Error> { + Err(query::error::Error::new(MockError::with_backtrace( + StatusCode::Internal, + ))) + } + + fn assert_internal_error(err: &Error) { + assert!(err.backtrace_opt().is_some()); + assert_eq!(StatusCode::Internal, err.status_code()); + } + + #[test] + fn test_error() { + let err = raise_query_error().context(ExecuteSqlSnafu).err().unwrap(); + assert_internal_error(&err); + let err = raise_query_error().context(NewCatalogSnafu).err().unwrap(); + assert_internal_error(&err); + } +} diff --git a/src/query/src/catalog/memory.rs b/src/query/src/catalog/memory.rs index e2c194c001..1245b855b5 100644 --- a/src/query/src/catalog/memory.rs +++ b/src/query/src/catalog/memory.rs @@ -153,7 +153,6 @@ impl SchemaProvider for MemorySchemaProvider { fn register_table(&self, name: String, table: TableRef) -> Result> { if self.table_exist(name.as_str()) { - // FIXME(yingwen): Define another error. return TableExistsSnafu { table: name }.fail()?; } let mut tables = self.tables.write().unwrap(); @@ -220,6 +219,8 @@ mod tests { assert!(provider.table_exist(table_name)); let other_table = NumbersTable::default(); let result = provider.register_table(table_name.to_string(), Arc::new(other_table)); - assert!(result.is_err()); + let err = result.err().unwrap(); + assert!(err.backtrace_opt().is_some()); + assert_eq!(StatusCode::TableAlreadyExists, err.status_code()); } } diff --git a/src/query/src/datafusion/error.rs b/src/query/src/datafusion/error.rs index 57a53a1755..425519de02 100644 --- a/src/query/src/datafusion/error.rs +++ b/src/query/src/datafusion/error.rs @@ -19,7 +19,10 @@ pub enum InnerError { // The sql error already contains the SQL. #[snafu(display("Cannot parse SQL, source: {}", source))] - ParseSql { source: sql::error::Error }, + ParseSql { + #[snafu(backtrace)] + source: sql::error::Error, + }, #[snafu(display("Cannot plan SQL: {}, source: {}", sql, source))] PlanSql { @@ -51,3 +54,51 @@ impl From for Error { Self::new(err) } } + +#[cfg(test)] +mod tests { + use super::*; + + fn raise_df_error() -> Result<(), DataFusionError> { + Err(DataFusionError::NotImplemented("test".to_string())) + } + + fn assert_internal_error(err: &InnerError) { + assert_eq!(StatusCode::Internal, err.status_code()); + assert!(err.backtrace_opt().is_some()); + } + + #[test] + fn test_datafusion_as_source() { + let err = raise_df_error() + .context(DatafusionSnafu { msg: "test df" }) + .err() + .unwrap(); + assert_internal_error(&err); + + let err = raise_df_error() + .context(PlanSqlSnafu { sql: "" }) + .err() + .unwrap(); + assert_internal_error(&err); + + let res: Result<(), InnerError> = PhysicalPlanDowncastSnafu {}.fail(); + let err = res.err().unwrap(); + assert_internal_error(&err); + } + + fn raise_sql_error() -> Result<(), sql::error::Error> { + Err(sql::error::Error::Unsupported { + sql: "".to_string(), + keyword: "".to_string(), + }) + } + + #[test] + fn test_parse_error() { + let err = raise_sql_error().context(ParseSqlSnafu).err().unwrap(); + assert!(err.backtrace_opt().is_none()); + let sql_err = raise_sql_error().err().unwrap(); + assert_eq!(sql_err.status_code(), err.status_code()); + } +} diff --git a/src/table/src/error.rs b/src/table/src/error.rs index 5b03f04628..e924a3d0ee 100644 --- a/src/table/src/error.rs +++ b/src/table/src/error.rs @@ -26,7 +26,7 @@ pub enum InnerError { } impl ErrorExt for InnerError { - fn backtrace_opt(&self) -> Option<&snafu::Backtrace> { + fn backtrace_opt(&self) -> Option<&Backtrace> { ErrorCompat::backtrace(self) } } @@ -42,3 +42,27 @@ impl From for DataFusionError { DataFusionError::External(Box::new(e)) } } + +#[cfg(test)] +mod tests { + use super::*; + + fn raise_df_error() -> Result<()> { + Err(DataFusionError::NotImplemented("table test".to_string())).context(DatafusionSnafu)? + } + + fn raise_repeatedly() -> Result<()> { + ExecuteRepeatedlySnafu {}.fail()? + } + + #[test] + fn test_error() { + let err = raise_df_error().err().unwrap(); + assert!(err.backtrace_opt().is_some()); + assert_eq!(StatusCode::Unknown, err.status_code()); + + let err = raise_repeatedly().err().unwrap(); + assert!(err.backtrace_opt().is_some()); + assert_eq!(StatusCode::Unknown, err.status_code()); + } +}