Bug 1081158 - Ensure we report all login related errors. r=rnewman, a=lmandel
authorMark Hammond <mhammond@skippinet.com.au>
Thu, 05 Mar 2015 16:12:51 +1100
changeset 250395 ad1f181d8593
parent 250394 3836553057c4
child 250396 3a27c2da51d1
push id4570
push userryanvm@gmail.com
push date2015-03-16 16:03 +0000
treeherdermozilla-beta@ad1f181d8593 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersrnewman, lmandel
bugs1081158
milestone37.0
Bug 1081158 - Ensure we report all login related errors. r=rnewman, a=lmandel
services/sync/modules/policies.js
services/sync/tests/unit/test_errorhandler.js
--- a/services/sync/modules/policies.js
+++ b/services/sync/modules/policies.js
@@ -832,16 +832,22 @@ ErrorHandler.prototype = {
       this._log.trace("shouldReportError: false (master password locked).");
       return false;
     }
 
     if (this.dontIgnoreErrors) {
       return true;
     }
 
+    if (Status.login == LOGIN_FAILED_LOGIN_REJECTED) {
+      // An explicit LOGIN_REJECTED state is always reported (bug 1081158)
+      this._log.trace("shouldReportError: true (login was rejected)");
+      return true;
+    }
+
     let lastSync = Svc.Prefs.get("lastSync");
     if (lastSync && ((Date.now() - Date.parse(lastSync)) >
         Svc.Prefs.get("errorhandler.networkFailureReportTimeout") * 1000)) {
       Status.sync = PROLONGED_SYNC_FAILURE;
       if (this.didReportProlongedError) {
         this._log.trace("shouldReportError: false (prolonged sync failure, but" +
                         " we've already reported it).");
         return false;
--- a/services/sync/tests/unit/test_errorhandler.js
+++ b/services/sync/tests/unit/test_errorhandler.js
@@ -465,16 +465,34 @@ add_identity_test(this, function test_sh
   // Clean up.
   Service.verifyLogin = Service._verifyLogin;
   clean();
   let deferred = Promise.defer();
   server.stop(deferred.resolve);
   yield deferred.promise;
 });
 
+// Test that even if we don't have a cluster URL, a login failure due to
+// authentication errors is always reported.
+add_identity_test(this, function test_shouldReportLoginFailureWithNoCluster() {
+  // Ensure no clusterURL - any error not specific to login should not be reported.
+  Service.serverURL  = "";
+  Service.clusterURL = "";
+
+  // Test explicit "login rejected" state.
+  Status.resetSync();
+  // If we have a LOGIN_REJECTED state, we always report the error.
+  Status.login = LOGIN_FAILED_LOGIN_REJECTED;
+  do_check_true(errorHandler.shouldReportError());
+  // But any other status with a missing clusterURL is treated as a mid-sync
+  // 401 (ie, should be treated as a node reassignment)
+  Status.login = LOGIN_SUCCEEDED;
+  do_check_false(errorHandler.shouldReportError());
+});
+
 // XXX - how to arrange for 'Service.identity.basicPassword = null;' in
 // an fxaccounts environment?
 add_task(function test_login_syncAndReportErrors_non_network_error() {
   // Test non-network errors are reported
   // when calling syncAndReportErrors
   let server = sync_httpd_setup();
   yield setUp(server);
   Service.identity.basicPassword = null;