From 6a8ea8a08343864d947a1c044787b9795d902c49 Mon Sep 17 00:00:00 2001
From: link2xt <link2xt@testrun.org>
Date: Wed, 17 Jan 2024 05:05:50 +0000
Subject: [PATCH] fix: set message download state to Failure on IMAP errors

Previously the message was removed from `download` table,
but message bubble was stuck in InProgress state.

Now download state is updated by the caller,
so it cannot be accidentally skipped.
---
 src/download.rs  | 34 ++++++++++++----------------------
 src/scheduler.rs | 12 +++++++++++-
 2 files changed, 23 insertions(+), 23 deletions(-)

diff --git a/src/download.rs b/src/download.rs
index 5c626fc71..1f3e1d56b 100644
--- a/src/download.rs
+++ b/src/download.rs
@@ -146,29 +146,19 @@ pub(crate) async fn download_msg(context: &Context, msg_id: MsgId, imap: &mut Im
         )
         .await?;
 
-    if let Some((server_uid, server_folder)) = row {
-        match imap
-            .fetch_single_msg(context, &server_folder, server_uid, msg.rfc724_mid.clone())
-            .await
-        {
-            ImapActionResult::RetryLater | ImapActionResult::Failed => {
-                msg.id
-                    .update_download_state(context, DownloadState::Failure)
-                    .await?;
-                Err(anyhow!("Call download_full() again to try over."))
-            }
-            ImapActionResult::Success => {
-                // update_download_state() not needed as receive_imf() already
-                // set the state and emitted the event.
-                Ok(())
-            }
-        }
-    } else {
+    let Some((server_uid, server_folder)) = row else {
         // No IMAP record found, we don't know the UID and folder.
-        msg.id
-            .update_download_state(context, DownloadState::Failure)
-            .await?;
-        Err(anyhow!("Call download_full() again to try over."))
+        return Err(anyhow!("Call download_full() again to try over."));
+    };
+
+    match imap
+        .fetch_single_msg(context, &server_folder, server_uid, msg.rfc724_mid.clone())
+        .await
+    {
+        ImapActionResult::RetryLater | ImapActionResult::Failed => {
+            Err(anyhow!("Call download_full() again to try over."))
+        }
+        ImapActionResult::Success => Ok(()),
     }
 }
 
diff --git a/src/scheduler.rs b/src/scheduler.rs
index dd3d60dae..ff4a96c22 100644
--- a/src/scheduler.rs
+++ b/src/scheduler.rs
@@ -15,7 +15,7 @@ use self::connectivity::ConnectivityStore;
 use crate::config::Config;
 use crate::contact::{ContactId, RecentlySeenLoop};
 use crate::context::Context;
-use crate::download::download_msg;
+use crate::download::{download_msg, DownloadState};
 use crate::ephemeral::{self, delete_expired_imap_messages};
 use crate::events::EventType;
 use crate::imap::{FolderMeaning, Imap};
@@ -350,6 +350,16 @@ async fn download_msgs(context: &Context, imap: &mut Imap) -> Result<()> {
     for msg_id in msg_ids {
         if let Err(err) = download_msg(context, msg_id, imap).await {
             warn!(context, "Failed to download message {msg_id}: {:#}.", err);
+
+            // Update download state to failure
+            // so it can be retried.
+            //
+            // On success update_download_state() is not needed
+            // as receive_imf() already
+            // set the state and emitted the event.
+            msg_id
+                .update_download_state(context, DownloadState::Failure)
+                .await?;
         }
         context
             .sql