Show better errors when configuring (#1917)

* Show all errors when configuring

* Shorten some overly long msgs

deltachat-ffi/deltachat.h

@@ -4938,8 +4938,9 @@ void dc_event_unref(dc_event_t* event);
 #define DC_STR_CONFIGURATION_FAILED       84
 #define DC_STR_BAD_TIME_MSG_BODY          85
 #define DC_STR_UPDATE_REMINDER_MSG_BODY   86
+#define DC_STR_ERROR_NO_NETWORK           87
 
-#define DC_STR_COUNT                      86
+#define DC_STR_COUNT                      87
 
 /*
  * @}

python/tests/test_account.py

@@ -1739,6 +1739,38 @@ class TestOnlineAccount:
 
         assert len(imap2.get_all_messages()) == 1
 
+    def test_configure_error_msgs(self, acfactory):
+        ac1, configdict = acfactory.get_online_config()
+        ac1.update_config(configdict)
+        ac1.set_config("mail_pw", "abc")  # Wrong mail pw
+        ac1.configure()
+        while True:
+            ev = ac1._evtracker.get_matching("DC_EVENT_CONFIGURE_PROGRESS")
+            if ev.data1 == 0:
+                break
+        # Password is wrong so it definitely has to say something about "password"
+        # but the error message should not be repeated:
+        assert ev.data2.count("password") == 1
+
+        ac2, configdict = acfactory.get_online_config()
+        ac2.update_config(configdict)
+        ac2.set_config("addr", "abc@def.invalid")  # mail server can't be reached
+        ac2.configure()
+        while True:
+            ev = ac2._evtracker.get_matching("DC_EVENT_CONFIGURE_PROGRESS")
+            if ev.data1 == 0:
+                break
+        # Can't connect so it probably should say something about "internet"
+        # again, should not repeat itself
+        # If this fails then probably `e.msg.to_lowercase().contains("could not resolve")`
+        # in configure/mod.rs returned false because the error message was changed
+        # (i.e. did not contain "could not resolve" anymore)
+        assert (ev.data2.count("internet") + ev.data2.count("network")) == 1
+        # Should mention that it can't connect:
+        assert ev.data2.count("connect") == 1
+        # The users do not know what "configuration" is
+        assert "configuration" not in ev.data2.lower()
+
     def test_name_changes(self, acfactory):
         ac1, ac2 = acfactory.get_two_online_accounts()
         ac1.set_config("displayname", "Account 1")

src/configure/mod.rs

@@ -8,6 +8,7 @@ mod server_params;
 use anyhow::{bail, ensure, Context as _, Result};
 use async_std::prelude::*;
 use async_std::task;
+use itertools::Itertools;
 use percent_encoding::{utf8_percent_encode, NON_ALPHANUMERIC};
 
 use crate::config::Config;
@@ -250,22 +251,28 @@ async fn configure(ctx: &Context, param: &mut LoginParam) -> Result<()> {
 
     let smtp_config_task = task::spawn(async move {
         let mut smtp_configured = false;
+        let mut errors = Vec::new();
         for smtp_server in smtp_servers {
             smtp_param.user = smtp_server.username.clone();
             smtp_param.server = smtp_server.hostname.clone();
             smtp_param.port = smtp_server.port;
             smtp_param.security = smtp_server.socket;
 
-            if try_smtp_one_param(&context_smtp, &smtp_param, &smtp_addr, oauth2, &mut smtp).await {
+            match try_smtp_one_param(&context_smtp, &smtp_param, &smtp_addr, oauth2, &mut smtp)
+                .await
+            {
+                Ok(_) => {
                     smtp_configured = true;
                     break;
                 }
+                Err(e) => errors.push(e),
+            }
         }
 
         if smtp_configured {
-            Some(smtp_param)
+            Ok(smtp_param)
         } else {
-            None
+            Err(errors)
         }
     });
 
@@ -281,32 +288,39 @@ async fn configure(ctx: &Context, param: &mut LoginParam) -> Result<()> {
         .filter(|params| params.protocol == Protocol::IMAP)
         .collect();
     let imap_servers_count = imap_servers.len();
+    let mut errors = Vec::new();
     for (imap_server_index, imap_server) in imap_servers.into_iter().enumerate() {
         param.imap.user = imap_server.username.clone();
         param.imap.server = imap_server.hostname.clone();
         param.imap.port = imap_server.port;
         param.imap.security = imap_server.socket;
 
-        if try_imap_one_param(ctx, &param.imap, &param.addr, oauth2, &mut imap).await {
+        match try_imap_one_param(ctx, &param.imap, &param.addr, oauth2, &mut imap).await {
+            Ok(_) => {
                 imap_configured = true;
                 break;
             }
+            Err(e) => errors.push(e),
+        }
         progress!(
             ctx,
             600 + (800 - 600) * (1 + imap_server_index) / imap_servers_count
         );
     }
     if !imap_configured {
-        bail!("IMAP autoconfig did not succeed");
+        bail!(nicer_configuration_error(ctx, errors).await);
     }
 
     progress!(ctx, 850);
 
     // Wait for SMTP configuration
-    if let Some(smtp_param) = smtp_config_task.await {
+    match smtp_config_task.await {
+        Ok(smtp_param) => {
             param.smtp = smtp_param;
-    } else {
+        }
-        bail!("SMTP autoconfig did not succeed");
+        Err(errors) => {
+            bail!(nicer_configuration_error(ctx, errors).await);
+        }
     }
 
     progress!(ctx, 900);
@@ -478,7 +492,7 @@ async fn try_imap_one_param(
     addr: &str,
     oauth2: bool,
     imap: &mut Imap,
-) -> bool {
+) -> Result<(), ConfigurationError> {
     let inf = format!(
         "imap: {}@{}:{} security={} certificate_checks={} oauth2={}",
         param.user, param.server, param.port, param.security, param.certificate_checks, oauth2
@@ -487,10 +501,13 @@ async fn try_imap_one_param(
 
     if let Err(err) = imap.connect(context, param, addr, oauth2).await {
         info!(context, "failure: {}", err);
-        false
+        Err(ConfigurationError {
+            config: inf,
+            msg: err.to_string(),
+        })
     } else {
         info!(context, "success: {}", inf);
-        true
+        Ok(())
     }
 }
 
@@ -500,7 +517,7 @@ async fn try_smtp_one_param(
     addr: &str,
     oauth2: bool,
     smtp: &mut Smtp,
-) -> bool {
+) -> Result<(), ConfigurationError> {
     let inf = format!(
         "smtp: {}@{}:{} security={} certificate_checks={} oauth2={}",
         param.user, param.server, param.port, param.security, param.certificate_checks, oauth2
@@ -509,14 +526,48 @@ async fn try_smtp_one_param(
 
     if let Err(err) = smtp.connect(context, param, addr, oauth2).await {
         info!(context, "failure: {}", err);
-        false
+        Err(ConfigurationError {
+            config: inf,
+            msg: err.to_string(),
+        })
     } else {
         info!(context, "success: {}", inf);
         smtp.disconnect().await;
-        true
+        Ok(())
     }
 }
 
+#[derive(Debug, thiserror::Error)]
+#[error("Trying {config}…\nError: {msg}")]
+pub struct ConfigurationError {
+    config: String,
+    msg: String,
+}
+
+async fn nicer_configuration_error(context: &Context, errors: Vec<ConfigurationError>) -> String {
+    let first_err = if let Some(f) = errors.first() {
+        f
+    } else {
+        return "".to_string();
+    };
+
+    if errors
+        .iter()
+        .all(|e| e.msg.to_lowercase().contains("could not resolve"))
+    {
+        return context
+            .stock_str(StockMessage::ErrorNoNetwork)
+            .await
+            .to_string();
+    }
+
+    if errors.iter().all(|e| e.msg == first_err.msg) {
+        return first_err.msg.to_string();
+    }
+
+    errors.iter().map(|e| e.to_string()).join("\n\n")
+}
+
 #[derive(Debug, thiserror::Error)]
 pub enum Error {
     #[error("Invalid email address: {0:?}")]

src/imap/mod.rs

@@ -229,19 +229,7 @@ impl Imap {
                 }
             }
             Err(err) => {
-                let message = {
+                bail!(err);
-                    let config = &self.config;
-                    let imap_server: &str = config.lp.server.as_ref();
-                    let imap_port = config.lp.port;
-                    context
-                        .stock_string_repl_str2(
-                            StockMessage::ServerResponse,
-                            format!("IMAP {}:{}", imap_server, imap_port),
-                            err.to_string(),
-                        )
-                        .await
-                };
-                bail!("{}: {}", message, err);
             }
         };
 
@@ -286,7 +274,7 @@ impl Imap {
                 }
 
                 self.trigger_reconnect();
-                Err(format_err!("{}: {}", message, err))
+                Err(format_err!("{}\n\n{}", message, err))
             }
         }
     }

src/smtp/mod.rs

@@ -30,7 +30,7 @@ pub enum Error {
         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:?}")]

src/stock.rs

@@ -217,7 +217,7 @@ pub enum StockMessage {
     #[strum(props(fallback = "You are invited to a video chat, click %1$s to join."))]
     VideochatInviteMsgBody = 83,
 
-    #[strum(props(fallback = "Configuration failed. Error: “%1$s”"))]
+    #[strum(props(fallback = "Error:\n\n“%1$s”"))]
     ConfigurationFailed = 84,
 
     #[strum(props(
@@ -231,6 +231,11 @@ pub enum StockMessage {
                     and you are missing the latest features 😳\n\
                     Please check https://get.delta.chat or your app store for updates."))]
     UpdateReminderMsgBody = 86,
+
+    #[strum(props(
+        fallback = "Could not find your mail server.\n\nPlease check your internet connection."
+    ))]
+    ErrorNoNetwork = 87,
 }
 
 /*