refactor: migrate hand-written RetryableError impls to #[derive(Retryable)]

apps/xnet/lib/src/wallet_funding.rs

@@ -11,7 +11,7 @@ use alloy::{
 };
 use color_eyre::eyre::{Context, Result};
 use tracing::info;
-use xmtp_common::{Retry, RetryableError, retry_async};
+use xmtp_common::{Retry, Retryable, retry_async};
 
 /// Default funding amount for new nodes (in ETH)
 const DEFAULT_FUNDING_AMOUNT_ETH: &str = "1000";
@@ -72,6 +72,8 @@ pub async fn fund_wallet(
     Ok(())
 }
 
+#[derive(Retryable)]
+#[retry(true)]
 pub struct SetBalanceFailure(Box<dyn std::error::Error + Send + Sync>);
 
 impl std::error::Error for SetBalanceFailure {
@@ -92,12 +94,6 @@ impl std::fmt::Debug for SetBalanceFailure {
     }
 }
 
-impl RetryableError for SetBalanceFailure {
-    fn is_retryable(&self) -> bool {
-        true
-    }
-}
-
 #[cfg(test)]
 mod tests {
     use super::*;

crates/xmtp_api/src/lib.rs

@@ -8,7 +8,7 @@ pub mod test_utils;
 
 use std::sync::Arc;
 
-use xmtp_common::{ErrorCode, ExponentialBackoff, Retry, RetryableError, retryable};
+use xmtp_common::{ErrorCode, ExponentialBackoff, Retry, Retryable, RetryableError};
 pub use xmtp_proto::api_client::XmtpApi;
 
 pub use identity::*;
@@ -29,12 +29,13 @@ pub fn dyn_err(e: impl RetryableError + 'static) -> ApiError {
     ApiError::Api(Box::new(e))
 }
 
-#[derive(Debug, thiserror::Error, ErrorCode)]
+#[derive(Debug, thiserror::Error, ErrorCode, Retryable)]
 pub enum ApiError {
     /// API client error.
     ///
     /// API operation error (network, deserialization, or other). May be retryable.
     #[error("api client error {0}")]
+    #[retry(inherit)]
     Api(Box<dyn RetryableError>),
     /// Mismatched key packages.
     ///
@@ -55,15 +56,6 @@ pub enum ApiError {
     ProtoConversion(#[from] xmtp_proto::ConversionError),
 }
 
-impl RetryableError for ApiError {
-    fn is_retryable(&self) -> bool {
-        match self {
-            Self::Api(e) => retryable!(e),
-            _ => false,
-        }
-    }
-}
-
 #[derive(Clone, Debug)]
 pub struct ApiClientWrapper<ApiClient> {
     // todo: this should be private to impl

crates/xmtp_api_d14n/src/middleware/multi_node_client/errors.rs

@@ -1,14 +1,15 @@
 use thiserror::Error;
 use xmtp_api_grpc::error::GrpcBuilderError;
-use xmtp_common::RetryableError;
+use xmtp_common::Retryable;
 use xmtp_proto::api::{ApiClientError, BodyError};
 
 /// Errors that can occur during multi-node client operations.
-#[derive(Debug, Error)]
+#[derive(Debug, Error, Retryable)]
 pub enum MultiNodeClientError {
     #[error("all node clients failed to build")]
     AllNodeClientsFailedToBuild,
     #[error(transparent)]
+    #[retry(inherit)]
     BodyError(#[from] BodyError),
     #[error("node {} timed out under {}ms latency", node_id, latency)]
     NodeTimedOut { node_id: u32, latency: u64 },
@@ -37,16 +38,6 @@ impl From<MultiNodeClientError> for ApiClientError {
     }
 }
 
-/// Implements RetryableError to enable proper retry behavior in the API client error handling system.
-impl RetryableError for MultiNodeClientError {
-    fn is_retryable(&self) -> bool {
-        match self {
-            Self::BodyError(e) => e.is_retryable(),
-            _ => false,
-        }
-    }
-}
-
 /// Errors that can occur when building a MultiNodeClient.
 #[derive(Debug, Error)]
 pub enum MultiNodeClientBuilderError {

crates/xmtp_api_d14n/src/protocol/extractors.rs

@@ -1,7 +1,7 @@
 //! Extractors transform [`ProtocolEnvelope`]'s into logical types usable by xmtp_mls
 
 use super::{EnvelopeCollection, EnvelopeError, Extractor, ProtocolEnvelope};
-use xmtp_common::{RetryableError, retryable};
+use xmtp_common::Retryable;
 
 mod aggregate;
 pub use aggregate::*;
@@ -35,22 +35,15 @@ pub use orphaned_envelope::*;
 #[cfg(any(test, feature = "test-utils"))]
 pub mod test_utils;
 
-#[derive(thiserror::Error, Debug)]
+#[derive(thiserror::Error, Debug, Retryable)]
 pub enum ExtractionError {
     #[error(transparent)]
+    #[retry(inherit)]
     Payload(#[from] PayloadExtractionError),
     #[error(transparent)]
+    #[retry(inherit)]
     Topic(#[from] TopicExtractionError),
     #[error(transparent)]
+    #[retry(inherit)]
     Conversion(#[from] xmtp_proto::ConversionError),
 }
-
-impl RetryableError for ExtractionError {
-    fn is_retryable(&self) -> bool {
-        match self {
-            Self::Payload(p) => retryable!(p),
-            Self::Topic(t) => retryable!(t),
-            Self::Conversion(c) => retryable!(c),
-        }
-    }
-}

crates/xmtp_api_d14n/src/protocol/extractors/payloads.rs

@@ -1,4 +1,4 @@
-use xmtp_common::RetryableError;
+use xmtp_common::Retryable;
 
 use crate::protocol::traits::EnvelopeVisitor;
 use crate::protocol::{EnvelopeError, ExtractionError};
@@ -23,18 +23,12 @@ impl PayloadExtractor {
     }
 }
 
-#[derive(thiserror::Error, Debug)]
+#[derive(thiserror::Error, Debug, Retryable)]
 pub enum PayloadExtractionError {
     #[error("Failed to extract payload, wrong ProtocolMessage?")]
     Failed,
 }
 
-impl RetryableError for PayloadExtractionError {
-    fn is_retryable(&self) -> bool {
-        false
-    }
-}
-
 impl From<PayloadExtractionError> for EnvelopeError {
     fn from(err: PayloadExtractionError) -> EnvelopeError {
         EnvelopeError::Extraction(ExtractionError::Payload(err))

crates/xmtp_api_d14n/src/protocol/extractors/topics.rs

@@ -1,6 +1,6 @@
 use hex::FromHexError;
 use openmls::framing::errors::ProtocolMessageError;
-use xmtp_common::RetryableError;
+use xmtp_common::Retryable;
 use xmtp_proto::ConversionError;
 
 use crate::protocol::ExtractionError;
@@ -50,7 +50,7 @@ impl TopicExtractor {
     }
 }
 
-#[derive(thiserror::Error, Debug)]
+#[derive(thiserror::Error, Debug, Retryable)]
 pub enum TopicExtractionError {
     #[error("Topic extraction failed, no topic available")]
     Failed,
@@ -66,12 +66,6 @@ pub enum TopicExtractionError {
     Conversion(#[from] ConversionError),
 }
 
-impl RetryableError for TopicExtractionError {
-    fn is_retryable(&self) -> bool {
-        false
-    }
-}
-
 impl From<TopicExtractionError> for EnvelopeError {
     fn from(err: TopicExtractionError) -> EnvelopeError {
         EnvelopeError::Extraction(ExtractionError::Topic(err))

crates/xmtp_api_d14n/src/protocol/traits.rs

@@ -16,8 +16,8 @@ use xmtp_proto::types::WelcomeMessage;
 use super::ExtractionError;
 use super::PayloadExtractor;
 use super::TopicExtractor;
+use xmtp_common::Retryable;
 use xmtp_common::RetryableError;
-use xmtp_common::retryable;
 use xmtp_proto::ConversionError;
 use xmtp_proto::xmtp::xmtpv4::envelopes::AuthenticatedData;
 use xmtp_proto::xmtp::xmtpv4::envelopes::ClientEnvelope;
@@ -53,11 +53,13 @@ pub use sort::*;
 mod ordered_collection;
 pub use ordered_collection::*;
 
-#[derive(thiserror::Error, Debug)]
+#[derive(thiserror::Error, Debug, Retryable)]
 pub enum EnvelopeError {
     #[error(transparent)]
+    #[retry(inherit)]
     Conversion(#[from] ConversionError),
     #[error(transparent)]
+    #[retry(inherit)]
     Extraction(#[from] ExtractionError),
     #[error("Each topic must have a payload")]
     TopicMismatch,
@@ -66,12 +68,15 @@ pub enum EnvelopeError {
     #[error(transparent)]
     MissingBuilderField(#[from] UninitializedFieldError),
     #[error(transparent)]
+    #[retry(inherit)]
     Store(#[from] CursorStoreError),
     #[error(transparent)]
+    #[retry(true)]
     Decode(#[from] prost::DecodeError),
     // for extractors defined outside of this crate or
     // generic implementations like Tuples
     #[error("{0}")]
+    #[retry(inherit)]
     DynError(Box<dyn RetryableError>),
 }
 
@@ -80,18 +85,3 @@ impl EnvelopeError {
         EnvelopeError::DynError(Box::new(self) as _)
     }
 }
-
-impl RetryableError for EnvelopeError {
-    fn is_retryable(&self) -> bool {
-        match self {
-            Self::Conversion(c) => retryable!(c),
-            Self::Extraction(e) => retryable!(e),
-            Self::TopicMismatch => false,
-            Self::DynError(d) => retryable!(d),
-            Self::NotFound(_) => false,
-            Self::MissingBuilderField(_) => false,
-            Self::Store(s) => retryable!(s),
-            Self::Decode(_) => true,
-        }
-    }
-}

crates/xmtp_api_d14n/src/protocol/traits/cursor_store.rs

@@ -1,12 +1,12 @@
 use std::collections::HashMap;
 use std::sync::Arc;
-use xmtp_common::{MaybeSend, MaybeSync, RetryableError};
+use xmtp_common::{MaybeSend, MaybeSync, Retryable, RetryableError};
 use xmtp_proto::{
     api::ApiClientError,
     types::{Cursor, GlobalCursor, OriginatorId, OrphanedEnvelope, Topic, TopicKind},
 };
 
-#[derive(thiserror::Error, Debug)]
+#[derive(thiserror::Error, Debug, Retryable)]
 pub enum CursorStoreError {
     #[error("error writing cursors to persistent store")]
     Write,
@@ -16,7 +16,9 @@ pub enum CursorStoreError {
     UnhandledTopicKind(TopicKind),
     #[error("no dependencies found for {_0:?}")]
     NoDependenciesFound(Vec<String>),
+    // retries should be an implementation detail; only Other forwards.
     #[error("{0}")]
+    #[retry(inherit)]
     Other(Box<dyn RetryableError>),
 }
 
@@ -26,16 +28,6 @@ impl CursorStoreError {
     }
 }
 
-impl RetryableError for CursorStoreError {
-    fn is_retryable(&self) -> bool {
-        match self {
-            Self::Other(s) => s.is_retryable(),
-            // retries should be an implementation detail
-            _ => false,
-        }
-    }
-}
-
 impl From<CursorStoreError> for ApiClientError {
     fn from(value: CursorStoreError) -> Self {
         ApiClientError::Other(Box::new(value) as Box<_>)

crates/xmtp_api_d14n/src/protocol/traits/dependency_resolution.rs

@@ -1,7 +1,7 @@
 use std::{collections::HashSet, marker::PhantomData};
 
 use derive_builder::UninitializedFieldError;
-use xmtp_common::{MaybeSend, MaybeSync, RetryableError};
+use xmtp_common::{MaybeSend, MaybeSync, Retryable, RetryableError};
 use xmtp_proto::api::BodyError;
 
 use crate::protocol::{CursorStoreError, Envelope, EnvelopeError, types::RequiredDependency};
@@ -96,36 +96,26 @@ where
     }
 }
 
-#[derive(thiserror::Error, Debug)]
+#[derive(thiserror::Error, Debug, Retryable)]
 pub enum ResolutionError {
     #[error(transparent)]
+    #[retry(inherit)]
     Envelope(#[from] EnvelopeError),
     #[error(transparent)]
+    #[retry(inherit)]
     Body(#[from] BodyError),
     #[error("{0}")]
+    #[retry(inherit)]
     Api(Box<dyn RetryableError>),
     #[error(transparent)]
     Build(#[from] UninitializedFieldError),
     #[error("Resolution failed  to find all missing dependant envelopes")]
     ResolutionFailed,
     #[error(transparent)]
+    #[retry(inherit)]
     Store(#[from] CursorStoreError),
 }
 
-impl RetryableError for ResolutionError {
-    fn is_retryable(&self) -> bool {
-        use ResolutionError::*;
-        match self {
-            Envelope(e) => e.is_retryable(),
-            Body(b) => b.is_retryable(),
-            Api(a) => a.is_retryable(),
-            Build(_) => false,
-            ResolutionFailed => false,
-            Store(s) => s.is_retryable(),
-        }
-    }
-}
-
 impl ResolutionError {
     pub fn api<E: RetryableError + 'static>(e: E) -> Self {
         ResolutionError::Api(Box::new(e))

crates/xmtp_api_d14n/src/queries/combined/tests.rs

@@ -1,7 +1,7 @@
 use std::sync::{Arc, OnceLock};
 
 use prost::Message;
-use xmtp_common::RetryableError;
+use xmtp_common::Retryable;
 use xmtp_configuration::CUTOVER_REFRESH_TIME;
 use xmtp_proto::api::mock::MockNetworkClient;
 use xmtp_proto::api::{ApiClientError, BytesStream, Client, IsConnectedCheck};
@@ -89,16 +89,11 @@ fn mock_v3_with_cutover(timestamp_ns: u64) -> TestNetworkClient {
 }
 
 /// A retryable error type for constructing migration-matching errors.
-#[derive(Debug, thiserror::Error)]
+#[derive(Debug, thiserror::Error, Retryable)]
 #[error("{0}")]
+#[retry(true)]
 struct FakeNetworkError(String);
 
-impl RetryableError for FakeNetworkError {
-    fn is_retryable(&self) -> bool {
-        true
-    }
-}
-
 #[xmtp_common::test]
 fn regex_does_not_panic() {
     assert!(!ERROR_REGEX.is_match("hi"))