[arrow-rs] branch master updated: Implement `std::error::Error::source` for `ArrowError` and `FlightError` (#3567)

[al...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

alamb pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/arrow-rs.git


The following commit(s) were added to refs/heads/master by this push:
     new 9bb6aaf38 Implement `std::error::Error::source` for `ArrowError` and `FlightError` (#3567)
9bb6aaf38 is described below

commit 9bb6aaf38c17d63b8b6201f03cd5adb330b64fa7
Author: Andrew Lamb <an...@nerdnetworks.org>
AuthorDate: Thu Jan 19 23:08:48 2023 +0100

    Implement `std::error::Error::source` for `ArrowError` and `FlightError` (#3567)
    
    * impl Error::source for ArrowError
    
    * Add source() for FlightError
    
    * clippy
    
    * Update arrow-flight/src/error.rs
    
    Co-authored-by: Liang-Chi Hsieh <vi...@gmail.com>
    
    Co-authored-by: Liang-Chi Hsieh <vi...@gmail.com>
---
 arrow-flight/src/error.rs | 65 ++++++++++++++++++++++++++++++++++++++++++++---
 arrow-schema/src/error.rs | 40 ++++++++++++++++++++++++++++-
 2 files changed, 100 insertions(+), 5 deletions(-)

diff --git a/arrow-flight/src/error.rs b/arrow-flight/src/error.rs
index 11e0ae5c9..7a43e537a 100644
--- a/arrow-flight/src/error.rs
+++ b/arrow-flight/src/error.rs
@@ -15,6 +15,8 @@
 // specific language governing permissions and limitations
 // under the License.
 
+use std::error::Error;
+
 use arrow_schema::ArrowError;
 
 /// Errors for the Apache Arrow Flight crate
@@ -30,8 +32,8 @@ pub enum FlightError {
     ProtocolError(String),
     /// An error occured during decoding
     DecodeError(String),
-    /// Some other (opaque) error
-    ExternalError(Box<dyn std::error::Error + Send + Sync>),
+    /// External error that can provide source of error by calling `Error::source`.
+    ExternalError(Box<dyn Error + Send + Sync>),
 }
 
 impl FlightError {
@@ -40,7 +42,7 @@ impl FlightError {
     }
 
     /// Wraps an external error in an `ArrowError`.
-    pub fn from_external_error(error: Box<dyn std::error::Error + Send + Sync>) -> Self {
+    pub fn from_external_error(error: Box<dyn Error + Send + Sync>) -> Self {
         Self::ExternalError(error)
     }
 }
@@ -52,7 +54,15 @@ impl std::fmt::Display for FlightError {
     }
 }
 
-impl std::error::Error for FlightError {}
+impl Error for FlightError {
+    fn source(&self) -> Option<&(dyn Error + 'static)> {
+        if let Self::ExternalError(e) = self {
+            Some(e.as_ref())
+        } else {
+            None
+        }
+    }
+}
 
 impl From<tonic::Status> for FlightError {
     fn from(status: tonic::Status) -> Self {
@@ -82,3 +92,50 @@ impl From<FlightError> for tonic::Status {
 }
 
 pub type Result<T> = std::result::Result<T, FlightError>;
+
+#[cfg(test)]
+mod test {
+    use super::*;
+
+    #[test]
+    fn error_source() {
+        let e1 = FlightError::DecodeError("foo".into());
+        assert!(e1.source().is_none());
+
+        // one level of wrapping
+        let e2 = FlightError::ExternalError(Box::new(e1));
+        let source = e2.source().unwrap().downcast_ref::<FlightError>().unwrap();
+        assert!(matches!(source, FlightError::DecodeError(_)));
+
+        let e3 = FlightError::ExternalError(Box::new(e2));
+        let source = e3
+            .source()
+            .unwrap()
+            .downcast_ref::<FlightError>()
+            .unwrap()
+            .source()
+            .unwrap()
+            .downcast_ref::<FlightError>()
+            .unwrap();
+
+        assert!(matches!(source, FlightError::DecodeError(_)));
+    }
+
+    #[test]
+    fn error_through_arrow() {
+        // flight error that wraps an arrow error that wraps a flight error
+        let e1 = FlightError::DecodeError("foo".into());
+        let e2 = ArrowError::ExternalError(Box::new(e1));
+        let e3 = FlightError::ExternalError(Box::new(e2));
+
+        // ensure we can find the lowest level error by following source()
+        let mut root_error: &dyn Error = &e3;
+        while let Some(source) = root_error.source() {
+            // walk the next level
+            root_error = source;
+        }
+
+        let source = root_error.downcast_ref::<FlightError>().unwrap();
+        assert!(matches!(source, FlightError::DecodeError(_)));
+    }
+}
diff --git a/arrow-schema/src/error.rs b/arrow-schema/src/error.rs
index 0d7a35a9d..ea60572b3 100644
--- a/arrow-schema/src/error.rs
+++ b/arrow-schema/src/error.rs
@@ -100,4 +100,42 @@ impl Display for ArrowError {
     }
 }
 
-impl Error for ArrowError {}
+impl Error for ArrowError {
+    fn source(&self) -> Option<&(dyn Error + 'static)> {
+        if let Self::ExternalError(e) = self {
+            Some(e.as_ref())
+        } else {
+            None
+        }
+    }
+}
+
+#[cfg(test)]
+mod test {
+    use super::*;
+
+    #[test]
+    fn error_source() {
+        let e1 = ArrowError::DivideByZero;
+        assert!(e1.source().is_none());
+
+        // one level of wrapping
+        let e2 = ArrowError::ExternalError(Box::new(e1));
+        let source = e2.source().unwrap().downcast_ref::<ArrowError>().unwrap();
+        assert!(matches!(source, ArrowError::DivideByZero));
+
+        // two levels of wrapping
+        let e3 = ArrowError::ExternalError(Box::new(e2));
+        let source = e3
+            .source()
+            .unwrap()
+            .downcast_ref::<ArrowError>()
+            .unwrap()
+            .source()
+            .unwrap()
+            .downcast_ref::<ArrowError>()
+            .unwrap();
+
+        assert!(matches!(source, ArrowError::DivideByZero));
+    }
+}