From 308403ad99a86312472042ee3769d4906e2adac4 Mon Sep 17 00:00:00 2001 From: Hocuri Date: Thu, 8 Jul 2021 22:50:11 +0200 Subject: [PATCH] Connectivity view (instead of spamming the user with error_network when sth fails) (#2319) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit See https://support.delta.chat/t/discussion-how-to-show-error-states/1363/10 It turns out that it's pretty easy to distinguish between lots of states (currently Error/NotConnected, Connecting…, Getting new messages… and Connected). What's not that easy is distinguishing between an actual error and no network, because if the server just doesn't respond, it could mean that we don't have network or that we are trying ipv6, but only ipv4 works. **WRT debouncing:** Sending of EVENT_CONNECTIVITY_CHANGED is not debounced, but emitted every time one of the 3 threads (Inbox, Mvbox and Sentbox) has a network error, starts fetching data, or is done fetching data. This means that it is emitted: - 9 times when dc_maybe_network() is called or we get network connection - 12 times when we lose network connection Some measurements: dc_get_connectivity() takes a little more than 1ms (in my measurements back in March), dc_get_connectivity_html() takes 10-20ms. This means that it's no immmediate problem to call them very often, might increase battery drain though. For the UI it may be a lot of work to update the title everytime; at least Android is smart enough to update the title only once. Possible problems (we don't have to worry about them now I think): - Due to the scan_folders feature, if the user has lots of folders, the state could be "Connecting..." for quite a long time, generally DC seemed a little unresponsive to me because it took so long for "Connecting..." to go away. Telegram has a state "Updating..." that sometimes comes after "Connecting...". To be done in other PRs: - Better handle the case that the password was changed on the server and authenticating fails, see https://github.com/deltachat/deltachat-core-rust/issues/1923 and https://github.com/deltachat/deltachat-core-rust/issues/1768 - maybe event debouncing (except for "Connected" connectivity events) fix https://github.com/deltachat/deltachat-android/issues/1760 --- CHANGELOG.md | 8 + deltachat-ffi/deltachat.h | 107 +++++++--- deltachat-ffi/src/lib.rs | 48 ++++- examples/repl/main.rs | 3 - examples/simple.rs | 2 +- python/src/deltachat/_build.py | 1 + python/src/deltachat/account.py | 9 + python/src/deltachat/events.py | 27 +++ python/tests/test_account.py | 81 +++++++- src/accounts.rs | 19 ++ src/events.rs | 22 +- src/imap.rs | 61 ++++-- src/imap/idle.rs | 24 +-- src/imap/scan_folders.rs | 16 +- src/imap/select_folder.rs | 8 +- src/job.rs | 6 +- src/log.rs | 11 - src/scheduler.rs | 54 +++-- src/scheduler/connectivity.rs | 349 ++++++++++++++++++++++++++++++++ src/smtp.rs | 24 +-- src/stock_str.rs | 20 +- src/test_utils.rs | 1 - 22 files changed, 747 insertions(+), 154 deletions(-) create mode 100644 src/scheduler/connectivity.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index f1a72d77a..94e4af52a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,13 @@ # Changelog +## Unreleased + +- possibly breaking change: + removed `DC_EVENT_ERROR_NETWORK` and `DC_STR_SERVER_RESPONSE` + Instead, there is a new api `dc_get_connectivity()` + and `dc_get_connectivity_html()`; + `DC_EVENT_CONNECTIVITY_CHANGED` is emitted on changes + ## 1.56.0 - fix downscaling images #2469 diff --git a/deltachat-ffi/deltachat.h b/deltachat-ffi/deltachat.h index 885fbf993..038cbac09 100644 --- a/deltachat-ffi/deltachat.h +++ b/deltachat-ffi/deltachat.h @@ -463,6 +463,60 @@ char* dc_get_info (const dc_context_t* context); char* dc_get_oauth2_url (dc_context_t* context, const char* addr, const char* redirect_uri); +#define DC_CONNECTIVITY_NOT_CONNECTED 1000 +#define DC_CONNECTIVITY_CONNECTING 2000 +#define DC_CONNECTIVITY_WORKING 3000 +#define DC_CONNECTIVITY_CONNECTED 4000 + + +/** + * Get the current connectivity, i.e. whether the device is connected to the IMAP server. + * One of: + * - DC_CONNECTIVITY_NOT_CONNECTED (1000-1999): Show e.g. the string "Not connected" or a red dot + * - DC_CONNECTIVITY_CONNECTING (2000-2999): Show e.g. the string "Connecting…" or a yellow dot + * - DC_CONNECTIVITY_WORKING (3000-3999): Show e.g. the string "Getting new messages" or a spinning wheel + * - DC_CONNECTIVITY_CONNECTED (>=4000): Show e.g. the string "Connected" or a green dot + * + * We don't use exact values but ranges here so that we can split up + * states into multiple states in the future. + * + * Meant as a rough overview that can be shown + * e.g. in the title of the main screen. + * + * If the connectivity changes, a DC_EVENT_CONNECTIVITY_CHANGED will be emitted. + * + * @memberof dc_context_t + * @param context The context object. + * @return The current connectivity. + */ +int dc_get_connectivity (dc_context_t* context); + + +/** + * Get an overview of the current connectivity, and possibly more statistics. + * Meant to give the user more insight about the current status than + * the basic connectivity info returned by dc_get_connectivity(); show this + * e.g., if the user taps on said basic connectivity info. + * + * If this page changes, a DC_EVENT_CONNECTIVITY_CHANGED will be emitted. + * + * This comes as an HTML from the core so that we can easily improve it + * and the improvement instantly reaches all UIs. + * + * @memberof dc_context_t + * @param context The context object. + * @return An HTML page with some info about the current connectivity and status. + */ +char* dc_get_connectivity_html (dc_context_t* context); + + +/** + * Standalone version of dc_accounts_all_work_done(). + * Only used by the python tests. + */ +int dc_all_work_done (dc_context_t* context); + + // connect /** @@ -2544,6 +2598,23 @@ dc_context_t* dc_accounts_get_selected_account (dc_accounts_t* accounts); int dc_accounts_select_account (dc_accounts_t* accounts, uint32_t account_id); +/** + * This is meant especially for iOS, because iOS needs to tell the system when its background work is done. + * + * iOS can: + * - call dc_start_io() (in case IO was not running) + * - call dc_maybe_network() + * - while dc_accounts_all_work_done() returns false: + * - Wait for DC_EVENT_CONNECTIVITY_CHANGED + * + * @memberof dc_accounts_t + * @param accounts Account manager as created by dc_accounts_new(). + * @return Whether all accounts finished their background work. + * DC_EVENT_CONNECTIVITY_CHANGED will be sent when this turns to true. + */ +int dc_accounts_all_work_done (dc_accounts_t* accounts); + + /** * Start job and IMAP/SMTP tasks for all accounts managed by the account manager. * If IO is already running, nothing happens. @@ -4955,26 +5026,6 @@ void dc_event_unref(dc_event_t* event); #define DC_EVENT_ERROR 400 -/** - * An action cannot be performed because there is no network available. - * - * The library will typically try over after a some time - * and when dc_maybe_network() is called. - * - * Network errors should be reported to users in a non-disturbing way, - * however, as network errors may come in a sequence, - * it is not useful to raise each an every error to the user. - * - * Moreover, if the UI detects that the device is offline, - * it is probably more useful to report this to the user - * instead of the string from data2. - * - * @param data1 0 - * @param data2 (char*) Error string, always set, never NULL. - */ -#define DC_EVENT_ERROR_NETWORK 401 - - /** * An action cannot be performed because the user is not in the group. * Reported e.g. after a call to @@ -5159,6 +5210,15 @@ void dc_event_unref(dc_event_t* event); */ #define DC_EVENT_SECUREJOIN_JOINER_PROGRESS 2061 + +/** + * The connectivity to the server changed. + * This means that you should refresh the connectivity view + * and possibly the connectivtiy HTML; see dc_get_connectivity() and + * dc_get_connectivity_html() for details. + */ +#define DC_EVENT_CONNECTIVITY_CHANGED 2100 + /** * @} */ @@ -5486,13 +5546,6 @@ void dc_event_unref(dc_event_t* event); /// - %1$s will be replaced by the failing login name #define DC_STR_CANNOT_LOGIN 60 -/// "Could not connect to %1$s: %2$s" -/// -/// Used in error strings. -/// - %1$s will be replaced by the failing server -/// - %2$s by a the error message as returned from the server -#define DC_STR_SERVER_RESPONSE 61 - /// "%1$s by %2$s" /// /// Used to concretize actions, diff --git a/deltachat-ffi/src/lib.rs b/deltachat-ffi/src/lib.rs index 01b7da575..b92f9d443 100644 --- a/deltachat-ffi/src/lib.rs +++ b/deltachat-ffi/src/lib.rs @@ -256,6 +256,38 @@ fn render_info( Ok(res) } +#[no_mangle] +pub unsafe extern "C" fn dc_get_connectivity(context: *const dc_context_t) -> libc::c_int { + if context.is_null() { + eprintln!("ignoring careless call to dc_get_connectivity()"); + return 0; + } + let ctx = &*context; + block_on(async move { ctx.get_connectivity().await as u32 as libc::c_int }) +} + +#[no_mangle] +pub unsafe extern "C" fn dc_get_connectivity_html( + context: *const dc_context_t, +) -> *mut libc::c_char { + if context.is_null() { + eprintln!("ignoring careless call to dc_get_connectivity_html()"); + return "".strdup(); + } + let ctx = &*context; + block_on(async move { ctx.get_connectivity_html().await.strdup() }) +} + +#[no_mangle] +pub unsafe extern "C" fn dc_all_work_done(context: *mut dc_context_t) -> libc::c_int { + if context.is_null() { + eprintln!("ignoring careless call to dc_all_work_done()"); + return 0; + } + let ctx = &*context; + block_on(async move { ctx.all_work_done().await as libc::c_int }) +} + #[no_mangle] pub unsafe extern "C" fn dc_get_oauth2_url( context: *mut dc_context_t, @@ -368,7 +400,7 @@ pub unsafe extern "C" fn dc_event_get_data1_int(event: *mut dc_event_t) -> libc: | EventType::DeletedBlobFile(_) | EventType::Warning(_) | EventType::Error(_) - | EventType::ErrorNetwork(_) + | EventType::ConnectivityChanged | EventType::ErrorSelfNotInGroup(_) => 0, EventType::MsgsChanged { chat_id, .. } | EventType::IncomingMsg { chat_id, .. } @@ -411,7 +443,6 @@ pub unsafe extern "C" fn dc_event_get_data2_int(event: *mut dc_event_t) -> libc: | EventType::DeletedBlobFile(_) | EventType::Warning(_) | EventType::Error(_) - | EventType::ErrorNetwork(_) | EventType::ErrorSelfNotInGroup(_) | EventType::ContactsChanged(_) | EventType::LocationChanged(_) @@ -419,6 +450,7 @@ pub unsafe extern "C" fn dc_event_get_data2_int(event: *mut dc_event_t) -> libc: | EventType::ImexProgress(_) | EventType::ImexFileWritten(_) | EventType::MsgsNoticed(_) + | EventType::ConnectivityChanged | EventType::ChatModified(_) => 0, EventType::MsgsChanged { msg_id, .. } | EventType::IncomingMsg { msg_id, .. } @@ -451,7 +483,6 @@ pub unsafe extern "C" fn dc_event_get_data2_str(event: *mut dc_event_t) -> *mut | EventType::DeletedBlobFile(msg) | EventType::Warning(msg) | EventType::Error(msg) - | EventType::ErrorNetwork(msg) | EventType::ErrorSelfNotInGroup(msg) => { let data2 = msg.to_c_string().unwrap_or_default(); data2.into_raw() @@ -468,6 +499,7 @@ pub unsafe extern "C" fn dc_event_get_data2_str(event: *mut dc_event_t) -> *mut | EventType::ImexProgress(_) | EventType::SecurejoinInviterProgress { .. } | EventType::SecurejoinJoinerProgress { .. } + | EventType::ConnectivityChanged | EventType::ChatEphemeralTimerModified { .. } => ptr::null_mut(), EventType::ConfigureProgress { comment, .. } => { if let Some(comment) = comment { @@ -3799,6 +3831,16 @@ pub unsafe extern "C" fn dc_accounts_get_all(accounts: *mut dc_accounts_t) -> *m Box::into_raw(Box::new(array)) } +#[no_mangle] +pub unsafe extern "C" fn dc_accounts_all_work_done(accounts: *mut dc_accounts_t) -> libc::c_int { + if accounts.is_null() { + eprintln!("ignoring careless call to dc_accounts_all_work_done()"); + return 0; + } + let accounts = &*accounts; + block_on(async move { accounts.all_work_done().await as libc::c_int }) +} + #[no_mangle] pub unsafe extern "C" fn dc_accounts_start_io(accounts: *mut dc_accounts_t) { if accounts.is_null() { diff --git a/examples/repl/main.rs b/examples/repl/main.rs index 328389870..68fb54947 100644 --- a/examples/repl/main.rs +++ b/examples/repl/main.rs @@ -57,9 +57,6 @@ fn receive_event(event: EventType) { EventType::Error(msg) => { error!("{}", msg); } - EventType::ErrorNetwork(msg) => { - error!("[NETWORK] msg={}", msg); - } EventType::ErrorSelfNotInGroup(msg) => { error!("[SELF_NOT_IN_GROUP] {}", msg); } diff --git a/examples/simple.rs b/examples/simple.rs index 9fe2dfc7c..2aa0ca960 100644 --- a/examples/simple.rs +++ b/examples/simple.rs @@ -19,7 +19,7 @@ fn cb(event: EventType) { EventType::Warning(msg) => { log::warn!("{}", msg); } - EventType::Error(msg) | EventType::ErrorNetwork(msg) => { + EventType::Error(msg) => { log::error!("{}", msg); } event => { diff --git a/python/src/deltachat/_build.py b/python/src/deltachat/_build.py index 13dffc74d..e2ff57645 100644 --- a/python/src/deltachat/_build.py +++ b/python/src/deltachat/_build.py @@ -150,6 +150,7 @@ def extract_defines(flags): | DC_PROVIDER | DC_KEY_GEN | DC_IMEX + | DC_CONNECTIVITY ) # End of prefix matching _[\w_]+ # Match the suffix, e.g. _RSA2048 in DC_KEY_GEN_RSA2048 ) # Close the capturing group, this contains diff --git a/python/src/deltachat/account.py b/python/src/deltachat/account.py index 36c05b255..8e718fb7b 100644 --- a/python/src/deltachat/account.py +++ b/python/src/deltachat/account.py @@ -574,6 +574,15 @@ class Account(object): """ Stop ongoing securejoin, configuration or other core jobs. """ lib.dc_stop_ongoing_process(self._dc_context) + def get_connectivity(self): + return lib.dc_get_connectivity(self._dc_context) + + def get_connectivity_html(self): + return from_dc_charpointer(lib.dc_get_connectivity_html(self._dc_context)) + + def all_work_done(self): + return lib.dc_all_work_done(self._dc_context) + def start_io(self): """ start this account's IO scheduling (Rust-core async scheduler) diff --git a/python/src/deltachat/events.py b/python/src/deltachat/events.py index de733fd73..2b1445244 100644 --- a/python/src/deltachat/events.py +++ b/python/src/deltachat/events.py @@ -111,6 +111,33 @@ class FFIEventTracker: if m is not None: return m.groups() + def wait_for_connectivity(self, connectivity): + """Wait for the specified connectivity. + This only works reliably if the connectivity doesn't change + again too quickly, otherwise we might miss it.""" + while 1: + if self.account.get_connectivity() == connectivity: + return + self.get_matching("DC_EVENT_CONNECTIVITY_CHANGED") + + def wait_for_connectivity_change(self, previous, expected_next): + """Wait until the connectivity changes to `expected_next`. + Fails the test if it changes to something else.""" + while 1: + current = self.account.get_connectivity() + if current == expected_next: + return + elif current != previous: + raise Exception("Expected connectivity " + str(expected_next) + " but got " + str(current)) + + self.get_matching("DC_EVENT_CONNECTIVITY_CHANGED") + + def wait_for_all_work_done(self): + while 1: + if self.account.all_work_done(): + return + self.get_matching("DC_EVENT_CONNECTIVITY_CHANGED") + def ensure_event_not_queued(self, event_name_regex): __tracebackhide__ = True rex = re.compile("(?:{}).*".format(event_name_regex)) diff --git a/python/tests/test_account.py b/python/tests/test_account.py index e16ed1635..24dd7ce55 100644 --- a/python/tests/test_account.py +++ b/python/tests/test_account.py @@ -2042,6 +2042,84 @@ class TestOnlineAccount: assert msg_back.chat == chat assert chat.get_profile_image() is None + @pytest.mark.parametrize("inbox_watch", ["0", "1"]) + def test_connectivity(self, acfactory, lp, inbox_watch): + ac1, ac2 = acfactory.get_two_online_accounts() + ac1.set_config("inbox_watch", inbox_watch) + ac1.set_config("scan_all_folders_debounce_secs", "0") + + ac1._evtracker.wait_for_connectivity(const.DC_CONNECTIVITY_CONNECTED) + + lp.sec("Test stop_io() and start_io()") + ac1.stop_io() + ac1._evtracker.wait_for_connectivity(const.DC_CONNECTIVITY_NOT_CONNECTED) + + ac1.start_io() + ac1._evtracker.wait_for_connectivity(const.DC_CONNECTIVITY_CONNECTING) + ac1._evtracker.wait_for_connectivity_change(const.DC_CONNECTIVITY_CONNECTING, const.DC_CONNECTIVITY_CONNECTED) + + lp.sec("Test that after calling start_io(), maybe_network() and waiting for `all_work_done()`, " + + "all messages are fetched") + + ac1.direct_imap.select_config_folder("inbox") + ac1.direct_imap.idle_start() + ac2.create_chat(ac1).send_text("Hi") + + ac1.direct_imap.idle_check(terminate=False) + ac1.maybe_network() + + ac1._evtracker.wait_for_all_work_done() + msgs = ac1.create_chat(ac2).get_messages() + assert len(msgs) == 1 + assert msgs[0].text == "Hi" + + lp.sec("Test that the connectivity changes to WORKING while new messages are fetched") + + ac2.create_chat(ac1).send_text("Hi 2") + + ac1.direct_imap.idle_check(terminate=True) + ac1.maybe_network() + ac1._evtracker.wait_for_connectivity_change(const.DC_CONNECTIVITY_CONNECTED, const.DC_CONNECTIVITY_WORKING) + ac1._evtracker.wait_for_connectivity_change(const.DC_CONNECTIVITY_WORKING, const.DC_CONNECTIVITY_CONNECTED) + + msgs = ac1.create_chat(ac2).get_messages() + assert len(msgs) == 2 + assert msgs[1].text == "Hi 2" + + lp.sec("Test that the connectivity doesn't flicker to WORKING if there are no new messages") + + ac1.maybe_network() + while 1: + assert ac1.get_connectivity() == const.DC_CONNECTIVITY_CONNECTED + if ac1.all_work_done(): + break + ac1._evtracker.get_matching("DC_EVENT_CONNECTIVITY_CHANGED") + + lp.sec("Test that the connectivity doesn't flicker to WORKING if the sender of the message is blocked") + ac1.create_contact(ac2).block() + + ac1.direct_imap.select_config_folder("inbox") + ac1.direct_imap.idle_start() + ac2.create_chat(ac1).send_text("Hi") + + ac1.direct_imap.idle_check(terminate=True) + ac1.maybe_network() + + while 1: + assert ac1.get_connectivity() == const.DC_CONNECTIVITY_CONNECTED + if ac1.all_work_done(): + break + ac1._evtracker.get_matching("DC_EVENT_CONNECTIVITY_CHANGED") + + lp.sec("Test that the connectivity is NOT_CONNECTED if the password is wrong") + + ac1.set_config("configured_mail_pw", "abc") + ac1.stop_io() + ac1._evtracker.wait_for_connectivity(const.DC_CONNECTIVITY_NOT_CONNECTED) + ac1.start_io() + ac1._evtracker.wait_for_connectivity(const.DC_CONNECTIVITY_CONNECTING) + ac1._evtracker.wait_for_connectivity(const.DC_CONNECTIVITY_NOT_CONNECTED) + def test_fetch_deleted_msg(self, acfactory, lp): """This is a regression test: Messages with \\Deleted flag were downloaded again and again, hundreds of times, because uid_next was not updated. @@ -2758,7 +2836,6 @@ class TestOnlineConfigureFails: configtracker = ac1.configure() configtracker.wait_progress(500) configtracker.wait_progress(0) - ac1._evtracker.ensure_event_not_queued("DC_EVENT_ERROR_NETWORK") def test_invalid_user(self, acfactory): ac1, configdict = acfactory.get_online_config() @@ -2766,7 +2843,6 @@ class TestOnlineConfigureFails: configtracker = ac1.configure() configtracker.wait_progress(500) configtracker.wait_progress(0) - ac1._evtracker.ensure_event_not_queued("DC_EVENT_ERROR_NETWORK") def test_invalid_domain(self, acfactory): ac1, configdict = acfactory.get_online_config() @@ -2774,4 +2850,3 @@ class TestOnlineConfigureFails: configtracker = ac1.configure() configtracker.wait_progress(500) configtracker.wait_progress(0) - ac1._evtracker.ensure_event_not_queued("DC_EVENT_ERROR_NETWORK") diff --git a/src/accounts.rs b/src/accounts.rs index 67427b971..27421ed8f 100644 --- a/src/accounts.rs +++ b/src/accounts.rs @@ -179,6 +179,25 @@ impl Accounts { self.accounts.read().await.keys().copied().collect() } + /// This is meant especially for iOS, because iOS needs to tell the system when its background work is done. + /// + /// Returns whether all accounts finished their background work. + /// DC_EVENT_CONNECTIVITY_CHANGED will be sent when this turns to true. + /// + /// iOS can: + /// - call dc_start_io() (in case IO was not running) + /// - call dc_maybe_network() + /// - while dc_accounts_all_work_done() returns false: + /// - Wait for DC_EVENT_CONNECTIVITY_CHANGED + pub async fn all_work_done(&self) -> bool { + for account in self.accounts.read().await.values() { + if !account.all_work_done().await { + return false; + } + } + true + } + pub async fn start_io(&self) { let accounts = &*self.accounts.read().await; for account in accounts.values() { diff --git a/src/events.rs b/src/events.rs index 308182e4c..d2c269894 100644 --- a/src/events.rs +++ b/src/events.rs @@ -185,21 +185,6 @@ pub enum EventType { #[strum(props(id = "400"))] Error(String), - /// An action cannot be performed because there is no network available. - /// - /// The library will typically try over after a some time - /// and when dc_maybe_network() is called. - /// - /// Network errors should be reported to users in a non-disturbing way, - /// however, as network errors may come in a sequence, - /// it is not useful to raise each an every error to the user. - /// - /// Moreover, if the UI detects that the device is offline, - /// it is probably more useful to report this to the user - /// instead of the string from data2. - #[strum(props(id = "401"))] - ErrorNetwork(String), - /// An action cannot be performed because the user is not in the group. /// Reported eg. after a call to /// dc_set_chat_name(), dc_set_chat_profile_image(), @@ -330,4 +315,11 @@ pub enum EventType { /// (Bob has verified alice and waits until Alice does the same for him) #[strum(props(id = "2061"))] SecurejoinJoinerProgress { contact_id: u32, progress: usize }, + + /// The connectivity to the server changed. + /// This means that you should refresh the connectivity view + /// and possibly the connectivtiy HTML; see dc_get_connectivity() and + /// dc_get_connectivity_html() for details. + #[strum(props(id = "2100"))] + ConnectivityChanged, } diff --git a/src/imap.rs b/src/imap.rs index f3e9f221d..4806ee52b 100644 --- a/src/imap.rs +++ b/src/imap.rs @@ -8,13 +8,12 @@ use std::{cmp, cmp::max, collections::BTreeMap}; use anyhow::{bail, format_err, Context as _, Result}; use async_imap::{ error::Result as ImapResult, - types::{Fetch, Flag, Mailbox, Name, NameAttribute}, + types::{Fetch, Flag, Mailbox, Name, NameAttribute, UnsolicitedResponse}, }; use async_std::channel::Receiver; use async_std::prelude::*; use num_traits::FromPrimitive; -use crate::config::Config; use crate::constants::{ Chattype, ShowEmails, Viewtype, DC_FETCH_EXISTING_MSGS_COUNT, DC_FOLDERS_CONFIGURED_VERSION, DC_LP_AUTH_OAUTH2, @@ -36,6 +35,7 @@ use crate::provider::Socket; use crate::scheduler::InterruptInfo; use crate::stock_str; use crate::{chat, constants::DC_CONTACT_ID_SELF}; +use crate::{config::Config, scheduler::connectivity::ConnectivityStore}; mod client; mod idle; @@ -96,6 +96,8 @@ pub struct Imap { /// True if CAPABILITY command was run successfully once and config.can_* contain correct /// values. capabilities_determined: bool, + + pub(crate) connectivity: ConnectivityStore, } #[derive(Debug)] @@ -194,6 +196,7 @@ impl Imap { interrupt: None, should_reconnect: false, login_failed_once: false, + connectivity: Default::default(), capabilities_determined: false, }; @@ -227,9 +230,6 @@ impl Imap { /// /// It is safe to call this function if already connected, actions are performed only as needed. /// - /// Does not emit network errors, can be used to try various parameters during - /// autoconfiguration. - /// /// 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 /// parameters. @@ -245,6 +245,8 @@ impl Imap { return Ok(()); } + self.connectivity.set_connecting(context).await; + let oauth2 = self.config.oauth2; let connection_res: ImapResult = if self.config.lp.security == Socket::Starttls @@ -343,7 +345,7 @@ impl Imap { self.login_failed_once = true; } - self.trigger_reconnect(); + self.trigger_reconnect(context).await; Err(format_err!("{}\n\n{}", message, err)) } } @@ -377,18 +379,15 @@ impl Imap { /// /// Ensure that IMAP client is connected, folders are created and IMAP capabilities are /// determined. - /// - /// This function emits network error if it fails. It should not be used during configuration - /// to avoid showing failed attempt errors to the user. pub async fn prepare(&mut self, context: &Context) -> Result<()> { - let res = self.connect(context).await; - if let Err(ref err) = res { - emit_event!(context, EventType::ErrorNetwork(err.to_string())); + if let Err(err) = self.connect(context).await { + self.connectivity.set_err(context, &err).await; + return Err(err); } self.ensure_configured_folders(context, true).await?; self.determine_capabilities().await?; - res + Ok(()) } async fn disconnect(&mut self, context: &Context) { @@ -417,7 +416,8 @@ impl Imap { self.should_reconnect } - pub fn trigger_reconnect(&mut self) { + pub async fn trigger_reconnect(&mut self, context: &Context) { + self.connectivity.set_connecting(context).await; self.should_reconnect = true; } @@ -678,6 +678,10 @@ impl Imap { } } + if !uids.is_empty() { + self.connectivity.set_working(context).await; + } + let (largest_uid_processed, error_cnt) = self .fetch_many_msgs(context, folder, uids, fetch_existing_msgs) .await; @@ -845,7 +849,7 @@ impl Imap { if self.session.is_none() { // we could not get a valid imap session, this should be retried - self.trigger_reconnect(); + self.trigger_reconnect(context).await; warn!(context, "Could not get IMAP session"); return (None, server_uids.len()); } @@ -1363,6 +1367,31 @@ impl Imap { info!(context, "FINISHED configuring IMAP-folders."); Ok(()) } + + /// Return whether the server sent an unsolicited EXISTS response. + /// Drains all responses from `session.unsolicited_responses` in the process. + /// If this returns `true`, this means that new emails arrived and you should + /// fetch again, even if you just fetched. + fn server_sent_unsolicited_exists(&self, context: &Context) -> bool { + let session = match &self.session { + Some(s) => s, + None => return false, + }; + let mut unsolicited_exists = false; + while let Ok(response) = session.unsolicited_responses.try_recv() { + match response { + UnsolicitedResponse::Exists(_) => { + info!( + context, + "Need to fetch again, got unsolicited EXISTS {:?}", response + ); + unsolicited_exists = true; + } + _ => info!(context, "ignoring unsolicited response {:?}", response), + } + } + unsolicited_exists + } } /// Try to get the folder meaning by the name of the folder only used if the server does not support XLIST. @@ -1595,7 +1624,7 @@ pub(crate) async fn prefetch_should_download( let is_reply_to_chat_message = parent.is_some(); if let Some(parent) = &parent { let chat = chat::Chat::load_from_db(context, parent.get_chat_id()).await?; - if chat.typ == Chattype::Group { + if chat.typ == Chattype::Group && !chat.id.is_special() { // This might be a group command, like removing a group member. // We really need to fetch this to avoid inconsistent group state. return Ok(true); diff --git a/src/imap/idle.rs b/src/imap/idle.rs index ce59e2be4..e397c435e 100644 --- a/src/imap/idle.rs +++ b/src/imap/idle.rs @@ -2,7 +2,6 @@ use super::Imap; use anyhow::{bail, format_err, Result}; use async_imap::extensions::idle::IdleResponse; -use async_imap::types::UnsolicitedResponse; use async_std::prelude::*; use std::time::{Duration, SystemTime}; @@ -32,24 +31,11 @@ impl Imap { let timeout = Duration::from_secs(23 * 60); let mut info = Default::default(); + if self.server_sent_unsolicited_exists(context) { + return Ok(info); + } + if let Some(session) = self.session.take() { - // if we have unsolicited responses we directly return - let mut unsolicited_exists = false; - while let Ok(response) = session.unsolicited_responses.try_recv() { - match response { - UnsolicitedResponse::Exists(_) => { - warn!(context, "skip idle, got unsolicited EXISTS {:?}", response); - unsolicited_exists = true; - } - _ => info!(context, "ignoring unsolicited response {:?}", response), - } - } - - if unsolicited_exists { - self.session = Some(session); - return Ok(info); - } - if let Ok(info) = self.idle_interrupt.try_recv() { info!(context, "skip idle, got interrupt {:?}", info); self.session = Some(session); @@ -181,7 +167,7 @@ impl Imap { } Err(err) => { error!(context, "could not fetch from folder: {:#}", err); - self.trigger_reconnect() + self.trigger_reconnect(context).await; } } } diff --git a/src/imap/scan_folders.rs b/src/imap/scan_folders.rs index 5c7d982f9..3ab09bf6d 100644 --- a/src/imap/scan_folders.rs +++ b/src/imap/scan_folders.rs @@ -61,9 +61,19 @@ impl Imap { // Don't scan folders that are watched anyway if !watched_folders.contains(&folder.name().to_string()) && !is_drafts { - self.fetch_new_messages(context, folder.name(), false) - .await - .ok_or_log_msg(context, "Can't fetch new msgs in scanned folder"); + // Drain leftover unsolicited EXISTS messages + self.server_sent_unsolicited_exists(context); + + loop { + self.fetch_new_messages(context, folder.name(), false) + .await + .ok_or_log_msg(context, "Can't fetch new msgs in scanned folder"); + + // If the server sent an unsocicited EXISTS during the fetch, we need to fetch again + if !self.server_sent_unsolicited_exists(context) { + break; + } + } } } diff --git a/src/imap/select_folder.rs b/src/imap/select_folder.rs index a07c94b0c..81efe1103 100644 --- a/src/imap/select_folder.rs +++ b/src/imap/select_folder.rs @@ -37,7 +37,7 @@ impl Imap { info!(context, "close/expunge succeeded"); } Err(err) => { - self.trigger_reconnect(); + self.trigger_reconnect(context).await; return Err(Error::CloseExpungeFailed(err)); } } @@ -70,7 +70,7 @@ impl Imap { if self.session.is_none() { self.config.selected_folder = None; self.config.selected_folder_needs_expunge = false; - self.trigger_reconnect(); + self.trigger_reconnect(context).await; return Err(Error::NoSession); } @@ -103,7 +103,7 @@ impl Imap { Ok(NewlySelected::Yes) } Err(async_imap::error::Error::ConnectionLost) => { - self.trigger_reconnect(); + self.trigger_reconnect(context).await; self.config.selected_folder = None; Err(Error::ConnectionLost) } @@ -112,7 +112,7 @@ impl Imap { } Err(err) => { self.config.selected_folder = None; - self.trigger_reconnect(); + self.trigger_reconnect(context).await; Err(Error::Other(err.to_string())) } } diff --git a/src/job.rs b/src/job.rs index 4612bec24..7e53996e5 100644 --- a/src/job.rs +++ b/src/job.rs @@ -249,10 +249,14 @@ impl Job { info!(context, "smtp-sending out mime message:"); println!("{}", String::from_utf8_lossy(&message)); } + + smtp.connectivity.set_working(context).await; + let status = match smtp.send(context, recipients, message, job_id).await { Err(crate::smtp::send::Error::SendError(err)) => { // Remote error, retry later. - warn!(context, "SMTP failed to send: {:?}", err); + warn!(context, "SMTP failed to send: {:?}", &err); + smtp.connectivity.set_err(context, &err).await; self.pending_error = Some(err.to_string()); let res = match err { diff --git a/src/log.rs b/src/log.rs index 64123e140..0dc9dc434 100644 --- a/src/log.rs +++ b/src/log.rs @@ -42,17 +42,6 @@ macro_rules! error { }}; } -#[macro_export] -macro_rules! error_network { - ($ctx:expr, $msg:expr) => { - error_network!($ctx, $msg,) - }; - ($ctx:expr, $msg:expr, $($args:expr),* $(,)?) => {{ - let formatted = format!($msg, $($args),*); - emit_event!($ctx, $crate::EventType::ErrorNetwork(formatted)); - }}; -} - #[macro_export] macro_rules! emit_event { ($ctx:expr, $event:expr) => { diff --git a/src/scheduler.rs b/src/scheduler.rs index c96510d70..7e15bb865 100644 --- a/src/scheduler.rs +++ b/src/scheduler.rs @@ -13,6 +13,10 @@ use crate::job::{self, Thread}; use crate::message::MsgId; use crate::smtp::Smtp; +use self::connectivity::ConnectivityStore; + +pub(crate) mod connectivity; + pub(crate) struct StopToken; /// Job and connection scheduler. @@ -35,7 +39,9 @@ pub(crate) enum Scheduler { impl Context { /// Indicate that the network likely has come back. pub async fn maybe_network(&self) { - self.scheduler.read().await.maybe_network().await; + let lock = self.scheduler.read().await; + lock.maybe_network().await; + connectivity::idle_interrupted(lock).await; } pub(crate) async fn interrupt_inbox(&self, info: InterruptInfo) { @@ -107,6 +113,9 @@ async fn inbox_loop(ctx: Context, started: Sender<()>, inbox_handlers: ImapConne } else { if let Err(err) = connection.scan_folders(&ctx).await { warn!(ctx, "{}", err); + connection.connectivity.set_err(&ctx, err).await; + } else { + connection.connectivity.set_not_configured(&ctx).await; } connection.fake_idle(&ctx, None).await }; @@ -132,26 +141,24 @@ async fn fetch(ctx: &Context, connection: &mut Imap) { match ctx.get_config(Config::ConfiguredInboxFolder).await { Ok(Some(watch_folder)) => { if let Err(err) = connection.prepare(ctx).await { - error_network!(ctx, "{}", err); + warn!(ctx, "Could not connect: {}", err); return; } // fetch if let Err(err) = connection.fetch(ctx, &watch_folder).await { - connection.trigger_reconnect(); + connection.trigger_reconnect(ctx).await; warn!(ctx, "{:#}", err); } } Ok(None) => { - warn!(ctx, "Can not fetch inbox folder, not set"); - connection.fake_idle(ctx, None).await; + info!(ctx, "Can not fetch inbox folder, not set"); } Err(err) => { warn!( ctx, "Can not fetch inbox folder, failed to get config: {:?}", err ); - connection.fake_idle(ctx, None).await; } } } @@ -167,8 +174,9 @@ async fn fetch_idle(ctx: &Context, connection: &mut Imap, folder: Config) -> Int // fetch if let Err(err) = connection.fetch(ctx, &watch_folder).await { - connection.trigger_reconnect(); + connection.trigger_reconnect(ctx).await; warn!(ctx, "{:#}", err); + return InterruptInfo::new(false, None); } if folder == Config::ConfiguredInboxFolder { @@ -180,22 +188,25 @@ async fn fetch_idle(ctx: &Context, connection: &mut Imap, folder: Config) -> Int } } + connection.connectivity.set_connected(ctx).await; + // idle if connection.can_idle() { - connection - .idle(ctx, Some(watch_folder)) - .await - .unwrap_or_else(|err| { - connection.trigger_reconnect(); + match connection.idle(ctx, Some(watch_folder)).await { + Ok(v) => v, + Err(err) => { + connection.trigger_reconnect(ctx).await; warn!(ctx, "{}", err); InterruptInfo::new(false, None) - }) + } + } } else { connection.fake_idle(ctx, Some(watch_folder)).await } } Ok(None) => { - warn!(ctx, "Can not watch {} folder, not set", folder); + connection.connectivity.set_not_configured(ctx).await; + info!(ctx, "Can not watch {} folder, not set", folder); connection.fake_idle(ctx, None).await } Err(err) => { @@ -280,6 +291,7 @@ async fn smtp_loop(ctx: Context, started: Sender<()>, smtp_handlers: SmtpConnect None => { // Fake Idle info!(ctx, "smtp fake idle - started"); + connection.connectivity.set_connected(&ctx).await; interrupt_info = idle_interrupt_receiver.recv().await.unwrap_or_default(); info!(ctx, "smtp fake idle - interrupted") } @@ -338,6 +350,11 @@ impl Scheduler { .send(()) .await .expect("mvbox start send, missing receiver"); + mvbox_handlers + .connection + .connectivity + .set_not_configured(&ctx) + .await } if ctx.get_config_bool(Config::SentboxWatch).await? { @@ -356,6 +373,11 @@ impl Scheduler { .send(()) .await .expect("sentbox start send, missing receiver"); + sentbox_handlers + .connection + .connectivity + .set_not_configured(&ctx) + .await } let smtp_handle = { @@ -508,6 +530,8 @@ struct ConnectionState { stop_sender: Sender<()>, /// Channel to interrupt idle. idle_interrupt_sender: Sender, + /// Mutex to pass connectivity info between IMAP/SMTP threads and the API + connectivity: ConnectivityStore, } impl ConnectionState { @@ -550,6 +574,7 @@ impl SmtpConnectionState { shutdown_receiver, stop_sender, idle_interrupt_sender, + connectivity: handlers.connection.connectivity.clone(), }; let conn = SmtpConnectionState { state }; @@ -597,6 +622,7 @@ impl ImapConnectionState { shutdown_receiver, stop_sender, idle_interrupt_sender, + connectivity: handlers.connection.connectivity.clone(), }; let conn = ImapConnectionState { state }; diff --git a/src/scheduler/connectivity.rs b/src/scheduler/connectivity.rs new file mode 100644 index 000000000..25eff175b --- /dev/null +++ b/src/scheduler/connectivity.rs @@ -0,0 +1,349 @@ +use core::fmt; +use std::{ops::Deref, sync::Arc}; + +use async_std::sync::{Mutex, RwLockReadGuard}; + +use crate::events::EventType; +use crate::{config::Config, scheduler::Scheduler}; +use crate::{context::Context, log::LogExt}; + +#[derive(Debug, Clone, Copy, PartialEq, Eq, EnumProperty, PartialOrd, Ord)] +pub enum Connectivity { + NotConnected = 1000, + Connecting = 2000, + /// Fetching or sending messages + Working = 3000, + Connected = 4000, +} + +// The order of the connectivities is important: worse connectivities (i.e. those at +// the top) take priority. This means that e.g. if any folder has an error - usually +// because there is no internet connection - the connectivity for the whole +// account will be `Notconnected`. +#[derive(Debug, Clone, PartialEq, Eq, EnumProperty)] +enum DetailedConnectivity { + Error(String), + Uninitialized, + Connecting, + Working, + InterruptingIdle, + Connected, + + /// The folder was configured not to be watched or configured_*_folder is not set + NotConfigured, +} + +impl Default for DetailedConnectivity { + fn default() -> Self { + DetailedConnectivity::Uninitialized + } +} + +impl DetailedConnectivity { + fn to_basic(&self) -> Option { + match self { + DetailedConnectivity::Error(_) => Some(Connectivity::NotConnected), + DetailedConnectivity::Uninitialized => Some(Connectivity::NotConnected), + DetailedConnectivity::Connecting => Some(Connectivity::Connecting), + DetailedConnectivity::Working => Some(Connectivity::Working), + DetailedConnectivity::InterruptingIdle => Some(Connectivity::Connected), + DetailedConnectivity::Connected => Some(Connectivity::Connected), + + // Just don't return a connectivity, probably the folder is configured not to be + // watched or there is e.g. no "Sent" folder, so we are not interested in it + DetailedConnectivity::NotConfigured => None, + } + } + + fn to_string_imap(&self, _context: &Context) -> String { + match self { + DetailedConnectivity::Error(e) => format!("🔴 Error: {}", e), + DetailedConnectivity::Uninitialized => "🔴 Not started".to_string(), + DetailedConnectivity::Connecting => "🟡 Connecting…".to_string(), + DetailedConnectivity::Working => "⬇️ Getting new messages…".to_string(), + DetailedConnectivity::InterruptingIdle | DetailedConnectivity::Connected => { + "🟢 Connected".to_string() + } + DetailedConnectivity::NotConfigured => "🔴 Not configured".to_string(), + } + } + + fn to_string_smtp(&self, _context: &Context) -> String { + match self { + DetailedConnectivity::Error(e) => format!("🔴 Error: {}", e), + DetailedConnectivity::Uninitialized => { + "(You did not try to send a message recently)".to_string() + } + DetailedConnectivity::Connecting => "🟡 Connecting…".to_string(), + DetailedConnectivity::Working => "⬆️ Sending…".to_string(), + + // We don't know any more than that the last message was sent successfully; + // since sending the last message, connectivity could have changed, which we don't notice + // until another message is sent + DetailedConnectivity::InterruptingIdle | DetailedConnectivity::Connected => { + "🟢 Your last message was sent successfully".to_string() + } + DetailedConnectivity::NotConfigured => "🔴 Not configured".to_string(), + } + } + + fn all_work_done(&self) -> bool { + match self { + DetailedConnectivity::Error(_) => true, + DetailedConnectivity::Uninitialized => false, + DetailedConnectivity::Connecting => false, + DetailedConnectivity::Working => false, + DetailedConnectivity::InterruptingIdle => false, + DetailedConnectivity::Connected => true, + DetailedConnectivity::NotConfigured => true, + } + } +} + +#[derive(Clone, Default)] +pub(crate) struct ConnectivityStore(Arc>); + +impl ConnectivityStore { + async fn set(&self, context: &Context, v: DetailedConnectivity) { + { + *self.0.lock().await = v; + } + context.emit_event(EventType::ConnectivityChanged); + } + + pub(crate) async fn set_err(&self, context: &Context, e: impl ToString) { + self.set(context, DetailedConnectivity::Error(e.to_string())) + .await; + } + pub(crate) async fn set_connecting(&self, context: &Context) { + self.set(context, DetailedConnectivity::Connecting).await; + } + pub(crate) async fn set_working(&self, context: &Context) { + self.set(context, DetailedConnectivity::Working).await; + } + pub(crate) async fn set_connected(&self, context: &Context) { + self.set(context, DetailedConnectivity::Connected).await; + } + pub(crate) async fn set_not_configured(&self, context: &Context) { + self.set(context, DetailedConnectivity::NotConfigured).await; + } + + async fn get_detailed(&self) -> DetailedConnectivity { + self.0.lock().await.deref().clone() + } + async fn get_basic(&self) -> Option { + self.0.lock().await.to_basic() + } + async fn get_all_work_done(&self) -> bool { + self.0.lock().await.all_work_done() + } +} + +/// Set all folder states to InterruptingIdle in case they were `Connected` before. +/// Called during `dc_maybe_network()` to make sure that `dc_accounts_all_work_done()` +/// returns false immediately after `dc_maybe_network()`. +pub(crate) async fn idle_interrupted(scheduler: RwLockReadGuard<'_, Scheduler>) { + let [inbox, mvbox, sentbox] = match &*scheduler { + Scheduler::Running { + inbox, + mvbox, + sentbox, + .. + } => [ + inbox.state.connectivity.clone(), + mvbox.state.connectivity.clone(), + sentbox.state.connectivity.clone(), + ], + Scheduler::Stopped => return, + }; + drop(scheduler); + + let mut connectivity_lock = inbox.0.lock().await; + // For the inbox, we also have to set the connectivity to InterruptingIdle if it was + // NotConfigured before: If all folders are NotConfigured, dc_get_connectivity() + // returns Connected. But after dc_maybe_network(), dc_get_connectivity() must not + // return Connected until DC is completely done with fetching folders; this also + // includes scan_folders() which happens on the inbox thread. + if *connectivity_lock == DetailedConnectivity::Connected + || *connectivity_lock == DetailedConnectivity::NotConfigured + { + *connectivity_lock = DetailedConnectivity::InterruptingIdle; + } + drop(connectivity_lock); + + for state in &[&mvbox, &sentbox] { + let mut connectivity_lock = state.0.lock().await; + if *connectivity_lock == DetailedConnectivity::Connected { + *connectivity_lock = DetailedConnectivity::InterruptingIdle; + } + } + // No need to send ConnectivityChanged, the user-facing connectivity doesn't change because + // of what we do here. +} + +impl fmt::Debug for ConnectivityStore { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + if let Some(guard) = self.0.try_lock() { + write!(f, "ConnectivityStore {:?}", &*guard) + } else { + write!(f, "ConnectivityStore [LOCKED]") + } + } +} + +impl Context { + /// Get the current connectivity, i.e. whether the device is connected to the IMAP server. + /// One of: + /// - DC_CONNECTIVITY_NOT_CONNECTED (1000-1999): Show e.g. the string "Not connected" or a red dot + /// - DC_CONNECTIVITY_CONNECTING (2000-2999): Show e.g. the string "Connecting…" or a yellow dot + /// - DC_CONNECTIVITY_WORKING (3000-3999): Show e.g. the string "Getting new messages" or a spinning wheel + /// - DC_CONNECTIVITY_CONNECTED (>=4000): Show e.g. the string "Connected" or a green dot + /// + /// We don't use exact values but ranges here so that we can split up + /// states into multiple states in the future. + /// + /// Meant as a rough overview that can be shown + /// e.g. in the title of the main screen. + /// + /// If the connectivity changes, a DC_EVENT_CONNECTIVITY_CHANGED will be emitted. + pub async fn get_connectivity(&self) -> Connectivity { + let lock = self.scheduler.read().await; + let stores: Vec<_> = match &*lock { + Scheduler::Running { + inbox, + mvbox, + sentbox, + .. + } => [&inbox.state, &mvbox.state, &sentbox.state] + .iter() + .map(|state| state.connectivity.clone()) + .collect(), + Scheduler::Stopped => return Connectivity::NotConnected, + }; + drop(lock); + + let mut connectivities = Vec::new(); + for s in stores { + if let Some(connectivity) = s.get_basic().await { + connectivities.push(connectivity); + } + } + connectivities + .into_iter() + .min() + .unwrap_or(Connectivity::Connected) + } + + /// Get an overview of the current connectivity, and possibly more statistics. + /// Meant to give the user more insight about the current status than + /// the basic connectivity info returned by dc_get_connectivity(); show this + /// e.g., if the user taps on said basic connectivity info. + /// + /// If this page changes, a DC_EVENT_CONNECTIVITY_CHANGED will be emitted. + /// + /// This comes as an HTML from the core so that we can easily improve it + /// and the improvement instantly reaches all UIs. + pub async fn get_connectivity_html(&self) -> String { + let mut ret = + "\n\n".to_string(); + + let lock = self.scheduler.read().await; + let (folders_states, smtp) = match &*lock { + Scheduler::Running { + inbox, + mvbox, + sentbox, + smtp, + .. + } => ( + [ + ( + Config::ConfiguredInboxFolder, + Config::InboxWatch, + inbox.state.connectivity.clone(), + ), + ( + Config::ConfiguredMvboxFolder, + Config::MvboxWatch, + mvbox.state.connectivity.clone(), + ), + ( + Config::ConfiguredSentboxFolder, + Config::SentboxWatch, + sentbox.state.connectivity.clone(), + ), + ], + smtp.state.connectivity.clone(), + ), + Scheduler::Stopped => { + ret += "Not started\n"; + return ret; + } + }; + drop(lock); + + ret += "

Incoming messages:

    "; + for (folder, watch, state) in &folders_states { + let w = self.get_config(*watch).await.ok_or_log(self); + + let mut folder_added = false; + if w.flatten() == Some("1".to_string()) { + let f = self.get_config(*folder).await.ok_or_log(self).flatten(); + + if let Some(foldername) = f { + ret += "
  • ""; + ret += &foldername; + ret += "": "; + ret += &state.get_detailed().await.to_string_imap(self); + ret += "
    • "; + + folder_added = true; + } + } + + if !folder_added && folder == &Config::ConfiguredInboxFolder { + let detailed = &state.get_detailed().await; + if let DetailedConnectivity::Error(_) = detailed { + // On the inbox thread, we also do some other things like scan_folders and run jobs + // so, maybe, the inbox is not watched, but something else went wrong + ret += "
  • "; + ret += &detailed.to_string_imap(self); + ret += "
    • "; + } + } + } + ret += "
"; + + ret += "

Outgoing messages:

  • "; + ret += &smtp.get_detailed().await.to_string_smtp(self); + ret += "
"; + + ret += "\n"; + ret + } + + pub async fn all_work_done(&self) -> bool { + let lock = self.scheduler.read().await; + let stores: Vec<_> = match &*lock { + Scheduler::Running { + inbox, + mvbox, + sentbox, + smtp, + .. + } => [&inbox.state, &mvbox.state, &sentbox.state, &smtp.state] + .iter() + .map(|state| state.connectivity.clone()) + .collect(), + Scheduler::Stopped => return false, + }; + drop(lock); + + for s in &stores { + if !s.get_all_work_done().await { + return false; + } + } + true + } +} diff --git a/src/smtp.rs b/src/smtp.rs index 0af07797e..635996028 100644 --- a/src/smtp.rs +++ b/src/smtp.rs @@ -8,12 +8,11 @@ use async_smtp::smtp::client::net::ClientTlsParameters; use async_smtp::{error, smtp, EmailAddress}; use crate::constants::DC_LP_AUTH_OAUTH2; -use crate::context::Context; use crate::events::EventType; use crate::login_param::{dc_build_tls, CertificateChecks, LoginParam, ServerLoginParam}; use crate::oauth2::dc_get_oauth2_access_token; use crate::provider::Socket; -use crate::stock_str; +use crate::{context::Context, scheduler::connectivity::ConnectivityStore}; /// SMTP write and read timeout in seconds. const SMTP_TIMEOUT: u64 = 30; @@ -28,11 +27,11 @@ pub enum Error { #[source] error: error::Error, }, - #[error("SMTP: failed to connect: {0}")] + #[error("SMTP failed to connect: {0}")] ConnectionFailure(#[source] smtp::error::Error), - #[error("SMTP: failed to setup connection {0:?}")] + #[error("SMTP failed to setup connection: {0}")] ConnectionSetupFailure(#[source] smtp::error::Error), - #[error("SMTP: oauth2 error {address}")] + #[error("SMTP oauth2 error {address}")] Oauth2Error { address: String }, #[error("TLS error {0}")] Tls(#[from] async_native_tls::Error), @@ -53,6 +52,8 @@ pub(crate) struct Smtp { /// (eg connect or send succeeded). On initialization and disconnect /// it is set to None. last_success: Option, + + pub(crate) connectivity: ConnectivityStore, } impl Smtp { @@ -97,6 +98,7 @@ impl Smtp { return Ok(()); } + self.connectivity.set_connecting(context).await; let lp = LoginParam::from_database(context, "configured_").await?; let res = self .connect( @@ -107,16 +109,10 @@ impl Smtp { lp.provider.map_or(false, |provider| provider.strict_tls), ) .await; - if let Err(ref err) = res { - let message = stock_str::server_response( - context, - format!("SMTP {}:{}", lp.smtp.server, lp.smtp.port), - err.to_string(), - ) - .await; - context.emit_event(EventType::ErrorNetwork(message)); - }; + if let Err(err) = &res { + self.connectivity.set_err(context, err).await; + } res } diff --git a/src/stock_str.rs b/src/stock_str.rs index 09905b36d..4db2c8a9f 100644 --- a/src/stock_str.rs +++ b/src/stock_str.rs @@ -130,9 +130,6 @@ pub enum StockMessage { ))] CannotLogin = 60, - #[strum(props(fallback = "Could not connect to %1$s: %2$s"))] - ServerResponse = 61, - #[strum(props(fallback = "%1$s by %2$s."))] MsgActionByUser = 62, @@ -583,18 +580,6 @@ pub(crate) async fn cannot_login(context: &Context, user: impl AsRef) -> St .replace1(user) } -/// Stock string: `Could not connect to %1$s: %2$s`. -pub(crate) async fn server_response( - context: &Context, - server: impl AsRef, - details: impl AsRef, -) -> String { - translated(context, StockMessage::ServerResponse) - .await - .replace1(server) - .replace2(details) -} - /// Stock string: `%1$s by %2$s.`. pub(crate) async fn msg_action_by_user( context: &Context, @@ -1000,10 +985,7 @@ mod tests { #[async_std::test] async fn test_stock_string_repl_str2() { let t = TestContext::new().await; - assert_eq!( - server_response(&t, "foo", "bar").await, - "Could not connect to foo: bar" - ); + assert_eq!(msg_action_by_user(&t, "foo", "bar").await, "foo by bar."); } #[async_std::test] diff --git a/src/test_utils.rs b/src/test_utils.rs index 3f2a10dcc..a90eed41d 100644 --- a/src/test_utils.rs +++ b/src/test_utils.rs @@ -631,7 +631,6 @@ fn receive_event(event: &Event) { EventType::SmtpMessageSent(msg) => format!("[SMTP_MESSAGE_SENT] {}", msg), EventType::Warning(msg) => format!("WARN: {}", yellow.paint(msg)), EventType::Error(msg) => format!("ERROR: {}", red.paint(msg)), - EventType::ErrorNetwork(msg) => format!("{}", red.paint(format!("[NETWORK] msg={}", msg))), EventType::ErrorSelfNotInGroup(msg) => { format!("{}", red.paint(format!("[SELF_NOT_IN_GROUP] {}", msg))) }