Skip to content

Improve IMAP logging and docs #6859

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
May 19, 2025
Merged

Improve IMAP logging and docs #6859

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
38ab86d
refactor: remove explicit lock drop at the end of scope
link2xt May 18, 2025
8084160
docs: update Imap.connect() documentation
link2xt May 18, 2025
ab4c1d2
docs: update Imap.prepare() documentation
link2xt May 18, 2025
5ffb326
feat: add more IMAP logging
link2xt May 18, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions python/tests/test_1_online.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1617,7 +1617,7 @@ def test_immediate_autodelete(acfactory, lp):

lp.sec("ac2: wait for close/expunge on autodelete")
ac2._evtracker.get_matching("DC_EVENT_IMAP_MESSAGE_DELETED")
ac2._evtracker.get_info_contains("close/expunge succeeded")
ac2._evtracker.get_info_contains("Close/expunge succeeded.")

lp.sec("ac2: check that message was autodeleted on server")
assert len(ac2.direct_imap.get_all_messages()) == 0
Expand Down Expand Up @@ -1653,7 +1653,7 @@ def test_delete_multiple_messages(acfactory, lp):
lp.sec("ac2: test that only one message is left")
while 1:
ac2._evtracker.get_matching("DC_EVENT_IMAP_MESSAGE_DELETED")
ac2._evtracker.get_info_contains("close/expunge succeeded")
ac2._evtracker.get_info_contains("Close/expunge succeeded.")
ac2.direct_imap.select_config_folder("inbox")
nr_msgs = len(ac2.direct_imap.get_all_messages())
assert nr_msgs > 0
Expand Down
16 changes: 7 additions & 9 deletions src/imap.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -285,9 +285,7 @@ impl Imap {
Ok(imap)
}

/// Connects or reconnects if needed.
///
/// It is safe to call this function if already connected, actions are performed only as needed.
/// Connects to IMAP server and returns a new IMAP session.
///
/// Calling this function is not enough to perform IMAP operations. Use [`Imap::prepare`]
/// instead if you are going to actually use connection rather than trying connection
Expand Down Expand Up @@ -326,7 +324,7 @@ impl Imap {
}
}

info!(context, "Connecting to IMAP server");
info!(context, "Connecting to IMAP server.");
self.connectivity.set_connecting(context).await;

self.conn_last_try = tools::Time::now();
Expand Down Expand Up @@ -411,7 +409,7 @@ impl Imap {
lp.user
)));
self.connectivity.set_preparing(context).await;
info!(context, "Successfully logged into IMAP server");
info!(context, "Successfully logged into IMAP server.");
return Ok(session);
}

Expand Down Expand Up @@ -459,10 +457,10 @@ impl Imap {
Err(first_error.unwrap_or_else(|| format_err!("No IMAP connection candidates provided")))
}

/// Prepare for IMAP operation.
/// Prepare a new IMAP session.
///
/// Ensure that IMAP client is connected, folders are created and IMAP capabilities are
/// determined.
/// This creates a new IMAP connection and ensures
/// that folders are created and IMAP capabilities are determined.
pub(crate) async fn prepare(&mut self, context: &Context) -> Result<Session> {
let configuring = false;
let mut session = match self.connect(context, configuring).await {
Expand Down Expand Up @@ -1048,7 +1046,7 @@ impl Session {
// Expunge folder if needed, e.g. if some jobs have
// deleted messages on the server.
if let Err(err) = self.maybe_close_folder(context).await {
warn!(context, "failed to close folder: {:?}", err);
warn!(context, "Failed to close folder: {err:#}.");
}

Ok(())
Expand Down
17 changes: 9 additions & 8 deletions src/imap/select_folder.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,10 +33,10 @@ impl ImapSession {
pub(super) async fn maybe_close_folder(&mut self, context: &Context) -> anyhow::Result<()> {
if let Some(folder) = &self.selected_folder {
if self.selected_folder_needs_expunge {
info!(context, "Expunge messages in \"{}\".", folder);
info!(context, "Expunge messages in {folder:?}.");

self.close().await.context("IMAP close/expunge failed")?;
info!(context, "close/expunge succeeded");
info!(context, "Close/expunge succeeded.");
self.selected_folder = None;
self.selected_folder_needs_expunge = false;
self.new_mail = false;
Expand Down Expand Up @@ -73,6 +73,7 @@ impl ImapSession {

match res {
Ok(mailbox) => {
info!(context, "Selected folder {folder:?}.");
self.selected_folder = Some(folder.to_string());
self.selected_mailbox = Some(mailbox);
Ok(NewlySelected::Yes)
Expand All @@ -94,7 +95,7 @@ impl ImapSession {
Ok(newly_selected) => Ok(newly_selected),
Err(err) => match err {
Error::NoFolder(..) => {
info!(context, "Failed to select folder {} because it does not exist, trying to create it.", folder);
info!(context, "Failed to select folder {folder:?} because it does not exist, trying to create it.");
let create_res = self.create(folder).await;
if let Err(ref err) = create_res {
info!(context, "Couldn't select folder, then create() failed: {err:#}.");
Expand Down Expand Up @@ -129,30 +130,30 @@ impl ImapSession {
let newly_selected = if create {
self.select_or_create_folder(context, folder)
.await
.with_context(|| format!("failed to select or create folder {folder}"))?
.with_context(|| format!("Failed to select or create folder {folder:?}"))?
} else {
match self.select_folder(context, folder).await {
Ok(newly_selected) => newly_selected,
Err(err) => match err {
Error::NoFolder(..) => return Ok(false),
_ => {
return Err(err)
.with_context(|| format!("failed to select folder {folder}"))?
.with_context(|| format!("Failed to select folder {folder:?}"))?
}
},
}
};
let mailbox = self
.selected_mailbox
.as_mut()
.with_context(|| format!("No mailbox selected, folder: {folder}"))?;
.with_context(|| format!("No mailbox selected, folder: {folder:?}"))?;

let old_uid_validity = get_uidvalidity(context, folder)
.await
.with_context(|| format!("failed to get old UID validity for folder {folder}"))?;
.with_context(|| format!("Failed to get old UID validity for folder {folder:?}"))?;
let old_uid_next = get_uid_next(context, folder)
.await
.with_context(|| format!("failed to get old UID NEXT for folder {folder}"))?;
.with_context(|| format!("Failed to get old UID NEXT for folder {folder:?}"))?;

let new_uid_validity = mailbox
.uid_validity
Expand Down
29 changes: 21 additions & 8 deletions src/scheduler.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -386,7 +386,7 @@ async fn inbox_loop(
) {
use futures::future::FutureExt;

info!(ctx, "starting inbox loop");
info!(ctx, "Starting inbox loop.");
let ImapConnectionHandlers {
mut connection,
stop_receiver,
Expand All @@ -396,7 +396,7 @@ async fn inbox_loop(
let fut = async move {
let ctx = ctx1;
if let Err(()) = started.send(()) {
warn!(ctx, "inbox loop, missing started receiver");
warn!(ctx, "Inbox loop, missing started receiver.");
return;
};

Expand All @@ -405,18 +405,23 @@ async fn inbox_loop(
let session = if let Some(session) = old_session.take() {
session
} else {
info!(ctx, "Preparing new IMAP session for inbox.");
match connection.prepare(&ctx).await {
Err(err) => {
warn!(ctx, "Failed to prepare INBOX connection: {:#}.", err);
warn!(ctx, "Failed to prepare inbox connection: {err:#}.");
continue;
}
Ok(session) => session,
}
};

match inbox_fetch_idle(&ctx, &mut connection, session).await {
Err(err) => warn!(ctx, "Failed fetch_idle: {err:#}"),
Err(err) => warn!(ctx, "Failed inbox fetch_idle: {err:#}."),
Ok(session) => {
info!(
ctx,
"IMAP loop iteration for inbox finished, keeping the session."
);
old_session = Some(session);
}
}
Expand All @@ -426,7 +431,7 @@ async fn inbox_loop(
stop_receiver
.recv()
.map(|_| {
info!(ctx, "shutting down inbox loop");
info!(ctx, "Shutting down inbox loop.");
})
.race(fut)
.await;
Expand Down Expand Up @@ -695,7 +700,7 @@ async fn simple_imap_loop(
) {
use futures::future::FutureExt;

info!(ctx, "starting simple loop for {}", folder_meaning);
info!(ctx, "Starting simple loop for {folder_meaning}.");
let ImapConnectionHandlers {
mut connection,
stop_receiver,
Expand All @@ -706,7 +711,10 @@ async fn simple_imap_loop(
let fut = async move {
let ctx = ctx1;
if let Err(()) = started.send(()) {
warn!(&ctx, "simple imap loop, missing started receiver");
warn!(
ctx,
"Simple imap loop for {folder_meaning}, missing started receiver."
);
return;
}

Expand All @@ -715,6 +723,7 @@ async fn simple_imap_loop(
let session = if let Some(session) = old_session.take() {
session
} else {
info!(ctx, "Preparing new IMAP session for {folder_meaning}.");
match connection.prepare(&ctx).await {
Err(err) => {
warn!(
Expand All @@ -730,6 +739,10 @@ async fn simple_imap_loop(
match fetch_idle(&ctx, &mut connection, session, folder_meaning).await {
Err(err) => warn!(ctx, "Failed fetch_idle: {err:#}"),
Ok(session) => {
info!(
ctx,
"IMAP loop iteration for {folder_meaning} finished, keeping the session"
);
old_session = Some(session);
}
}
Expand All @@ -739,7 +752,7 @@ async fn simple_imap_loop(
stop_receiver
.recv()
.map(|_| {
info!(ctx, "shutting down simple loop");
info!(ctx, "Shutting down IMAP loop for {folder_meaning}.");
})
.race(fut)
.await;
Expand Down
1 change: 0 additions & 1 deletion src/scheduler/connectivity.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -241,7 +241,6 @@ pub(crate) async fn maybe_network_lost(context: &Context, stores: Vec<Connectivi
) {
*connectivity_lock = DetailedConnectivity::Error("Connection lost".to_string());
}
drop(connectivity_lock);
}
context.emit_event(EventType::ConnectivityChanged);
}
Expand Down
Loading