Back out bug 692714 and bug 684798 part 4.
authorPhilipp von Weitershausen <philipp@weitershausen.de>
Thu, 20 Oct 2011 16:19:47 -0700
changeset 79720 b957b1741f6823aef6f8980cc3a7e1802c676d4d
parent 79719 a90cea9fee2113cf4ac0a849268a115fc287a07c
child 79722 96c48e41ce91f96ff3e69e6abba997e64010f624
push id506
push userclegnitto@mozilla.com
push dateWed, 09 Nov 2011 02:03:18 +0000
treeherdermozilla-aurora@63587fc7bb93 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
bugs692714, 684798
milestone10.0a1
Back out bug 692714 and bug 684798 part 4.
services/sync/modules/policies.js
services/sync/modules/service.js
services/sync/tests/unit/head_helpers.js
services/sync/tests/unit/test_errorhandler.js
services/sync/tests/unit/test_node_reassignment.js
services/sync/tests/unit/test_service_sync_401.js
services/sync/tests/unit/test_syncscheduler.js
services/sync/tests/unit/xpcshell.ini
--- a/services/sync/modules/policies.js
+++ b/services/sync/modules/policies.js
@@ -17,17 +17,16 @@
  * the Mozilla Foundation.
  * Portions created by the Initial Developer are Copyright (C) 2011
  * the Initial Developer. All Rights Reserved.
  *
  * Contributor(s):
  *  Marina Samuel <msamuel@mozilla.com>
  *  Philipp von Weitershausen <philipp@weitershausen.de>
  *  Chenxia Liu <liuche@mozilla.com>
- *  Richard Newman <rnewman@mozilla.com>
  *
  * Alternatively, the contents of this file may be used under the terms of
  * either the GNU General Public License Version 2 or later (the "GPL"), or
  * the GNU Lesser General Public License Version 2.1 or later (the "LGPL"),
  * in which case the provisions of the GPL or the LGPL are applicable instead
  * of those above. If you wish to allow use of your version of this file only
  * under the terms of either the GPL or the LGPL, and not to allow others to
  * use your version of this file under the terms of the MPL, indicate your
@@ -51,22 +50,16 @@ Cu.import("resource://services-sync/engi
 Cu.import("resource://services-sync/engines/clients.js");
 Cu.import("resource://services-sync/status.js");
 
 Cu.import("resource://services-sync/main.js");    // So we can get to Service for callbacks.
 
 let SyncScheduler = {
   _log: Log4Moz.repository.getLogger("Sync.SyncScheduler"),
 
-  _fatalLoginStatus: [LOGIN_FAILED_NO_USERNAME,
-                      LOGIN_FAILED_NO_PASSWORD,
-                      LOGIN_FAILED_NO_PASSPHRASE,
-                      LOGIN_FAILED_INVALID_PASSPHRASE,
-                      LOGIN_FAILED_LOGIN_REJECTED],
-
   /**
    * The nsITimer object that schedules the next sync. See scheduleNextSync().
    */
   syncTimer: null,
 
   setDefaults: function setDefaults() {
     this._log.trace("Setting SyncScheduler policy values to defaults.");
 
@@ -118,17 +111,16 @@ let SyncScheduler = {
     Svc.Obs.add("weave:service:start-over", this);
 
     if (Status.checkSetup() == STATUS_OK) {
       Svc.Idle.addIdleObserver(this, Svc.Prefs.get("scheduler.idleTime"));
     }
   },
 
   observe: function observe(subject, topic, data) {
-    this._log.trace("Handling " + topic);
     switch(topic) {
       case "weave:engine:score:updated":
         if (Status.login == LOGIN_SUCCEEDED) {
           Utils.namedTimer(this.calculateScore, SCORE_UPDATE_DELAY, this,
                            "_scoreTimer");
         }
         break;
       case "network:offline-status-changed":
@@ -166,35 +158,31 @@ let SyncScheduler = {
         break;
       case "weave:engine:sync:finish":
         if (data == "clients") {
           // Update the client mode because it might change what we sync.
           this.updateClientMode();
         }
         break;
       case "weave:engine:sync:error":
-        // `subject` is the exception thrown by an engine's sync() method.
+        // subject is the exception thrown by an engine's sync() method
         let exception = subject;
         if (exception.status >= 500 && exception.status <= 504) {
           this.requiresBackoff = true;
         }
         break;
       case "weave:service:login:error":
         this.clearSyncTriggers();
 
+        // Try again later, just as if we threw an error... only without the
+        // error count.
         if (Status.login == MASTER_PASSWORD_LOCKED) {
-          // Try again later, just as if we threw an error... only without the
-          // error count.
           this._log.debug("Couldn't log in: master password is locked.");
           this._log.trace("Scheduling a sync at MASTER_PASSWORD_LOCKED_RETRY_INTERVAL");
           this.scheduleAtInterval(MASTER_PASSWORD_LOCKED_RETRY_INTERVAL);
-        } else if (this._fatalLoginStatus.indexOf(Status.login) == -1) {
-          // Not a fatal login error, just an intermittent network or server
-          // issue. Keep on syncin'.
-          this.checkSyncStatus();
         }
         break;
       case "weave:service:logout:finish":
         // Start or cancel the sync timer depending on if
         // logged in or logged out
         this.checkSyncStatus();
         break;
       case "weave:service:sync:error":
@@ -456,28 +444,25 @@ let SyncScheduler = {
     }
   },
 
   _syncErrors: 0,
   /**
    * Deal with sync errors appropriately
    */
   handleSyncError: function handleSyncError() {
-    this._log.trace("In handleSyncError. Error count: " + this._syncErrors);
     this._syncErrors++;
 
     // Do nothing on the first couple of failures, if we're not in
     // backoff due to 5xx errors.
     if (!Status.enforceBackoff) {
       if (this._syncErrors < MAX_ERROR_COUNT_BEFORE_BACKOFF) {
         this.scheduleNextSync();
         return;
       }
-      this._log.debug("Sync error count has exceeded " +
-                      MAX_ERROR_COUNT_BEFORE_BACKOFF + "; enforcing backoff.");
       Status.enforceBackoff = true;
     }
 
     this.scheduleAtInterval();
   },
 
 
   /**
@@ -531,17 +516,16 @@ let ErrorHandler = {
     root.addAppender(dapp);
 
     let fapp = this._logAppender = new Log4Moz.StorageStreamAppender(formatter);
     fapp.level = Log4Moz.Level[Svc.Prefs.get("log.appender.file.level")];
     root.addAppender(fapp);
   },
 
   observe: function observe(subject, topic, data) {
-    this._log.trace("Handling " + topic);
     switch(topic) {
       case "weave:engine:sync:applied":
         if (subject.newFailed) {
           // An engine isn't able to apply one or more incoming records.
           // We don't fail hard on this, but it usually indicates a bug,
           // so for now treat it as sync error (c.f. Service._syncEngine())
           Status.engines = [data, ENGINE_APPLY_FAIL];
           this._log.debug(data + " failed to apply some records.");
@@ -580,24 +564,16 @@ let ErrorHandler = {
           this.notifyOnNextTick("weave:ui:sync:error");
         } else {
           this.notifyOnNextTick("weave:ui:sync:finish");
         }
 
         this.dontIgnoreErrors = false;
         break;
       case "weave:service:sync:finish":
-        this._log.trace("Status.service is " + Status.service);
-
-        if (Status.sync == SYNC_SUCCEEDED) {
-          // Great. Let's clear our mid-sync 401 note.
-          this._log.trace("Clearing lastSyncReassigned.");
-          Svc.Prefs.reset("lastSyncReassigned");
-        }
-
         if (Status.service == SYNC_FAILED_PARTIAL) {
           this._log.debug("Some engines did not sync correctly.");
           this.resetFileLog(Svc.Prefs.get("log.appender.file.logOnError"),
                             LOG_PREFIX_ERROR);
 
           if (this.shouldReportError()) {
             this.dontIgnoreErrors = false;
             this.notifyOnNextTick("weave:ui:sync:error");
@@ -734,40 +710,29 @@ let ErrorHandler = {
       return "weak-password";
     default:
       return "generic-server-error";
     }
   },
 
   shouldReportError: function shouldReportError() {
     if (Status.login == MASTER_PASSWORD_LOCKED) {
-      this._log.trace("shouldReportError: false (master password locked).");
       return false;
     }
 
     if (this.dontIgnoreErrors) {
       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;
-      this._log.trace("shouldReportError: true (prolonged sync failure).");
       return true;
     }
- 
-    // We got a 401 mid-sync. Wait for the next sync before actually handling
-    // an error. This assumes that we'll get a 401 again on a login fetch in
-    // order to report the error.
-    if (!Weave.Service.clusterURL) {
-      this._log.trace("shouldReportError: false (no cluster URL; " +
-                      "possible node reassignment).");
-      return false;
-    }
 
     return ([Status.login, Status.sync].indexOf(SERVER_MAINTENANCE) == -1 &&
             [Status.login, Status.sync].indexOf(LOGIN_FAILED_NETWORK_ERROR) == -1);
   },
 
   /**
    * Handle HTTP response results or exceptions and set the appropriate
    * Status.* bits.
@@ -777,34 +742,17 @@ let ErrorHandler = {
       case 400:
         if (resp == RESPONSE_OVER_QUOTA) {
           Status.sync = OVER_QUOTA;
         }
         break;
 
       case 401:
         Weave.Service.logout();
-        this._log.info("Got 401 response; resetting clusterURL.");
-        Svc.Prefs.reset("clusterURL");
-
-        let delay = 0;
-        if (Svc.Prefs.get("lastSyncReassigned")) {
-          // We got a 401 in the middle of the previous sync, and we just got
-          // another. Login must have succeeded in order for us to get here, so
-          // the password should be correct.
-          // This is likely to be an intermittent server issue, so back off and
-          // give it time to recover.
-          this._log.warn("Last sync also failed for 401. Delaying next sync.");
-          delay = MINIMUM_BACKOFF_INTERVAL;
-        } else {
-          this._log.debug("New mid-sync 401 failure. Making a note.");
-          Svc.Prefs.set("lastSyncReassigned", true);
-        }
-        this._log.info("Attempting to schedule another sync.");
-        SyncScheduler.scheduleNextSync(delay);
+        Status.login = LOGIN_FAILED_LOGIN_REJECTED;
         break;
 
       case 500:
       case 502:
       case 503:
       case 504:
         Status.enforceBackoff = true;
         if (resp.status == 503 && resp.headers["retry-after"]) {
--- a/services/sync/modules/service.js
+++ b/services/sync/modules/service.js
@@ -518,20 +518,18 @@ WeaveSvc.prototype = {
           Status.login = LOGIN_FAILED_LOGIN_REJECTED;
           fail = "Find cluster denied: " + ErrorHandler.errorStr(node);
           break;
         case 404:
           this._log.debug("Using serverURL as data cluster (multi-cluster support disabled)");
           return this.serverURL;
         case 0:
         case 200:
-          if (node == "null") {
+          if (node == "null")
             node = null;
-          }
-          this._log.trace("_findCluster successfully returning " + node);
           return node;
         default:
           ErrorHandler.checkServerError(node);
           fail = "Unexpected response code: " + node.status;
           break;
       }
     } catch (e) {
       this._log.debug("Network error on findCluster");
@@ -731,19 +729,19 @@ WeaveSvc.prototype = {
       } catch (ex) {
         this._log.debug("Fetching passphrase threw " + ex +
                         "; assuming master password locked.");
         Status.login = MASTER_PASSWORD_LOCKED;
         return false;
       }
 
       try {
-        // Make sure we have a cluster to verify against.
-        // This is a little weird, if we don't get a node we pretend
-        // to succeed, since that probably means we just don't have storage.
+        // Make sure we have a cluster to verify against
+        // this is a little weird, if we don't get a node we pretend
+        // to succeed, since that probably means we just don't have storage
         if (this.clusterURL == "" && !this._setCluster()) {
           Status.sync = NO_SYNC_NODE_FOUND;
           Svc.Obs.notify("weave:service:sync:delayed");
           return true;
         }
 
         // Fetch collection info on every startup.
         let test = new Resource(this.infoURL).get();
@@ -916,17 +914,16 @@ WeaveSvc.prototype = {
       CollectionKeys.clear();
 
       /* Login and sync. This also generates new keys. */
       this.sync();
       return true;
     }))(),
 
   startOver: function() {
-    this._log.trace("Invoking Service.startOver.");
     Svc.Obs.notify("weave:engine:stop-tracking");
     Status.resetSync();
 
     // We want let UI consumers of the following notification know as soon as
     // possible, so let's fake for the CLIENT_NOT_CONFIGURED status for now
     // by emptying the passphrase (we still need the password).
     Service.passphrase = "";
     Status.login = LOGIN_FAILED_NO_PASSPHRASE;
@@ -1489,28 +1486,32 @@ WeaveSvc.prototype = {
     this._ignorePrefObserver = false;
   },
 
   // Returns true if sync should proceed.
   // false / no return value means sync should be aborted.
   _syncEngine: function WeaveSvc__syncEngine(engine) {
     try {
       engine.sync();
+      return true;
     }
     catch(e) {
+      // Maybe a 401, cluster update perhaps needed?
       if (e.status == 401) {
-        // Maybe a 401, cluster update perhaps needed?
-        // We rely on ErrorHandler observing the sync failure notification to
-        // schedule another sync and clear node assignment values.
-        // Here we simply want to muffle the exception and return an
-        // appropriate value.
+        // Log out and clear the cluster URL pref. That will make us perform
+        // cluster detection and password check on next sync, which handles
+        // both causes of 401s; in either case, we won't proceed with this
+        // sync so return false, but kick off a sync for next time.
+        this.logout();
+        Svc.Prefs.reset("clusterURL");
+        Utils.nextTick(this.sync, this);
         return false;
       }
+      return true;
     }
-    return true;
   },
 
   /**
    * Silently fixes case issues.
    */
   syncKeyNeedsUpgrade: function syncKeyNeedsUpgrade() {
     let p = this.passphrase;
 
--- a/services/sync/tests/unit/head_helpers.js
+++ b/services/sync/tests/unit/head_helpers.js
@@ -12,33 +12,16 @@ let provider = {
       default:
         throw Cr.NS_ERROR_FAILURE;
     }
   },
   QueryInterface: XPCOMUtils.generateQI([Ci.nsIDirectoryServiceProvider])
 };
 Services.dirsvc.QueryInterface(Ci.nsIDirectoryService).registerProvider(provider);
 
-let timer;
-function waitForZeroTimer(callback) {
-  // First wait >100ms (nsITimers can take up to that much time to fire, so
-  // we can account for the timer in delayedAutoconnect) and then two event
-  // loop ticks (to account for the Utils.nextTick() in autoConnect).
-  let ticks = 2;
-  function wait() {
-    if (ticks) {
-      ticks -= 1;
-      Utils.nextTick(wait);
-      return;
-    }
-    callback();
-  }
-  timer = Utils.namedTimer(wait, 150, {}, "timer");
-}
-
 btoa = Cu.import("resource://services-sync/log4moz.js").btoa;
 function getTestLogger(component) {
   return Log4Moz.repository.getLogger("Testing");
 }
 
 function initTestLogging(level) {
   function LogStats() {
     this.errorsLogged = 0;
--- a/services/sync/tests/unit/test_errorhandler.js
+++ b/services/sync/tests/unit/test_errorhandler.js
@@ -132,44 +132,26 @@ add_test(function test_401_logout() {
   let server = sync_httpd_setup();
   setUp();
 
   // By calling sync, we ensure we're logged in.
   Service.sync();
   do_check_eq(Status.sync, SYNC_SUCCEEDED);
   do_check_true(Service.isLoggedIn);
 
-  Svc.Obs.add("weave:service:sync:error", onSyncError);
-  function onSyncError() {
-    _("Got weave:service:sync:error in first sync.");
-    Svc.Obs.remove("weave:service:sync:error", onSyncError);
-
-    // Wait for the automatic next sync.
-    function onLoginError() {
-      _("Got weave:service:login:error in second sync.");
-      Svc.Obs.remove("weave:service:login:error", onLoginError);
-
-      do_check_eq(Status.login, LOGIN_FAILED_LOGIN_REJECTED);
-      do_check_false(Service.isLoggedIn);
-
-      // Clean up.
-      Utils.nextTick(function () {
-        Service.startOver();
-        server.stop(run_next_test);
-      });
-    }
-    Svc.Obs.add("weave:service:login:error", onLoginError);
-  }
-
   // Make sync fail due to login rejected.
   Service.username = "janedoe";
+  Service.sync();
 
-  _("Starting first sync.");
-  Service.sync();
-  _("First sync done.");
+  do_check_eq(Status.login, LOGIN_FAILED_LOGIN_REJECTED);
+  do_check_false(Service.isLoggedIn);
+
+  // Clean up.
+  Service.startOver();
+  server.stop(run_next_test);
 });
 
 add_test(function test_credentials_changed_logout() {
   let server = sync_httpd_setup();
   setUp();
 
   // By calling sync, we ensure we're logged in.
   Service.sync();
@@ -202,20 +184,16 @@ add_test(function test_no_lastSync_pref(
 
   run_next_test();
 });
 
 add_test(function test_shouldReportError() {
   Status.login = MASTER_PASSWORD_LOCKED;
   do_check_false(ErrorHandler.shouldReportError());
 
-  // Give ourselves a clusterURL so that the temporary 401 no-error situation
-  // doesn't come into play.
-  Service.clusterURL = "http://localhost:8080/";
-
   // Test dontIgnoreErrors, non-network, non-prolonged, login error reported
   Status.resetSync();
   setLastSync(NON_PROLONGED_ERROR_DURATION);
   ErrorHandler.dontIgnoreErrors = true;
   Status.login = LOGIN_FAILED_NO_PASSWORD;
   do_check_true(ErrorHandler.shouldReportError());
 
   // Test dontIgnoreErrors, non-network, non-prolonged, sync error reported
deleted file mode 100644
--- a/services/sync/tests/unit/test_node_reassignment.js
+++ /dev/null
@@ -1,332 +0,0 @@
-/* Any copyright is dedicated to the Public Domain.
-   http://creativecommons.org/publicdomain/zero/1.0/ */
-
-_("Test that node reassignment responses are respected on all kinds of " +
-  "requests.");
-
-// Don't sync any engines by default.
-Svc.DefaultPrefs.set("registerEngines", "")
-
-Cu.import("resource://services-sync/constants.js");
-Cu.import("resource://services-sync/policies.js");
-Cu.import("resource://services-sync/rest.js");
-Cu.import("resource://services-sync/service.js");
-Cu.import("resource://services-sync/status.js");
-Cu.import("resource://services-sync/log4moz.js");
-
-function run_test() {
-  Log4Moz.repository.getLogger("Sync.AsyncResource").level = Log4Moz.Level.Trace;
-  Log4Moz.repository.getLogger("Sync.ErrorHandler").level  = Log4Moz.Level.Trace;
-  Log4Moz.repository.getLogger("Sync.Resource").level      = Log4Moz.Level.Trace;
-  Log4Moz.repository.getLogger("Sync.RESTRequest").level   = Log4Moz.Level.Trace;
-  Log4Moz.repository.getLogger("Sync.Service").level       = Log4Moz.Level.Trace;
-  Log4Moz.repository.getLogger("Sync.SyncScheduler").level = Log4Moz.Level.Trace;
-  initTestLogging();
-
-  Engines.register(RotaryEngine);
-
-  // None of the failures in this file should result in a UI error.
-  function onUIError() {
-    do_throw("Errors should not be presented in the UI.");
-  }
-  Svc.Obs.add("weave:ui:login:error", onUIError);
-  Svc.Obs.add("weave:ui:sync:error", onUIError);
-
-  run_next_test();
-}
-
-/**
- * Emulate the following Zeus config:
- * $draining = data.get($prefix . $host . " draining");
- * if ($draining == "drain.") {
- *   log.warn($log_host_db_status . " migrating=1 (node-reassignment)" .
- *            $log_suffix);
- *   http.sendResponse("401 Node reassignment", $content_type,
- *                     '"server request: node reassignment"', "");
- * }
- */
-const reassignBody = "\"server request: node reassignment\"";
-
-// API-compatible with SyncServer handler. Bind `handler` to something to use
-// as a ServerCollection handler.
-function handleReassign(handler, req, resp) {
-  resp.setStatusLine(req.httpVersion, 401, "Node reassignment");
-  resp.setHeader("Content-Type", "application/json");
-  resp.bodyOutputStream.write(reassignBody, reassignBody.length);
-}
-
-/**
- * A node assignment handler.
- */
-const newNodeBody = "http://localhost:8080/";
-function installNodeHandler(server, next) {
-  function handleNodeRequest(req, resp) {
-    _("Client made a request for a node reassignment.");
-    resp.setStatusLine(req.httpVersion, 200, "OK");
-    resp.setHeader("Content-Type", "text/plain");
-    resp.bodyOutputStream.write(newNodeBody, newNodeBody.length);
-    Utils.nextTick(next);
-  }
-  let nodePath = "/user/1.0/johndoe/node/weave";
-  server.server.registerPathHandler(nodePath, handleNodeRequest);
-  _("Registered node handler at " + nodePath);
-}
-
-function prepareServer() {
-  Service.username   = "johndoe";
-  Service.passphrase = "abcdeabcdeabcdeabcdeabcdea";
-  Service.password   = "ilovejane";
-  Service.serverURL  = "http://localhost:8080/";
-  Service.clusterURL = "http://localhost:8080/";
-
-  do_check_eq(Service.userAPI, "http://localhost:8080/user/1.0/");
-  let server = new SyncServer();
-  server.registerUser("johndoe");
-  server.start();
-  return server;
-}
-
-/**
- * Make a test request to `url`, then watch the result of two syncs
- * to ensure that a node request was made.
- * Runs `undo` between the two.
- */
-function syncAndExpectNodeReassignment(server, firstNotification, undo,
-                                       secondNotification, url) {
-  function onwards() {
-    let nodeFetched = false;
-    function onFirstSync() {
-      _("First sync completed.");
-      Svc.Obs.remove(firstNotification, onFirstSync);
-      Svc.Obs.add(secondNotification, onSecondSync);
-
-      do_check_eq(Service.clusterURL, "");
-
-      // Track whether we fetched node/weave. We want to wait for the second
-      // sync to finish so that we're cleaned up for the next test, so don't
-      // run_next_test in the node handler.
-      nodeFetched = false;
-
-      // Verify that the client requests a node reassignment.
-      // Install a node handler to watch for these requests.
-      installNodeHandler(server, function () {
-        nodeFetched = true;
-      });
-
-      // Allow for tests to clean up error conditions.
-      _("Undoing test changes.");
-      undo();
-    }
-    function onSecondSync() {
-      _("Second sync completed.");
-      Svc.Obs.remove(secondNotification, onSecondSync);
-      SyncScheduler.clearSyncTriggers();
-
-      // Make absolutely sure that any event listeners are done with their work
-      // before we proceed.
-      waitForZeroTimer(function () {
-        _("Second sync nextTick.");
-        do_check_true(nodeFetched);
-        Service.startOver();
-        server.stop(run_next_test);
-      });
-    }
-
-    Svc.Obs.add(firstNotification, onFirstSync);
-    Service.sync();
-  }
-
-  // Make sure that it works!
-  let request = new RESTRequest(url);
-  request.get(function () {
-    do_check_eq(request.response.status, 401);
-    Utils.nextTick(onwards);
-  });
-}
-
-add_test(function test_momentary_401_engine() {
-  _("Test a failure for engine URLs that's resolved by reassignment.");
-  let server = prepareServer();
-  let john   = server.user("johndoe");
-
-  _("Enabling the Rotary engine.");
-  let engine = Engines.get("rotary");
-  engine.enabled = true;
-
-  // We need the server to be correctly set up prior to experimenting. Do this
-  // through a sync.
-  let global = {syncID: Service.syncID,
-                storageVersion: STORAGE_VERSION,
-                rotary: {version: engine.version,
-                         syncID:  engine.syncID}}
-  john.createCollection("meta").insert("global", global);
-
-  _("First sync to prepare server contents.");
-  Service.sync();
-
-  _("Setting up Rotary collection to 401.");
-  let rotary = john.createCollection("rotary");
-  let oldHandler = rotary.collectionHandler;
-  rotary.collectionHandler = handleReassign.bind(this, undefined);
-
-  // We want to verify that the clusterURL pref has been cleared after a 401
-  // inside a sync. Flag the Rotary engine to need syncing.
-  john.collection("rotary").timestamp += 1000;
-
-  function undo() {
-    rotary.collectionHandler = oldHandler;
-  }
-
-  syncAndExpectNodeReassignment(server,
-                                "weave:service:sync:finish",
-                                undo,
-                                "weave:service:sync:finish",
-                                Service.storageURL + "rotary");
-});
-
-// This test ends up being a failing fetch *after we're already logged in*.
-add_test(function test_momentary_401_info_collections() {
-  _("Test a failure for info/collections that's resolved by reassignment.");
-  let server = prepareServer();
-
-  _("First sync to prepare server contents.");
-  Service.sync();
-
-  // Return a 401 for info requests, particularly info/collections.
-  let oldHandler = server.toplevelHandlers.info;
-  server.toplevelHandlers.info = handleReassign;
-
-  function undo() {
-    server.toplevelHandlers.info = oldHandler;
-  }
-
-  syncAndExpectNodeReassignment(server,
-                                "weave:service:sync:error",
-                                undo,
-                                "weave:service:sync:finish",
-                                Service.infoURL);
-});
-
-add_test(function test_momentary_401_storage() {
-  _("Test a failure for any storage URL, not just engine parts. " +
-    "Resolved by reassignment.");
-  let server = prepareServer();
-
-  // Return a 401 for all storage requests.
-  let oldHandler = server.toplevelHandlers.storage;
-  server.toplevelHandlers.storage = handleReassign;
-
-  function undo() {
-    server.toplevelHandlers.storage = oldHandler;
-  }
-
-  syncAndExpectNodeReassignment(server,
-                                "weave:service:login:error",
-                                undo,
-                                "weave:service:sync:finish",
-                                Service.storageURL + "meta/global");
-});
-
-add_test(function test_loop_avoidance() {
-  _("Test that a repeated failure doesn't result in a sync loop " +
-    "if node reassignment cannot resolve the failure.");
-
-  let server = prepareServer();
-
-  // Return a 401 for all storage requests.
-  let oldHandler = server.toplevelHandlers.storage;
-  server.toplevelHandlers.storage = handleReassign;
-
-  let firstNotification  = "weave:service:login:error";
-  let secondNotification = "weave:service:login:error";
-  let thirdNotification  = "weave:service:sync:finish";
-
-  let nodeFetched = false;
-
-  // Track the time. We want to make sure the duration between the first and
-  // second sync is small, and then that the duration between second and third
-  // is set to be large.
-  let now;
-
-  function getReassigned() {
-    return Services.prefs.getBoolPref("services.sync.lastSyncReassigned");
-  }
-
-  function onFirstSync() {
-    _("First sync completed.");
-    Svc.Obs.remove(firstNotification, onFirstSync);
-    Svc.Obs.add(secondNotification, onSecondSync);
-
-    do_check_eq(Service.clusterURL, "");
-
-    // We got a 401 mid-sync, and set the pref accordingly.
-    do_check_true(Services.prefs.getBoolPref("services.sync.lastSyncReassigned"));
-
-    // Track whether we fetched node/weave. We want to wait for the second
-    // sync to finish so that we're cleaned up for the next test, so don't
-    // run_next_test in the node handler.
-    nodeFetched = false;
-
-    // Verify that the client requests a node reassignment.
-    // Install a node handler to watch for these requests.
-    installNodeHandler(server, function () {
-      nodeFetched = true;
-    });
-
-    // Update the timestamp.
-    now = Date.now();
-  }
-
-  function onSecondSync() {
-    _("Second sync completed.");
-    Svc.Obs.remove(secondNotification, onSecondSync);
-    Svc.Obs.add(thirdNotification, onThirdSync);
-
-    // This sync occurred within the backoff interval.
-    let elapsedTime = Date.now() - now;
-    do_check_true(elapsedTime < MINIMUM_BACKOFF_INTERVAL);
-
-    // This pref will be true until a sync completes successfully.
-    do_check_true(getReassigned());
-
-    // The timer will be set for some distant time.
-    // We store nextSync in prefs, which offers us only limited resolution.
-    // Include that logic here.
-    let expectedNextSync = 1000 * Math.floor((now + MINIMUM_BACKOFF_INTERVAL) / 1000);
-    _("Next sync scheduled for " + SyncScheduler.nextSync);
-    _("Expected to be slightly greater than " + expectedNextSync);
-
-    do_check_true(SyncScheduler.nextSync >= expectedNextSync);
-    do_check_true(!!SyncScheduler.syncTimer);
-
-    // Undo our evil scheme.
-    server.toplevelHandlers.storage = oldHandler;
-
-    // Bring the timer forward to kick off a successful sync, so we can watch
-    // the pref get cleared.
-    SyncScheduler.scheduleNextSync(0);
-  }
-  function onThirdSync() {
-    Svc.Obs.remove(thirdNotification, onThirdSync);
-
-    // That'll do for now; no more syncs.
-    SyncScheduler.clearSyncTriggers();
-
-    // Make absolutely sure that any event listeners are done with their work
-    // before we proceed.
-    waitForZeroTimer(function () {
-      _("Third sync nextTick.");
-
-      // A missing pref throws.
-      do_check_throws(getReassigned, Cr.NS_ERROR_UNEXPECTED);
-      do_check_true(nodeFetched);
-      Service.startOver();
-      server.stop(run_next_test);
-    });
-  }
-
-  Svc.Obs.add(firstNotification, onFirstSync);
-
-  now = Date.now();
-  Service.sync();
-});
--- a/services/sync/tests/unit/test_service_sync_401.js
+++ b/services/sync/tests/unit/test_service_sync_401.js
@@ -46,37 +46,31 @@ function run_test() {
       threw = true;
     });
 
     _("Initial state: We're successfully logged in.");
     Weave.Service.login();
     do_check_true(Weave.Service.isLoggedIn);
     do_check_eq(Weave.Status.login, Weave.LOGIN_SUCCEEDED);
 
-    _("Simulate having changed the password somewhere else.");
+    _("Simulate having changed the password somehwere else.");
     Weave.Service.password = "ilovejosephine";
 
     _("Let's try to sync.");
     Weave.Service.sync();
 
     _("Verify that sync() threw an exception.");
     do_check_true(threw);
 
     _("We're no longer logged in.");
     do_check_false(Weave.Service.isLoggedIn);
 
-    _("Sync status won't have changed yet, because we haven't tried again.");
+    _("Sync status.");
+    do_check_eq(Weave.Status.login, Weave.LOGIN_FAILED_LOGIN_REJECTED);
 
     _("globalScore is reset upon starting a sync.");
     do_check_eq(SyncScheduler.globalScore, 0);
 
-    _("Our next sync will fail appropriately.");
-    try {
-      Weave.Service.sync();
-    } catch (ex) {
-    }
-    do_check_eq(Weave.Status.login, Weave.LOGIN_FAILED_LOGIN_REJECTED);
-
   } finally {
     Weave.Svc.Prefs.resetBranch("");
     server.stop(do_test_finished);
   }
 }
--- a/services/sync/tests/unit/test_syncscheduler.js
+++ b/services/sync/tests/unit/test_syncscheduler.js
@@ -66,16 +66,33 @@ function cleanUpAndGo(server) {
     if (server) {
       server.stop(run_next_test);
     } else {
       run_next_test();
     }
   });
 }
 
+let timer;
+function waitForZeroTimer(callback) {
+  // First wait >100ms (nsITimers can take up to that much time to fire, so
+  // we can account for the timer in delayedAutoconnect) and then two event
+  // loop ticks (to account for the Utils.nextTick() in autoConnect).
+  let ticks = 2;
+  function wait() {
+    if (ticks) {
+      ticks -= 1;
+      Utils.nextTick(wait);
+      return;
+    }
+    callback();
+  }
+  timer = Utils.namedTimer(wait, 150, {}, "timer");
+}
+
 function run_test() {
   initTestLogging("Trace");
 
   Log4Moz.repository.getLogger("Sync.Service").level = Log4Moz.Level.Trace;
   Log4Moz.repository.getLogger("Sync.SyncScheduler").level = Log4Moz.Level.Trace;
 
   run_next_test();
 }
@@ -841,75 +858,8 @@ add_test(function test_sync_503_Retry_Af
   do_check_true(Status.minimumNextSync >= Date.now() + minimumExpectedDelay);
 
   // Verify that the next sync is actually going to wait that long.
   do_check_true(SyncScheduler.nextSync >= Date.now() + minimumExpectedDelay);
   do_check_true(SyncScheduler.syncTimer.delay >= minimumExpectedDelay);
 
   cleanUpAndGo(server);
 });
-
-add_test(function test_loginError_recoverable_reschedules() {
-  _("Verify that a recoverable login error schedules a new sync.");
-  Service.username = "johndoe";
-  Service.password = "ilovejane";
-  Service.passphrase = "abcdeabcdeabcdeabcdeabcdea";
-  Service.clusterURL = "http://localhost:8080/";
-  Service.persistLogin();
-  Status.resetSync(); // reset Status.login
-
-  Svc.Obs.add("weave:service:login:error", function onLoginError() {
-    Svc.Obs.remove("weave:service:login:error", onLoginError);
-    Utils.nextTick(function aLittleBitAfterLoginError() {
-      do_check_eq(Status.login, LOGIN_FAILED_NETWORK_ERROR);
-
-      let expectedNextSync = Date.now() + SyncScheduler.syncInterval;
-      do_check_true(SyncScheduler.nextSync > Date.now());
-      do_check_true(SyncScheduler.nextSync <= expectedNextSync);
-      do_check_true(SyncScheduler.syncTimer.delay > 0);
-      do_check_true(SyncScheduler.syncTimer.delay <= SyncScheduler.syncInterval);
-
-      cleanUpAndGo();
-    });
-  });
-
-  // Sanity check.
-  do_check_eq(SyncScheduler.nextSync, 0);
-  do_check_eq(SyncScheduler.syncTimer, null);
-  do_check_eq(Status.checkSetup(), STATUS_OK);
-  do_check_eq(Status.login, LOGIN_SUCCEEDED);
-
-  SyncScheduler.scheduleNextSync(0);
-});
-
-add_test(function test_loginError_fatal_clearsTriggers() {
-  _("Verify that a fatal login error clears sync triggers.");
-  Service.username = "johndoe";
-  Service.password = "ilovejane";
-  Service.passphrase = "abcdeabcdeabcdeabcdeabcdea";
-  Service.clusterURL = "http://localhost:8080/";
-  Service.persistLogin();
-  Status.resetSync(); // reset Status.login
-
-  let server = httpd_setup({
-    "/1.1/johndoe/info/collections": httpd_handler(401, "Unauthorized")
-  });
-
-  Svc.Obs.add("weave:service:login:error", function onLoginError() {
-    Svc.Obs.remove("weave:service:login:error", onLoginError);
-    Utils.nextTick(function aLittleBitAfterLoginError() {
-      do_check_eq(Status.login, LOGIN_FAILED_LOGIN_REJECTED);
-
-      do_check_eq(SyncScheduler.nextSync, 0);
-      do_check_eq(SyncScheduler.syncTimer, null);
-
-      cleanUpAndGo(server);
-    });
-  });
-
-  // Sanity check.
-  do_check_eq(SyncScheduler.nextSync, 0);
-  do_check_eq(SyncScheduler.syncTimer, null);
-  do_check_eq(Status.checkSetup(), STATUS_OK);
-  do_check_eq(Status.login, LOGIN_SUCCEEDED);
-
-  SyncScheduler.scheduleNextSync(0);
-});
--- a/services/sync/tests/unit/xpcshell.ini
+++ b/services/sync/tests/unit/xpcshell.ini
@@ -43,17 +43,16 @@ skip-if = (os == "mac" && debug) || os =
 [test_interval_triggers.js]
 [test_jpakeclient.js]
 # Bug 618233: this test produces random failures on Windows 7.
 # Bug 676978: test hangs on Android (see also testing/xpcshell/xpcshell.ini)
 skip-if = os == "win" || os == "android"
 [test_keys.js]
 [test_load_modules.js]
 [test_log4moz.js]
-[test_node_reassignment.js]
 [test_notifications.js]
 [test_password_store.js]
 [test_password_tracker.js]
 [test_places_guid_downgrade.js]
 [test_prefs_store.js]
 [test_prefs_tracker.js]
 [test_records_crypto.js]
 [test_records_crypto_generateEntry.js]