From 93a28f67b3c36f9f21ac2e450bf0791758796da1 Mon Sep 17 00:00:00 2001 From: Benjamin Bouvier Date: Thu, 27 Jun 2024 16:43:56 +0200 Subject: [PATCH] send queue: mark `ConcurrentRequestFailed` as recoverable This will still disable the room's send queue, but the embedder may then decide whether to re-enable the queue or not, based on network connectivity. Ideally, we'd implement some retry mechanism here too, but since we're at a different layer than the HTTP one, we can't get that "for free", so let the embedder decide what to do here. --- crates/matrix-sdk/src/error.rs | 5 +++-- crates/matrix-sdk/src/send_queue.rs | 19 ++++++++++++++----- 2 files changed, 17 insertions(+), 7 deletions(-) diff --git a/crates/matrix-sdk/src/error.rs b/crates/matrix-sdk/src/error.rs index 0e1041a5066..f27e11df8f6 100644 --- a/crates/matrix-sdk/src/error.rs +++ b/crates/matrix-sdk/src/error.rs @@ -320,8 +320,9 @@ pub enum Error { #[error("a concurrent request failed; see logs for details")] ConcurrentRequestFailed, - /// An other error was raised - /// this might happen because encryption was enabled on the base-crate + /// An other error was raised. + /// + /// This might happen because encryption was enabled on the base-crate /// but not here and that raised. #[error("unknown error: {0}")] UnknownError(Box), diff --git a/crates/matrix-sdk/src/send_queue.rs b/crates/matrix-sdk/src/send_queue.rs index bf616844185..3a60952e79c 100644 --- a/crates/matrix-sdk/src/send_queue.rs +++ b/crates/matrix-sdk/src/send_queue.rs @@ -471,11 +471,20 @@ impl RoomSendQueue { } Err(err) => { - let is_recoverable = if let crate::Error::Http(ref http_err) = err { - // All transient errors are recoverable. - matches!(http_err.retry_kind(), RetryKind::Transient { .. }) - } else { - false + let is_recoverable = match err { + crate::Error::Http(ref http_err) => { + // All transient errors are recoverable. + matches!(http_err.retry_kind(), RetryKind::Transient { .. }) + } + + // `ConcurrentRequestFailed` typically happens because of an HTTP failure; + // since we don't get the underlying error, be lax and consider it + // recoverable, and let observers decide to retry it or not. At some point + // we'll get the actual underlying error. + crate::Error::ConcurrentRequestFailed => true, + + // As of 2024-06-27, all other error types are considered unrecoverable. + _ => false, }; if is_recoverable {