[geary/wip/778276-better-flag-updates] Retry an account operation after a network error.



commit 4cb9f2fbbaec1f89569095f9331443575f17b23b
Author: Michael James Gratton <mike vee net>
Date:   Fri Jan 12 11:39:44 2018 +1100

    Retry an account operation after a network error.

 .../imap-engine/imap-engine-account-operation.vala |   24 ++++++++--
 .../imap-engine/imap-engine-account-processor.vala |   44 ++++++++++++++++----
 2 files changed, 55 insertions(+), 13 deletions(-)
---
diff --git a/src/engine/imap-engine/imap-engine-account-operation.vala 
b/src/engine/imap-engine/imap-engine-account-operation.vala
index ffab9ca..ea9641b 100644
--- a/src/engine/imap-engine/imap-engine-account-operation.vala
+++ b/src/engine/imap-engine/imap-engine-account-operation.vala
@@ -8,10 +8,24 @@
 /**
  * A unit of work to be executed by {@link GenericAccount}.
  *
+ * It is important that account operations are idempotent in that they
+ * can be safely re-executed multiple times, and perform the same task
+ * each time. This means that in practice instance properties should
+ * only be used to store state passed to the operation via its
+ * constructor (e.g. a target folder to be updated) and this state
+ * should not be modified when the operation is executed (e.g. the
+ * target folder should not be changed or set to `null` during or
+ * after execution), any state needed to be maintained when executing
+ * should be passed as arguments to internal methods (e.g. the list of
+ * messages to be checked in the target folder should be passed around
+ * as arguments), and the operation should perform any needed sanity
+ * checks before proceeding (e.g. check the target folder sill exists
+ * before updating it).
+ *
  * To queue an operation for execution, pass an instance to {@link
- * GenericAccount.queue_operation} when the account is opened. It will
- * added to the accounts queue and executed asynchronously when it
- * reaches the front.
+ * GenericAccount.queue_operation} after the account has been
+ * opened. It will added to the accounts queue and executed
+ * asynchronously when it reaches the front.
  *
  * Execution of the operation is managed by {@link
  * AccountProcessor}. Since the processor will not en-queue duplicate
@@ -23,7 +37,7 @@ public abstract class Geary.ImapEngine.AccountOperation : Geary.BaseObject {
 
 
     /** The account this operation applies to. */
-    protected weak Geary.Account account;
+    protected weak Geary.Account account { get; private set; }
 
 
     /**
@@ -113,7 +127,7 @@ public abstract class Geary.ImapEngine.FolderOperation : AccountOperation {
 
 
     /** The folder this operation applies to. */
-    protected weak Geary.Folder folder;
+    protected weak Geary.Folder folder { get; private set; }
 
 
     /**
diff --git a/src/engine/imap-engine/imap-engine-account-processor.vala 
b/src/engine/imap-engine/imap-engine-account-processor.vala
index 0e6faea..9729032 100644
--- a/src/engine/imap-engine/imap-engine-account-processor.vala
+++ b/src/engine/imap-engine/imap-engine-account-processor.vala
@@ -1,5 +1,5 @@
 /*
- * Copyright 2017 Michael Gratton <mike vee net>
+ * Copyright 2017-2018 Michael Gratton <mike vee net>
  *
  * This software is licensed under the GNU Lesser General Public License
  * (version 2.1 or later).  See the COPYING file in this distribution.
@@ -12,11 +12,18 @@
  * in the queue will not be re-queued.
  *
  * Errors thrown are reported to the user via the account's
- * `problem_report` signal.
+ * `problem_report` signal. Normally if an operation throws an error
+ * it will not be re-queued, however if a network connection error
+ * occurs the error will be suppressed and it will be re-attempted
+ * once, to allow for the network dropping out mid-execution.
  */
 internal class Geary.ImapEngine.AccountProcessor : Geary.BaseObject {
 
 
+    // Retry ops after network failures at least once before giving up
+    private const int MAX_NETWORK_ERRORS = 1;
+
+
     private static bool op_equal(AccountOperation a, AccountOperation b) {
         return a.equal_to(b);
     }
@@ -72,14 +79,35 @@ internal class Geary.ImapEngine.AccountProcessor : Geary.BaseObject {
             if (op != null) {
                 debug("%s: Executing operation: %s", id, op.to_string());
                 this.current_op = op;
-                try {
-                    yield op.execute(this.cancellable);
-                    op.succeeded();
-                } catch (Error err) {
-                    op.failed(err);
-                    operation_error(op, err);
+
+                Error? op_error = null;
+                int network_errors = 0;
+                while (op_error == null) {
+                    try {
+                        yield op.execute(this.cancellable);
+                        op.succeeded();
+                        break;
+                    } catch (ImapError err) {
+                        if (err is ImapError.NOT_CONNECTED &&
+                            ++network_errors <= MAX_NETWORK_ERRORS) {
+                            debug(
+                                "Retrying operation due to network error: %s",
+                                err.message
+                            );
+                        } else {
+                            op_error = err;
+                        }
+                    } catch (Error err) {
+                        op_error = err;
+                    }
+                }
+
+                if (op_error != null) {
+                    op.failed(op_error);
+                    operation_error(op, op_error);
                 }
                 op.completed();
+
                 this.current_op = null;
             }
         }


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