From e23fad2e14572937aced94a5668c213d7b77ac22 Mon Sep 17 00:00:00 2001 From: Andrew Plaza Date: Tue, 9 Jun 2026 12:08:51 -0400 Subject: [PATCH] refactor: migrate hand-written RetryableError impls to #[derive(Retryable)] MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replaces ~47 hand-written `impl RetryableError` blocks across 8 crates (+xnet) with the derive. Net -456 lines. Behavior preserved per-variant: - forwarding arms -> #[retry(inherit)] - hardcoded trues -> #[retry(true)]; all-true / inverted enums -> #[retry(default = true)] - hardcoded falses -> unannotated (baseline) - multi-field forwards & collection aggregation -> #[retry(when = ...)] - ClientError::Generic -> #[retry(when = this.contains("database is locked"))] - GroupMessageProcessingError's nested match collapsed via a new derive on ProcessMessageWithAppDataError (gains a RetryableError where-bound) Equivalence was verified by an exhaustive variant-by-variant audit against the deleted impls (~350 variants total, zero mismatches). Per-enum golden tests were intentionally NOT kept: the macro's own test suite (18 behavior tests + 11 trybuild fixtures in #3753) is the behavioral contract, and annotations are the source of truth thereafter — same trust model as the hand-written impls had. Still hand-written by design: the 22 specialized RetryableError impls for foreign openmls/diesel types, GroupAppDataError (instantiation-specific), ReceiveErrors/SyncSummary one-line aggregators, and test mocks. Co-Authored-By: Claude Opus 4.8 (1M context) --- apps/xnet/lib/src/wallet_funding.rs | 10 +- crates/xmtp_api/src/lib.rs | 14 +- .../middleware/multi_node_client/errors.rs | 15 +- .../xmtp_api_d14n/src/protocol/extractors.rs | 17 +- .../src/protocol/extractors/payloads.rs | 10 +- .../src/protocol/extractors/topics.rs | 10 +- crates/xmtp_api_d14n/src/protocol/traits.rs | 24 +-- .../src/protocol/traits/cursor_store.rs | 16 +- .../protocol/traits/dependency_resolution.rs | 22 +-- .../src/queries/combined/tests.rs | 11 +- crates/xmtp_api_d14n/src/queries/mod.rs | 17 +- .../src/queries/stream/ordered.rs | 12 +- crates/xmtp_api_grpc/src/error.rs | 11 +- .../src/encrypted_store/database/native.rs | 36 ++-- .../src/encrypted_store/database/wasm.rs | 15 +- .../src/encrypted_store/group_intent/error.rs | 14 +- crates/xmtp_db/src/encrypted_store/mod.rs | 22 +-- crates/xmtp_db/src/errors.rs | 56 ++---- crates/xmtp_db/src/sql_key_store.rs | 20 +-- crates/xmtp_id/src/associations/signature.rs | 22 +-- crates/xmtp_id/src/scw_verifier/mod.rs | 21 +-- crates/xmtp_mls/src/client.rs | 33 ++-- .../xmtp_mls/src/groups/app_data/migration.rs | 30 +--- crates/xmtp_mls/src/groups/app_data/mod.rs | 26 ++- crates/xmtp_mls/src/groups/commit_log.rs | 35 ++-- crates/xmtp_mls/src/groups/error.rs | 161 +++++------------- crates/xmtp_mls/src/groups/mls_sync.rs | 72 ++------ .../xmtp_mls/src/groups/validated_commit.rs | 14 +- crates/xmtp_mls/src/identity.rs | 19 +-- crates/xmtp_mls/src/identity_updates.rs | 17 +- crates/xmtp_mls/src/intents.rs | 15 +- crates/xmtp_mls/src/messages/enrichment.rs | 15 +- crates/xmtp_mls/src/mls_store.rs | 19 +-- .../xmtp_mls/src/subscriptions/d14n_compat.rs | 10 +- crates/xmtp_mls/src/subscriptions/mod.rs | 50 ++---- .../src/subscriptions/stream_conversations.rs | 13 +- .../src/subscriptions/stream_messages.rs | 9 - .../subscriptions/stream_messages/types.rs | 2 +- crates/xmtp_mls/src/test/group_test_utils.rs | 11 +- crates/xmtp_mls/src/worker/device_sync/mod.rs | 29 ++-- .../src/mls_ext/payload_encryption.rs | 18 +- crates/xmtp_proto/src/error.rs | 9 +- crates/xmtp_proto/src/traits/error.rs | 47 ++--- 43 files changed, 296 insertions(+), 753 deletions(-) diff --git a/apps/xnet/lib/src/wallet_funding.rs b/apps/xnet/lib/src/wallet_funding.rs index 72f03def67..dec9aedcff 100644 --- a/apps/xnet/lib/src/wallet_funding.rs +++ b/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); 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::*; diff --git a/crates/xmtp_api/src/lib.rs b/crates/xmtp_api/src/lib.rs index 0eb7017fd8..8e0f518ffa 100644 --- a/crates/xmtp_api/src/lib.rs +++ b/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), /// 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 { // todo: this should be private to impl diff --git a/crates/xmtp_api_d14n/src/middleware/multi_node_client/errors.rs b/crates/xmtp_api_d14n/src/middleware/multi_node_client/errors.rs index 85aaf864d4..86120165d8 100644 --- a/crates/xmtp_api_d14n/src/middleware/multi_node_client/errors.rs +++ b/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 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 { diff --git a/crates/xmtp_api_d14n/src/protocol/extractors.rs b/crates/xmtp_api_d14n/src/protocol/extractors.rs index c08491099f..10eb984aee 100644 --- a/crates/xmtp_api_d14n/src/protocol/extractors.rs +++ b/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), - } - } -} diff --git a/crates/xmtp_api_d14n/src/protocol/extractors/payloads.rs b/crates/xmtp_api_d14n/src/protocol/extractors/payloads.rs index cbd25b8e6c..32acc497a3 100644 --- a/crates/xmtp_api_d14n/src/protocol/extractors/payloads.rs +++ b/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 for EnvelopeError { fn from(err: PayloadExtractionError) -> EnvelopeError { EnvelopeError::Extraction(ExtractionError::Payload(err)) diff --git a/crates/xmtp_api_d14n/src/protocol/extractors/topics.rs b/crates/xmtp_api_d14n/src/protocol/extractors/topics.rs index 3bee804d55..78827498bc 100644 --- a/crates/xmtp_api_d14n/src/protocol/extractors/topics.rs +++ b/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 for EnvelopeError { fn from(err: TopicExtractionError) -> EnvelopeError { EnvelopeError::Extraction(ExtractionError::Topic(err)) diff --git a/crates/xmtp_api_d14n/src/protocol/traits.rs b/crates/xmtp_api_d14n/src/protocol/traits.rs index 3788b8c0a6..89fdbe9d26 100644 --- a/crates/xmtp_api_d14n/src/protocol/traits.rs +++ b/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), } @@ -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, - } - } -} diff --git a/crates/xmtp_api_d14n/src/protocol/traits/cursor_store.rs b/crates/xmtp_api_d14n/src/protocol/traits/cursor_store.rs index 3e19854c8b..3583d87465 100644 --- a/crates/xmtp_api_d14n/src/protocol/traits/cursor_store.rs +++ b/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), + // retries should be an implementation detail; only Other forwards. #[error("{0}")] + #[retry(inherit)] Other(Box), } @@ -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 for ApiClientError { fn from(value: CursorStoreError) -> Self { ApiClientError::Other(Box::new(value) as Box<_>) diff --git a/crates/xmtp_api_d14n/src/protocol/traits/dependency_resolution.rs b/crates/xmtp_api_d14n/src/protocol/traits/dependency_resolution.rs index 3c836e5181..7cdbe19549 100644 --- a/crates/xmtp_api_d14n/src/protocol/traits/dependency_resolution.rs +++ b/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), #[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: E) -> Self { ResolutionError::Api(Box::new(e)) diff --git a/crates/xmtp_api_d14n/src/queries/combined/tests.rs b/crates/xmtp_api_d14n/src/queries/combined/tests.rs index bddfec18e4..87be13d63a 100644 --- a/crates/xmtp_api_d14n/src/queries/combined/tests.rs +++ b/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")) diff --git a/crates/xmtp_api_d14n/src/queries/mod.rs b/crates/xmtp_api_d14n/src/queries/mod.rs index bf8491b482..4b770dee90 100644 --- a/crates/xmtp_api_d14n/src/queries/mod.rs +++ b/crates/xmtp_api_d14n/src/queries/mod.rs @@ -19,7 +19,7 @@ pub use stream::*; pub use v3::*; use std::collections::HashMap; -use xmtp_common::{RetryableError, retryable}; +use xmtp_common::Retryable; use xmtp_proto::{ ConversionError, api::{self, ApiClientError, BodyError, Client, Query}, @@ -69,15 +69,17 @@ pub(crate) async fn build_node_clients( Ok(clients) } -#[derive(thiserror::Error, Debug)] +#[derive(thiserror::Error, Debug, Retryable)] pub enum QueryError { #[error(transparent)] + #[retry(inherit)] ApiClient(#[from] ApiClientError), #[error(transparent)] Envelope(#[from] crate::protocol::EnvelopeError), #[error(transparent)] Conversion(#[from] ConversionError), #[error(transparent)] + #[retry(inherit)] Body(#[from] BodyError), } @@ -86,14 +88,3 @@ impl From for ApiClientError { ApiClientError::Other(Box::new(e)) } } - -impl RetryableError for QueryError { - fn is_retryable(&self) -> bool { - match self { - Self::ApiClient(c) => retryable!(c), - Self::Envelope(_e) => false, - Self::Conversion(_c) => false, - Self::Body(b) => retryable!(b), - } - } -} diff --git a/crates/xmtp_api_d14n/src/queries/stream/ordered.rs b/crates/xmtp_api_d14n/src/queries/stream/ordered.rs index d7c0bea426..10ba6c6a58 100644 --- a/crates/xmtp_api_d14n/src/queries/stream/ordered.rs +++ b/crates/xmtp_api_d14n/src/queries/stream/ordered.rs @@ -10,7 +10,7 @@ use std::{ marker::PhantomData, task::{Poll, ready}, }; -use xmtp_common::RetryableError; +use xmtp_common::Retryable; use xmtp_proto::{api::ApiClientError, types::TopicCursor}; #[pin_project] @@ -24,8 +24,9 @@ pub struct OrderedStream { // this is an error which should never occur, // and if it does is a bug in libxmtp -#[derive(thiserror::Error, Debug)] +#[derive(thiserror::Error, Debug, Retryable)] pub enum OrderedStreamError { + // Hardcoded non-retryable: this error indicates a bug, not a transient failure. #[error(transparent)] Resolver(#[from] ResolutionError), } @@ -36,12 +37,6 @@ impl From for ApiClientError { } } -impl RetryableError for OrderedStreamError { - fn is_retryable(&self) -> bool { - false - } -} - impl From for EnvelopeError { fn from(value: OrderedStreamError) -> Self { EnvelopeError::DynError(Box::new(value) as _) @@ -117,6 +112,7 @@ where #[cfg(test)] mod test { use super::*; + use crate::protocol::{InMemoryCursorStore, test::missing_dependencies}; use futures::{FutureExt, StreamExt, future, stream}; use proptest::prelude::*; diff --git a/crates/xmtp_api_grpc/src/error.rs b/crates/xmtp_api_grpc/src/error.rs index 36d321a10c..150d92ac1c 100644 --- a/crates/xmtp_api_grpc/src/error.rs +++ b/crates/xmtp_api_grpc/src/error.rs @@ -1,5 +1,5 @@ use thiserror::Error; -use xmtp_common::ErrorCode; +use xmtp_common::{ErrorCode, Retryable}; use xmtp_proto::ConversionError; #[derive(Debug, Error, ErrorCode)] @@ -47,7 +47,8 @@ pub enum GrpcBuilderError { Transport(#[from] tonic::transport::Error), } -#[derive(Debug, Error, ErrorCode)] +#[derive(Debug, Error, ErrorCode, Retryable)] +#[retry(default = true)] pub enum GrpcError { /// Invalid URI. /// @@ -105,9 +106,3 @@ impl From for GrpcError { GrpcError::NotFound(error.to_string()) } } - -impl xmtp_common::retry::RetryableError for GrpcError { - fn is_retryable(&self) -> bool { - true - } -} diff --git a/crates/xmtp_db/src/encrypted_store/database/native.rs b/crates/xmtp_db/src/encrypted_store/database/native.rs index 70205b2ba2..d5cb628f21 100644 --- a/crates/xmtp_db/src/encrypted_store/database/native.rs +++ b/crates/xmtp_db/src/encrypted_store/database/native.rs @@ -15,7 +15,7 @@ use diesel::{ use parking_lot::Mutex; use std::sync::Arc; use thiserror::Error; -use xmtp_common::{BoxDynError, ErrorCode, RetryableError, retryable}; +use xmtp_common::{BoxDynError, ErrorCode, Retryable}; use xmtp_configuration::{BUSY_TIMEOUT, MAX_DB_POOL_SIZE, MIN_DB_POOL_SIZE}; use pool::*; @@ -150,23 +150,29 @@ impl StorageOption { } } -#[derive(Debug, Error, ErrorCode)] +#[derive(Debug, Error, ErrorCode, Retryable)] pub enum PlatformStorageError { /// Pool error. /// /// Database connection pool error. Retryable. #[error("Pool error: {0}")] + #[retry(true)] Pool(#[from] diesel::r2d2::PoolError), /// DB connection error. /// /// R2D2 connection manager error (e.g. a failed `on_acquire` while /// establishing a connection). Transient — retryable. + // An r2d2 connection-setup error (e.g. a failed `on_acquire` when + // establishing the single connection) is transient — retryable, in + // line with how the pooled checkout path classifies the same failure. #[error("Error with connection to Sqlite {0}")] + #[retry(true)] DbConnection(#[from] diesel::r2d2::Error), /// Pool needs connection. /// /// Pool must reconnect before use. Retryable. #[error("Pool needs to reconnect before use")] + #[retry(true)] PoolNeedsConnection, /// Pool requires path. /// @@ -177,6 +183,7 @@ pub enum PlatformStorageError { /// /// Encryption key given but SQLCipher not available. Retryable. #[error("The SQLCipher Sqlite extension is not present, but an encryption key is given")] + #[retry(true)] SqlCipherNotLoaded, /// SQLCipher key incorrect. /// @@ -187,11 +194,13 @@ pub enum PlatformStorageError { /// /// Database file is locked by another process. Retryable. #[error("Database is locked")] + #[retry(true)] DatabaseLocked, /// Diesel result error. /// /// Database query error. May be retryable. #[error(transparent)] + #[retry(inherit)] DieselResult(#[from] diesel::result::Error), /// Not found. /// @@ -202,6 +211,7 @@ pub enum PlatformStorageError { /// /// File system I/O error. Retryable. #[error(transparent)] + #[retry(true)] Io(#[from] std::io::Error), /// Hex decode error. /// @@ -212,6 +222,7 @@ pub enum PlatformStorageError { /// /// Failed to establish connection. Retryable. #[error(transparent)] + #[retry(true)] DieselConnect(#[from] diesel::ConnectionError), /// Boxed error. /// @@ -220,27 +231,6 @@ pub enum PlatformStorageError { Boxed(#[from] BoxDynError), } -impl RetryableError for PlatformStorageError { - fn is_retryable(&self) -> bool { - match self { - Self::Pool(_) => true, - // An r2d2 connection-setup error (e.g. a failed `on_acquire` when - // establishing the single connection) is transient — retryable, in - // line with how the pooled checkout path classifies the same failure. - Self::DbConnection(_) => true, - Self::SqlCipherNotLoaded => true, - Self::PoolNeedsConnection => true, - Self::SqlCipherKeyIncorrect => false, - Self::DatabaseLocked => true, - Self::DieselResult(result) => retryable!(result), - Self::Io(_) => true, - Self::DieselConnect(_) => true, - - _ => false, - } - } -} - /// Database used in `native` (everywhere but web) #[derive(Clone, Debug)] pub struct NativeDb { diff --git a/crates/xmtp_db/src/encrypted_store/database/wasm.rs b/crates/xmtp_db/src/encrypted_store/database/wasm.rs index d42e027e77..af4295dab3 100644 --- a/crates/xmtp_db/src/encrypted_store/database/wasm.rs +++ b/crates/xmtp_db/src/encrypted_store/database/wasm.rs @@ -11,9 +11,10 @@ use std::rc::Rc; use std::sync::Arc; use thiserror::Error; use web_sys::wasm_bindgen::JsCast; -use xmtp_common::ErrorCode; +use xmtp_common::{ErrorCode, Retryable}; -#[derive(Debug, Error, ErrorCode)] +#[derive(Debug, Error, ErrorCode, Retryable)] +#[retry(default = true)] pub enum PlatformStorageError { /// OPFS error. /// @@ -32,16 +33,6 @@ pub enum PlatformStorageError { DieselResult(#[from] diesel::result::Error), } -impl xmtp_common::RetryableError for PlatformStorageError { - fn is_retryable(&self) -> bool { - match self { - Self::SAH(_) => true, - Self::Connection(_) => true, - Self::DieselResult(_) => true, - } - } -} - #[derive(Clone)] pub struct WasmDb { conn: Arc>, diff --git a/crates/xmtp_db/src/encrypted_store/group_intent/error.rs b/crates/xmtp_db/src/encrypted_store/group_intent/error.rs index 150c467e15..aad8068eec 100644 --- a/crates/xmtp_db/src/encrypted_store/group_intent/error.rs +++ b/crates/xmtp_db/src/encrypted_store/group_intent/error.rs @@ -1,11 +1,12 @@ use thiserror::Error; -use xmtp_common::{ErrorCode, RetryableError}; +use xmtp_common::{ErrorCode, Retryable}; use xmtp_proto::types::{CursorList, GroupId}; use crate::group_intent::PayloadHash; -#[derive(Debug, Error, ErrorCode)] +#[derive(Debug, Error, ErrorCode, Retryable)] #[error_code(internal)] +#[retry(default = true)] pub enum GroupIntentError { /// More than one dependency. /// @@ -26,12 +27,3 @@ pub enum GroupIntentError { #[error("intent with hash {hash} has no known dependencies")] NoDependencyFound { hash: PayloadHash }, } - -impl RetryableError for GroupIntentError { - fn is_retryable(&self) -> bool { - match self { - Self::MoreThanOneDependency { .. } => true, - Self::NoDependencyFound { .. } => true, - } - } -} diff --git a/crates/xmtp_db/src/encrypted_store/mod.rs b/crates/xmtp_db/src/encrypted_store/mod.rs index d0b7f486e8..0ef29c4dd9 100644 --- a/crates/xmtp_db/src/encrypted_store/mod.rs +++ b/crates/xmtp_db/src/encrypted_store/mod.rs @@ -51,7 +51,7 @@ pub use diesel::{ }; use openmls::storage::OpenMlsProvider; use prost::DecodeError; -use xmtp_common::{ErrorCode, MaybeSend, MaybeSync, RetryableError}; +use xmtp_common::{ErrorCode, MaybeSend, MaybeSync, Retryable}; use xmtp_proto::ConversionError; use zeroize::ZeroizeOnDrop; @@ -136,15 +136,17 @@ impl std::fmt::Display for StorageOption { } } -#[derive(thiserror::Error, Debug, ErrorCode)] +#[derive(thiserror::Error, Debug, ErrorCode, Retryable)] pub enum ConnectionError { /// Database error. /// /// Diesel database query error. May be retryable. #[error(transparent)] + #[retry(inherit)] Database(#[from] diesel::result::Error), #[error(transparent)] #[error_code(inherit)] + #[retry(inherit)] Platform(#[from] PlatformStorageError), /// Decode error. /// @@ -155,11 +157,13 @@ pub enum ConnectionError { /// /// Cannot disconnect while transaction is active. Retryable. #[error("disconnect not possible in transaction")] + #[retry(true)] DisconnectInTransaction, /// Reconnect in transaction. /// /// Cannot reconnect while transaction is active. Retryable. #[error("reconnect not possible in transaction")] + #[retry(true)] ReconnectInTransaction, /// Invalid query. /// @@ -177,20 +181,6 @@ pub enum ConnectionError { InvalidVersion { expected: String, found: String }, } -impl RetryableError for ConnectionError { - fn is_retryable(&self) -> bool { - match self { - Self::Database(d) => d.is_retryable(), - Self::Platform(n) => n.is_retryable(), - Self::DecodeError(_) => false, - Self::DisconnectInTransaction => true, - Self::ReconnectInTransaction => true, - Self::InvalidQuery(_) => false, - Self::InvalidVersion { .. } => false, - } - } -} - impl ConnectionError { /// True when the pool can't currently hand out a connection. Mirrors /// [`StorageError::db_needs_connection`]. diff --git a/crates/xmtp_db/src/errors.rs b/crates/xmtp_db/src/errors.rs index a7c2f9ead2..ee6c776164 100644 --- a/crates/xmtp_db/src/errors.rs +++ b/crates/xmtp_db/src/errors.rs @@ -1,6 +1,6 @@ use diesel::result::DatabaseErrorKind; use thiserror::Error; -use xmtp_common::ErrorCode; +use xmtp_common::{ErrorCode, Retryable}; use crate::group_intent::GroupIntentError; @@ -13,17 +13,19 @@ use xmtp_proto::types::{Cursor, GroupId, InstallationId}; pub struct Mls; -#[derive(Debug, Error, ErrorCode)] +#[derive(Debug, Error, ErrorCode, Retryable)] pub enum StorageError { /// Diesel connection error. /// /// Failed to connect to SQLite. Retryable. #[error(transparent)] + #[retry(true)] DieselConnect(#[from] diesel::ConnectionError), /// Diesel result error. /// /// Database query returned an error. May be retryable. #[error(transparent)] + #[retry(inherit)] DieselResult(#[from] diesel::result::Error), /// Migration error. /// @@ -39,11 +41,13 @@ pub enum StorageError { /// /// Attempted to insert a duplicate record. Not retryable. #[error(transparent)] + #[retry(inherit)] Duplicate(DuplicateItem), /// OpenMLS storage error. /// /// OpenMLS key store operation failed. Not retryable. #[error(transparent)] + #[retry(inherit)] OpenMlsStorage(#[from] SqlKeyStoreError), /// Intentional rollback. /// @@ -69,6 +73,7 @@ pub enum StorageError { /// /// Platform-specific storage error. May be retryable. #[error(transparent)] + #[retry(inherit)] Platform(#[from] crate::database::PlatformStorageError), /// Protobuf decode error. /// @@ -84,6 +89,7 @@ pub enum StorageError { /// /// Database connection error. Retryable. #[error(transparent)] + #[retry(inherit)] Connection(#[from] crate::ConnectionError), /// Invalid HMAC length. /// @@ -94,6 +100,7 @@ pub enum StorageError { /// /// Group intent processing failed. May be retryable. #[error(transparent)] + #[retry(inherit)] GroupIntent(#[from] GroupIntentError), } @@ -127,7 +134,9 @@ impl StorageError { } } -#[derive(Error, Debug, ErrorCode)] +#[derive(Error, Debug, ErrorCode, Retryable)] +// All NotFound errors are transient — the row may appear on a later attempt. +#[retry(default = true)] // Monolithic enum for all things lost pub enum NotFound { /// Group with welcome ID not found. @@ -217,7 +226,7 @@ pub enum NotFound { KeyPackage(Vec), } -#[derive(Error, Debug, ErrorCode)] +#[derive(Error, Debug, ErrorCode, Retryable)] #[error_code(internal)] pub enum DuplicateItem { /// Duplicate welcome ID. @@ -232,16 +241,6 @@ pub enum DuplicateItem { CommitLogPublicKey(Vec), } -impl RetryableError for DuplicateItem { - fn is_retryable(&self) -> bool { - use DuplicateItem::*; - match self { - WelcomeId(_) => false, - CommitLogPublicKey(_) => false, - } - } -} - impl RetryableError for diesel::result::Error { fn is_retryable(&self) -> bool { use DatabaseErrorKind::*; @@ -260,35 +259,6 @@ impl RetryableError for diesel::result::Error { } } -impl RetryableError for StorageError { - fn is_retryable(&self) -> bool { - match self { - Self::DieselConnect(_) => true, - Self::DieselResult(result) => retryable!(result), - Self::Duplicate(d) => retryable!(d), - Self::OpenMlsStorage(storage) => retryable!(storage), - Self::Platform(p) => retryable!(p), - Self::Connection(e) => retryable!(e), - Self::GroupIntent(e) => retryable!(e), - Self::MigrationError(_) - | Self::Conversion(_) - | Self::NotFound(_) - | Self::IntentionalRollback - | Self::DbDeserialize - | Self::DbSerialize - | Self::Builder(_) - | Self::InvalidHmacLength - | Self::Prost(_) => false, - } - } -} - -impl RetryableError for NotFound { - fn is_retryable(&self) -> bool { - true - } -} - // OpenMLS KeyStore errors impl RetryableError for openmls::group::AddMembersError { fn is_retryable(&self) -> bool { diff --git a/crates/xmtp_db/src/sql_key_store.rs b/crates/xmtp_db/src/sql_key_store.rs index 79076d3a97..6a34e39683 100644 --- a/crates/xmtp_db/src/sql_key_store.rs +++ b/crates/xmtp_db/src/sql_key_store.rs @@ -1,4 +1,4 @@ -use xmtp_common::{ErrorCode, RetryableError, retryable}; +use xmtp_common::{ErrorCode, Retryable}; use self::transactions::MutableTransactionConnection; use crate::{ConnectionExt, TransactionalKeyStore, XmtpMlsStorageProvider}; @@ -277,7 +277,7 @@ where /// Errors thrown by the key store. /// General error type for Mls Storage Trait -#[derive(thiserror::Error, Debug, ErrorCode)] +#[derive(thiserror::Error, Debug, ErrorCode, Retryable)] pub enum SqlKeyStoreError { /// Unsupported value type. /// @@ -303,28 +303,16 @@ pub enum SqlKeyStoreError { /// /// Underlying Diesel database error. May be retryable. #[error("database error: {0}")] + #[retry(inherit)] Storage(#[from] diesel::result::Error), /// Connection error. /// /// Database connection error. Retryable. #[error("connection {0}")] + #[retry(inherit)] Connection(#[from] crate::ConnectionError), } -impl RetryableError for SqlKeyStoreError { - fn is_retryable(&self) -> bool { - use SqlKeyStoreError::*; - match self { - Storage(err) => retryable!(err), - SerializationError => false, - UnsupportedMethod => false, - UnsupportedValueTypeBytes => false, - NotFound => false, - Connection(c) => retryable!(c), - } - } -} - const KEY_PACKAGE_LABEL: &[u8] = b"KeyPackage"; const ENCRYPTION_KEY_PAIR_LABEL: &[u8] = b"EncryptionKeyPair"; const SIGNATURE_KEY_PAIR_LABEL: &[u8] = b"SignatureKeyPair"; diff --git a/crates/xmtp_id/src/associations/signature.rs b/crates/xmtp_id/src/associations/signature.rs index 93bc01f357..61682d757f 100644 --- a/crates/xmtp_id/src/associations/signature.rs +++ b/crates/xmtp_id/src/associations/signature.rs @@ -4,7 +4,7 @@ use prost::Message; use sha2::{Digest as _, Sha512}; use std::array::TryFromSliceError; use thiserror::Error; -use xmtp_common::{ErrorCode, RetryableError}; +use xmtp_common::{ErrorCode, Retryable}; use xmtp_cryptography::{ CredentialSign, CredentialVerify, SignerError, SigningContextProvider, XmtpInstallationCredential, @@ -20,7 +20,7 @@ use super::{ use alloy::signers::k256::ecdsa::Signature as K256Signature; -#[derive(Debug, Error, ErrorCode)] +#[derive(Debug, Error, ErrorCode, Retryable)] pub enum SignatureError { /// Malformed legacy key. /// @@ -30,8 +30,13 @@ pub enum SignatureError { #[error(transparent)] #[error_code(inherit)] CryptoSignatureError(#[from] xmtp_cryptography::signature::SignatureError), + // Smart contract wallet verification goes through an RPC provider; + // transient provider/IO failures must surface as retryable so the + // welcome sync path does not permanently advance the cursor past + // welcomes involving SCW users. See xmtp/libxmtp#3394. #[error(transparent)] #[error_code(inherit)] + #[retry(inherit)] VerifierError(#[from] crate::scw_verifier::VerifierError), /// Ed25519 signature failed. /// @@ -91,19 +96,6 @@ pub enum SignatureError { Signature(#[from] alloy::primitives::SignatureError), } -impl RetryableError for SignatureError { - fn is_retryable(&self) -> bool { - match self { - // Smart contract wallet verification goes through an RPC provider; - // transient provider/IO failures must surface as retryable so the - // welcome sync path does not permanently advance the cursor past - // welcomes involving SCW users. See xmtp/libxmtp#3394. - SignatureError::VerifierError(e) => e.is_retryable(), - _ => false, - } - } -} - /// Xmtp Installation Credential for Specialized for XMTP Identity pub struct InboxIdInstallationCredential; diff --git a/crates/xmtp_id/src/scw_verifier/mod.rs b/crates/xmtp_id/src/scw_verifier/mod.rs index 5eb1f3d21f..91f1535c5f 100644 --- a/crates/xmtp_id/src/scw_verifier/mod.rs +++ b/crates/xmtp_id/src/scw_verifier/mod.rs @@ -11,11 +11,11 @@ use std::{collections::HashMap, fs, path::Path, sync::Arc}; use thiserror::Error; use tracing::info; use url::Url; -use xmtp_common::{ErrorCode, MaybeSend, MaybeSync, RetryableError}; +use xmtp_common::{ErrorCode, MaybeSend, MaybeSync, Retryable, RetryableError}; static DEFAULT_CHAIN_URLS: &str = include_str!("chain_urls_default.json"); -#[derive(Debug, Error, ErrorCode)] +#[derive(Debug, Error, ErrorCode, Retryable)] pub enum VerifierError { /// Unexpected ERC-6492 result. /// @@ -29,6 +29,7 @@ pub enum VerifierError { /// /// Ethereum RPC provider error. Retryable. #[error(transparent)] + #[retry(true)] Provider(#[from] alloy::transports::RpcError), /// URL parse error. /// @@ -39,6 +40,7 @@ pub enum VerifierError { /// /// I/O operation failed. May be retryable. #[error(transparent)] + #[retry(true)] Io(#[from] std::io::Error), /// Serialization error. /// @@ -54,6 +56,7 @@ pub enum VerifierError { /// /// Verifier not configured for the given chain ID. Retryable. #[error("verifier not present for chain ID {0}")] + #[retry(true)] NoVerifier(String), /// Invalid hash. /// @@ -64,22 +67,10 @@ pub enum VerifierError { /// /// Unclassified verifier error. May be retryable. #[error("{0}")] + #[retry(inherit)] Other(Box), } -impl RetryableError for VerifierError { - fn is_retryable(&self) -> bool { - use VerifierError::*; - match self { - Io(_) => true, - NoVerifier(_) => true, - Provider(_) => true, - Other(o) => o.is_retryable(), - _ => false, - } - } -} - #[xmtp_common::async_trait] pub trait SmartContractSignatureVerifier: MaybeSend + MaybeSync { /// Verifies an ERC-6492 signature. diff --git a/crates/xmtp_mls/src/client.rs b/crates/xmtp_mls/src/client.rs index 9af631c3f9..9a8dab6991 100644 --- a/crates/xmtp_mls/src/client.rs +++ b/crates/xmtp_mls/src/client.rs @@ -29,7 +29,7 @@ use std::{collections::HashMap, sync::Arc}; use thiserror::Error; use tokio::sync::broadcast; use xmtp_api::{ApiClientWrapper, XmtpApi}; -use xmtp_common::{ErrorCode, Event, Retry, retry_async, retryable}; +use xmtp_common::{ErrorCode, Event, Retry, Retryable, retry_async}; use xmtp_configuration::{CREATE_PQ_KEY_PACKAGE_EXTENSION, KEY_PACKAGE_ROTATION_INTERVAL_NS}; use xmtp_cryptography::signature::IdentifierValidationError; use xmtp_db::{ @@ -75,7 +75,7 @@ pub enum Network { Prod, } -#[derive(Debug, Error, ErrorCode)] +#[derive(Debug, Error, ErrorCode, Retryable)] pub enum ClientError { #[error(transparent)] #[error_code(inherit)] @@ -89,11 +89,13 @@ pub enum ClientError { /// /// Database operation failed. May be retryable. #[error("storage error: {0}")] + #[retry(inherit)] Storage(#[from] StorageError), /// API error. /// /// Network request to XMTP backend failed. Retryable. #[error("API error: {0}")] + #[retry(inherit)] Api(#[from] xmtp_api::ApiError), /// Identity error. /// @@ -122,8 +124,11 @@ pub enum ClientError { Association(#[from] AssociationError), /// Signature validation error. /// - /// A signature failed verification. Not retryable. + /// A signature failed verification. SCW verification errors carry + /// retryability through SignatureError; transient RPC provider failures + /// must not advance the welcome cursor. See xmtp/libxmtp#3394. #[error("signature validation error: {0}")] + #[retry(inherit)] SignatureValidation(#[from] SignatureError), /// Identity update error. /// @@ -140,6 +145,7 @@ pub enum ClientError { /// Group operation failed. May be retryable. // the box is to prevent infinite cycle between client and group errors #[error(transparent)] + #[retry(inherit)] Group(Box), /// Local event error. /// @@ -150,11 +156,13 @@ pub enum ClientError { /// /// Connection to database failed. Retryable. #[error(transparent)] + #[retry(inherit)] Db(#[from] xmtp_db::ConnectionError), /// Generic error. /// /// Unclassified error. May be retryable. #[error("generic:{0}")] + #[retry(when = this.contains("database is locked"))] Generic(String), /// MLS store error. /// @@ -180,6 +188,7 @@ pub enum ClientError { /// /// Registration envelopes haven't propagated to the node yet. Retryable. #[error("Envelopes not yet visible on node {node_id}")] + #[retry(true)] EnvelopesNotYetVisible { node_id: u32 }, /// Client is closed. /// @@ -211,24 +220,6 @@ impl From for ClientError { } } -impl xmtp_common::RetryableError for ClientError { - fn is_retryable(&self) -> bool { - match self { - ClientError::Group(group_error) => retryable!(group_error), - ClientError::Api(api_error) => retryable!(api_error), - ClientError::Storage(storage_error) => retryable!(storage_error), - ClientError::Db(db) => retryable!(db), - // SCW verification errors carry retryability through SignatureError; - // transient RPC provider failures must not advance the welcome cursor. - // See xmtp/libxmtp#3394. - ClientError::SignatureValidation(e) => retryable!(e), - ClientError::Generic(err) => err.contains("database is locked"), - ClientError::EnvelopesNotYetVisible { .. } => true, - _ => false, - } - } -} - impl From for ClientError { fn from(value: String) -> Self { Self::Generic(value) diff --git a/crates/xmtp_mls/src/groups/app_data/migration.rs b/crates/xmtp_mls/src/groups/app_data/migration.rs index deda78d55d..0ff37db703 100644 --- a/crates/xmtp_mls/src/groups/app_data/migration.rs +++ b/crates/xmtp_mls/src/groups/app_data/migration.rs @@ -44,7 +44,7 @@ use xmtp_proto::xmtp::mls::message_contents::{ use crate::{context::XmtpSharedContext, identity_updates::IdentityUpdates}; /// Sender-side bootstrap synthesis errors. -#[derive(Debug, thiserror::Error)] +#[derive(Debug, thiserror::Error, xmtp_common::Retryable)] pub enum BootstrapSynthesisError { #[error(transparent)] Common(#[from] CommonMigrationError), @@ -52,7 +52,15 @@ pub enum BootstrapSynthesisError { /// Bootstrap can't fall back silently: a missing installation list /// means we can't partition `failed_installations`, so we surface /// the lookup error rather than emit incorrect bytes. + // Most synthesis failures are deterministic over the inputs and + // not retryable (decode errors, registry-shape mismatches, common + // migration validation). The exception is `IdentityUpdateLookup`, + // which wraps a [`crate::client::ClientError`] that can carry a + // transient API failure (network blip, server 5xx). Delegate + // retryability to the wrapped client error so a momentary blip + // during bootstrap doesn't permanently fail the intent. #[error("identity-update lookup failed for inbox {inbox_id}: {source}")] + #[retry(when = source.is_retryable())] IdentityUpdateLookup { inbox_id: String, #[source] @@ -94,26 +102,6 @@ pub enum BootstrapSynthesisError { TlsCodec(#[from] tls_codec::Error), } -impl xmtp_common::retry::RetryableError for BootstrapSynthesisError { - /// Most synthesis failures are deterministic over the inputs and - /// not retryable (decode errors, registry-shape mismatches, common - /// migration validation). The exception is `IdentityUpdateLookup`, - /// which wraps a [`crate::client::ClientError`] that can carry a - /// transient API failure (network blip, server 5xx). Delegate - /// retryability to the wrapped client error so a momentary blip - /// during bootstrap doesn't permanently fail the intent. - fn is_retryable(&self) -> bool { - match self { - Self::IdentityUpdateLookup { source, .. } => source.is_retryable(), - Self::Common(_) - | Self::LegacyMembershipDecode(_) - | Self::RegistryReEncode(_) - | Self::SequenceIdOverflow { .. } - | Self::TlsCodec(_) => false, - } - } -} - /// Synthesize the full `AppDataUpdate` payload set the bootstrap /// commit ships, keyed by `ComponentId`. /// diff --git a/crates/xmtp_mls/src/groups/app_data/mod.rs b/crates/xmtp_mls/src/groups/app_data/mod.rs index f9b8e3dd4b..159c64d7c7 100644 --- a/crates/xmtp_mls/src/groups/app_data/mod.rs +++ b/crates/xmtp_mls/src/groups/app_data/mod.rs @@ -35,6 +35,7 @@ use openmls::{ prelude::CommitMessageBundle, storage::OpenMlsProvider, }; +use xmtp_common::{Retryable, RetryableError}; use xmtp_mls_common::app_data::{component_id::ComponentId, component_registry::ComponentRegistry}; use self::component_source::{ @@ -58,21 +59,27 @@ tokio::task_local! { /// payload). Splitting them keeps "the message was bad in OpenMLS terms" /// distinct from "we couldn't decode an AppData payload" so callers can /// log / retry / surface them differently. -#[derive(Debug, thiserror::Error)] -pub enum ProcessMessageWithAppDataError { +#[derive(Debug, thiserror::Error, Retryable)] +pub enum ProcessMessageWithAppDataError +where + StorageError: std::error::Error, + ProcessMessageError: RetryableError, +{ /// Standard OpenMLS processing failure (decryption, validation, …). #[error(transparent)] + #[retry(inherit)] OpenMls(#[from] ProcessMessageError), /// Failed to decode an incoming `AppDataUpdate` payload via /// [`apply_app_data_update_payload`]. Almost always indicates a /// malformed proposal from a peer (or a wire-format mismatch with a /// future version we don't understand yet). /// - /// **Not retryable.** Decode failures are deterministic over the - /// exact bytes on the wire, so retrying the same message will fail - /// the same way. `GroupMessageProcessingError::is_retryable` and - /// `commit_result` treat this as a terminal wire-format violation - /// (mapped to `CommitResult::Invalid`). + /// **Not retryable.** Decode failures are wire-format violations from + /// the peer — deterministic over the exact bytes on the wire, so + /// retrying the same message will fail the same way. + /// `GroupMessageProcessingError::is_retryable` and `commit_result` + /// treat this as a terminal wire-format violation (mapped to + /// `CommitResult::Invalid`). #[error("failed to decode incoming AppDataUpdate payload: {0}")] AppDataDecode(#[from] ComponentSourceError), } @@ -194,7 +201,10 @@ pub(crate) fn process_message_with_app_data( mls_group: &mut OpenMlsGroup, provider: &Provider, message: impl Into, -) -> Result> { +) -> Result> +where + ProcessMessageError: RetryableError, +{ let unverified = mls_group.unprotect_message(provider, message)?; let app_data_updates: Option = match unverified.committed_proposals() { diff --git a/crates/xmtp_mls/src/groups/commit_log.rs b/crates/xmtp_mls/src/groups/commit_log.rs index 055b0ce59e..15e4b6d08a 100644 --- a/crates/xmtp_mls/src/groups/commit_log.rs +++ b/crates/xmtp_mls/src/groups/commit_log.rs @@ -11,8 +11,8 @@ use std::collections::HashSet; use std::{collections::HashMap, time::Duration}; use thiserror::Error; use xmtp_api::ApiError; -use xmtp_common::RetryableError; use xmtp_common::hex::NormalizeHex; +use xmtp_common::{Retryable, RetryableError}; use xmtp_configuration::MAX_PAGE_SIZE; use xmtp_configuration::Originators; use xmtp_db::consent_record::ConsentState; @@ -77,21 +77,27 @@ where } } -#[derive(Debug, Error)] +#[derive(Debug, Error, Retryable)] pub enum CommitLogError { #[error("generic storage error: {0}")] + #[retry(inherit)] Storage(#[from] StorageError), #[error("diesel error: {0}")] + #[retry(inherit)] Diesel(#[from] xmtp_db::diesel::result::Error), #[error("generic api error: {0}")] + #[retry(inherit)] Api(#[from] ApiError), #[error("connection error: {0}")] + #[retry(inherit)] Connection(#[from] xmtp_db::ConnectionError), #[error("prost decode error: {0}")] Prost(#[from] prost::DecodeError), #[error("keystore error: {0}")] + #[retry(inherit)] KeystoreError(#[from] xmtp_db::sql_key_store::SqlKeyStoreError), #[error("group error: {0}")] + #[retry(inherit)] GroupError(#[from] GroupError), #[error("crypto error: {0}")] CryptoError(#[from] openmls_traits::types::CryptoError), @@ -102,40 +108,21 @@ pub enum CommitLogError { #[error("Group did not pass readd validation: {0}")] GroupReaddValidationError(String), #[error("sync error: {0}")] + #[retry(inherit)] SyncError(#[from] SyncSummary), #[error("failed to send readd request for group {group_id}: {source}")] + #[retry(when = source.is_retryable())] FailedToSendReadd { group_id: GroupId, source: Box, }, #[error("{count} readd request(s) failed: {errors:?}", count = errors.len())] + #[retry(when = errors.iter().any(|e| e.is_retryable()))] FailedReadds { errors: Vec }, #[error("no latest commit sequence id found for forked group {group_id}")] MissingLatestCommitSequenceId { group_id: GroupId }, } -impl RetryableError for CommitLogError { - fn is_retryable(&self) -> bool { - match self { - Self::Storage(storage_error) => storage_error.is_retryable(), - Self::Diesel(diesel_error) => diesel_error.is_retryable(), - Self::Api(api_error) => api_error.is_retryable(), - Self::Connection(connection_error) => connection_error.is_retryable(), - Self::Prost(_prost_error) => false, - Self::KeystoreError(keystore_error) => keystore_error.is_retryable(), - Self::GroupError(group_error) => group_error.is_retryable(), - Self::CryptoError(_crypto_error) => false, - Self::TryFromSliceError(_try_from_slice_error) => false, - Self::Conversion(_) => false, - Self::GroupReaddValidationError(_group_readd_validation_error) => false, - Self::SyncError(sync_error) => sync_error.is_retryable(), - Self::FailedToSendReadd { source, .. } => source.is_retryable(), - Self::FailedReadds { errors } => errors.iter().any(|e| e.is_retryable()), - Self::MissingLatestCommitSequenceId { .. } => false, - } - } -} - impl NeedsDbReconnect for CommitLogError { fn needs_db_reconnect(&self) -> bool { match self { diff --git a/crates/xmtp_mls/src/groups/error.rs b/crates/xmtp_mls/src/groups/error.rs index 371a08c5c3..b5c932cae5 100644 --- a/crates/xmtp_mls/src/groups/error.rs +++ b/crates/xmtp_mls/src/groups/error.rs @@ -20,6 +20,7 @@ use openmls::{ use std::collections::HashSet; use thiserror::Error; use xmtp_common::ErrorCode; +use xmtp_common::Retryable; use xmtp_common::retry::RetryableError; use xmtp_content_types::CodecError; use xmtp_cryptography::signature::IdentifierValidationError; @@ -78,7 +79,7 @@ impl std::fmt::Display for ReceiveErrors { Ok(()) } } -#[derive(Debug, Error, ErrorCode)] +#[derive(Debug, Error, ErrorCode, Retryable)] pub enum GroupError { #[error(transparent)] #[error_code(inherit)] @@ -102,6 +103,7 @@ pub enum GroupError { /// /// Network request failed. Retryable. #[error("api error: {0}")] + #[retry(inherit)] WrappedApi(#[from] xmtp_api::ApiError), /// Invalid group membership. /// @@ -112,11 +114,13 @@ pub enum GroupError { /// /// Group leave validation failed. Not retryable. #[error(transparent)] + #[retry(inherit)] LeaveCantProcessed(#[from] GroupLeaveValidationError), /// Storage error. /// /// Database operation failed. May be retryable. #[error("storage error: {0}")] + #[retry(inherit)] Storage(#[from] xmtp_db::StorageError), /// Intent error. /// @@ -137,6 +141,7 @@ pub enum GroupError { /// /// Failed to update group membership. May be retryable. #[error("add members: {0}")] + #[retry(inherit)] UpdateGroupMembership( #[from] openmls::prelude::UpdateGroupMembershipError, ), @@ -144,16 +149,19 @@ pub enum GroupError { /// /// MLS group creation failed. May be retryable. #[error("group create: {0}")] + #[retry(inherit)] GroupCreate(#[from] openmls::group::NewGroupError), /// Self update error. /// /// MLS self-update operation failed. May be retryable. #[error("self update: {0}")] + #[retry(inherit)] SelfUpdate(#[from] openmls::group::SelfUpdateError), /// Welcome error. /// /// Processing MLS welcome message failed. May be retryable. #[error("welcome error: {0}")] + #[retry(inherit)] WelcomeError(#[from] openmls::prelude::WelcomeError), /// Invalid extension. /// @@ -169,16 +177,19 @@ pub enum GroupError { /// /// Client operation failed within group. May be retryable. #[error("client: {0}")] + #[retry(inherit)] Client(#[from] ClientError), /// Receive error. /// /// Processing received group message failed. May be retryable. #[error("receive error: {0}")] + #[retry(inherit)] ReceiveError(#[from] GroupMessageProcessingError), /// Receive errors. /// /// Multiple message processing failures. May be retryable. #[error("Receive errors: {0}")] + #[retry(inherit)] ReceiveErrors(ReceiveErrors), /// Address validation error. /// @@ -189,6 +200,7 @@ pub enum GroupError { /// /// Failed to process local event. Not retryable. #[error(transparent)] + #[retry(inherit)] LocalEvent(#[from] LocalEventError), /// Invalid public keys. /// @@ -199,11 +211,13 @@ pub enum GroupError { /// /// MLS commit validation failed. May be retryable. #[error("Commit validation error {0}")] + #[retry(inherit)] CommitValidation(#[from] CommitValidationError), /// Identity error. /// /// Identity operation failed. Not retryable. #[error("identity error: {0}")] + #[retry(inherit)] Identity(#[from] IdentityError), /// Conversion error. /// @@ -219,6 +233,7 @@ pub enum GroupError { /// /// Failed to create group context extension proposal. May be retryable. #[error("create group context proposal error: {0}")] + #[retry(inherit)] CreateGroupContextExtProposalError( #[from] CreateGroupContextExtProposalError, ), @@ -226,21 +241,25 @@ pub enum GroupError { /// /// Failed to create an add-member proposal. May be retryable. #[error("propose add member error: {0}")] + #[retry(inherit)] ProposeAddMember(#[from] ProposeAddMemberError), /// Propose remove member error. /// /// Failed to create a remove-member proposal. May be retryable. #[error("propose remove member error: {0}")] + #[retry(inherit)] ProposeRemoveMember(#[from] ProposeRemoveMemberError), /// Proposal error. /// /// Generic MLS proposal creation/handling failure. May be retryable. #[error("proposal error: {0}")] + #[retry(inherit)] Proposal(#[from] ProposalError), /// Commit to pending proposals error. /// /// Failed to commit pending proposals into an MLS commit. May be retryable. #[error("commit to pending proposals error: {0}")] + #[retry(inherit)] CommitToPendingProposals( #[from] CommitToPendingProposalsError, ), @@ -248,6 +267,7 @@ pub enum GroupError { /// /// Failed to merge a pending commit into local state. May be retryable. #[error("merge pending commit error: {0}")] + #[retry(inherit)] MergePendingCommit( #[from] openmls::group::MergePendingCommitError, ), @@ -288,6 +308,7 @@ pub enum GroupError { /// [`stage_app_data_propose_and_commit`] so the underlying OpenMLS create/stage /// failure is preserved instead of being string-flattened. #[error("app data commit error: {0}")] + #[retry(inherit)] AppDataCommit(#[from] super::app_data::GroupAppDataError), /// Bootstrap synthesis failure — sender-side couldn't build the /// complete set of initial component values for the migration @@ -298,6 +319,11 @@ pub enum GroupError { /// identity-update API error is itself retryable. Decode/registry-shape /// failures are deterministic and not retryable. #[error("bootstrap synthesis error: {0}")] + // Bootstrap synthesis can fail on a transient identity-update + // API blip — delegate to the inner error so we retry on + // network errors and stay non-retryable on deterministic + // wire-format / registry-shape failures. + #[retry(inherit)] BootstrapSynthesis(#[from] super::app_data::migration::BootstrapSynthesisError), /// Bootstrap commit-build failure. /// @@ -322,6 +348,7 @@ pub enum GroupError { /// /// Installation diff computation failed. May be retryable. #[error("Installation diff error: {0}")] + #[retry(inherit)] InstallationDiff(#[from] InstallationDiffError), /// No PSK support. /// @@ -332,11 +359,13 @@ pub enum GroupError { /// /// OpenMLS key store operation failed. May be retryable. #[error("sql key store error: {0}")] + #[retry(inherit)] SqlKeyStore(#[from] sql_key_store::SqlKeyStoreError), /// Sync failed to wait. /// /// Waiting for intent sync failed. Retryable. #[error("Sync failed to wait for intent: {}", _0)] + #[retry(true)] SyncFailedToWait(Box), /// Missing pending commit. /// @@ -347,11 +376,13 @@ pub enum GroupError { /// /// Failed to process group intent. May be retryable. #[error(transparent)] + #[retry(inherit)] ProcessIntent(#[from] ProcessIntentError), /// Failed to load lock. /// /// Concurrency lock acquisition failed. Retryable. #[error("Failed to load lock")] + #[retry(true)] LockUnavailable, /// Exceeded max characters. /// @@ -372,21 +403,25 @@ pub enum GroupError { /// /// Sync operation completed with errors. May be retryable. #[error("{}", _0.to_string())] + #[retry(inherit)] Sync(#[from] Box), /// Database connection error. /// /// Database connection failed. Retryable. #[error(transparent)] + #[retry(inherit)] Db(#[from] xmtp_db::ConnectionError), /// MLS store error. /// /// OpenMLS key store failed. Not retryable. #[error(transparent)] + #[retry(inherit)] MlsStore(#[from] MlsStoreError), /// Metadata permissions error. /// /// Metadata permission check failed. Not retryable. #[error(transparent)] + #[retry(inherit)] MetadataPermissionsError(#[from] MetadataPermissionsError), /// Failed to verify installations. /// @@ -402,16 +437,19 @@ pub enum GroupError { /// /// Content type codec failed. Retryable. #[error("Codec error: {0}")] + #[retry(true)] CodecError(#[from] CodecError), /// Wrap welcome error. /// /// Failed to wrap welcome message. Not retryable. #[error(transparent)] + #[retry(inherit)] WrapWelcome(#[from] WrapPayloadError), /// Unwrap welcome error. /// /// Failed to unwrap welcome message. Not retryable. #[error(transparent)] + #[retry(inherit)] UnwrapWelcome(#[from] UnwrapPayloadError), /// Welcome data not found. /// @@ -427,6 +465,7 @@ pub enum GroupError { /// /// Raw database query failed. May be retryable. #[error(transparent)] + #[retry(inherit)] Diesel(#[from] xmtp_db::diesel::result::Error), /// Uninitialized field. /// @@ -437,15 +476,17 @@ pub enum GroupError { /// /// Failed to delete message. Not retryable. #[error(transparent)] + #[retry(inherit)] DeleteMessage(#[from] DeleteMessageError), /// Device sync error. /// /// Device sync operation failed. May be retryable. #[error(transparent)] + #[retry(inherit)] DeviceSync(#[from] Box), } -#[derive(Error, Debug)] +#[derive(Error, Debug, Retryable)] pub enum DeleteMessageError { #[error("Message not found: {0}")] MessageNotFound(String), @@ -457,12 +498,6 @@ pub enum DeleteMessageError { MessageAlreadyDeleted, } -impl RetryableError for DeleteMessageError { - fn is_retryable(&self) -> bool { - false - } -} - impl From for GroupError { fn from(value: prost::EncodeError) -> Self { GroupError::ConversionError(value.into()) @@ -481,7 +516,7 @@ impl From for GroupError { } } -#[derive(Error, Debug)] +#[derive(Error, Debug, Retryable)] pub enum MetadataPermissionsError { #[error(transparent)] Permissions(#[from] GroupMutablePermissionsError), @@ -506,13 +541,7 @@ pub enum MetadataPermissionsError { ComponentSource(#[from] crate::groups::app_data::component_source::ComponentSourceError), } -impl RetryableError for MetadataPermissionsError { - fn is_retryable(&self) -> bool { - false - } -} - -#[derive(Error, Debug)] +#[derive(Error, Debug, Retryable)] pub enum GroupLeaveValidationError { #[error("cannot leave a DM conversation")] DmLeaveForbidden, @@ -528,13 +557,7 @@ pub enum GroupLeaveValidationError { NotAGroupMember, } -impl RetryableError for GroupLeaveValidationError { - fn is_retryable(&self) -> bool { - false - } -} - -#[derive(Error, Debug)] +#[derive(Error, Debug, Retryable)] pub enum DmValidationError { #[error("DM group must have DmMembers set")] OurInboxMustBeMember, @@ -550,98 +573,6 @@ pub enum DmValidationError { InvalidPermissions, } -impl RetryableError for DmValidationError { - fn is_retryable(&self) -> bool { - match self { - Self::OurInboxMustBeMember - | Self::MustHaveMembersSet - | Self::InvalidConversationType - | Self::ExpectedInboxesDoNotMatch - | Self::MustHaveEmptyAdminAndSuperAdmin - | Self::InvalidPermissions => false, - } - } -} - -impl RetryableError for GroupError { - fn is_retryable(&self) -> bool { - match self { - Self::ReceiveErrors(errors) => errors.is_retryable(), - Self::Client(client_error) => client_error.is_retryable(), - Self::Storage(storage) => storage.is_retryable(), - Self::ReceiveError(msg) => msg.is_retryable(), - Self::Identity(identity) => identity.is_retryable(), - Self::UpdateGroupMembership(update) => update.is_retryable(), - Self::GroupCreate(group) => group.is_retryable(), - Self::SelfUpdate(update) => update.is_retryable(), - Self::WelcomeError(welcome) => welcome.is_retryable(), - Self::SqlKeyStore(sql) => sql.is_retryable(), - Self::InstallationDiff(diff) => diff.is_retryable(), - Self::CreateGroupContextExtProposalError(create) => create.is_retryable(), - Self::ProposeAddMember(e) => e.is_retryable(), - Self::ProposeRemoveMember(e) => e.is_retryable(), - Self::Proposal(e) => e.is_retryable(), - Self::CommitToPendingProposals(e) => e.is_retryable(), - Self::ProposalsNotSupported(_) => false, - Self::MinVersionExceedsOwnVersion { .. } => false, - Self::MinVersionDowngrade { .. } => false, - Self::InvalidMinVersion { .. } => false, - Self::ComponentSource(_) => false, - Self::AppDataCommit(e) => e.is_retryable(), - // Bootstrap synthesis can fail on a transient identity-update - // API blip — delegate to the inner error so we retry on - // network errors and stay non-retryable on deterministic - // wire-format / registry-shape failures. - Self::BootstrapSynthesis(e) => e.is_retryable(), - Self::BootstrapCommit(_) => false, - Self::CommitValidation(err) => err.is_retryable(), - Self::WrappedApi(err) => err.is_retryable(), - Self::ProcessIntent(err) => err.is_retryable(), - Self::LocalEvent(err) => err.is_retryable(), - Self::LockUnavailable => true, - Self::SyncFailedToWait(_) => true, - Self::CodecError(_) => true, - Self::Sync(s) => s.is_retryable(), - Self::Db(e) => e.is_retryable(), - Self::MlsStore(e) => e.is_retryable(), - Self::MetadataPermissionsError(e) => e.is_retryable(), - Self::WrapWelcome(e) => e.is_retryable(), - Self::UnwrapWelcome(e) => e.is_retryable(), - Self::Diesel(e) => e.is_retryable(), - Self::LeaveCantProcessed(e) => e.is_retryable(), - Self::DeleteMessage(e) => e.is_retryable(), - Self::DeviceSync(e) => e.is_retryable(), - Self::MergePendingCommit(e) => e.is_retryable(), - Self::NotFound(_) - | Self::UserLimitExceeded - | Self::InvalidGroupMembership - | Self::Intent(_) - | Self::CreateMessage(_) - | Self::TlsError(_) - | Self::MissingSequenceId - | Self::AddressNotFound(_) - | Self::InvalidExtension(_) - | Self::Signature(_) - | Self::LeafNodeError(_) - | Self::NoPSKSupport - | Self::MissingPendingCommit - | Self::AddressValidation(_) - | Self::InvalidPublicKeys(_) - | Self::CredentialError(_) - | Self::ConversionError(_) - | Self::CryptoError(_) - | Self::TooManyCharacters { .. } - | Self::GroupPausedUntilUpdate(_) - | Self::GroupInactive - | Self::FailedToVerifyInstallations - | Self::NoWelcomesToSend - | Self::WelcomeDataNotFound(_) - | Self::UninitializedField(_) - | Self::UninitializedResult => false, - } - } -} - impl crate::worker::NeedsDbReconnect for GroupError { /// Forwards a dropped-pool signal from storage-bearing variants so a worker /// catching `GroupError`s per item can stop on disconnect; else `false`. diff --git a/crates/xmtp_mls/src/groups/mls_sync.rs b/crates/xmtp_mls/src/groups/mls_sync.rs index 108cd8f1f5..9eff6e54bd 100644 --- a/crates/xmtp_mls/src/groups/mls_sync.rs +++ b/crates/xmtp_mls/src/groups/mls_sync.rs @@ -76,7 +76,7 @@ use thiserror::Error; use tracing::debug; use update_group_membership::apply_update_group_membership_intent; use xmtp_common::{ - Event, ExponentialBackoff, Retry, RetryableError, Strategy, log_event, retry_async, + Event, ExponentialBackoff, Retry, Retryable, RetryableError, Strategy, log_event, retry_async, time::now_ns, }; use xmtp_configuration::{ @@ -135,7 +135,7 @@ use zeroize::Zeroizing; pub mod update_group_membership; -#[derive(Debug, Error)] +#[derive(Debug, Error, Retryable)] pub enum GroupMessageProcessingError { #[error("intent already processed")] IntentAlreadyProcessed, @@ -154,10 +154,13 @@ pub enum GroupMessageProcessingError { #[error("invalid payload")] InvalidPayload, #[error("storage error: {0}")] + #[retry(inherit)] Storage(#[from] xmtp_db::StorageError), #[error(transparent)] + #[retry(inherit)] Identity(#[from] IdentityError), #[error("openmls process message error: {0}")] + #[retry(inherit)] OpenMlsProcessMessage( #[from] openmls::prelude::ProcessMessageError, ), @@ -169,20 +172,24 @@ pub enum GroupMessageProcessingError { /// `OpenMlsProcessMessage` so the AppData-decode failure mode is /// greppable in logs. #[error("app-data process message error: {0}")] + #[retry(inherit)] OpenMlsProcessMessageWithAppData( #[from] super::app_data::ProcessMessageWithAppDataError, ), #[error("merge staged commit: {0}")] + #[retry(inherit)] MergeStagedCommit(#[from] openmls::group::MergeCommitError), #[error("TLS Codec error: {0}")] TlsError(#[from] TlsCodecError), #[error("unsupported message type: {0:?}")] UnsupportedMessageType(Discriminant), #[error("commit validation")] + #[retry(inherit)] CommitValidation(#[from] CommitValidationError), #[error("epoch increment not allowed")] EpochIncrementNotAllowed, #[error("clear pending commit error: {0}")] + #[retry(inherit)] ClearPendingCommit(#[from] sql_key_store::SqlKeyStoreError), #[error("Serialization/Deserialization Error {0}")] Serde(#[from] serde_json::Error), @@ -199,10 +206,12 @@ pub enum GroupMessageProcessingError { #[error("wrong credential type")] WrongCredentialType(#[from] BasicCredentialError), #[error(transparent)] + #[retry(inherit)] ProcessIntent(#[from] ProcessIntentError), #[error(transparent)] AssociationDeserialization(#[from] xmtp_id::associations::DeserializationError), #[error(transparent)] + #[retry(inherit)] Client(#[from] ClientError), #[error("Group paused due to minimum protocol version requirement")] GroupPaused, @@ -211,12 +220,15 @@ pub enum GroupMessageProcessingError { #[error("Message epoch [{0}] is greater than group epoch [{1}]")] FutureEpoch(u64, u64), #[error(transparent)] + #[retry(inherit)] Db(#[from] xmtp_db::ConnectionError), #[error(transparent)] Builder(#[from] derive_builder::UninitializedFieldError), #[error(transparent)] + #[retry(inherit)] Diesel(#[from] xmtp_db::diesel::result::Error), #[error(transparent)] + #[retry(inherit)] EnrichMessage(#[from] EnrichMessageError), #[error("pre-commit proposal phase complete, re-queuing intent")] PreCommitProposalPhaseComplete, @@ -224,53 +236,6 @@ pub enum GroupMessageProcessingError { Conversion(#[from] xmtp_proto::ConversionError), } -impl RetryableError for GroupMessageProcessingError { - fn is_retryable(&self) -> bool { - match self { - Self::Storage(err) => err.is_retryable(), - Self::Diesel(err) => err.is_retryable(), - Self::Identity(err) => err.is_retryable(), - Self::OpenMlsProcessMessage(err) => err.is_retryable(), - Self::OpenMlsProcessMessageWithAppData(err) => match err { - super::app_data::ProcessMessageWithAppDataError::OpenMls(e) => e.is_retryable(), - // Decode failures are wire-format violations from the - // peer — retrying won't help. - super::app_data::ProcessMessageWithAppDataError::AppDataDecode(_) => false, - }, - Self::MergeStagedCommit(err) => err.is_retryable(), - Self::ProcessIntent(err) => err.is_retryable(), - Self::CommitValidation(err) => err.is_retryable(), - Self::ClearPendingCommit(err) => err.is_retryable(), - Self::Client(err) => err.is_retryable(), - Self::Db(e) => e.is_retryable(), - Self::EnrichMessage(e) => e.is_retryable(), - Self::IntentAlreadyProcessed - | Self::MessageIdentifierNotFound - | Self::WrongCredentialType(_) - | Self::Codec(_) - | Self::MessageAlreadyProcessed(_) - | Self::WelcomeAlreadyProcessed(_) - | Self::InvalidSender { .. } - | Self::DecodeProto(_) - | Self::InvalidPayload - | Self::Intent(_) - | Self::EpochIncrementNotAllowed - | Self::EncodeProto(_) - | Self::IntentMissingStagedCommit - | Self::Serde(_) - | Self::AssociationDeserialization(_) - | Self::TlsError(_) - | Self::UnsupportedMessageType(_) - | Self::GroupPaused - | Self::FutureEpoch(_, _) - | Self::OldEpoch(_, _) - | Self::PreCommitProposalPhaseComplete => false, - Self::Builder(_) => false, - Self::Conversion(_) => false, - } - } -} - impl GroupMessageProcessingError { pub(crate) fn commit_result(&self) -> CommitResult { use super::app_data::ProcessMessageWithAppDataError; @@ -304,7 +269,8 @@ impl GroupMessageProcessingError { } } -#[derive(Debug, Error)] +#[derive(Debug, Error, Retryable)] +#[retry(when = self.processing_error.is_retryable())] pub struct IntentResolutionError { processing_error: GroupMessageProcessingError, // The next intent state to transition to, if the error is non-retriable. @@ -318,12 +284,6 @@ impl std::fmt::Display for IntentResolutionError { } } -impl RetryableError for IntentResolutionError { - fn is_retryable(&self) -> bool { - self.processing_error.is_retryable() - } -} - #[derive(Debug)] pub(crate) struct PublishIntentData { pub(crate) staged_commit: Option>, diff --git a/crates/xmtp_mls/src/groups/validated_commit.rs b/crates/xmtp_mls/src/groups/validated_commit.rs index 4ce547dee4..e1d861a700 100644 --- a/crates/xmtp_mls/src/groups/validated_commit.rs +++ b/crates/xmtp_mls/src/groups/validated_commit.rs @@ -25,7 +25,7 @@ use prost::Message; use serde::Serialize; use std::collections::HashSet; use thiserror::Error; -use xmtp_common::{retry::RetryableError, retryable}; +use xmtp_common::Retryable; use xmtp_db::StorageError; use xmtp_db::local_commit_log::CommitType; #[cfg(doc)] @@ -47,7 +47,7 @@ use xmtp_proto::xmtp::{ }, }; -#[derive(Debug, Error)] +#[derive(Debug, Error, Retryable)] pub enum CommitValidationError { #[error("Actor could not be found")] ActorCouldNotBeFound, @@ -88,6 +88,7 @@ pub enum CommitValidationError { #[error(transparent)] ProtoDecode(#[from] prost::DecodeError), #[error(transparent)] + #[retry(inherit)] InstallationDiff(#[from] InstallationDiffError), #[error("Failed to parse group mutable permissions: {0}")] GroupMutablePermissions(#[from] GroupMutablePermissionsError), @@ -134,15 +135,6 @@ pub enum CommitValidationError { Conversion(#[from] xmtp_proto::ConversionError), } -impl RetryableError for CommitValidationError { - fn is_retryable(&self) -> bool { - match self { - CommitValidationError::InstallationDiff(diff_error) => retryable!(diff_error), - _ => false, - } - } -} - #[derive(Clone, PartialEq, Hash, Serialize)] pub struct CommitParticipant { pub inbox_id: String, diff --git a/crates/xmtp_mls/src/identity.rs b/crates/xmtp_mls/src/identity.rs index 3f86617a09..a88ef29d3a 100644 --- a/crates/xmtp_mls/src/identity.rs +++ b/crates/xmtp_mls/src/identity.rs @@ -29,8 +29,8 @@ use tracing::debug; use tracing::info; use xmtp_api::ApiClientWrapper; use xmtp_common::ErrorCode; +use xmtp_common::Retryable; use xmtp_common::time::now_ns; -use xmtp_common::{RetryableError, retryable}; use xmtp_configuration::{ CIPHERSUITE, CREATE_PQ_KEY_PACKAGE_EXTENSION, GROUP_MEMBERSHIP_EXTENSION_ID, GROUP_PERMISSIONS_EXTENSION_ID, KEY_PACKAGE_ROTATION_INTERVAL_NS, MAX_INSTALLATIONS_PER_INBOX, @@ -188,7 +188,7 @@ impl IdentityStrategy { } } -#[derive(Debug, Error, ErrorCode)] +#[derive(Debug, Error, ErrorCode, Retryable)] pub enum IdentityError { /// Credential serialization error. /// @@ -258,9 +258,11 @@ pub enum IdentityError { OpenMls(#[from] openmls::prelude::Error), #[error(transparent)] #[error_code(inherit)] + #[retry(inherit)] StorageError(#[from] xmtp_db::StorageError), #[error(transparent)] #[error_code(inherit)] + #[retry(inherit)] OpenMlsStorageError(#[from] SqlKeyStoreError), /// Key package generation error. /// @@ -300,6 +302,7 @@ pub enum IdentityError { Signer(#[from] xmtp_cryptography::SignerError), #[error(transparent)] #[error_code(inherit)] + #[retry(inherit)] ApiClient(#[from] xmtp_api::ApiError), #[error(transparent)] #[error_code(inherit)] @@ -353,17 +356,6 @@ impl NeedsDbReconnect for IdentityError { } } -impl RetryableError for IdentityError { - fn is_retryable(&self) -> bool { - match self { - Self::ApiClient(err) => retryable!(err), - Self::StorageError(err) => retryable!(err), - Self::OpenMlsStorageError(err) => retryable!(err), - _ => false, - } - } -} - #[derive(Debug)] pub struct Identity { pub(crate) inbox_id: InboxId, @@ -1050,6 +1042,7 @@ pub(crate) fn store_key_package_references( Ok(()) } +#[cfg(test)] #[cfg(test)] mod tests { use crate::context::XmtpSharedContext; diff --git a/crates/xmtp_mls/src/identity_updates.rs b/crates/xmtp_mls/src/identity_updates.rs index bf82da3cf3..65a3ef9487 100644 --- a/crates/xmtp_mls/src/identity_updates.rs +++ b/crates/xmtp_mls/src/identity_updates.rs @@ -9,7 +9,7 @@ use crate::{ use futures::{StreamExt, future::try_join_all, stream::FuturesUnordered}; use std::collections::{HashMap, HashSet}; use thiserror::Error; -use xmtp_common::{Event, Retry, RetryableError, retry_async, retryable}; +use xmtp_common::{Event, Retry, Retryable, retry_async}; use xmtp_configuration::Originators; use xmtp_cryptography::CredentialSign; use xmtp_db::StorageError; @@ -51,26 +51,19 @@ pub struct InstallationDiff { pub removed_installations: HashSet>, } -#[derive(Debug, Error)] +#[derive(Debug, Error, Retryable)] pub enum InstallationDiffError { #[error(transparent)] + #[retry(inherit)] Client(#[from] ClientError), #[error(transparent)] + #[retry(inherit)] Db(#[from] xmtp_db::ConnectionError), #[error(transparent)] + #[retry(inherit)] Storage(#[from] StorageError), } -impl RetryableError for InstallationDiffError { - fn is_retryable(&self) -> bool { - match self { - InstallationDiffError::Client(client_error) => retryable!(client_error), - InstallationDiffError::Storage(e) => retryable!(e), - InstallationDiffError::Db(e) => retryable!(e), - } - } -} - pub struct IdentityUpdates { context: Context, } diff --git a/crates/xmtp_mls/src/intents.rs b/crates/xmtp_mls/src/intents.rs index 2ecad3b93b..f7bea99720 100644 --- a/crates/xmtp_mls/src/intents.rs +++ b/crates/xmtp_mls/src/intents.rs @@ -9,27 +9,18 @@ //! intent is fully resolved (success or failure) once it use thiserror::Error; -use xmtp_common::RetryableError; +use xmtp_common::Retryable; use xmtp_proto::types::Cursor; use crate::groups::summary::MessageIdentifier; -#[derive(Debug, Error)] +#[derive(Debug, Error, Retryable)] pub enum ProcessIntentError { #[error("message with cursor [{}] for group [{}] already processed", _0.cursor, xmtp_common::fmt::debug_hex(_0.group_id))] MessageAlreadyProcessed(MessageIdentifier), #[error("welcome with cursor [{0}] already processed")] WelcomeAlreadyProcessed(Cursor), #[error("storage error: {0}")] + #[retry(inherit)] Storage(#[from] xmtp_db::StorageError), } - -impl RetryableError for ProcessIntentError { - fn is_retryable(&self) -> bool { - match self { - Self::MessageAlreadyProcessed(_) => false, - Self::WelcomeAlreadyProcessed(_) => false, - Self::Storage(err) => err.is_retryable(), - } - } -} diff --git a/crates/xmtp_mls/src/messages/enrichment.rs b/crates/xmtp_mls/src/messages/enrichment.rs index da0049c952..83dd480e8c 100644 --- a/crates/xmtp_mls/src/messages/enrichment.rs +++ b/crates/xmtp_mls/src/messages/enrichment.rs @@ -2,7 +2,7 @@ use crate::messages::decoded_message::{DecodedMessage, DeletedBy, MessageBody}; use hex::ToHexExt; use std::collections::HashMap; use thiserror::Error; -use xmtp_common::{ErrorCode, RetryableError}; +use xmtp_common::{ErrorCode, Retryable}; use xmtp_db::DbQuery; use xmtp_db::group_message::{ ContentType as DbContentType, Deletable, RelationCounts, RelationQuery, StoredGroupMessage, @@ -21,10 +21,11 @@ pub fn deleted_message_content_type() -> ContentTypeId { } } -#[derive(Debug, Error, ErrorCode)] +#[derive(Debug, Error, ErrorCode, Retryable)] pub enum EnrichMessageError { #[error("DB error: {0}")] #[error_code(inherit)] + #[retry(inherit)] DbConnection(#[from] xmtp_db::ConnectionError), /// Codec decode error. /// @@ -38,16 +39,6 @@ pub enum EnrichMessageError { DecodeError(#[from] prost::DecodeError), } -impl RetryableError for EnrichMessageError { - fn is_retryable(&self) -> bool { - match self { - Self::DbConnection(e) => e.is_retryable(), - Self::CodecError(_) => false, - Self::DecodeError(_) => false, - } - } -} - // Mapping of reactions, keyed by the ID of the message being reacted to. type ReactionMap = HashMap, Vec>; // Mapping of referenced messages, keyed by ID (stores both stored and decoded) diff --git a/crates/xmtp_mls/src/mls_store.rs b/crates/xmtp_mls/src/mls_store.rs index 879863375a..6c702db012 100644 --- a/crates/xmtp_mls/src/mls_store.rs +++ b/crates/xmtp_mls/src/mls_store.rs @@ -4,7 +4,7 @@ use std::collections::HashMap; use xmtp_api::ApiError; -use xmtp_common::RetryableError; +use xmtp_common::Retryable; use xmtp_db::{ Fetch, NotFound, XmtpOpenMlsProvider, group::{GroupQueryArgs, StoredGroup}, @@ -17,29 +17,22 @@ use xmtp_id::key_package::{KeyPackageVerificationError, VerifiedKeyPackageV2}; use thiserror::Error; use xmtp_db::prelude::*; -#[derive(Error, Debug)] +#[derive(Error, Debug, Retryable)] pub enum MlsStoreError { #[error(transparent)] + #[retry(inherit)] Storage(#[from] xmtp_db::StorageError), #[error(transparent)] + #[retry(inherit)] Api(#[from] ApiError), #[error(transparent)] + #[retry(inherit)] Connection(#[from] xmtp_db::ConnectionError), #[error(transparent)] + #[retry(inherit)] NotFound(#[from] NotFound), } -impl RetryableError for MlsStoreError { - fn is_retryable(&self) -> bool { - match self { - Self::Storage(e) => e.is_retryable(), - Self::Api(e) => e.is_retryable(), - Self::Connection(e) => e.is_retryable(), - Self::NotFound(e) => e.is_retryable(), - } - } -} - impl crate::worker::NeedsDbReconnect for MlsStoreError { /// Forwards a dropped-pool signal from the storage/connection variants so a /// worker loading groups can stop on disconnect. `Api`/`NotFound` return `false`. diff --git a/crates/xmtp_mls/src/subscriptions/d14n_compat.rs b/crates/xmtp_mls/src/subscriptions/d14n_compat.rs index aeff573d3e..0d1addaee2 100644 --- a/crates/xmtp_mls/src/subscriptions/d14n_compat.rs +++ b/crates/xmtp_mls/src/subscriptions/d14n_compat.rs @@ -4,14 +4,14 @@ //! with d14n. returns both errors on failure. use prost::Message; use std::fmt; -use xmtp_common::RetryableError; +use xmtp_common::Retryable; use xmtp_proto::xmtp::mls::api::v1::GroupMessage as V3GroupMessage; use xmtp_proto::xmtp::mls::api::v1::WelcomeMessage as V3WelcomeMessage; use xmtp_proto::xmtp::xmtpv4::envelopes::OriginatorEnvelope; use crate::subscriptions::SubscribeError; -#[derive(thiserror::Error, Debug)] +#[derive(thiserror::Error, Debug, Retryable)] enum D14nCompatDecodeError { #[error( "unable to decode externally streamed message `{}` for v3 or d14n\ @@ -28,12 +28,6 @@ enum D14nCompatDecodeError { }, } -impl RetryableError for D14nCompatDecodeError { - fn is_retryable(&self) -> bool { - false - } -} - impl D14nCompatDecodeError { fn err(v3: prost::DecodeError, d14n: prost::DecodeError) -> SubscribeError { SubscribeError::dyn_err(D14nCompatDecodeError::FallbackFailure { diff --git a/crates/xmtp_mls/src/subscriptions/mod.rs b/crates/xmtp_mls/src/subscriptions/mod.rs index 893f46d890..68aed96608 100644 --- a/crates/xmtp_mls/src/subscriptions/mod.rs +++ b/crates/xmtp_mls/src/subscriptions/mod.rs @@ -36,7 +36,7 @@ use crate::{ subscriptions::d14n_compat::{V3OrD14n, decode_welcome_message}, }; use thiserror::Error; -use xmtp_common::{ErrorCode, MaybeSend, RetryableError, StreamHandle, retryable}; +use xmtp_common::{ErrorCode, MaybeSend, Retryable, RetryableError, StreamHandle}; use xmtp_db::{ NotFound, StorageError, consent_record::{ConsentState, StoredConsentRecord}, @@ -46,18 +46,13 @@ use xmtp_db::{ pub(crate) type Result = std::result::Result; -#[derive(Debug, Error)] +#[derive(Debug, Error, Retryable)] +#[retry(default = true)] pub enum LocalEventError { #[error("Unable to send event: {0}")] Send(String), } -impl RetryableError for LocalEventError { - fn is_retryable(&self) -> bool { - true - } -} - /// Events local to this client /// are broadcast across all senders/receivers of streams #[derive(Debug, Clone)] @@ -176,33 +171,38 @@ impl StreamMessages for broadcast::Receiver { } } -#[derive(thiserror::Error, Debug, ErrorCode)] +#[derive(thiserror::Error, Debug, ErrorCode, Retryable)] pub enum SubscribeError { /// Group error. /// /// Group operation failed during subscription. May be retryable. #[error(transparent)] + #[retry(inherit)] Group(#[from] Box), /// Not found. /// /// Subscribed resource not found. Retryable. #[error(transparent)] + #[retry(inherit)] NotFound(#[from] NotFound), /// Group message not found. /// /// Expected message missing from database. Retryable. // TODO: Add this to `NotFound` #[error("group message expected in database but is missing")] + #[retry(true)] GroupMessageNotFound, /// Receive group error. /// /// Processing streamed group message failed. May be retryable. #[error("processing group message in stream: {0}")] + #[retry(inherit)] ReceiveGroup(#[from] Box), /// Storage error. /// /// Database operation failed. May be retryable. #[error(transparent)] + #[retry(inherit)] Storage(#[from] StorageError), /// Decode error. /// @@ -213,45 +213,54 @@ pub enum SubscribeError { /// /// Message stream failed. Retryable. #[error(transparent)] + #[retry(inherit)] MessageStream(#[from] stream_messages::MessageStreamError), /// Conversation stream error. /// /// Conversation stream failed. Retryable. #[error(transparent)] + #[retry(inherit)] ConversationStream(#[from] stream_conversations::ConversationStreamError), /// API client error. /// /// Network request failed. Retryable. #[error(transparent)] + #[retry(inherit)] ApiClient(#[from] xmtp_api::ApiError), /// Boxed error. /// /// Wrapped dynamic error. May be retryable. #[error("{0}")] + #[retry(inherit)] BoxError(Box), /// Database connection error. /// /// Database connection failed. Retryable. #[error(transparent)] + #[retry(inherit)] Db(#[from] xmtp_db::ConnectionError), /// Conversion error. /// /// Proto conversion failed. Not retryable. #[error(transparent)] + #[retry(inherit)] Conversion(#[from] xmtp_proto::ConversionError), /// Envelope error. /// /// Decentralized API envelope error. May be retryable. #[error(transparent)] + #[retry(inherit)] Envelope(#[from] xmtp_api_d14n::protocol::EnvelopeError), /// Enriched Message Error. #[error("error occured during subscription {0}")] + #[retry(inherit)] Enriched(#[from] EnrichMessageError), /// Stream liveness watchdog tripped. /// /// No activity arrived within the idle timeout, so the stream was terminated to force /// a reconnect (resuming from the persisted cursor). Retryable. #[error("stream went stale: no activity within the idle timeout")] + #[retry(true)] StreamStale, } @@ -273,29 +282,6 @@ impl From for SubscribeError { } } -impl RetryableError for SubscribeError { - fn is_retryable(&self) -> bool { - use SubscribeError::*; - match self { - Group(e) => retryable!(e), - GroupMessageNotFound => true, - ReceiveGroup(e) => retryable!(e), - Storage(e) => retryable!(e), - Decode(_) => false, - NotFound(e) => retryable!(e), - MessageStream(e) => retryable!(e), - ConversationStream(e) => retryable!(e), - ApiClient(e) => retryable!(e), - BoxError(e) => retryable!(e), - Db(c) => retryable!(c), - Conversion(c) => retryable!(c), - Envelope(c) => retryable!(c), - Enriched(c) => retryable!(c), - StreamStale => true, - } - } -} - impl crate::worker::NeedsDbReconnect for SubscribeError { /// Forwards a dropped-pool signal so the device-sync worker stops on /// disconnect. `BoxError` wraps an opaque error we can't introspect → `false`. diff --git a/crates/xmtp_mls/src/subscriptions/stream_conversations.rs b/crates/xmtp_mls/src/subscriptions/stream_conversations.rs index 4411baae65..156057c3aa 100644 --- a/crates/xmtp_mls/src/subscriptions/stream_conversations.rs +++ b/crates/xmtp_mls/src/subscriptions/stream_conversations.rs @@ -23,26 +23,17 @@ use xmtp_macro::log_event; use xmtp_proto::api_client::XmtpMlsStreams; use xmtp_proto::types::{Cursor, OriginatorId, SequenceId, WelcomeMessage}; -#[derive(thiserror::Error, Debug)] +#[derive(thiserror::Error, Debug, xmtp_common::Retryable)] pub enum ConversationStreamError { #[error("unexpected message type in welcome")] InvalidPayload, #[error("the conversation was filtered because of the given conversation type")] InvalidConversationType, #[error("the welcome pointer was not found")] + #[retry(true)] WelcomePointerNotFound, } -impl xmtp_common::RetryableError for ConversationStreamError { - fn is_retryable(&self) -> bool { - use ConversationStreamError::*; - match self { - InvalidPayload | InvalidConversationType => false, - WelcomePointerNotFound => true, - } - } -} - pub enum WelcomeOrGroup { Group(xmtp_proto::types::GroupId), Welcome(xmtp_proto::types::WelcomeMessage), diff --git a/crates/xmtp_mls/src/subscriptions/stream_messages.rs b/crates/xmtp_mls/src/subscriptions/stream_messages.rs index dccf6a78f4..9ed76bd0cd 100644 --- a/crates/xmtp_mls/src/subscriptions/stream_messages.rs +++ b/crates/xmtp_mls/src/subscriptions/stream_messages.rs @@ -35,15 +35,6 @@ use xmtp_proto::types::{Cursor, GlobalCursor, OriginatorId, SequenceId}; use xmtp_proto::types::{GroupId, Topic}; use xmtp_proto::{api_client::XmtpMlsStreams, types::TopicCursor}; -impl xmtp_common::RetryableError for MessageStreamError { - fn is_retryable(&self) -> bool { - use MessageStreamError::*; - match self { - NotSubscribed(_) | InvalidPayload => false, - } - } -} - type AddingResult = (Out, Vec, Option); #[pin_project(PinnedDrop)] diff --git a/crates/xmtp_mls/src/subscriptions/stream_messages/types.rs b/crates/xmtp_mls/src/subscriptions/stream_messages/types.rs index 620a675311..b43a5d0dd3 100644 --- a/crates/xmtp_mls/src/subscriptions/stream_messages/types.rs +++ b/crates/xmtp_mls/src/subscriptions/stream_messages/types.rs @@ -2,7 +2,7 @@ use std::collections::HashSet; use xmtp_proto::types::{Cursor, GlobalCursor, Topic, TopicCursor}; -#[derive(thiserror::Error, Debug)] +#[derive(thiserror::Error, Debug, xmtp_common::Retryable)] pub enum MessageStreamError { #[error("received message for not subscribed group {id}", id = hex::encode(_0))] NotSubscribed(Vec), diff --git a/crates/xmtp_mls/src/test/group_test_utils.rs b/crates/xmtp_mls/src/test/group_test_utils.rs index 61235f1a9f..61821b125b 100644 --- a/crates/xmtp_mls/src/test/group_test_utils.rs +++ b/crates/xmtp_mls/src/test/group_test_utils.rs @@ -9,14 +9,15 @@ use crate::{ use thiserror::Error; use xmtp_api::{ApiError, XmtpApi}; use xmtp_api_d14n::protocol::{EnvelopeError, XmtpQuery}; -use xmtp_common::RetryableError; +use xmtp_common::Retryable; use xmtp_db::{ XmtpDb, group_message::{GroupMessageKind, MsgQueryArgs}, }; use xmtp_proto::types::{GroupMessage, TopicKind}; -#[derive(Error, Debug)] +#[derive(Error, Debug, Retryable)] +#[retry(default = true)] pub enum TestError { #[error("{0}")] Generic(String), @@ -30,12 +31,6 @@ pub enum TestError { Envelope(#[from] EnvelopeError), } -impl RetryableError for TestError { - fn is_retryable(&self) -> bool { - true - } -} - impl MlsGroup where Context: XmtpSharedContext, diff --git a/crates/xmtp_mls/src/worker/device_sync/mod.rs b/crates/xmtp_mls/src/worker/device_sync/mod.rs index e71408e324..2802f4cc47 100644 --- a/crates/xmtp_mls/src/worker/device_sync/mod.rs +++ b/crates/xmtp_mls/src/worker/device_sync/mod.rs @@ -22,10 +22,10 @@ use tracing::instrument; use worker::SyncMetric; use xmtp_archive::{ArchiveError, BackupMetadata}; use xmtp_common::ErrorCode; -use xmtp_common::{NS_IN_DAY, RetryableError, time::now_ns}; +use xmtp_common::{NS_IN_DAY, Retryable, time::now_ns}; use xmtp_content_types::encoded_content_to_bytes; use xmtp_db::{ - NotFound, StorageError, consent_record::ConsentState, group::GroupQueryArgs, + StorageError, consent_record::ConsentState, group::GroupQueryArgs, group_message::StoredGroupMessage, }; use xmtp_db::{XmtpDb, group::ConversationType, prelude::*}; @@ -51,7 +51,9 @@ pub use xmtp_archive::archive_options::{ArchiveOptions, BackupElementSelection}; #[cfg(test)] mod tests; -#[derive(Debug, Error, ErrorCode)] +#[derive(Debug, Error, ErrorCode, Retryable)] +// Inverted logic: every variant is retryable EXCEPT the explicit exceptions below. +#[retry(default = true)] pub enum DeviceSyncError { /// I/O error. /// @@ -109,6 +111,7 @@ pub enum DeviceSyncError { /// /// Device sync kind not specified. Not retryable. #[error("unspecified device sync kind")] + #[retry(false)] UnspecifiedDeviceSyncKind, /// Sync payload too old. /// @@ -140,6 +143,7 @@ pub enum DeviceSyncError { /// /// Sync interaction already acknowledged. Not retryable. #[error("Sync interaction is already acknowledged by another installation")] + #[retry(false)] AlreadyAcknowledged, /// Missing options. /// @@ -150,11 +154,13 @@ pub enum DeviceSyncError { /// /// Sync server URL not configured. Not retryable. #[error("Missing sync server url")] + #[retry(false)] MissingSyncServerUrl, /// Missing sync group. /// /// Sync group not found. Not retryable. #[error("Missing sync group")] + #[retry(false)] MissingSyncGroup, #[error(transparent)] #[error_code(inherit)] @@ -217,20 +223,9 @@ impl NeedsDbReconnect for DeviceSyncError { } } -impl RetryableError for DeviceSyncError { - fn is_retryable(&self) -> bool { - !matches!( - self, - Self::AlreadyAcknowledged - | Self::MissingSyncGroup - | Self::MissingSyncServerUrl - | Self::UnspecifiedDeviceSyncKind - ) - } -} - -impl From for DeviceSyncError { - fn from(value: NotFound) -> Self { +#[cfg(test)] +impl From for DeviceSyncError { + fn from(value: xmtp_db::NotFound) -> Self { DeviceSyncError::Storage(StorageError::NotFound(value)) } } diff --git a/crates/xmtp_mls_common/src/mls_ext/payload_encryption.rs b/crates/xmtp_mls_common/src/mls_ext/payload_encryption.rs index e357509364..af04c9aa8f 100644 --- a/crates/xmtp_mls_common/src/mls_ext/payload_encryption.rs +++ b/crates/xmtp_mls_common/src/mls_ext/payload_encryption.rs @@ -4,7 +4,7 @@ use openmls_traits::crypto::OpenMlsCrypto; use openmls_traits::types::HpkeCiphertext; use thiserror::Error; use tls_codec::{Deserialize, Serialize}; -use xmtp_common::RetryableError; +use xmtp_common::Retryable; use xmtp_id::key_package::WrapperAlgorithm; static LIBCRUX_CRYPTO_PROVIDER: std::sync::LazyLock = @@ -12,7 +12,7 @@ static LIBCRUX_CRYPTO_PROVIDER: std::sync::LazyLock bool { - false - } -} - -impl RetryableError for UnwrapPayloadError { - fn is_retryable(&self) -> bool { - false - } -} - /// Wrap a payload (plus optional secondary payload) in an outer layer of HPKE /// encryption using the specified [WrapperAlgorithm]. The algorithm and public /// key type MUST match. diff --git a/crates/xmtp_proto/src/error.rs b/crates/xmtp_proto/src/error.rs index 7daed1b03a..a0c7cc74b7 100644 --- a/crates/xmtp_proto/src/error.rs +++ b/crates/xmtp_proto/src/error.rs @@ -1,7 +1,7 @@ use std::array::TryFromSliceError; use thiserror::Error; -use xmtp_common::{ErrorCode, RetryableError}; +use xmtp_common::{ErrorCode, Retryable}; #[derive(Clone, Debug, PartialEq, Eq)] pub enum ApiEndpoint { @@ -61,7 +61,7 @@ impl std::fmt::Display for ApiEndpoint { /// Loosely Modeled after serdes [error](https://docs.rs/serde/latest/serde/de/value/struct.Error.html) type. /// This general error type avoid circular hard-dependencies on crates further up the tree /// (xmtp_id/xmtp_mls) if they had defined the error themselves. -#[derive(thiserror::Error, Debug, ErrorCode)] +#[derive(thiserror::Error, Debug, ErrorCode, Retryable)] pub enum ConversionError { /// Missing field. /// @@ -173,11 +173,6 @@ pub enum ConversionError { // The API call is what should be retried. // If retry on a conversion error is desired a new error enum + custom Retrayble implementation // should be preferred. -impl RetryableError for ConversionError { - fn is_retryable(&self) -> bool { - false - } -} /// Error resulting from proto conversions/mutations #[derive(Debug, Error, ErrorCode)] diff --git a/crates/xmtp_proto/src/traits/error.rs b/crates/xmtp_proto/src/traits/error.rs index cb7e808323..6437a87c30 100644 --- a/crates/xmtp_proto/src/traits/error.rs +++ b/crates/xmtp_proto/src/traits/error.rs @@ -2,23 +2,27 @@ use std::fmt::Display; use crate::{ApiEndpoint, ProtoError}; use thiserror::Error; -use xmtp_common::{BoxDynError, RetryableError, retryable}; +use xmtp_common::{BoxDynError, Retryable, RetryableError}; -#[derive(Debug, Error)] +#[derive(Debug, Error, Retryable)] #[non_exhaustive] pub enum ApiClientError { /// The client encountered an error. #[error("api client at endpoint \"{}\" has error {}", endpoint, source)] + #[retry(when = source.is_retryable())] ClientWithEndpoint { endpoint: String, /// The client error. source: NetworkError, }, #[error("client errored {}", source)] + #[retry(inherit)] Client { source: NetworkError }, #[error(transparent)] + #[retry(true)] Http(#[from] http::Error), #[error(transparent)] + #[retry(inherit)] Body(#[from] BodyError), #[error(transparent)] DecodeError(#[from] prost::DecodeError), @@ -29,8 +33,10 @@ pub enum ApiClientError { #[error(transparent)] InvalidUri(#[from] http::uri::InvalidUri), #[error(transparent)] + #[retry(true)] Expired(#[from] xmtp_common::time::Expired), #[error("{0}")] + #[retry(inherit)] Other(Box), #[error("{0}")] OtherUnretryable(BoxDynError), @@ -41,7 +47,8 @@ pub enum ApiClientError { /// A lower level NetworkError, like gRPC/QUIC/HTTP/1.1 errors go here. /// use [`ApiClientError::new`] to construct // needed because of AsDynError sealed trait -#[derive(Debug)] +#[derive(Debug, Retryable)] +#[retry(when = self.source.is_retryable())] pub struct NetworkError { source: Box, } @@ -58,12 +65,6 @@ impl Display for NetworkError { } } -impl RetryableError for NetworkError { - fn is_retryable(&self) -> bool { - self.source.is_retryable() - } -} - impl NetworkError { pub fn new(e: impl RetryableError + 'static) -> Self { NetworkError { @@ -114,26 +115,6 @@ impl ApiClientError { } } -impl RetryableError for ApiClientError { - fn is_retryable(&self) -> bool { - use ApiClientError::*; - match self { - Client { source } => retryable!(*source), - ClientWithEndpoint { source, .. } => retryable!(source), - Body(e) => retryable!(e), - Http(_) => true, - DecodeError(_) => false, - Conversion(_) => false, - ProtoError(_) => false, - InvalidUri(_) => false, - Expired(_) => true, - Other(r) => retryable!(r), - OtherUnretryable(_) => false, - WritesDisabled => false, - } - } -} - // Infallible errors by definition can never occur impl From for ApiClientError { fn from(_v: std::convert::Infallible) -> ApiClientError { @@ -141,16 +122,10 @@ impl From for ApiClientError { } } -#[derive(Debug, Error)] +#[derive(Debug, Error, Retryable)] pub enum BodyError { #[error(transparent)] UninitializedField(#[from] derive_builder::UninitializedFieldError), #[error(transparent)] Conversion(#[from] crate::ConversionError), } - -impl RetryableError for BodyError { - fn is_retryable(&self) -> bool { - false - } -}