Bug 1386027 - Simplify handleError interfaces for SessionCallback and TelemetryCollector r=rnewman
authorGrigory Kruglov <gkruglov@mozilla.com>
Tue, 08 Aug 2017 13:45:29 -0400
changeset 422958 2b09cdaae71cf5e2f98825ce960954eb1c36a435
parent 422957 46b4a667c1e43ad4c3153187051b1dcff5e4b0ea
child 422959 640c562833fcd157368fb86a7da3231b23dedcb3
push id7761
push userjlund@mozilla.com
push dateFri, 15 Sep 2017 00:19:52 +0000
treeherdermozilla-beta@c38455951db4 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersrnewman
bugs1386027
milestone57.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1386027 - Simplify handleError interfaces for SessionCallback and TelemetryCollector r=rnewman Error reporting, and especially the split between per-stage and global session errors, are a bit of a mess; at the very least, this patch unifies things a little bit, and ensures we're not passing around nulls unexpectedly. MozReview-Commit-ID: JTSp7GuOji0
mobile/android/base/android-services.mozbuild
mobile/android/services/src/main/java/org/mozilla/gecko/fxa/sync/FxAccountSyncAdapter.java
mobile/android/services/src/main/java/org/mozilla/gecko/sync/BackoffException.java
mobile/android/services/src/main/java/org/mozilla/gecko/sync/CryptoKeysChangedException.java
mobile/android/services/src/main/java/org/mozilla/gecko/sync/GlobalSession.java
mobile/android/services/src/main/java/org/mozilla/gecko/sync/OutdatedStorageVersionException.java
mobile/android/services/src/main/java/org/mozilla/gecko/sync/delegates/GlobalSessionCallback.java
mobile/android/services/src/main/java/org/mozilla/gecko/sync/stage/EnsureCrypto5KeysStage.java
mobile/android/services/src/main/java/org/mozilla/gecko/sync/stage/FetchMetaGlobalStage.java
mobile/android/services/src/main/java/org/mozilla/gecko/sync/telemetry/TelemetryCollector.java
mobile/android/tests/background/junit3/src/org/mozilla/gecko/background/sync/TestResetting.java
mobile/android/tests/background/junit3/src/org/mozilla/gecko/background/testhelpers/DefaultGlobalSessionCallback.java
mobile/android/tests/background/junit4/src/org/mozilla/android/sync/test/TestResetCommands.java
mobile/android/tests/background/junit4/src/org/mozilla/android/sync/test/helpers/MockGlobalSessionCallback.java
mobile/android/tests/background/junit4/src/org/mozilla/gecko/background/testhelpers/DefaultGlobalSessionCallback.java
mobile/android/tests/background/junit4/src/org/mozilla/gecko/sync/telemetry/TelemetryCollectorTest.java
--- a/mobile/android/base/android-services.mozbuild
+++ b/mobile/android/base/android-services.mozbuild
@@ -859,32 +859,34 @@ sync_java_files = [TOPSRCDIR + '/mobile/
     'fxa/sync/FxAccountSyncStatusHelper.java',
     'fxa/sync/SchedulePolicy.java',
     'fxa/SyncStatusListener.java',
     'push/autopush/AutopushClient.java',
     'push/autopush/AutopushClientException.java',
     'push/RegisterUserAgentResponse.java',
     'push/SubscribeChannelResponse.java',
     'sync/AlreadySyncingException.java',
+    'sync/BackoffException.java',
     'sync/BackoffHandler.java',
     'sync/BadRequiredFieldJSONException.java',
     'sync/CollectionConcurrentModificationException.java',
     'sync/CollectionKeys.java',
     'sync/CommandProcessor.java',
     'sync/CommandRunner.java',
     'sync/CredentialException.java',
     'sync/crypto/CryptoException.java',
     'sync/crypto/CryptoInfo.java',
     'sync/crypto/HKDF.java',
     'sync/crypto/HMACVerificationException.java',
     'sync/crypto/KeyBundle.java',
     'sync/crypto/MissingCryptoInputException.java',
     'sync/crypto/NoKeyBundleException.java',
     'sync/crypto/PBKDF2.java',
     'sync/crypto/PersistedCrypto5Keys.java',
+    'sync/CryptoKeysChangedException.java',
     'sync/CryptoRecord.java',
     'sync/DelayedWorkTracker.java',
     'sync/delegates/ClientsDataDelegate.java',
     'sync/delegates/FreshStartDelegate.java',
     'sync/delegates/GlobalSessionCallback.java',
     'sync/delegates/JSONRecordFetchDelegate.java',
     'sync/delegates/KeyUploadDelegate.java',
     'sync/delegates/MetaGlobalDelegate.java',
@@ -938,16 +940,17 @@ sync_java_files = [TOPSRCDIR + '/mobile/
     'sync/net/TLSSocketFactory.java',
     'sync/net/WBOCollectionRequestDelegate.java',
     'sync/net/WBORequestDelegate.java',
     'sync/NoCollectionKeysSetException.java',
     'sync/NodeAuthenticationException.java',
     'sync/NonArrayJSONException.java',
     'sync/NonObjectJSONException.java',
     'sync/NullClusterURLException.java',
+    'sync/OutdatedStorageVersionException.java',
     'sync/PersistedMetaGlobal.java',
     'sync/PrefsBackoffHandler.java',
     'sync/ReflowIsNecessaryException.java',
     'sync/repositories/android/AndroidBrowserBookmarksDataAccessor.java',
     'sync/repositories/android/AndroidBrowserBookmarksRepository.java',
     'sync/repositories/android/AndroidBrowserBookmarksRepositorySession.java',
     'sync/repositories/android/AndroidBrowserHistoryDataAccessor.java',
     'sync/repositories/android/AndroidBrowserHistoryRepository.java',
--- a/mobile/android/services/src/main/java/org/mozilla/gecko/fxa/sync/FxAccountSyncAdapter.java
+++ b/mobile/android/services/src/main/java/org/mozilla/gecko/fxa/sync/FxAccountSyncAdapter.java
@@ -28,16 +28,17 @@ import org.mozilla.gecko.fxa.authenticat
 import org.mozilla.gecko.fxa.authenticator.AndroidFxAccount;
 import org.mozilla.gecko.fxa.authenticator.FxADefaultLoginStateMachineDelegate;
 import org.mozilla.gecko.fxa.authenticator.FxAccountAuthenticator;
 import org.mozilla.gecko.fxa.login.FxAccountLoginStateMachine;
 import org.mozilla.gecko.fxa.login.Married;
 import org.mozilla.gecko.fxa.login.State;
 import org.mozilla.gecko.fxa.login.State.StateLabel;
 import org.mozilla.gecko.fxa.sync.FxAccountSyncDelegate.Result;
+import org.mozilla.gecko.sync.BackoffException;
 import org.mozilla.gecko.sync.BackoffHandler;
 import org.mozilla.gecko.sync.GlobalSession;
 import org.mozilla.gecko.sync.PrefsBackoffHandler;
 import org.mozilla.gecko.sync.SharedPreferencesClientsDataDelegate;
 import org.mozilla.gecko.sync.SyncConfiguration;
 import org.mozilla.gecko.sync.ThreadPool;
 import org.mozilla.gecko.sync.Utils;
 import org.mozilla.gecko.sync.crypto.KeyBundle;
@@ -171,38 +172,28 @@ public class FxAccountSyncAdapter extend
       recordTelemetry();
     }
 
     @Override
     public void handleError(GlobalSession globalSession, Exception ex, String reason) {
       super.handleError(globalSession, ex, reason);
       // If an error hasn't been set downstream, record what we know at this point.
       if (!telemetryCollector.hasError()) {
-        telemetryCollector.setError(TelemetryCollector.KEY_ERROR_INTERNAL, reason, ex);
-      }
-      recordTelemetry();
-    }
-
-    @Override
-    public void handleError(GlobalSession globalSession, Exception ex) {
-      super.handleError(globalSession, ex);
-      // If an error hasn't been set downstream, record what we know at this point.
-      if (!telemetryCollector.hasError()) {
-        telemetryCollector.setError(TelemetryCollector.KEY_ERROR_INTERNAL, ex);
+        telemetryCollector.setError(TelemetryCollector.KEY_ERROR_INTERNAL, ex, reason);
       }
       recordTelemetry();
     }
 
     @Override
     public void handleAborted(GlobalSession globalSession, String reason) {
       super.handleAborted(globalSession, reason);
       // Note to future maintainers: while there are reasons, other than 'backoff', this method
       // might be called, in practice that _is_ the only reason it gets called at the moment of
       // writing this. If this changes, please do expand this telemetry handling.
-      this.telemetryCollector.setError(TelemetryCollector.KEY_ERROR_INTERNAL, "backoff");
+      this.telemetryCollector.setError(TelemetryCollector.KEY_ERROR_INTERNAL, new BackoffException(), reason);
       recordTelemetry();
     }
 
     private void recordTelemetry() {
       telemetryCollector.setFinished(SystemClock.elapsedRealtime());
       final Intent telemetryIntent = new Intent();
       telemetryIntent.setAction(ACTION_BACKGROUND_TELEMETRY);
       telemetryIntent.putExtra(TelemetryContract.KEY_TYPE, TelemetryContract.KEY_TYPE_SYNC);
@@ -288,23 +279,18 @@ public class FxAccountSyncAdapter extend
       } finally {
         // Continue with the usual success flow.
         syncDelegate.handleSuccess();
       }
     }
 
     @Override
     public void handleError(GlobalSession globalSession, Exception ex, String reason) {
-      this.handleError(globalSession, ex);
-    }
-
-    @Override
-    public void handleError(GlobalSession globalSession, Exception e) {
       Logger.warn(LOG_TAG, "Global session failed."); // Exception will be dumped by delegate below.
-      syncDelegate.handleError(e);
+      syncDelegate.handleError(ex);
       // TODO: should we reduce the periodic sync interval?
     }
 
     @Override
     public void handleAborted(GlobalSession globalSession, String reason) {
       Logger.warn(LOG_TAG, "Global session aborted: " + reason);
       syncDelegate.handleError(null);
       // TODO: should we reduce the periodic sync interval?
@@ -423,50 +409,43 @@ public class FxAccountSyncAdapter extend
           Collection<String> knownStageNames = SyncConfiguration.validEngineNames();
           syncConfig.stagesToSync = Utils.getStagesToSyncFromBundle(knownStageNames, extras);
           syncConfig.setClusterURL(storageServerURI);
 
           globalSession = new GlobalSession(syncConfig, callback, context, clientsDataDelegate, callback.getCollector());
           callback.getCollector().setIDs(token.hashedFxaUid, clientsDataDelegate.getAccountGUID());
           globalSession.start(syncDeadline);
         } catch (Exception e) {
-          callback.handleError(globalSession, e);
-          return;
+          callback.handleError(globalSession, e, "Unexpected error while starting a sync");
         }
       }
 
       @Override
       public void handleFailure(TokenServerException e) {
         Logger.error(LOG_TAG, "Failed to get token.", e);
         try {
           // We should only get here *after* we're locked into the married state.
           State state = fxAccount.getState();
           if (state.getStateLabel() == StateLabel.Married) {
             Married married = (Married) state;
             fxAccount.setState(married.makeCohabitingState());
           }
         } finally {
           fxAccount.releaseSharedAccountStateLock();
         }
-        callback.getCollector().setError(
-                TelemetryCollector.KEY_ERROR_TOKEN,
-                e.getClass().getSimpleName()
-        );
-        callback.handleError(null, e);
+        callback.getCollector().setError(TelemetryCollector.KEY_ERROR_TOKEN, e);
+        callback.handleError(null, e, "Failure processing a token");
       }
 
       @Override
       public void handleError(Exception e) {
         Logger.error(LOG_TAG, "Failed to get token.", e);
         fxAccount.releaseSharedAccountStateLock();
-        callback.getCollector().setError(
-                TelemetryCollector.KEY_ERROR_TOKEN,
-                e.getClass().getSimpleName()
-        );
-        callback.handleError(null, e);
+        callback.getCollector().setError(TelemetryCollector.KEY_ERROR_TOKEN, e);
+        callback.handleError(null, e, "Error getting a token");
       }
 
       @Override
       public void handleBackoff(int backoffSeconds) {
         // This is the token server telling us to back off.
         Logger.info(LOG_TAG, "Token server requesting backoff of " + backoffSeconds + "s. Backoff handler: " + tokenBackoffHandler);
         didReceiveBackoff = true;
 
new file mode 100644
--- /dev/null
+++ b/mobile/android/services/src/main/java/org/mozilla/gecko/sync/BackoffException.java
@@ -0,0 +1,9 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+package org.mozilla.gecko.sync;
+
+public class BackoffException extends SyncException {
+    private static final long serialVersionUID = 2966836737310897884L;
+}
new file mode 100644
--- /dev/null
+++ b/mobile/android/services/src/main/java/org/mozilla/gecko/sync/CryptoKeysChangedException.java
@@ -0,0 +1,9 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+package org.mozilla.gecko.sync;
+
+public class CryptoKeysChangedException extends SyncException {
+    private static final long serialVersionUID = 5729127849933343128L;
+}
--- a/mobile/android/services/src/main/java/org/mozilla/gecko/sync/GlobalSession.java
+++ b/mobile/android/services/src/main/java/org/mozilla/gecko/sync/GlobalSession.java
@@ -1,16 +1,18 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 package org.mozilla.gecko.sync;
 
 import android.content.Context;
 import android.os.SystemClock;
+import android.support.annotation.NonNull;
+import android.support.annotation.Nullable;
 import android.support.annotation.VisibleForTesting;
 
 import org.json.simple.JSONArray;
 import org.mozilla.gecko.background.common.log.Logger;
 import org.mozilla.gecko.sync.crypto.CryptoException;
 import org.mozilla.gecko.sync.crypto.KeyBundle;
 import org.mozilla.gecko.sync.delegates.ClientsDataDelegate;
 import org.mozilla.gecko.sync.delegates.FreshStartDelegate;
@@ -272,17 +274,17 @@ public class GlobalSession implements Ht
 
   /**
    * Move to the next stage in the syncing process.
    */
   public void advance() {
     // If we have a backoff, request a backoff and don't advance to next stage.
     long existingBackoff = largestBackoffObserved.get();
     if (existingBackoff > 0) {
-      this.abort(null, "Aborting sync because of backoff of " + existingBackoff + " milliseconds.");
+      this.abort(new BackoffException(), "Aborting sync because of backoff of " + existingBackoff + " milliseconds.");
       return;
     }
 
     this.callback.handleStageCompleted(this.currentState, this);
     Stage next = nextStage(this.currentState);
     GlobalSyncStage nextStage;
     try {
       nextStage = this.getSyncStageByName(next);
@@ -507,17 +509,16 @@ public class GlobalSession implements Ht
         Logger.warn(LOG_TAG, "Got exception trying to upload updated meta/global record; ignoring.", e);
         synchronized (monitor) {
           monitor.notify();
         }
       }
     };
   }
 
-
   public void abort(Exception e, String reason) {
     Logger.warn(LOG_TAG, "Aborting sync: " + reason, e);
     cleanUp();
     long existingBackoff = largestBackoffObserved.get();
     if (existingBackoff > 0) {
       callback.requestBackoff(existingBackoff);
     }
     if (!(e instanceof HTTPFailureException)) {
@@ -1132,17 +1133,17 @@ public class GlobalSession implements Ht
 
   /**
    * Suggest that your Sync client needs to be upgraded to work
    * with this server.
    */
   public void requiresUpgrade() {
     Logger.info(LOG_TAG, "Client outdated storage version; requires update.");
     // TODO: notify UI.
-    this.abort(null, "Requires upgrade from " + STORAGE_VERSION);
+    this.abort(new OutdatedStorageVersionException(), "Requires upgrade from " + STORAGE_VERSION);
   }
 
   /**
    * If meta/global is missing or malformed, throws a MetaGlobalException.
    * Otherwise, returns true if there is an entry for this engine in the
    * meta/global "engines" object.
    * <p>
    * This is a global/permanent setting, not a local/temporary setting. For the
new file mode 100644
--- /dev/null
+++ b/mobile/android/services/src/main/java/org/mozilla/gecko/sync/OutdatedStorageVersionException.java
@@ -0,0 +1,9 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+package org.mozilla.gecko.sync;
+
+/* package-private */ class OutdatedStorageVersionException extends SyncException {
+    private static final long serialVersionUID = 1703691318986965545L;
+}
--- a/mobile/android/services/src/main/java/org/mozilla/gecko/sync/delegates/GlobalSessionCallback.java
+++ b/mobile/android/services/src/main/java/org/mozilla/gecko/sync/delegates/GlobalSessionCallback.java
@@ -31,17 +31,16 @@ public interface GlobalSessionCallback {
    * Called when a migration sentinel has been found and processed successfully.
    * <p>
    * This account should stop syncing immediately, and arrange to delete itself.
    */
   void informMigrated(GlobalSession session);
 
   void handleAborted(GlobalSession globalSession, String reason);
   void handleError(GlobalSession globalSession, Exception ex, String reason);
-  void handleError(GlobalSession globalSession, Exception ex);
   void handleSuccess(GlobalSession globalSession);
   void handleStageCompleted(Stage currentState, GlobalSession globalSession);
   void handleIncompleteStage(Stage currentState, GlobalSession globalSession);
   void handleFullSyncNecessary();
 
   /**
    * Called when a {@link GlobalSession} wants to know if it should continue
    * to make storage requests.
--- a/mobile/android/services/src/main/java/org/mozilla/gecko/sync/stage/EnsureCrypto5KeysStage.java
+++ b/mobile/android/services/src/main/java/org/mozilla/gecko/sync/stage/EnsureCrypto5KeysStage.java
@@ -7,16 +7,17 @@ package org.mozilla.gecko.sync.stage;
 import android.os.SystemClock;
 
 import java.net.URISyntaxException;
 import java.util.HashSet;
 import java.util.Set;
 
 import org.mozilla.gecko.background.common.log.Logger;
 import org.mozilla.gecko.sync.CollectionKeys;
+import org.mozilla.gecko.sync.CryptoKeysChangedException;
 import org.mozilla.gecko.sync.CryptoRecord;
 import org.mozilla.gecko.sync.ExtendedJSONObject;
 import org.mozilla.gecko.sync.HTTPFailureException;
 import org.mozilla.gecko.sync.InfoCollections;
 import org.mozilla.gecko.sync.NoCollectionKeysSetException;
 import org.mozilla.gecko.sync.crypto.KeyBundle;
 import org.mozilla.gecko.sync.crypto.PersistedCrypto5Keys;
 import org.mozilla.gecko.sync.net.AuthHeaderProvider;
@@ -45,17 +46,17 @@ implements SyncStorageRequestDelegate {
   @Override
   public void execute() throws NoSuchStageException {
     InfoCollections infoCollections = session.config.infoCollections;
     if (infoCollections == null) {
       telemetryStageCollector.finished = SystemClock.elapsedRealtime();
       telemetryStageCollector.error = new TelemetryCollector
               .StageErrorBuilder(TELEMETRY_ERROR_NAME, TELEMETRY_ERROR_NO_COLLECTIONS)
               .build();
-      session.abort(null, "No info/collections set in EnsureCrypto5KeysStage.");
+      session.abort(new IllegalStateException(), "No info/collections set in EnsureCrypto5KeysStage.");
       return;
     }
 
     PersistedCrypto5Keys pck = session.config.persistedCryptoKeys();
     long lastModified = pck.lastModified();
     if (retrying || !infoCollections.updateNeeded(CRYPTO_COLLECTION, lastModified)) {
       // Try to use our local collection keys for this session.
       Logger.debug(LOG_TAG, "Trying to use persisted collection keys for this session.");
@@ -180,17 +181,17 @@ implements SyncStorageRequestDelegate {
       // New keys, different from old keys.
       Logger.trace(LOG_TAG, "Fetched keys are not the same as persisted keys; " +
           "setting fetched keys for this session before resetting changed engines.");
       setAndPersist(pck, keys, responseTimestamp);
       session.resetStagesByName(changedCollections);
       telemetryStageCollector.error = new TelemetryCollector
               .StageErrorBuilder(TELEMETRY_ERROR_NAME, TELEMETRY_ERROR_KEYS_CHANGED)
               .build();
-      session.abort(null, "crypto/keys changed on server.");
+      session.abort(new CryptoKeysChangedException(), "crypto/keys changed on server.");
       return;
     }
 
     // New keys don't differ from old keys; persist timestamp and move on.
     Logger.trace(LOG_TAG, "Fetched keys are the same as persisted keys; persisting only last modified.");
     session.config.setCollectionKeys(oldKeys);
     pck.persistLastModified(response.normalizedWeaveTimestamp());
     doAdvance();
--- a/mobile/android/services/src/main/java/org/mozilla/gecko/sync/stage/FetchMetaGlobalStage.java
+++ b/mobile/android/services/src/main/java/org/mozilla/gecko/sync/stage/FetchMetaGlobalStage.java
@@ -74,17 +74,17 @@ public class FetchMetaGlobalStage extend
   @Override
   public void execute() throws NoSuchStageException {
     InfoCollections infoCollections = session.config.infoCollections;
     if (infoCollections == null) {
       telemetryStageCollector.finished = SystemClock.elapsedRealtime();
       telemetryStageCollector.error = new TelemetryCollector
               .StageErrorBuilder(TELEMETRY_ERROR_NAME, TELEMETRY_ERROR_NO_INFO_COLLECTIONS)
               .build();
-      session.abort(null, "No info/collections set in FetchMetaGlobalStage.");
+      session.abort(new IllegalStateException(), "No info/collections set in FetchMetaGlobalStage.");
       return;
     }
 
     final long lastModified = session.config.persistedMetaGlobal().lastModified();
     if (!infoCollections.updateNeeded(META_COLLECTION, lastModified)) {
       // Try to use our local collection keys for this session.
       Logger.info(LOG_TAG, "Trying to use persisted meta/global for this session.");
       MetaGlobal global = session.config.persistedMetaGlobal().metaGlobal(session.config.metaURL(), session.getAuthHeaderProvider());
--- a/mobile/android/services/src/main/java/org/mozilla/gecko/sync/telemetry/TelemetryCollector.java
+++ b/mobile/android/services/src/main/java/org/mozilla/gecko/sync/telemetry/TelemetryCollector.java
@@ -77,30 +77,27 @@ public class TelemetryCollector {
                     ));
         } catch (UnsupportedEncodingException | NoSuchAlgorithmException e) {
             // Should not happen.
             Log.e(LOG_TAG, "Either UTF-8 or SHA-256 are not supported", e);
         }
     }
 
     public void setError(@NonNull String name, @NonNull Exception e) {
-        setError(name, e.getClass().getSimpleName());
-    }
-
-    public void setError(@NonNull String name, @NonNull String details) {
-        setError(name, details, null);
+        setError(name, e, null);
     }
 
-    public void setError(@NonNull String name, @NonNull String details, @Nullable Exception e) {
+    public void setError(@NonNull String name, @NonNull Exception e, @Nullable String details) {
         final ExtendedJSONObject error = new ExtendedJSONObject();
         error.put("name", name);
-        if (e != null) {
-            error.put("error", e.getClass().getSimpleName() + ":" + details);
+        final String exceptionName = e.getClass().getSimpleName();
+        if (details != null) {
+            error.put("error", exceptionName + ":" + details);
         } else {
-            error.put("error", details);
+            error.put("error", exceptionName);
         }
         this.error = error;
     }
 
     public boolean hasError() {
         return this.error != null;
     }
 
--- a/mobile/android/tests/background/junit3/src/org/mozilla/gecko/background/sync/TestResetting.java
+++ b/mobile/android/tests/background/junit3/src/org/mozilla/gecko/background/sync/TestResetting.java
@@ -177,17 +177,17 @@ public class TestResetting extends Andro
     return new DefaultGlobalSessionCallback() {
 
       @Override
       public void handleAborted(GlobalSession globalSession, String reason) {
         performNotify(new Exception("Aborted"));
       }
 
       @Override
-      public void handleError(GlobalSession globalSession, Exception ex) {
+      public void handleError(GlobalSession globalSession, Exception ex, String reason) {
         performNotify(ex);
       }
     };
   }
 
   private static void assertConfigTimestampsGreaterThan(SynchronizerConfiguration config, long local, long remote) {
     assertTrue(local <= config.localBundle.getTimestamp());
     assertTrue(remote <= config.remoteBundle.getTimestamp());
--- a/mobile/android/tests/background/junit3/src/org/mozilla/gecko/background/testhelpers/DefaultGlobalSessionCallback.java
+++ b/mobile/android/tests/background/junit3/src/org/mozilla/gecko/background/testhelpers/DefaultGlobalSessionCallback.java
@@ -39,20 +39,16 @@ public class DefaultGlobalSessionCallbac
 
   }
 
   @Override
   public void handleAborted(GlobalSession globalSession, String reason) {
   }
 
   @Override
-  public void handleError(GlobalSession globalSession, Exception ex) {
-  }
-
-  @Override
   public void handleError(GlobalSession globalSession, Exception ex, String reason) {
   }
 
   @Override
   public void handleSuccess(GlobalSession globalSession) {
   }
 
   @Override
--- a/mobile/android/tests/background/junit4/src/org/mozilla/android/sync/test/TestResetCommands.java
+++ b/mobile/android/tests/background/junit4/src/org/mozilla/android/sync/test/TestResetCommands.java
@@ -136,17 +136,17 @@ public class TestResetCommands {
     return new DefaultGlobalSessionCallback() {
 
       @Override
       public void handleAborted(GlobalSession globalSession, String reason) {
         performNotify(new Exception("Aborted"));
       }
 
       @Override
-      public void handleError(GlobalSession globalSession, Exception ex) {
+      public void handleError(GlobalSession globalSession, Exception ex, String reason) {
         performNotify(ex);
       }
 
       @Override
       public void handleSuccess(GlobalSession globalSession) {
       }
     };
   }
--- a/mobile/android/tests/background/junit4/src/org/mozilla/android/sync/test/helpers/MockGlobalSessionCallback.java
+++ b/mobile/android/tests/background/junit4/src/org/mozilla/android/sync/test/helpers/MockGlobalSessionCallback.java
@@ -49,21 +49,16 @@ public class MockGlobalSessionCallback i
   @Override
   public void handleAborted(GlobalSession globalSession, String reason) {
     this.calledAborted = true;
     this.testWaiter().performNotify();
   }
 
   @Override
   public void handleError(GlobalSession globalSession, Exception ex, String reason) {
-    this.handleError(globalSession, ex);
-  }
-
-  @Override
-  public void handleError(GlobalSession globalSession, Exception ex) {
     this.calledError = true;
     this.calledErrorException = ex;
     this.testWaiter().performNotify();
   }
 
   @Override
   public void handleIncompleteStage(Stage currentState,
                                     GlobalSession globalSession) {
--- a/mobile/android/tests/background/junit4/src/org/mozilla/gecko/background/testhelpers/DefaultGlobalSessionCallback.java
+++ b/mobile/android/tests/background/junit4/src/org/mozilla/gecko/background/testhelpers/DefaultGlobalSessionCallback.java
@@ -31,20 +31,16 @@ public class DefaultGlobalSessionCallbac
   public void handleAborted(GlobalSession globalSession, String reason) {
   }
 
   @Override
   public void handleError(GlobalSession globalSession, Exception ex, String reason) {
   }
 
   @Override
-  public void handleError(GlobalSession globalSession, Exception ex) {
-  }
-
-  @Override
   public void handleSuccess(GlobalSession globalSession) {
   }
 
   @Override
   public void handleStageCompleted(Stage currentState,
                                    GlobalSession globalSession) {
   }
 
--- a/mobile/android/tests/background/junit4/src/org/mozilla/gecko/sync/telemetry/TelemetryCollectorTest.java
+++ b/mobile/android/tests/background/junit4/src/org/mozilla/gecko/sync/telemetry/TelemetryCollectorTest.java
@@ -137,48 +137,31 @@ public class TelemetryCollectorTest {
         collector.setFinished(2L);
 
         // Test that we can build without setting an error.
         assertFalse(collector.hasError());
         Bundle data = collector.build();
         assertFalse(data.containsKey("error"));
 
         // Test various ways to set an error.
-        // Just details.
-        collector.setError("testError", "unexpectedStuff");
-        data = collector.build();
-        assertTrue(data.containsKey("error"));
-        JSONObject errorJson = (JSONObject) data.getSerializable("error");
-        assertEquals("testError", errorJson.get("name"));
-        assertEquals("unexpectedStuff", errorJson.get("error"));
-        assertTrue(collector.hasError());
-
         // Just exception.
         collector.setError("exceptionTest", new IllegalArgumentException());
         data = collector.build();
         assertTrue(data.containsKey("error"));
-        errorJson = (JSONObject) data.getSerializable("error");
+        JSONObject errorJson = (JSONObject) data.getSerializable("error");
         assertEquals("exceptionTest", errorJson.get("name"));
         assertEquals("IllegalArgumentException", errorJson.get("error"));
 
         // Details and exception.
-        collector.setError("anotherTest", "Error details", new ConcurrentModificationException());
+        collector.setError("anotherTest", new ConcurrentModificationException(), "Error details");
         data = collector.build();
         assertTrue(data.containsKey("error"));
         errorJson = (JSONObject) data.getSerializable("error");
         assertEquals("anotherTest", errorJson.get("name"));
         assertEquals("ConcurrentModificationException:Error details", errorJson.get("error"));
-
-        // Details and explicit null exception.
-        collector.setError("noExceptionTest", "Error details", null);
-        data = collector.build();
-        assertTrue(data.containsKey("error"));
-        errorJson = (JSONObject) data.getSerializable("error");
-        assertEquals("noExceptionTest", errorJson.get("name"));
-        assertEquals("Error details", errorJson.get("error"));
     }
 
     @Test
     public void testRestarted() throws Exception {
         collector.setStarted(5L);
         collector.setFinished(10L);
 
         Bundle data = collector.build();