send queue: add a test that simulates unreachable network/server

Author: bnjbvr

crates/matrix-sdk/src/http_client/mod.rs

@@ -278,7 +278,10 @@ impl RetryKind {
     }
 }
 
-/// Returns whether an API error is transient or permanent,
+/// Returns whether an HTTP error response should be qualified as transient or
+/// permanent.
+///
+/// This is what decides whether to retry requests, in the [`native`] module.
 pub(crate) fn characterize_retry_kind(err: HttpError) -> RetryKind {
     use ruma::api::client::error::{ErrorBody, ErrorKind, RetryAfter};
 

crates/matrix-sdk/src/send_queue.rs

@@ -380,9 +380,9 @@ impl RoomSendQueue {
                         let http_err = retry_kind.error();
 
                         if !is_recoverable {
-                            // Also classify raw HTTP-level request errors as recoverable, because
-                            // `characterize_retry_kind` doesn't do that, and bnjbvr isn't clear
-                            // whether it should.
+                            // Also flag all HTTP errors as recoverable: they might indicate that
+                            // the server was unreachable, or that the device is entirely
+                            // disconnected.
                             is_recoverable =
                                 matches!(http_err, crate::error::HttpError::Reqwest(..));
                         }

crates/matrix-sdk/tests/integration/send_queue.rs

@@ -9,7 +9,7 @@ use std::{
 use assert_matches2::{assert_let, assert_matches};
 use matrix_sdk::{
     send_queue::{LocalEcho, RoomSendQueueError, RoomSendQueueUpdate},
-    test_utils::logged_in_client_with_server,
+    test_utils::{logged_in_client, logged_in_client_with_server},
 };
 use matrix_sdk_test::{async_test, InvitedRoomBuilder, JoinedRoomBuilder, LeftRoomBuilder};
 use ruma::{
@@ -974,3 +974,77 @@ async fn test_unrecoverable_errors() {
     assert!(room.send_queue().is_enabled());
     assert!(client.send_queue().is_enabled());
 }
+
+#[async_test]
+async fn test_no_network_access_error_is_recoverable() {
+    // This is subtle, but for the `drop(server)` below to be effectful, it needs to
+    // not be a pooled wiremock server (the default), which will keep the dropped
+    // server in a static. Using the line below will create a "bare" server,
+    // which is effectively dropped upon `drop()`.
+    let server = wiremock::MockServer::builder().start().await;
+
+    let client = logged_in_client(Some(server.uri().to_string())).await;
+
+    // Mark the room as joined.
+    let room_id = room_id!("!a:b.c");
+
+    let room = mock_sync_with_new_room(
+        |builder| {
+            builder.add_joined_room(JoinedRoomBuilder::new(room_id));
+        },
+        &client,
+        &server,
+        room_id,
+    )
+    .await;
+
+    // Dropping the server: any subsequent attempt to connect mimics an unreachable
+    // server, which might be caused by missing network.
+    drop(server);
+
+    let mut errors = client.send_queue().subscribe_errors();
+    assert!(errors.is_empty());
+
+    // Start with an enabled sending queue.
+    client.send_queue().set_enabled(true);
+    assert!(client.send_queue().is_enabled());
+
+    let q = room.send_queue();
+    let (local_echoes, mut watch) = q.subscribe().await;
+
+    assert!(local_echoes.is_empty());
+    assert!(watch.is_empty());
+
+    // Queue two messages.
+    q.send(RoomMessageEventContent::text_plain("is there anyone around here").into())
+        .await
+        .unwrap();
+
+    // First message is seen as a local echo.
+    assert_let!(
+        Ok(Ok(RoomSendQueueUpdate::NewLocalEvent(LocalEcho {
+            content: AnyMessageLikeEventContent::RoomMessage(msg),
+            transaction_id: txn1,
+            ..
+        }))) = timeout(Duration::from_secs(1), watch.recv()).await
+    );
+    assert_eq!(msg.body(), format!("is there anyone around here"));
+
+    // There will be an error report for the first message, indicating that the
+    // error is recoverable: because network is unreachable.
+    let report = errors.recv().await.unwrap();
+    assert_eq!(report.room_id, room.room_id());
+    assert!(report.is_recoverable);
+
+    // The room updates will report the error for the first message as recoverable
+    // too.
+    assert_let!(
+        Ok(Ok(RoomSendQueueUpdate::SendError { is_recoverable: true, transaction_id, .. })) =
+            timeout(Duration::from_secs(1), watch.recv()).await
+    );
+    assert_eq!(transaction_id, txn1);
+
+    // The room queue is disabled, because the error was recoverable.
+    assert!(!room.send_queue().is_enabled());
+    assert!(client.send_queue().is_enabled());
+}