Connectivity view: Only set smtp to "connected" if the last message was actually sent (#2541)

fix #2539

It's always a bit ambiguous which function should do set_err or set last_send_error, I used this rule here:

If some code returns Status::RetryLater, then it sets last_send_error, because Status::RetryLater means that the user won't see the error directly on the message (if we returned Status::Failed(Err(_)), then the message would be shown as failed to the user) Also, smtp_send always sets last_send_error because, well, sending just failed or succeeded.

Also, remove unused field pending_error.

src/job.rs

@@ -148,7 +148,6 @@ pub struct Job {
     pub added_timestamp: i64,
     pub tries: u32,
     pub param: Params,
-    pub pending_error: Option<String>,
 }
 
 impl fmt::Display for Job {
@@ -169,7 +168,6 @@ impl Job {
             added_timestamp: timestamp,
             tries: 0,
             param,
-            pending_error: None,
         }
     }
 
@@ -251,12 +249,13 @@ impl Job {
 
         smtp.connectivity.set_working(context).await;
 
-        let status = match smtp.send(context, recipients, message, job_id).await {
+        let send_result = smtp.send(context, recipients, message, job_id).await;
+        smtp.last_send_error = send_result.as_ref().err().map(|e| e.to_string());
+
+        let status = match send_result {
             Err(crate::smtp::send::Error::SmtpSend(err)) => {
                 // Remote error, retry later.
                 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 {
                     async_smtp::smtp::error::Error::Permanent(ref response) => {
@@ -365,6 +364,7 @@ impl Job {
         //  SMTP server, if not yet done
         if let Err(err) = smtp.connect_configured(context).await {
             warn!(context, "SMTP connection failure: {:?}", err);
+            smtp.last_send_error = Some(format!("SMTP connection failure: {:#}", err));
             return Status::RetryLater;
         }
 
@@ -407,6 +407,8 @@ impl Job {
                 }
                 Err(err) => {
                     warn!(context, "failed to check message existence: {:?}", err);
+                    smtp.last_send_error =
+                        Some(format!("failed to check message existence: {:#}", err));
                     return Status::RetryLater;
                 }
             }
@@ -521,6 +523,7 @@ impl Job {
         // connect to SMTP server, if not yet done
         if let Err(err) = smtp.connect_configured(context).await {
             warn!(context, "SMTP connection failure: {:?}", err);
+            smtp.last_send_error = Some(err.to_string());
             return Status::RetryLater;
         }
 
@@ -1321,7 +1324,6 @@ LIMIT 1;
                     added_timestamp: row.get("added_timestamp")?,
                     tries: row.get("tries")?,
                     param: row.get::<_, String>("param")?.parse().unwrap_or_default(),
-                    pending_error: None,
                 };
 
                 Ok(job)

src/scheduler.rs

@@ -298,7 +298,11 @@ 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;
+                    match &connection.last_send_error {
+                        None => connection.connectivity.set_connected(&ctx).await,
+                        Some(err) => connection.connectivity.set_err(&ctx, err).await,
+                    }
+
                     interrupt_info = idle_interrupt_receiver.recv().await.unwrap_or_default();
                     info!(ctx, "smtp fake idle - interrupted")
                 }

src/smtp.rs

@@ -54,6 +54,9 @@ pub(crate) struct Smtp {
     last_success: Option<SystemTime>,
 
     pub(crate) connectivity: ConnectivityStore,
+
+    /// If sending the last message failed, contains the error message.
+    pub(crate) last_send_error: Option<String>,
 }
 
 impl Smtp {
@@ -100,20 +103,14 @@ impl Smtp {
 
         self.connectivity.set_connecting(context).await;
         let lp = LoginParam::from_database(context, "configured_").await?;
-        let res = self
+        self.connect(
-            .connect(
+            context,
-                context,
+            &lp.smtp,
-                &lp.smtp,
+            &lp.addr,
-                &lp.addr,
+            lp.server_flags & DC_LP_AUTH_OAUTH2 != 0,
-                lp.server_flags & DC_LP_AUTH_OAUTH2 != 0,
+            lp.provider.map_or(false, |provider| provider.strict_tls),
-                lp.provider.map_or(false, |provider| provider.strict_tls),
+        )
-            )
+        .await
-            .await;
-
-        if let Err(err) = &res {
-            self.connectivity.set_err(context, err).await;
-        }
-        res
     }
 
     /// Connect using the provided login params.