diff --git a/CHANGELOG.md b/CHANGELOG.md index 2cbcc49ad..323cfac6a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,6 +21,7 @@ - Fix uncaught exception in JSON-RPC tests #3884 - Fix STARTTLS connection and add a test for it #3907 - Trigger reconnection when failing to fetch existing messages #3911 +- Do not retry fetching existing messages after failure, prevents infinite reconnection loop #3913 ## 1.104.0 diff --git a/src/imap.rs b/src/imap.rs index 2df278f29..cb3843ee0 100644 --- a/src/imap.rs +++ b/src/imap.rs @@ -677,7 +677,10 @@ impl Imap { return Ok(false); } - let new_emails = self.select_with_uidvalidity(context, folder).await?; + let new_emails = self + .select_with_uidvalidity(context, folder) + .await + .with_context(|| format!("failed to select folder {}", folder))?; if !new_emails && !fetch_existing_msgs { info!(context, "No new emails in folder {}", folder); @@ -837,9 +840,15 @@ impl Imap { } self.prepare(context).await.context("could not connect")?; - add_all_recipients_as_contacts(context, self, Config::ConfiguredSentboxFolder).await; - add_all_recipients_as_contacts(context, self, Config::ConfiguredMvboxFolder).await; - add_all_recipients_as_contacts(context, self, Config::ConfiguredInboxFolder).await; + add_all_recipients_as_contacts(context, self, Config::ConfiguredSentboxFolder) + .await + .context("failed to get recipients from the sentbox")?; + add_all_recipients_as_contacts(context, self, Config::ConfiguredMvboxFolder) + .await + .context("failed to ge recipients from the movebox")?; + add_all_recipients_as_contacts(context, self, Config::ConfiguredInboxFolder) + .await + .context("failed to get recipients from the inbox")?; if context.get_config_bool(Config::FetchExistingMsgs).await? { for config in &[ @@ -848,6 +857,10 @@ impl Imap { Config::ConfiguredSentboxFolder, ] { if let Some(folder) = context.get_config(*config).await? { + info!( + context, + "Fetching existing messages from folder \"{}\"", folder + ); self.fetch_new_messages(context, &folder, false, true) .await .context("could not fetch existing messages")?; @@ -856,9 +869,6 @@ impl Imap { } info!(context, "Done fetching existing messages."); - context - .set_config_bool(Config::FetchedExistingMsgs, true) - .await?; Ok(()) } } @@ -1207,8 +1217,10 @@ impl Imap { /// Prefetch all messages greater than or equal to `uid_next`. Returns a list of fetch results /// in the order of ascending delivery time to the server (INTERNALDATE). async fn prefetch(&mut self, uid_next: u32) -> Result> { - let session = self.session.as_mut(); - let session = session.context("fetch_after(): IMAP No Connection established")?; + let session = self + .session + .as_mut() + .context("no IMAP connection established")?; // fetch messages with larger UID than the last one seen let set = format!("{}:*", uid_next); @@ -1233,7 +1245,6 @@ impl Imap { } } } - drop(list); Ok(msgs.into_iter().map(|((_, uid), msg)| (uid, msg)).collect()) } @@ -2306,49 +2317,58 @@ impl std::fmt::Display for UidRange { } } } -async fn add_all_recipients_as_contacts(context: &Context, imap: &mut Imap, folder: Config) { - let mailbox = if let Ok(Some(m)) = context.get_config(folder).await { +async fn add_all_recipients_as_contacts( + context: &Context, + imap: &mut Imap, + folder: Config, +) -> Result<()> { + let mailbox = if let Some(m) = context.get_config(folder).await? { m } else { - return; + info!( + context, + "Folder {} is not configured, skipping fetching contacts from it.", folder + ); + return Ok(()); }; - if let Err(e) = imap.select_with_uidvalidity(context, &mailbox).await { - // We are using Anyhow's .context() and to show the inner error, too, we need the {:#}: - warn!(context, "Could not select {}: {:#}", mailbox, e); - return; - } - match imap.get_all_recipients(context).await { - Ok(contacts) => { - let mut any_modified = false; - for contact in contacts { - let display_name_normalized = contact - .display_name - .as_ref() - .map(|s| normalize_name(s)) - .unwrap_or_default(); + imap.select_with_uidvalidity(context, &mailbox) + .await + .with_context(|| format!("could not select {}", mailbox))?; - match Contact::add_or_lookup( - context, - &display_name_normalized, - &contact.addr, - Origin::OutgoingTo, - ) - .await - { - Ok((_, modified)) => { - if modified != Modifier::None { - any_modified = true; - } - } - Err(e) => warn!(context, "Could not add recipient: {}", e), + let contacts = imap + .get_all_recipients(context) + .await + .context("could not get recipients")?; + + let mut any_modified = false; + for contact in contacts { + let display_name_normalized = contact + .display_name + .as_ref() + .map(|s| normalize_name(s)) + .unwrap_or_default(); + + match Contact::add_or_lookup( + context, + &display_name_normalized, + &contact.addr, + Origin::OutgoingTo, + ) + .await + { + Ok((_, modified)) => { + if modified != Modifier::None { + any_modified = true; } } - if any_modified { - context.emit_event(EventType::ContactsChanged(None)); - } + Err(e) => warn!(context, "Could not add recipient: {}", e), } - Err(e) => warn!(context, "Could not add recipients: {}", e), - }; + } + if any_modified { + context.emit_event(EventType::ContactsChanged(None)); + } + + Ok(()) } #[cfg(test)] diff --git a/src/scheduler.rs b/src/scheduler.rs index d3d3bca2c..92e569010 100644 --- a/src/scheduler.rs +++ b/src/scheduler.rs @@ -141,6 +141,16 @@ async fn inbox_loop(ctx: Context, started: Sender<()>, inbox_handlers: ImapConne match ctx.get_config_bool(Config::FetchedExistingMsgs).await { Ok(fetched_existing_msgs) => { if !fetched_existing_msgs { + // Consider it done even if we fail. + // + // This operation is not critical enough to retry, + // especially if the error is persistent. + if let Err(err) = + ctx.set_config_bool(Config::FetchedExistingMsgs, true).await + { + warn!(ctx, "Can't set Config::FetchedExistingMsgs: {:#}", err); + } + if let Err(err) = connection.fetch_existing_msgs(&ctx).await { warn!(ctx, "Failed to fetch existing messages: {:#}", err); connection.trigger_reconnect(&ctx);