[geary/wip/713006-better-error-reporting: 6/6] Implement retry for send and receive errors.



commit 9733210642ef3120969eaef251c0832ac390052e
Author: Michael James Gratton <mike vee net>
Date:   Thu Nov 9 18:03:44 2017 +1100

    Implement retry for send and receive errors.
    
    * src/client/application/geary-controller.vala (GearyController): Make
      on_report_problem a proper method so it can be called internally, add a
      trivial handler shim for compat.
      (GearyController::on_retry_problem): Restart SMTP and IMAP clients when
      requested.
    
    * src/engine/api/geary-account.vala (Account): Add
      start_incoming_client() and start_outgoing_client() methods to allow
      clients to be restarted. Implement in subclasses.
    
    * src/engine/imap-db/outbox/smtp-outbox-folder.vala
      (SmtpOutboxFolder::start_postman_async): Make public so it can be
      called from GenericAccount.

 src/client/application/geary-controller.vala       |   30 +++++-
 src/engine/api/geary-account.vala                  |   22 +++-
 src/engine/imap-db/outbox/smtp-outbox-folder.vala  |  128 ++++++++++---------
 .../imap-engine/imap-engine-generic-account.vala   |   26 ++++-
 4 files changed, 141 insertions(+), 65 deletions(-)
---
diff --git a/src/client/application/geary-controller.vala b/src/client/application/geary-controller.vala
index 365ce7b..55954bd 100644
--- a/src/client/application/geary-controller.vala
+++ b/src/client/application/geary-controller.vala
@@ -873,7 +873,7 @@ public class GearyController : Geary.BaseObject {
         }
     }
 
-    private void on_report_problem(Geary.Account account, Geary.Account.Problem problem, Error? err) {
+    private void report_problem(Geary.Account.Problem problem, Geary.Account account, Error? err) {
         debug("Reported problem: %s Error: %s", problem.to_string(), err != null ? err.message : "(N/A)");
 
         switch (problem) {
@@ -904,10 +904,32 @@ public class GearyController : Geary.BaseObject {
         switch (info_bar.problem) {
         case Geary.Account.Problem.RECV_EMAIL_ERROR:
         case Geary.Account.Problem.RECV_EMAIL_LOGIN_FAILED:
+            info_bar.account.start_incoming_client.begin((obj, ret) => {
+                    try {
+                        info_bar.account.start_incoming_client.end(ret);
+                    } catch (Error err) {
+                        report_problem(
+                            Geary.Account.Problem.RECV_EMAIL_LOGIN_FAILED,
+                            info_bar.account,
+                            err
+                        );
+                    }
+                });
             break;
 
         case Geary.Account.Problem.SEND_EMAIL_ERROR:
         case Geary.Account.Problem.SEND_EMAIL_LOGIN_FAILED:
+            info_bar.account.start_outgoing_client.begin((obj, ret) => {
+                    try {
+                        info_bar.account.start_outgoing_client.end(ret);
+                    } catch (Error err) {
+                        report_problem(
+                            Geary.Account.Problem.SEND_EMAIL_ERROR,
+                            info_bar.account,
+                            err
+                        );
+                    }
+                });
             break;
 
         default:
@@ -961,7 +983,11 @@ public class GearyController : Geary.BaseObject {
             }
         }
     }
-    
+
+    private void on_report_problem(Geary.Account account, Geary.Account.Problem problem, Error? err) {
+        report_problem(problem, account, err);
+    }
+
     private void on_account_email_removed(Geary.Folder folder, Gee.Collection<Geary.EmailIdentifier> ids) {
         if (folder.special_folder_type == Geary.SpecialFolderType.OUTBOX) {
             main_window.status_bar.deactivate_message(StatusBar.Message.OUTBOX_SEND_FAILURE);
diff --git a/src/engine/api/geary-account.vala b/src/engine/api/geary-account.vala
index ffc16a4..f541674 100644
--- a/src/engine/api/geary-account.vala
+++ b/src/engine/api/geary-account.vala
@@ -251,7 +251,27 @@ public abstract class Geary.Account : BaseObject {
      * Unlike most methods in Account, this should only be called when the Account is closed.
      */
     public abstract async void rebuild_async(Cancellable? cancellable = null) throws Error;
-    
+
+    /**
+     * Starts delivery of messages to the outgoing server.
+     *
+     * Outgoing delivery will be started by default when the account
+     * is opened. This method is mostly useful when re-starting it
+     * after an error has occurred.
+     */
+    public abstract async void start_outgoing_client()
+        throws Error;
+
+    /**
+     * Starts receiving messages from the incoming server.
+     *
+     * The incoming client will be started by default when the account
+     * is opened. This method is mostly useful when re-starting it
+     * after an error has occurred.
+     */
+    public abstract async void start_incoming_client()
+        throws Error;
+
     /**
      * Lists all the currently-available folders found under the parent path
      * unless it's null, in which case it lists all the root folders.  If the
diff --git a/src/engine/imap-db/outbox/smtp-outbox-folder.vala 
b/src/engine/imap-db/outbox/smtp-outbox-folder.vala
index e582176..bae4525 100644
--- a/src/engine/imap-db/outbox/smtp-outbox-folder.vala
+++ b/src/engine/imap-db/outbox/smtp-outbox-folder.vala
@@ -110,9 +110,73 @@ private class Geary.SmtpOutboxFolder :
         );
     }
 
-    // create_email_async() requires the Outbox be open according to contract, but enqueuing emails
-    // for background delivery can happen at any time, so this is the mechanism to do so.
-    // email_count is the number of emails in the Outbox after enqueueing the message.
+    /**
+     * Starts delivery of messages in the outbox.
+     */
+    public async void start_postman_async() {
+        debug("Starting outbox postman with %u messages queued", this.outbox_queue.size);
+        if (this.queue_cancellable != null) {
+            debug("Postman already started, not starting another");
+            return;
+        }
+
+        Cancellable cancellable = this.queue_cancellable = new Cancellable();
+        uint send_retry_seconds = MIN_SEND_RETRY_INTERVAL_SEC;
+
+        // Start the send queue.
+        while (!cancellable.is_cancelled()) {
+            // yield until a message is ready
+            OutboxRow? row = null;
+            try {
+                row = yield this.outbox_queue.recv_async(cancellable);
+                if (yield postman_send(row, cancellable)) {
+                    // send was good, reset nap length
+                    send_retry_seconds = MIN_SEND_RETRY_INTERVAL_SEC;
+                } else {
+                    // send was bad, try sending again later
+                    this.outbox_queue.send(row);
+
+                    if (!cancellable.is_cancelled()) {
+                        debug("Outbox napping for %u seconds...", send_retry_seconds);
+                        // Take a brief nap before continuing to allow
+                        // connection problems to resolve.
+                        yield Geary.Scheduler.sleep_async(send_retry_seconds);
+                        send_retry_seconds = Geary.Numeric.uint_ceiling(
+                            send_retry_seconds * 2,
+                            MAX_SEND_RETRY_INTERVAL_SEC
+                        );
+                    }
+                }
+            } catch (Error err) {
+                // A hard error occurred. This will cause the postman
+                // to exit but we still want to re-queue the row in
+                // case it restarts.
+                if (row != null) {
+                    this.outbox_queue.send(row);
+                }
+                debug("Outbox postman error: %s", err.message);
+                if (err is SmtpError.AUTHENTICATION_FAILED) {
+                    report_problem(Geary.Account.Problem.SEND_EMAIL_LOGIN_FAILED, err);
+                } else if (!(err is IOError.CANCELLED)) {
+                    report_problem(Geary.Account.Problem.SEND_EMAIL_ERROR, err);
+                }
+                // Get out of here
+                cancellable.cancel();
+            }
+        }
+
+        this.queue_cancellable = null;
+        debug("Exiting outbox postman");
+    }
+
+    /**
+     * Queues a message in the outbox for delivery.
+     *
+     * This should be used instead of {@link create_email_async()},
+     * since that requires the Outbox be open according to contract,
+     * but enqueuing emails for background delivery can happen at any
+     * time, so this is the mechanism to do so.
+     */
     public async SmtpOutboxEmailIdentifier enqueue_email_async(Geary.RFC822.Message rfc822,
         Cancellable? cancellable) throws Error {
         debug("Queuing message for sending: %s",
@@ -382,64 +446,6 @@ private class Geary.SmtpOutboxFolder :
         return row_to_email(row);
     }
 
-    private async void start_postman_async() {
-        debug("Starting outbox postman with %u messages queued", this.outbox_queue.size);
-        if (this.queue_cancellable != null) {
-            debug("Postman already started, not starting another");
-            return;
-        }
-
-        Cancellable cancellable = this.queue_cancellable = new Cancellable();
-        uint send_retry_seconds = MIN_SEND_RETRY_INTERVAL_SEC;
-
-        // Start the send queue.
-        while (!cancellable.is_cancelled()) {
-            // yield until a message is ready
-            OutboxRow? row = null;
-            try {
-                row = yield this.outbox_queue.recv_async(cancellable);
-                if (yield postman_send(row, cancellable)) {
-                    // send was good, reset nap length
-                    send_retry_seconds = MIN_SEND_RETRY_INTERVAL_SEC;
-                } else {
-                    // send was bad, try sending again later
-                    this.outbox_queue.send(row);
-
-                    if (!cancellable.is_cancelled()) {
-                        debug("Outbox napping for %u seconds...", send_retry_seconds);
-                        // Take a brief nap before continuing to allow
-                        // connection problems to resolve.
-                        yield Geary.Scheduler.sleep_async(send_retry_seconds);
-                        send_retry_seconds = Geary.Numeric.uint_ceiling(
-                            send_retry_seconds * 2,
-                            MAX_SEND_RETRY_INTERVAL_SEC
-                        );
-                    }
-                }
-            } catch (Error err) {
-                // A hard error occurred. This will cause the postman
-                // to exit but we still want to re-queue the row in
-                // case it restarts.
-                if (row != null) {
-                    this.outbox_queue.send(row);
-                }
-                if (!(err is IOError.CANCELLED)) {
-                    debug("Outbox postman error: %s", err.message);
-                    if (err is SmtpError.AUTHENTICATION_FAILED) {
-                        report_problem(Geary.Account.Problem.SEND_EMAIL_LOGIN_FAILED, err);
-                    } else {
-                        report_problem(Geary.Account.Problem.SEND_EMAIL_ERROR, err);
-                    }
-                }
-                // Get out of here
-                cancellable.cancel();
-            }
-        }
-
-        this.queue_cancellable = null;
-        debug("Exiting outbox postman");
-    }
-
     // Returns true if row was successfully processed, else false
     private async bool postman_send(OutboxRow row, Cancellable cancellable)
         throws Error {
diff --git a/src/engine/imap-engine/imap-engine-generic-account.vala 
b/src/engine/imap-engine/imap-engine-generic-account.vala
index f085b4b..5155220 100644
--- a/src/engine/imap-engine/imap-engine-generic-account.vala
+++ b/src/engine/imap-engine/imap-engine-generic-account.vala
@@ -262,7 +262,31 @@ private abstract class Geary.ImapEngine.GenericAccount : Geary.Account {
         
         message("%s: Rebuild complete", to_string());
     }
-    
+
+    /**
+     * This starts the outbox postman running.
+     */
+    public override async void start_outgoing_client()
+        throws Error {
+        check_open();
+        this.local.outbox.start_postman_async.begin();
+    }
+
+    /**
+     * This closes then reopens the IMAP account.
+     */
+    public override async void start_incoming_client()
+        throws Error {
+        check_open();
+        try {
+            yield this.remote.close_async();
+        } catch (Error err) {
+            debug("Ignoring error closing IMAP account for restart: %s", err.message);
+        }
+
+        yield this.remote.open_async();
+    }
+
     // Subclasses should implement this to return their flavor of a MinimalFolder with the
     // appropriate interfaces attached.  The returned folder should have its SpecialFolderType
     // set using either the properties from the local folder or its path.


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]