From eecec551c1e5de0f0aa54c3e6f8eb3d55c774bda Mon Sep 17 00:00:00 2001 From: Manos Pitsidianakis Date: Sat, 23 Nov 2019 17:50:22 +0200 Subject: [PATCH] Display watch thread errors to user Show a proper notification with the error message to the user instead of just logging it on debug-tracing. --- melib/src/backends/imap/connection.rs | 52 ++++++++++++++++++++++++--- melib/src/backends/imap/watch.rs | 32 +++++++++++++---- ui/src/conf/accounts.rs | 8 +++++ ui/src/state.rs | 14 +++++--- 4 files changed, 91 insertions(+), 15 deletions(-) diff --git a/melib/src/backends/imap/connection.rs b/melib/src/backends/imap/connection.rs index c2993f41..45abddff 100644 --- a/melib/src/backends/imap/connection.rs +++ b/melib/src/backends/imap/connection.rs @@ -240,11 +240,45 @@ impl ImapStream { .as_bytes(), )?; let mut res = String::with_capacity(8 * 1024); - ret.read_lines(&mut res, &String::new())?; - let capabilities = protocol_parser::capabilities(res.as_bytes()).to_full_result()?; - let capabilities = FnvHashSet::from_iter(capabilities.into_iter().map(|s| s.to_vec())); + let tag_start = format!("M{} ", (ret.cmd_id - 1)); + loop { + ret.read_lines(&mut res, &String::new())?; + if res.starts_with("* OK") { + if let Some(pos) = res.as_bytes().find(b"\r\n") { + let pos = pos + "\r\n".len(); + res.replace_range(..pos, ""); + } + } - Ok((capabilities, ret)) + if res.starts_with(tag_start.as_str()) { + if !res[tag_start.len()..].trim().starts_with("OK ") { + return Err(MeliError::new(format!( + "Could not connect. Server replied with '{}'", + res[tag_start.len()..].trim() + ))); + } + break; + } + } + + let capabilities: std::result::Result, _> = + protocol_parser::capabilities(res.as_bytes()).to_full_result(); + + if capabilities.is_err() { + /* sending CAPABILITY after LOGIN automatically is an RFC recommendation, so check + * for lazy servers */ + drop(capabilities); + ret.send_command(b"CAPABILITY")?; + ret.read_response(&mut res).unwrap(); + let capabilities = protocol_parser::capabilities(res.as_bytes()).to_full_result()?; + let capabilities = FnvHashSet::from_iter(capabilities.into_iter().map(|s| s.to_vec())); + Ok((capabilities, ret)) + } else { + let capabilities = capabilities?; + let capabilities = FnvHashSet::from_iter(capabilities.into_iter().map(|s| s.to_vec())); + + Ok((capabilities, ret)) + } } } @@ -369,6 +403,7 @@ pub struct ImapBlockingConnection { result: Vec, prev_res_length: usize, pub conn: ImapConnection, + err: Option, } impl From for ImapBlockingConnection { @@ -398,6 +433,7 @@ impl From for ImapBlockingConnection { conn, prev_res_length: 0, result: Vec::with_capacity(8 * 1024), + err: None, } } } @@ -406,6 +442,10 @@ impl ImapBlockingConnection { pub fn into_conn(self) -> ImapConnection { self.conn } + + pub fn err(&self) -> Option<&str> { + self.err.as_ref().map(String::as_str) + } } impl Iterator for ImapBlockingConnection { @@ -418,6 +458,7 @@ impl Iterator for ImapBlockingConnection { ref mut result, ref mut conn, ref mut buf, + ref mut err, } = self; loop { if conn.stream.is_err() { @@ -436,7 +477,8 @@ impl Iterator for ImapBlockingConnection { } Err(e) => { debug!(&conn.stream); - debug!(e); + debug!(&e); + *err = Some(e.to_string()); return None; } } diff --git a/melib/src/backends/imap/watch.rs b/melib/src/backends/imap/watch.rs index 05af897e..ff2c009e 100644 --- a/melib/src/backends/imap/watch.rs +++ b/melib/src/backends/imap/watch.rs @@ -108,13 +108,28 @@ pub fn idle(kit: ImapWatchKit) -> Result<()> { std::thread::sleep(std::time::Duration::from_millis(100)); } let thread_id: std::thread::ThreadId = std::thread::current().id(); - let folder: ImapFolder = folders + let folder: ImapFolder = match folders .read() .unwrap() .values() .find(|f| f.parent.is_none() && f.path().eq_ignore_ascii_case("INBOX")) .map(std::clone::Clone::clone) - .unwrap(); + { + Some(folder) => folder, + None => { + let err = MeliError::new("INBOX mailbox not found in local folder index. meli may have not parsed the IMAP folders correctly"); + debug!("failure: {}", err.to_string()); + work_context + .set_status + .send((thread_id, err.to_string())) + .unwrap(); + sender.send(RefreshEvent { + hash: 0, + kind: RefreshEventKind::Failure(err.clone()), + }); + return Err(err); + } + }; let folder_hash = folder.hash(); let mut response = String::with_capacity(8 * 1024); exit_on_error!( @@ -428,16 +443,21 @@ pub fn idle(kit: ImapWatchKit) -> Result<()> { .send((thread_id, "IDLEing".to_string())) .unwrap(); } - debug!("IDLE exited"); + debug!("IDLE connection dropped"); + let err: &str = iter.err().unwrap_or("Unknown reason."); work_context .set_status - .send((thread_id, "IDLE exited".to_string())) + .send((thread_id, "IDLE connection dropped".to_string())) .unwrap(); + work_context.finished.send(thread_id).unwrap(); sender.send(RefreshEvent { hash: folder_hash, - kind: RefreshEventKind::Failure(MeliError::new("IDLE exited".to_string())), + kind: RefreshEventKind::Failure(MeliError::new(format!( + "IDLE connection dropped: {}", + &err + ))), }); - Err(MeliError::new("IDLE exited".to_string())) + Err(MeliError::new(format!("IDLE connection dropped: {}", err))) } fn examine_updates( diff --git a/ui/src/conf/accounts.rs b/ui/src/conf/accounts.rs index 402ed51f..dcc50dbd 100644 --- a/ui/src/conf/accounts.rs +++ b/ui/src/conf/accounts.rs @@ -638,6 +638,14 @@ impl Account { } RefreshEventKind::Failure(e) => { debug!("RefreshEvent Failure: {}", e.to_string()); + context + .1 + .send(ThreadEvent::UIEvent(UIEvent::Notification( + Some(format!("{} watcher exited with error", &self.name)), + e.to_string(), + Some(crate::types::NotificationType::ERROR), + ))) + .expect("Could not send event on main channel"); self.watch(context); } } diff --git a/ui/src/state.rs b/ui/src/state.rs index 10ef6813..359eeae8 100644 --- a/ui/src/state.rs +++ b/ui/src/state.rs @@ -317,10 +317,16 @@ impl State { self.rcv_event(notification); } } else { - debug!( - "BUG: mailbox with hash {} not found in mailbox_hashes.", - hash - ); + if let melib::backends::RefreshEventKind::Failure(e) = event.kind() { + self.context + .sender + .send(ThreadEvent::UIEvent(UIEvent::Notification( + Some("watcher thread exited with error".to_string()), + e.to_string(), + Some(crate::types::NotificationType::ERROR), + ))) + .expect("Could not send event on main channel"); + } } }