sdk: don't clobber the DM list if we failed to deserialize the previous m.direct event

Report a deserialization failure and do not mark the room as a DM,
instead of incorrectly restarting from an empty DM list.

Fixes #3410.

Author: bnjbvr

crates/matrix-sdk/src/account.rs

@@ -839,22 +839,21 @@ impl Account {
         // existing `m.direct` event and append the room to the list of DMs we
         // have with this user.
 
-        // TODO: Uncomment this once we can rely on `/sync`. Because of the
-        // `unwrap_or_default()` we're using, this may lead to us resetting the
-        // `m.direct` account data event and unmarking the user's DMs.
-        // let mut content = self
-        //     .account_data::<DirectEventContent>()
-        //     .await?
-        //     .map(|c| c.deserialize())
-        //     .transpose()?
-        //     .unwrap_or_default();
-
         // We are fetching the content from the server because we currently can't rely
         // on `/sync` giving us the correct data in a timely manner.
         let raw_content = self.fetch_account_data(GlobalAccountDataEventType::Direct).await?;
-        let mut content = raw_content
-            .and_then(|content| content.deserialize_as::<DirectEventContent>().ok())
-            .unwrap_or_default();
+
+        let mut content = if let Some(raw_content) = raw_content {
+            // Log the error and pass it upwards if we fail to deserialize the m.direct
+            // event.
+            raw_content.deserialize_as::<DirectEventContent>().map_err(|err| {
+                error!("unable to deserialize m.direct event content; aborting request to mark {room_id} as dm: {err}");
+                err
+            })?
+        } else {
+            // If there was no m.direct event server-side, create a default one.
+            Default::default()
+        };
 
         for user_id in user_ids {
             content.entry(user_id.to_owned()).or_default().push(room_id.to_owned());

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

@@ -1,6 +1,6 @@
 use std::{collections::BTreeMap, time::Duration};
 
-use assert_matches2::assert_let;
+use assert_matches2::{assert_let, assert_matches};
 use eyeball_im::VectorDiff;
 use futures_util::FutureExt;
 use matrix_sdk::{
@@ -442,7 +442,7 @@ async fn test_marking_room_as_dm() {
     client
         .sync_once(SyncSettings::default())
         .await
-        .expect("We should be able to performa an initial sync");
+        .expect("We should be able to perform an initial sync");
 
     let account_data = client
         .account()
@@ -504,6 +504,48 @@ async fn test_marking_room_as_dm() {
     server.verify().await;
 }
 
+// Check that we're fetching account data from the server when marking a room as
+// a DM, and that we don't clobber the previous entry if it was impossible to
+// deserialize.
+#[async_test]
+async fn test_marking_room_as_dm_fails_if_undeserializable() {
+    let (client, server) = logged_in_client_with_server().await;
+
+    mock_sync(&server, &*test_json::SYNC, None).await;
+    client
+        .sync_once(SyncSettings::default())
+        .await
+        .expect("We should be able to perform an initial sync");
+
+    let account_data = client
+        .account()
+        .account_data::<DirectEventContent>()
+        .await
+        .expect("We should be able to fetch the account data event from the store");
+
+    assert!(account_data.is_none(), "We should not have any account data initially");
+
+    let bob = user_id!("@bob:example.com");
+    let users = vec![bob.to_owned()];
+
+    // The response must be valid JSON, but not a valid `DirectEventContent`
+    // representation.
+    Mock::given(method("GET"))
+        .and(path("_matrix/client/r0/user/@example:localhost/account_data/m.direct"))
+        .and(header("authorization", "Bearer 1234"))
+        .respond_with(ResponseTemplate::new(200).set_body_json(json!(["hey", null, true, 42])))
+        .expect(1)
+        .named("m.direct account data GET")
+        .mount(&server)
+        .await;
+
+    let result = client.account().mark_as_dm(&DEFAULT_TEST_ROOM_ID, &users).await;
+
+    assert_matches!(result, Err(matrix_sdk::Error::SerdeJson(_)));
+
+    server.verify().await;
+}
+
 #[cfg(feature = "e2e-encryption")]
 #[async_test]
 async fn test_get_own_device() {