Box FlightErrror::tonic to reduce size (fixes nightly clippy) (#7229)

* make tonic error boxed * test with all features * Add test for FlightError size, suggested by @crepererum --------- Co-authored-by: Andrew Lamb <[email protected]>
apache · Mar 7, 2025 · 9e91029 · 9e91029
1 parent 40af786
commit 9e91029
Show file tree

Hide file tree

Showing 4 changed files with 15 additions and 9 deletions.
diff --git a/arrow-flight/src/client.rs b/arrow-flight/src/client.rs
@@ -210,7 +210,7 @@ impl FlightClient {
         let (response_stream, trailers) = extract_lazy_trailers(response_stream);
 
         Ok(FlightRecordBatchStream::new_from_flight_data(
-            response_stream.map_err(FlightError::Tonic),
+            response_stream.map_err(|status| status.into()),
         )
         .with_headers(md)
         .with_trailers(trailers))
@@ -470,7 +470,7 @@ impl FlightClient {
             .list_flights(request)
             .await?
             .into_inner()
-            .map_err(FlightError::Tonic);
+            .map_err(|status| status.into());
 
         Ok(response.boxed())
     }
@@ -537,7 +537,7 @@ impl FlightClient {
             .list_actions(request)
             .await?
             .into_inner()
-            .map_err(FlightError::Tonic);
+            .map_err(|status| status.into());
 
         Ok(action_stream.boxed())
     }
@@ -575,7 +575,7 @@ impl FlightClient {
             .do_action(request)
             .await?
             .into_inner()
-            .map_err(FlightError::Tonic)
+            .map_err(|status| status.into())
             .map(|r| {
                 r.map(|r| {
                     // unwrap inner bytes

diff --git a/arrow-flight/src/error.rs b/arrow-flight/src/error.rs
@@ -27,7 +27,7 @@ pub enum FlightError {
     /// Returned when functionality is not yet available.
     NotYetImplemented(String),
     /// Error from the underlying tonic library
-    Tonic(tonic::Status),
+    Tonic(Box<tonic::Status>),
     /// Some unexpected message was received
     ProtocolError(String),
     /// An error occurred during decoding
@@ -74,7 +74,7 @@ impl Error for FlightError {
 
 impl From<tonic::Status> for FlightError {
     fn from(status: tonic::Status) -> Self {
-        Self::Tonic(status)
+        Self::Tonic(Box::new(status))
     }
 }
 
@@ -91,7 +91,7 @@ impl From<FlightError> for tonic::Status {
         match value {
             FlightError::Arrow(e) => tonic::Status::internal(e.to_string()),
             FlightError::NotYetImplemented(e) => tonic::Status::internal(e),
-            FlightError::Tonic(status) => status,
+            FlightError::Tonic(status) => *status,
             FlightError::ProtocolError(e) => tonic::Status::internal(e),
             FlightError::DecodeError(e) => tonic::Status::internal(e),
             FlightError::ExternalError(e) => tonic::Status::internal(e.to_string()),
@@ -147,4 +147,10 @@ mod test {
         let source = root_error.downcast_ref::<FlightError>().unwrap();
         assert!(matches!(source, FlightError::DecodeError(_)));
     }
+
+    #[test]
+    fn test_error_size() {
+        // use Box in variants to keep this size down
+        assert_eq!(std::mem::size_of::<FlightError>(), 32);
+    }
 }
diff --git a/arrow-flight/src/sql/client.rs b/arrow-flight/src/sql/client.rs
@@ -309,7 +309,7 @@ impl FlightSqlServiceClient<Channel> {
         let (response_stream, trailers) = extract_lazy_trailers(response_stream);
 
         Ok(FlightRecordBatchStream::new_from_flight_data(
-            response_stream.map_err(FlightError::Tonic),
+            response_stream.map_err(|status| status.into()),
         )
         .with_headers(md)
         .with_trailers(trailers))

diff --git a/arrow-flight/src/streams.rs b/arrow-flight/src/streams.rs
@@ -127,7 +127,7 @@ impl<T> Stream for FallibleTonicResponseStream<T> {
 
         match ready!(pinned.response_stream.poll_next_unpin(cx)) {
             Some(Ok(res)) => Poll::Ready(Some(Ok(res))),
-            Some(Err(status)) => Poll::Ready(Some(Err(FlightError::Tonic(status)))),
+            Some(Err(status)) => Poll::Ready(Some(Err(status.into()))),
             None => Poll::Ready(None),
         }
     }