Bug 1308337 - Pre: Don't throw away store and fetch exceptions as they're encountered r=nalexander draft
authorGrigory Kruglov <gkruglov@mozilla.com>
Fri, 26 May 2017 17:44:27 -0400
changeset 588422 501ff746ecfb3022a0fe89844e307153bfdb5164
parent 588421 d7424fec748b9a2d07d1c98b78ce89fd418750e4
child 588423 d1790feae1c0f46dc5f420aeed347da12a6ac85c
push id62031
push userbmo:gkruglov@mozilla.com
push dateFri, 02 Jun 2017 19:52:26 +0000
reviewersnalexander
bugs1308337, 1362208
milestone55.0a1
Bug 1308337 - Pre: Don't throw away store and fetch exceptions as they're encountered r=nalexander We will need them later for telemetry reporting. For now we're just keeping the last exception which we encountered (which agrees with desktop's behaviour), and Bug 1362208 explores follow-up work to aggregate and count the exceptions as we see them. MozReview-Commit-ID: 8yKkZVGJZ9e
mobile/android/services/src/main/java/org/mozilla/gecko/sync/synchronizer/SynchronizerSession.java
--- a/mobile/android/services/src/main/java/org/mozilla/gecko/sync/synchronizer/SynchronizerSession.java
+++ b/mobile/android/services/src/main/java/org/mozilla/gecko/sync/synchronizer/SynchronizerSession.java
@@ -70,18 +70,26 @@ implements RecordsChannelDelegate,
   // that a concurrently syncing client has uploaded.
   private long pendingATimestamp = -1;
   private long pendingBTimestamp = -1;
   private long storeEndATimestamp = -1;
   private long storeEndBTimestamp = -1;
   private boolean flowAToBCompleted = false;
   private boolean flowBToACompleted = false;
 
-  protected final AtomicInteger numInboundRecords = new AtomicInteger(-1);
-  protected final AtomicInteger numOutboundRecords = new AtomicInteger(-1);
+  private final AtomicInteger numInboundRecords = new AtomicInteger(-1);
+  private final AtomicInteger numInboundRecordsStored = new AtomicInteger(-1);
+  private final AtomicInteger numInboundRecordsFailed = new AtomicInteger(-1);
+  private final AtomicInteger numInboundRecordsReconciled = new AtomicInteger(-1);
+  private final AtomicInteger numOutboundRecords = new AtomicInteger(-1);
+  private final AtomicInteger numOutboundRecordsStored = new AtomicInteger(-1);
+  private final AtomicInteger numOutboundRecordsFailed = new AtomicInteger(-1);
+
+  private Exception fetchFailedCauseException;
+  private Exception storeFailedCauseException;
 
   /*
    * Public API: constructor, init, synchronize.
    */
   public SynchronizerSession(Synchronizer synchronizer, SynchronizerSessionDelegate delegate) {
     this.setSynchronizer(synchronizer);
     this.delegate = delegate;
   }
@@ -121,16 +129,32 @@ implements RecordsChannelDelegate,
    * Valid only after second flow has completed.
    *
    * @return number of records, or -1 if not valid.
    */
   public int getOutboundCount() {
     return numOutboundRecords.get();
   }
 
+  public int getOutboundCountStored() {
+    return numOutboundRecordsStored.get();
+  }
+
+  public int getOutboundCountFailed() {
+    return numOutboundRecordsFailed.get();
+  }
+
+  public Exception getFetchFailedCauseException() {
+    return fetchFailedCauseException;
+  }
+
+  public Exception getStoreFailedCauseException() {
+    return storeFailedCauseException;
+  }
+
   // These are accessed by `abort` and `synchronize`, both of which are synchronized.
   // Guarded by `this`.
   protected RecordsChannel channelAToB;
   protected RecordsChannel channelBToA;
 
   /**
    * Please don't call this until you've been notified with onInitialized.
    */
@@ -167,21 +191,26 @@ implements RecordsChannelDelegate,
       public void onFlowBeginFailed(RecordsChannel recordsChannel, Exception ex) {
         Logger.warn(LOG_TAG, "First RecordsChannel onFlowBeginFailed. Logging session error.", ex);
         session.delegate.onSynchronizeFailed(session, ex, "Failed to begin first flow.");
       }
 
       @Override
       public void onFlowFetchFailed(RecordsChannel recordsChannel, Exception ex) {
         Logger.warn(LOG_TAG, "First RecordsChannel onFlowFetchFailed. Logging remote fetch error.", ex);
+        fetchFailedCauseException = ex;
       }
 
       @Override
       public void onFlowStoreFailed(RecordsChannel recordsChannel, Exception ex, String recordGuid) {
         Logger.warn(LOG_TAG, "First RecordsChannel onFlowStoreFailed. Logging local store error.", ex);
+        // Currently we're just recording the very last exception which occurred. This is a reasonable
+        // approach, but ideally we'd want to categorize the exceptions and count them for the purposes
+        // of better telemetry. See Bug 1362208.
+        storeFailedCauseException = ex;
       }
 
       @Override
       public void onFlowFinishFailed(RecordsChannel recordsChannel, Exception ex) {
         Logger.warn(LOG_TAG, "First RecordsChannel onFlowFinishedFailed. Logging session error.", ex);
         session.delegate.onSynchronizeFailed(session, ex, "Failed to finish first flow.");
       }
     };