feat: Impl ErrorExt for opaque error and ParseError

src/common/error/src/ext.rs

@@ -2,14 +2,15 @@ use crate::status_code::StatusCode;
 
 /// Extension to [`Error`](std::error::Error) in std.
 pub trait ErrorExt: std::error::Error {
-    /// Returns the [StatusCode] of this holder.
+    /// Map this error to [StatusCode].
     fn status_code(&self) -> StatusCode {
         StatusCode::Unknown
     }
 
-    /// Get the reference to the backtrace of this error.
-    // Add `_opt` suffix to avoid confusing with similar method in `std::error::Error`.
-    fn backtrace_opt(&self) -> Option<&snafu::Backtrace>;
+    /// Get the reference to the backtrace of this error, None if the backtrace is unavailable.
+    // Add `_opt` suffix to avoid confusing with similar method in `std::error::Error`, once backtrace
+    // in std is stable, we can deprecate this method.
+    fn backtrace_opt(&self) -> Option<&crate::snafu::Backtrace>;
 }
 
 /// A helper macro to define a opaque boxed error based on errors that implement [ErrorExt] trait.
@@ -25,7 +26,6 @@ macro_rules! define_opaque_error {
         }
 
         impl $Error {
-            /// Create a new error.
             pub fn new<E: $crate::ext::ErrorExt + Send + Sync + 'static>(err: E) -> Self {
                 Self {
                     inner: Box::new(err),
@@ -57,7 +57,13 @@ macro_rules! define_opaque_error {
                 self.inner.status_code()
             }
 
-            fn backtrace_opt(&self) -> Option<&snafu::Backtrace> {
+            fn backtrace_opt(&self) -> Option<&$crate::snafu::Backtrace> {
+                self.inner.backtrace_opt()
+            }
+        }
+
+        impl $crate::snafu::ErrorCompat for $Error {
+            fn backtrace(&self) -> Option<&$crate::snafu::Backtrace> {
                 self.inner.backtrace_opt()
             }
         }
@@ -68,7 +74,7 @@ macro_rules! define_opaque_error {
 mod tests {
     use std::error::Error as StdError;
 
-    use snafu::{prelude::*, Backtrace};
+    use snafu::{prelude::*, Backtrace, ErrorCompat};
 
     use super::*;
 
@@ -88,7 +94,7 @@ mod tests {
 
     impl ErrorExt for InnerError {
         fn backtrace_opt(&self) -> Option<&snafu::Backtrace> {
-            snafu::ErrorCompat::backtrace(self)
+            ErrorCompat::backtrace(self)
         }
     }
 
@@ -111,7 +117,7 @@ mod tests {
     }
 
     #[test]
-    fn test_opaque_error() {
+    fn test_inner_error() {
         let leaf = throw_leaf().err().unwrap();
         assert!(leaf.backtrace_opt().is_some());
         assert!(leaf.source().is_none());
@@ -120,4 +126,18 @@ mod tests {
         assert!(internal.backtrace_opt().is_some());
         assert!(internal.source().is_some());
     }
+
+    #[test]
+    fn test_opaque_error() {
+        let err: Error = throw_leaf().map_err(Into::into).err().unwrap();
+        let msg = format!("{:?}", err);
+        assert!(msg.contains("\nBacktrace:\n"));
+        assert!(ErrorCompat::backtrace(&err).is_some());
+
+        let err: Error = throw_internal().map_err(Into::into).err().unwrap();
+        let msg = format!("{:?}", err);
+        assert!(msg.contains("\nBacktrace:\n"));
+        assert!(msg.contains("Caused by"));
+        assert!(ErrorCompat::backtrace(&err).is_some());
+    }
 }

src/common/error/src/lib.rs

@@ -1,3 +1,13 @@
 pub mod ext;
 pub mod format;
 pub mod status_code;
+
+pub mod prelude {
+    pub use snafu::Backtrace;
+
+    pub use crate::ext::ErrorExt;
+    pub use crate::format::DebugFormat;
+    pub use crate::status_code::StatusCode;
+}
+
+pub use snafu;

src/common/error/src/status_code.rs

@@ -1,6 +1,15 @@
 /// Common status code for public API.
 #[derive(Debug, Clone, Copy)]
 pub enum StatusCode {
-    /// Unknown status.
+    // ====== Begin of common status code ===========
+    /// Unknown error.
     Unknown,
+    /// Unsupported operation.
+    Unsupported,
+    // ====== End of common status code =============
+
+    // ====== Begin of SQL related status code ======
+    /// SQL Syntax error.
+    InvalidSyntax,
+    // ====== End of SQL related status code ========
 }

src/query/src/error.rs

@@ -1,4 +1,5 @@
 use datafusion::error::DataFusionError;
+use snafu::Snafu;
 
 common_error::define_opaque_error!(Error);
 

src/query/src/query_engine/datafusion/error.rs

@@ -33,6 +33,7 @@ pub enum InnerError {
     Execution { message: String },
 }
 
+// TODO(yingwen): Implement status_code().
 impl ErrorExt for InnerError {
     fn backtrace_opt(&self) -> Option<&snafu::Backtrace> {
         ErrorCompat::backtrace(self)

src/sql/Cargo.toml

@@ -6,5 +6,6 @@ edition = "2021"
 # See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html
 
 [dependencies]
+common-error = { path = "../common/error" }
 snafu = { version = "0.7", features = ["backtraces"] }
 sqlparser = "0.15.0"

src/sql/src/errors.rs

@@ -1,4 +1,5 @@
-use snafu::prelude::*;
+use common_error::prelude::*;
+use snafu::{prelude::*, ErrorCompat};
 use sqlparser::parser::ParserError as SpParserError;
 
 /// SQL parser errors.
@@ -29,6 +30,19 @@ pub enum ParserError {
     SpSyntax { sql: String, source: SpParserError },
 }
 
+impl ErrorExt for ParserError {
+    fn status_code(&self) -> StatusCode {
+        match self {
+            Self::Unsupported { .. } => StatusCode::Unsupported,
+            Self::Unexpected { .. } | Self::SpSyntax { .. } => StatusCode::InvalidSyntax,
+        }
+    }
+
+    fn backtrace_opt(&self) -> Option<&Backtrace> {
+        ErrorCompat::backtrace(self)
+    }
+}
+
 #[cfg(test)]
 mod tests {
     use std::assert_matches::assert_matches;