fix cause drops on conversion

Author: chris-olszewski

crates/common/src/data_converters/failure_converter.rs

@@ -154,12 +154,8 @@ impl FailureDecodeHint for ChildWorkflowSignalDecodeHint {
 
     fn adapt(self, normalized: IncomingError) -> Self::Output {
         let failure = normalized.failure().clone();
-        let payload_converter = PayloadConverter::default();
-        let cause = failure.cause.clone().map(|cause| {
-            decode_failure(*cause, &payload_converter, &SerializationContextData::None)
-        });
         ChildWorkflowSignalError::Failed(Box::new(ChildWorkflowSignalFailureError::new(
-            failure, normalized, cause,
+            failure, normalized,
         )))
     }
 }
@@ -314,7 +310,10 @@ impl EncodeFailure for ApplicationFailure {
             .transpose()?;
         Ok(Failure {
             message: self.to_string(),
-            cause: encode_application_failure_cause(self.source_error().as_ref()),
+            cause: self
+                .cause()
+                .map(|cause| Box::new(cause.failure().clone()))
+                .or_else(|| encode_application_failure_cause(self.source_error().as_ref())),
             failure_info: Some(FailureInfo::ApplicationFailureInfo(
                 ApplicationFailureInfo {
                     r#type: self.type_name().unwrap_or_default().to_owned(),
@@ -785,6 +784,30 @@ mod tests {
         assert_eq!(converted.cause.unwrap().as_ref(), &activity_failure);
     }
 
+    #[test]
+    fn application_failures_fall_back_to_source_error() {
+        let activity_failure = Failure {
+            message: "activity failed".to_owned(),
+            failure_info: Some(FailureInfo::ActivityFailureInfo(
+                ActivityFailureInfo::default(),
+            )),
+            ..Default::default()
+        };
+        let app = ApplicationFailure::new(anyhow::Error::new(ActivityExecutionError::Failed(
+            ActivityFailureError::new(
+                activity_failure.clone(),
+                ActivityFailureInfo::default(),
+                None,
+            ),
+        )));
+
+        assert!(app.cause().is_none());
+
+        let converted = convert(OutgoingWorkflowError::Application(Box::new(app)));
+
+        assert_eq!(converted.cause.unwrap().as_ref(), &activity_failure);
+    }
+
     #[test]
     fn application_failures_skip_generic_wrappers_around_known_causes() {
         let activity_failure = Failure {
@@ -940,6 +963,48 @@ mod tests {
         assert!(matches!(app.cause(), Some(IncomingError::Timeout(_))));
     }
 
+    #[test]
+    fn decoded_application_failures_preserve_cause() {
+        let failure = Failure {
+            message: "app boom".to_owned(),
+            cause: Some(Box::new(timeout_failure("timed out"))),
+            failure_info: Some(FailureInfo::ApplicationFailureInfo(
+                ApplicationFailureInfo::default(),
+            )),
+            ..Default::default()
+        };
+
+        let decoded = DefaultFailureConverter
+            .to_error(
+                failure.clone(),
+                &PayloadConverter::default(),
+                &SerializationContextData::Workflow,
+            )
+            .unwrap();
+
+        let IncomingError::Application(app) = decoded else {
+            panic!("expected application error");
+        };
+        assert!(app.as_timeout().is_some());
+
+        let reencoded = convert(OutgoingWorkflowError::Application(Box::new(app)));
+
+        assert_eq!(reencoded.message, failure.message);
+        assert_eq!(reencoded.cause.as_deref(), failure.cause.as_deref());
+
+        let decoded_reencoded = DefaultFailureConverter
+            .to_error(
+                reencoded,
+                &PayloadConverter::default(),
+                &SerializationContextData::Workflow,
+            )
+            .unwrap();
+        let IncomingError::Application(roundtripped) = decoded_reencoded else {
+            panic!("expected application error");
+        };
+        assert!(roundtripped.as_timeout().is_some());
+    }
+
     #[test]
     fn application_failures_decode_wrapped_known_causes_without_collapsing_wrapper() {
         let failure = Failure {

crates/common/src/error.rs

@@ -2,9 +2,9 @@
 
 use crate::{
     data_converters::{
-        DecodablePayloads, DefaultFailureConverter, FailureConverter, GenericPayloadConverter,
-        PayloadConversionError, PayloadConverter, RawValue, SerializationContext,
-        SerializationContextData, TemporalDeserializable, TemporalSerializable,
+        DecodablePayloads, GenericPayloadConverter, PayloadConversionError, PayloadConverter,
+        RawValue, SerializationContext, SerializationContextData, TemporalDeserializable,
+        TemporalSerializable,
     },
     protos::{
         coresdk::child_workflow::StartChildWorkflowExecutionFailedCause,
@@ -210,6 +210,11 @@ impl ApplicationFailure {
         self.failure.as_ref()
     }
 
+    /// Consumes this application failure and returns the retained proto failure, if one exists.
+    pub fn into_failure(self) -> Option<Failure> {
+        self.failure
+    }
+
     /// Returns the normalized cause, if any.
     pub fn cause(&self) -> Option<&IncomingError> {
         self.cause.as_deref()
@@ -284,16 +289,6 @@ impl From<PayloadConversionError> for ApplicationFailure {
     }
 }
 
-impl From<ApplicationFailure> for Failure {
-    fn from(value: ApplicationFailure) -> Self {
-        DefaultFailureConverter.to_failure(
-            OutgoingError::Workflow(OutgoingWorkflowError::Application(Box::new(value))),
-            &PayloadConverter::default(),
-            &SerializationContextData::None,
-        )
-    }
-}
-
 /// A typed outbound error surface used before encoding to a Temporal failure proto.
 #[derive(Debug, thiserror::Error)]
 pub enum OutgoingError {
@@ -444,7 +439,9 @@ impl IncomingError {
     /// Consumes this normalized error and returns the retained proto failure.
     pub fn into_failure(self) -> Failure {
         match self {
-            IncomingError::Application(err) => err.into(),
+            IncomingError::Application(err) => err
+                .into_failure()
+                .expect("decoded application failures retain their original proto"),
             IncomingError::Timeout(err) => err.into_failure(),
             IncomingError::Cancelled(err) => err.into_failure(),
             IncomingError::Terminated(err) => err.into_failure(),
@@ -1038,38 +1035,53 @@ impl ChildWorkflowSignalError {
 pub struct ChildWorkflowSignalFailureError {
     failure: Failure,
     error: Box<IncomingError>,
-    cause: Option<Box<IncomingError>>,
 }
 
 impl ChildWorkflowSignalFailureError {
     /// Creates a child-workflow signal failure wrapper.
-    pub(crate) fn new(
-        failure: Failure,
-        error: IncomingError,
-        cause: Option<IncomingError>,
-    ) -> Self {
+    pub(crate) fn new(failure: Failure, error: IncomingError) -> Self {
         Self {
             failure,
             error: Box::new(error),
-            cause: cause.map(Box::new),
         }
     }
 
+    /// Returns the retained top-level proto failure.
+    pub fn failure(&self) -> &Failure {
+        &self.failure
+    }
+
+    /// Returns the normalized direct cause of the child-workflow signal failure, if any.
+    pub fn cause(&self) -> Option<&IncomingError> {
+        self.error.cause()
+    }
+
     /// Returns the direct decoded incoming error represented by the top-level proto failure.
     pub fn error(&self) -> &IncomingError {
         &self.error
     }
 }
 
-impl_incoming_failure_wrapper!(ChildWorkflowSignalFailureError);
+impl std::fmt::Display for ChildWorkflowSignalFailureError {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        self.failure.fmt(f)
+    }
+}
+
+impl std::error::Error for ChildWorkflowSignalFailureError {
+    fn source(&self) -> Option<&(dyn std::error::Error + 'static)> {
+        self.cause()
+            .map(|cause| cause as &(dyn std::error::Error + 'static))
+    }
+}
 
 #[cfg(test)]
 mod tests {
     use super::*;
     use crate::{
         data_converters::{
-            GenericPayloadConverter, PayloadConverter, SerializationContext,
-            SerializationContextData,
+            DefaultFailureConverter, FailureConverter, GenericPayloadConverter, PayloadConverter,
+            SerializationContext, SerializationContextData,
         },
         protos::temporal::api::{common::v1::Payload, failure::v1::failure::FailureInfo},
     };
@@ -1098,14 +1110,19 @@ mod tests {
                 ..Default::default()
             }],
         };
-        let failure: Failure = ApplicationFailure::builder(anyhow::anyhow!("oops"))
-            .type_name("MyType".to_owned())
-            .non_retryable(true)
-            .next_retry_delay(Duration::from_secs(3))
-            .category(ApplicationErrorCategory::Benign)
-            .details(RawValue::new(payloads.payloads.clone()))
-            .build()
-            .into();
+        let failure = DefaultFailureConverter.to_failure(
+            OutgoingError::Workflow(OutgoingWorkflowError::Application(Box::new(
+                ApplicationFailure::builder(anyhow::anyhow!("oops"))
+                    .type_name("MyType".to_owned())
+                    .non_retryable(true)
+                    .next_retry_delay(Duration::from_secs(3))
+                    .category(ApplicationErrorCategory::Benign)
+                    .details(RawValue::new(payloads.payloads.clone()))
+                    .build(),
+            ))),
+            &PayloadConverter::default(),
+            &SerializationContextData::None,
+        );
         let Some(FailureInfo::ApplicationFailureInfo(info)) = failure.failure_info else {
             panic!("expected application failure info");
         };
@@ -1123,10 +1140,15 @@ mod tests {
             data: b"details".to_vec(),
             ..Default::default()
         };
-        let failure: Failure = ApplicationFailure::builder(anyhow::anyhow!("oops"))
-            .details(RawValue::new(vec![payload.clone()]))
-            .build()
-            .into();
+        let failure = DefaultFailureConverter.to_failure(
+            OutgoingError::Workflow(OutgoingWorkflowError::Application(Box::new(
+                ApplicationFailure::builder(anyhow::anyhow!("oops"))
+                    .details(RawValue::new(vec![payload.clone()]))
+                    .build(),
+            ))),
+            &PayloadConverter::default(),
+            &SerializationContextData::None,
+        );
 
         let Some(FailureInfo::ApplicationFailureInfo(info)) = failure.failure_info else {
             panic!("expected application failure info");
@@ -1136,10 +1158,15 @@ mod tests {
 
     #[test]
     fn builder_accepts_serializable_details() {
-        let failure: Failure = ApplicationFailure::builder(anyhow::anyhow!("oops"))
-            .details("details".to_string())
-            .build()
-            .into();
+        let failure = DefaultFailureConverter.to_failure(
+            OutgoingError::Workflow(OutgoingWorkflowError::Application(Box::new(
+                ApplicationFailure::builder(anyhow::anyhow!("oops"))
+                    .details("details".to_string())
+                    .build(),
+            ))),
+            &PayloadConverter::default(),
+            &SerializationContextData::None,
+        );
 
         let Some(FailureInfo::ApplicationFailureInfo(info)) = failure.failure_info else {
             panic!("expected application failure info");
@@ -1159,11 +1186,16 @@ mod tests {
     }
 
     #[test]
-    fn application_failure_into_failure_surfaces_detail_encoding_errors() {
-        let failure: Failure = ApplicationFailure::builder(anyhow::anyhow!("oops"))
-            .details(AlwaysFailsSerialize)
-            .build()
-            .into();
+    fn application_failure_encoding_surfaces_detail_encoding_errors() {
+        let failure = DefaultFailureConverter.to_failure(
+            OutgoingError::Workflow(OutgoingWorkflowError::Application(Box::new(
+                ApplicationFailure::builder(anyhow::anyhow!("oops"))
+                    .details(AlwaysFailsSerialize)
+                    .build(),
+            ))),
+            &PayloadConverter::default(),
+            &SerializationContextData::None,
+        );
 
         assert_eq!(
             failure.message,

crates/common/src/payload_visitor.rs

@@ -194,7 +194,8 @@ include!(concat!(env!("OUT_DIR"), "/payload_visitor_impl.rs"));
 mod tests {
     use super::*;
     use crate::{
-        error::ApplicationFailure,
+        data_converters::{DefaultFailureConverter, FailureConverter, PayloadConverter},
+        error::{ApplicationFailure, OutgoingError, OutgoingWorkflowError},
         protos::{
             coresdk::{
                 activity_result::{
@@ -215,7 +216,7 @@ mod tests {
             },
             temporal::api::{
                 common::v1::{Memo, SearchAttributes},
-                failure::v1::{Failure, failure::FailureInfo},
+                failure::v1::failure::FailureInfo,
             },
         },
     };
@@ -654,12 +655,17 @@ mod tests {
 
     #[tokio::test]
     async fn test_encode_failure_encodes_application_failure_details() {
-        let mut failure: Failure = ApplicationFailure::builder(anyhow::anyhow!("app boom"))
-            .details(crate::data_converters::RawValue::new(vec![make_payload(
-                "detail",
-            )]))
-            .build()
-            .into();
+        let mut failure = DefaultFailureConverter.to_failure(
+            OutgoingError::Workflow(OutgoingWorkflowError::Application(Box::new(
+                ApplicationFailure::builder(anyhow::anyhow!("app boom"))
+                    .details(crate::data_converters::RawValue::new(vec![make_payload(
+                        "detail",
+                    )]))
+                    .build(),
+            ))),
+            &PayloadConverter::default(),
+            &SerializationContextData::Workflow,
+        );
 
         encode_payloads(
             &mut failure,