Skip to content

Commit

Permalink
fix: set message download state to Failure on IMAP errors
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
link2xt committed Jan 18, 2024
1 parent e0e56cd commit 6a8ea8a
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 23 deletions.
34 changes: 12 additions & 22 deletions src/download.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(()),
}
}

Expand Down
12 changes: 11 additions & 1 deletion src/scheduler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit 6a8ea8a

Please sign in to comment.