test: Add MockError and add more tests for error

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 {

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<Backtrace>,
+}
+
+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());
+    }
+}

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);
+    }
+}

src/query/src/catalog/memory.rs

@@ -153,7 +153,6 @@ impl SchemaProvider for MemorySchemaProvider {
 
     fn register_table(&self, name: String, table: TableRef) -> Result<Option<TableRef>> {
         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());
     }
 }

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<InnerError> 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());
+    }
+}

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<InnerError> 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());
+    }
+}