refactor: remove location_opt and DebugFormat (#3830)

Signed-off-by: Ruihang Xia <waynestxia@gmail.com>

src/common/base/src/buffer.rs

@@ -53,14 +53,6 @@ impl ErrorExt for Error {
     fn as_any(&self) -> &dyn Any {
         self
     }
-
-    fn location_opt(&self) -> Option<common_error::snafu::Location> {
-        match self {
-            Error::Overflow { location, .. } => Some(*location),
-            Error::Underflow { location, .. } => Some(*location),
-            Error::Eof { location, .. } => Some(*location),
-        }
-    }
 }
 
 macro_rules! impl_read_le {

src/common/datasource/src/error.rs

@@ -213,34 +213,4 @@ impl ErrorExt for Error {
     fn as_any(&self) -> &dyn Any {
         self
     }
-
-    fn location_opt(&self) -> Option<common_error::snafu::Location> {
-        use Error::*;
-        match self {
-            OrcReader { location, .. } => Some(*location),
-            BuildBackend { location, .. } => Some(*location),
-            ReadObject { location, .. } => Some(*location),
-            ListObjects { location, .. } => Some(*location),
-            InferSchema { location, .. } => Some(*location),
-            ReadParquetSnafu { location, .. } => Some(*location),
-            ParquetToSchema { location, .. } => Some(*location),
-            JoinHandle { location, .. } => Some(*location),
-            ParseFormat { location, .. } => Some(*location),
-            MergeSchema { location, .. } => Some(*location),
-            WriteObject { location, .. } => Some(*location),
-            ReadRecordBatch { location, .. } => Some(*location),
-            WriteRecordBatch { location, .. } => Some(*location),
-            AsyncWrite { location, .. } => Some(*location),
-            EncodeRecordBatch { location, .. } => Some(*location),
-            BufferedWriterClosed { location, .. } => Some(*location),
-
-            UnsupportedBackendProtocol { location, .. } => Some(*location),
-            EmptyHostPath { location, .. } => Some(*location),
-            InvalidUrl { location, .. } => Some(*location),
-            InvalidConnection { location, .. } => Some(*location),
-            UnsupportedCompressionType { location, .. } => Some(*location),
-            UnsupportedFormat { location, .. } => Some(*location),
-            WriteParquet { location, .. } => Some(*location),
-        }
-    }
 }

src/common/decimal/src/error.rs

@@ -56,14 +56,6 @@ impl ErrorExt for Error {
         }
     }
 
-    fn location_opt(&self) -> Option<common_error::snafu::Location> {
-        match self {
-            Error::BigDecimalOutOfRange { location, .. } => Some(*location),
-            Error::InvalidPrecisionOrScale { location, .. } => Some(*location),
-            Error::ParseRustDecimalStr { .. } | Error::ParseBigDecimalStr { .. } => None,
-        }
-    }
-
     fn as_any(&self) -> &dyn std::any::Any {
         self
     }

src/common/error/src/ext.rs

@@ -24,13 +24,6 @@ pub trait ErrorExt: StackError {
         StatusCode::Unknown
     }
 
-    // TODO(ruihang): remove this default implementation
-    /// Get the location of this error, None if the location is unavailable.
-    /// Add `_opt` suffix to avoid confusing with similar method in `std::error::Error`
-    fn location_opt(&self) -> Option<crate::snafu::Location> {
-        None
-    }
-
     /// Returns the error as [Any](std::any::Any) so that it can be
     /// downcast to a specific implementation.
     fn as_any(&self) -> &dyn Any;
@@ -116,9 +109,9 @@ impl BoxedError {
 
 impl std::fmt::Debug for BoxedError {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
-        // Use the pretty debug format of inner error for opaque error.
-        let debug_format = crate::format::DebugFormat::new(&*self.inner);
-        debug_format.fmt(f)
+        let mut buf = vec![];
+        self.debug_fmt(0, &mut buf);
+        write!(f, "{}", buf.join("\n"))
     }
 }
 
@@ -139,10 +132,6 @@ impl crate::ext::ErrorExt for BoxedError {
         self.inner.status_code()
     }
 
-    fn location_opt(&self) -> Option<crate::snafu::Location> {
-        self.inner.location_opt()
-    }
-
     fn as_any(&self) -> &dyn std::any::Any {
         self.inner.as_any()
     }
@@ -196,10 +185,6 @@ impl crate::ext::ErrorExt for PlainError {
         self.status_code
     }
 
-    fn location_opt(&self) -> Option<crate::snafu::Location> {
-        None
-    }
-
     fn as_any(&self) -> &dyn std::any::Any {
         self as _
     }

src/common/error/src/format.rs

@@ -1,154 +0,0 @@
-// Copyright 2023 Greptime Team
-//
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-//
-//     http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
-
-use std::fmt;
-
-use crate::ext::ErrorExt;
-
-/// Pretty debug format for error, also prints source and backtrace.
-pub struct DebugFormat<'a, E: ?Sized>(&'a E);
-
-impl<'a, E: ?Sized> DebugFormat<'a, E> {
-    /// Create a new format struct from `err`.
-    pub fn new(err: &'a E) -> Self {
-        Self(err)
-    }
-}
-
-impl<'a, E: ErrorExt + ?Sized> fmt::Debug for DebugFormat<'a, E> {
-    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        write!(f, "{}.", self.0)?;
-        if let Some(source) = self.0.source() {
-            // Source error use debug format for more verbose info.
-            write!(f, " Caused by: {source:?}")?;
-        }
-        if let Some(location) = self.0.location_opt() {
-            // Add a newline to separate causes and backtrace.
-            write!(f, " at: {location}")?;
-        }
-
-        Ok(())
-    }
-}
-
-#[cfg(test)]
-mod tests {
-    use std::any::Any;
-
-    use snafu::prelude::*;
-    use snafu::{GenerateImplicitData, Location};
-
-    use super::*;
-    use crate::ext::StackError;
-
-    #[derive(Debug, Snafu)]
-    #[snafu(display("This is a leaf error"))]
-    struct Leaf;
-
-    impl ErrorExt for Leaf {
-        fn location_opt(&self) -> Option<Location> {
-            None
-        }
-
-        fn as_any(&self) -> &dyn Any {
-            self
-        }
-    }
-
-    impl StackError for Leaf {
-        fn debug_fmt(&self, _: usize, _: &mut Vec<String>) {}
-
-        fn next(&self) -> Option<&dyn StackError> {
-            None
-        }
-    }
-
-    #[derive(Debug, Snafu)]
-    #[snafu(display("This is a leaf with location"))]
-    struct LeafWithLocation {
-        location: Location,
-    }
-
-    impl ErrorExt for LeafWithLocation {
-        fn location_opt(&self) -> Option<Location> {
-            None
-        }
-
-        fn as_any(&self) -> &dyn Any {
-            self
-        }
-    }
-
-    impl StackError for LeafWithLocation {
-        fn debug_fmt(&self, _: usize, _: &mut Vec<String>) {}
-
-        fn next(&self) -> Option<&dyn StackError> {
-            None
-        }
-    }
-
-    #[derive(Debug, Snafu)]
-    #[snafu(display("Internal error"))]
-    struct Internal {
-        #[snafu(source)]
-        source: Leaf,
-        location: Location,
-    }
-
-    impl ErrorExt for Internal {
-        fn location_opt(&self) -> Option<Location> {
-            None
-        }
-
-        fn as_any(&self) -> &dyn Any {
-            self
-        }
-    }
-
-    impl StackError for Internal {
-        fn debug_fmt(&self, layer: usize, buf: &mut Vec<String>) {
-            buf.push(format!("{}: Internal error, at {}", layer, self.location));
-            self.source.debug_fmt(layer + 1, buf);
-        }
-
-        fn next(&self) -> Option<&dyn StackError> {
-            Some(&self.source)
-        }
-    }
-
-    #[test]
-    fn test_debug_format() {
-        let err = Leaf;
-
-        let msg = format!("{:?}", DebugFormat::new(&err));
-        assert_eq!("This is a leaf error.", msg);
-
-        let err = LeafWithLocation {
-            location: Location::generate(),
-        };
-
-        // TODO(ruihang): display location here
-        let msg = format!("{:?}", DebugFormat::new(&err));
-        assert!(msg.starts_with("This is a leaf with location."));
-
-        let err = Internal {
-            source: Leaf,
-            location: Location::generate(),
-        };
-
-        // TODO(ruihang): display location here
-        let msg = format!("{:?}", DebugFormat::new(&err));
-        assert!(msg.contains("Internal error. Caused by: Leaf"));
-    }
-}

src/common/error/src/lib.rs

@@ -15,7 +15,6 @@
 #![feature(error_iter)]
 
 pub mod ext;
-pub mod format;
 pub mod mock;
 pub mod status_code;
 

src/common/error/src/mock.rs

@@ -17,8 +17,6 @@
 use std::any::Any;
 use std::fmt;
 
-use snafu::Location;
-
 use crate::ext::{ErrorExt, StackError};
 use crate::status_code::StatusCode;
 
@@ -61,10 +59,6 @@ impl ErrorExt for MockError {
         self.code
     }
 
-    fn location_opt(&self) -> Option<Location> {
-        None
-    }
-
     fn as_any(&self) -> &dyn Any {
         self
     }

src/common/runtime/src/error.rs

@@ -48,12 +48,4 @@ impl ErrorExt for Error {
     fn as_any(&self) -> &dyn Any {
         self
     }
-
-    fn location_opt(&self) -> Option<common_error::snafu::Location> {
-        match self {
-            Error::BuildRuntime { location, .. }
-            | Error::IllegalState { location, .. }
-            | Error::WaitGcTaskStop { location, .. } => Some(*location),
-        }
-    }
 }

src/common/time/src/error.rs

@@ -98,21 +98,6 @@ impl ErrorExt for Error {
     fn as_any(&self) -> &dyn Any {
         self
     }
-
-    fn location_opt(&self) -> Option<common_error::snafu::Location> {
-        match self {
-            Error::ParseTimestamp { location, .. }
-            | Error::Format { location, .. }
-            | Error::TimestampOverflow { location, .. }
-            | Error::ArithmeticOverflow { location, .. } => Some(*location),
-            Error::ParseDateStr { .. }
-            | Error::InvalidTimezoneOffset { .. }
-            | Error::ParseOffsetStr { .. }
-            | Error::ParseTimezoneName { .. } => None,
-            Error::InvalidDateStr { location, .. } => Some(*location),
-            Error::ParseInterval { location, .. } => Some(*location),
-        }
-    }
 }
 
 pub type Result<T> = std::result::Result<T, Error>;