Bug 659067 - Part 1: Move error handling and logging from Service to ErrorHandler. r=philikon
authorMarina Samuel <msamuel@mozilla.com>
Fri, 26 Aug 2011 14:01:35 -0700
changeset 76304 298e7535687c453f49f4185ded4011afa3fdfb3f
parent 76303 459aa245e7f052196e60e935edf35c23b98b6b87
child 76305 bdd0d53dcaff2e0616b08086aa575de4697fe24c
push id3
push userfelipc@gmail.com
push dateFri, 30 Sep 2011 20:09:13 +0000
reviewersphilikon
bugs659067
milestone9.0a1
Bug 659067 - Part 1: Move error handling and logging from Service to ErrorHandler. r=philikon
browser/base/content/browser-syncui.js
services/sync/modules/main.js
services/sync/modules/policies.js
services/sync/modules/service.js
services/sync/tests/unit/test_errorhandler.js
services/sync/tests/unit/test_errorhandler_filelog.js
services/sync/tests/unit/test_errorhandler_sync_checkServerError.js
services/sync/tests/unit/test_service_filelog.js
services/sync/tests/unit/test_service_login.js
services/sync/tests/unit/test_service_sync_checkServerError.js
services/sync/tests/unit/xpcshell.ini
--- a/browser/base/content/browser-syncui.js
+++ b/browser/base/content/browser-syncui.js
@@ -220,17 +220,17 @@ let gSyncUI = {
     this.updateUI();
   },
 
   onLoginError: function SUI_onLoginError() {
     // if login fails, any other notifications are essentially moot
     Weave.Notifications.removeAll();
 
     // if we haven't set up the client, don't show errors
-    if (this._needsSetup() || Weave.Service.shouldIgnoreError()) {
+    if (this._needsSetup() || Weave.ErrorHandler.shouldIgnoreError()) {
       this.updateUI();
       return;
     }
 
     let title = this._stringBundle.GetStringFromName("error.login.title");
     let reason = Weave.Utils.getErrorString(Weave.Status.login);
     let description =
       this._stringBundle.formatStringFromName("error.login.description", [reason], 1);
@@ -347,17 +347,17 @@ let gSyncUI = {
     if (!success) {
       if (Weave.Status.login != Weave.LOGIN_SUCCEEDED) {
         this.onLoginError();
         return;
       }
 
       // Ignore network related errors unless we haven't been able to
       // sync for a while.
-      if (Weave.Service.shouldIgnoreError()) {
+      if (Weave.ErrorHandler.shouldIgnoreError()) {
         this.updateUI();
         return;
       }
 
       let error = Weave.Utils.getErrorString(Weave.Status.sync);
       let description =
         this._stringBundle.formatStringFromName("error.sync.description", [error], 1);
 
--- a/services/sync/modules/main.js
+++ b/services/sync/modules/main.js
@@ -47,17 +47,17 @@ let lazies = {
   "engines/forms.js":     ["FormEngine"],
   "engines/history.js":   ["HistoryEngine"],
   "engines/prefs.js":     ["PrefsEngine"],
   "engines/passwords.js": ["PasswordEngine"],
   "engines/tabs.js":      ["TabEngine"],
   "identity.js":          ["Identity", "ID"],
   "jpakeclient.js":       ["JPAKEClient"],
   "notifications.js":     ["Notifications", "Notification", "NotificationButton"],
-  "policies.js":          ["SyncScheduler"],
+  "policies.js":          ["SyncScheduler", "ErrorHandler"],
   "resource.js":          ["Resource", "AsyncResource", "Auth",
                            "BasicAuthenticator", "NoOpAuthenticator"],
   "service.js":           ["Service"],
   "status.js":            ["Status"],
   "util.js":              ['Utils', 'Svc', 'Str']
 };
 
 function lazyImport(module, dest, props) {
--- a/services/sync/modules/policies.js
+++ b/services/sync/modules/policies.js
@@ -32,19 +32,19 @@
  * decision by deleting the provisions above and replace them with the notice
  * and other provisions required by the GPL or the LGPL. If you do not delete
  * the provisions above, a recipient may use your version of this file under
  * the terms of any one of the MPL, the GPL or the LGPL.
  *
  * ***** END LICENSE BLOCK ***** */
 
 
-const EXPORTED_SYMBOLS = ["SyncScheduler"];
+const EXPORTED_SYMBOLS = ["SyncScheduler", "ErrorHandler"];
 
-const Cu = Components.utils;
+const {classes: Cc, interfaces: Ci, utils: Cu, results: Cr} = Components;
 
 Cu.import("resource://services-sync/constants.js");
 Cu.import("resource://services-sync/log4moz.js");
 Cu.import("resource://services-sync/util.js");
 Cu.import("resource://services-sync/engines.js");
 Cu.import("resource://services-sync/engines/clients.js");
 Cu.import("resource://services-sync/status.js");
 
@@ -408,8 +408,190 @@ let SyncScheduler = {
     this._log.debug("Clearing sync triggers.");
 
     // Clear out any scheduled syncs
     if (this.syncTimer)
       this.syncTimer.clear();
   }
 
 };
+
+const LOG_PREFIX_SUCCESS = "success-";
+const LOG_PREFIX_ERROR   = "error-";
+
+let ErrorHandler = {
+
+  init: function init() {
+    Svc.Obs.add("weave:service:login:error", this);
+    Svc.Obs.add("weave:service:sync:error", this);
+    Svc.Obs.add("weave:service:sync:finish", this);
+
+    this.initLogs();
+  },
+
+  initLogs: function initLogs() {
+    this._log = Log4Moz.repository.getLogger("Sync.ErrorHandler");
+		this._log.level = Log4Moz.Level[Svc.Prefs.get("log.logger.service.main")];
+
+    let root = Log4Moz.repository.getLogger("Sync");
+    root.level = Log4Moz.Level[Svc.Prefs.get("log.rootLogger")];
+
+    let formatter = new Log4Moz.BasicFormatter();
+    let capp = new Log4Moz.ConsoleAppender(formatter);
+    capp.level = Log4Moz.Level[Svc.Prefs.get("log.appender.console")];
+    root.addAppender(capp);
+
+    let dapp = new Log4Moz.DumpAppender(formatter);
+    dapp.level = Log4Moz.Level[Svc.Prefs.get("log.appender.dump")];
+    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) {
+    switch(topic) {
+      case "weave:service:login:error":
+        if (Status.login == LOGIN_FAILED_NETWORK_ERROR &&
+            !Services.io.offline) {
+          this._ignorableErrorCount += 1;
+        } else {
+          this.resetFileLog(Svc.Prefs.get("log.appender.file.logOnError"),
+                            LOG_PREFIX_ERROR);
+        }
+        break;
+      case "weave:service:sync:error":
+        switch (Status.sync) {
+          case LOGIN_FAILED_NETWORK_ERROR:
+            if (!Services.io.offline) {
+              this._ignorableErrorCount += 1;
+            }
+            break;
+          case CREDENTIALS_CHANGED:
+            Weave.Service.logout();
+            break;
+          default:
+            this.resetFileLog(Svc.Prefs.get("log.appender.file.logOnError"),
+                              LOG_PREFIX_ERROR);
+            break;
+        }
+        break;
+      case "weave:service:sync:finish":
+        this.resetFileLog(Svc.Prefs.get("log.appender.file.logOnSuccess"),
+                          LOG_PREFIX_SUCCESS);
+        this._ignorableErrorCount = 0;
+        break;
+    }
+  },
+
+  /**
+   * Generate a log file for the sync that just completed
+   * and refresh the input & output streams.
+   * 
+   * @param flushToFile
+   *        the log file to be flushed/reset
+   *
+   * @param filenamePrefix
+   *        a value of either LOG_PREFIX_SUCCESS or LOG_PREFIX_ERROR
+   *        to be used as the log filename prefix
+   */
+  resetFileLog: function resetFileLog(flushToFile, filenamePrefix) {
+    let inStream = this._logAppender.getInputStream();
+    this._logAppender.reset();
+    if (flushToFile && inStream) {
+      try {
+        let filename = filenamePrefix + Date.now() + ".txt";
+        let file = FileUtils.getFile("ProfD", ["weave", "logs", filename]);
+        let outStream = FileUtils.openFileOutputStream(file);
+        NetUtil.asyncCopy(inStream, outStream, function () {
+          Svc.Obs.notify("weave:service:reset-file-log");
+        });
+      } catch (ex) {
+        Svc.Obs.notify("weave:service:reset-file-log");
+      }
+    } else {
+      Svc.Obs.notify("weave:service:reset-file-log");
+    }
+  },
+
+  /**
+   * Translates server error codes to meaningful strings.
+   * 
+   * @param code
+   *        server error code as an integer
+   */
+  errorStr: function errorStr(code) {
+    switch (code.toString()) {
+    case "1":
+      return "illegal-method";
+    case "2":
+      return "invalid-captcha";
+    case "3":
+      return "invalid-username";
+    case "4":
+      return "cannot-overwrite-resource";
+    case "5":
+      return "userid-mismatch";
+    case "6":
+      return "json-parse-failure";
+    case "7":
+      return "invalid-password";
+    case "8":
+      return "invalid-record";
+    case "9":
+      return "weak-password";
+    default:
+      return "generic-server-error";
+    }
+  },
+
+  _ignorableErrorCount: 0,
+  shouldIgnoreError: function shouldIgnoreError() {
+    // Never show an error bar for a locked master password.
+    return (Status.login == MASTER_PASSWORD_LOCKED) ||
+           ([Status.login, Status.sync].indexOf(LOGIN_FAILED_NETWORK_ERROR) != -1
+            && this._ignorableErrorCount < MAX_IGNORE_ERROR_COUNT);
+  },
+
+  /**
+   * Handle HTTP response results or exceptions and set the appropriate
+   * Status.* bits.
+   */
+  checkServerError: function checkServerError(resp) {
+    switch (resp.status) {
+      case 400:
+        if (resp == RESPONSE_OVER_QUOTA) {
+          Status.sync = OVER_QUOTA;
+        }
+        break;
+
+      case 401:
+        Weave.Service.logout();
+        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"]) {
+          Svc.Obs.notify("weave:service:backoff:interval",
+                         parseInt(resp.headers["retry-after"], 10));
+        }
+        break;
+    }
+
+    switch (resp.result) {
+      case Cr.NS_ERROR_UNKNOWN_HOST:
+      case Cr.NS_ERROR_CONNECTION_REFUSED:
+      case Cr.NS_ERROR_NET_TIMEOUT:
+      case Cr.NS_ERROR_NET_RESET:
+      case Cr.NS_ERROR_NET_INTERRUPT:
+      case Cr.NS_ERROR_PROXY_CONNECTION_REFUSED:
+        // The constant says it's about login, but in fact it just
+        // indicates general network error.
+        Status.sync = LOGIN_FAILED_NETWORK_ERROR;
+        break;
+    }
+  },
+};
--- a/services/sync/modules/service.js
+++ b/services/sync/modules/service.js
@@ -53,19 +53,16 @@ const CLUSTER_BACKOFF = 5 * 60 * 1000; /
 // How long a key to generate from an old passphrase.
 const PBKDF2_KEY_BYTES = 16;
 
 const CRYPTO_COLLECTION = "crypto";
 const KEYS_WBO = "keys";
 
 const LOG_DATE_FORMAT = "%Y-%m-%d %H:%M:%S";
 
-const LOG_PREFIX_SUCCESS = "success-";
-const LOG_PREFIX_ERROR   = "error-";
-
 Cu.import("resource://gre/modules/XPCOMUtils.jsm");
 Cu.import("resource://services-sync/record.js");
 Cu.import("resource://services-sync/constants.js");
 Cu.import("resource://services-sync/engines.js");
 Cu.import("resource://services-sync/engines/clients.js");
 Cu.import("resource://services-sync/ext/Preferences.js");
 Cu.import("resource://services-sync/identity.js");
 Cu.import("resource://services-sync/log4moz.js");
@@ -361,17 +358,22 @@ WeaveSvc.prototype = {
     return false;
   },
 
   /**
    * Prepare to initialize the rest of Weave after waiting a little bit
    */
   onStartup: function onStartup() {
     this._migratePrefs();
-    this._initLogs();
+    ErrorHandler.init();
+
+    this._log = Log4Moz.repository.getLogger("Sync.Service");
+    this._log.level =
+      Log4Moz.Level[Svc.Prefs.get("log.logger.service.main")];
+
     this._log.info("Loading Weave " + WEAVE_VERSION);
 
     this.enabled = true;
 
     this._registerEngines();
 
     let ua = Cc["@mozilla.org/network/protocol;1?name=http"].
       getService(Ci.nsIHttpProtocolHandler).userAgent;
@@ -379,21 +381,17 @@ WeaveSvc.prototype = {
 
     if (!this._checkCrypto()) {
       this.enabled = false;
       this._log.info("Could not load the Weave crypto component. Disabling " +
                       "Weave, since it will not work correctly.");
     }
 
     Svc.Obs.add("weave:service:setup-complete", this);
-    Svc.Obs.add("weave:service:sync:finish", this);
-    Svc.Obs.add("weave:service:login:error", this);
-    Svc.Obs.add("weave:service:sync:error", this);
     Svc.Obs.add("weave:engine:sync:applied", this);
-    Svc.Obs.add("weave:resource:status:401", this);
     Svc.Prefs.observe("engine.", this);
 
     SyncScheduler.init();
 
     if (!this.enabled)
       this._log.info("Weave Sync disabled");
 
     // Create Weave identities (for logging in, and for encryption)
@@ -455,57 +453,16 @@ WeaveSvc.prototype = {
     for each (let pref in oldPrefNames)
       Svc.Prefs.set(pref, oldPref.get(pref));
 
     // Remove all the old prefs and remember that we've migrated
     oldPref.resetBranch("");
     Svc.Prefs.set("migrated", true);
   },
 
-  _initLogs: function WeaveSvc__initLogs() {
-    this._log = Log4Moz.repository.getLogger("Sync.Service");
-    this._log.level =
-      Log4Moz.Level[Svc.Prefs.get("log.logger.service.main")];
-
-    let root = Log4Moz.repository.getLogger("Sync");
-    root.level = Log4Moz.Level[Svc.Prefs.get("log.rootLogger")];
-
-    let formatter = new Log4Moz.BasicFormatter();
-    let capp = new Log4Moz.ConsoleAppender(formatter);
-    capp.level = Log4Moz.Level[Svc.Prefs.get("log.appender.console")];
-    root.addAppender(capp);
-
-    let dapp = new Log4Moz.DumpAppender(formatter);
-    dapp.level = Log4Moz.Level[Svc.Prefs.get("log.appender.dump")];
-    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);
-  },
-
-  _resetFileLog: function _resetFileLog(flushToFile, filenamePrefix) {
-    let inStream = this._logAppender.getInputStream();
-    this._logAppender.reset();
-    if (flushToFile && inStream) {
-      try {
-        let filename = filenamePrefix + Date.now() + ".txt";
-        let file = FileUtils.getFile("ProfD", ["weave", "logs", filename]);
-        let outStream = FileUtils.openFileOutputStream(file);
-        NetUtil.asyncCopy(inStream, outStream, function () {
-          Svc.Obs.notify("weave:service:reset-file-log");
-        });
-      } catch (ex) {
-        Svc.Obs.notify("weave:service:reset-file-log");
-      }
-    } else {
-      Svc.Obs.notify("weave:service:reset-file-log");
-    }
-  },
-
   /**
    * Register the built-in engines for certain applications
    */
   _registerEngines: function WeaveSvc__registerEngines() {
     let engines = [];
     // Applications can provide this preference (comma-separated list)
     // to specify which engines should be registered on startup.
     let pref = Svc.Prefs.get("registerEngines");
@@ -524,59 +481,26 @@ WeaveSvc.prototype = {
 
   observe: function WeaveSvc__observe(subject, topic, data) {
     switch (topic) {
       case "weave:service:setup-complete":
         let status = this._checkSetup();
         if (status != STATUS_DISABLED && status != CLIENT_NOT_CONFIGURED)
             Svc.Obs.notify("weave:engine:start-tracking");
         break;
-      case "weave:service:login:error":
-        if (Status.login == LOGIN_FAILED_NETWORK_ERROR &&
-            !Services.io.offline) {
-          this._ignorableErrorCount += 1;
-        } else {
-          this._resetFileLog(Svc.Prefs.get("log.appender.file.logOnError"),
-                             LOG_PREFIX_ERROR);
-        }
-        break;
-      case "weave:service:sync:error":
-        switch (Status.sync) {
-          case LOGIN_FAILED_NETWORK_ERROR:
-            if (!Services.io.offline) {
-              this._ignorableErrorCount += 1;
-            }
-            break;
-          case CREDENTIALS_CHANGED:
-            this.logout();
-            break;
-          default:
-            this._resetFileLog(Svc.Prefs.get("log.appender.file.logOnError"),
-                               LOG_PREFIX_ERROR);
-            break;
-        }
-        break;
-      case "weave:service:sync:finish":
-        this._resetFileLog(Svc.Prefs.get("log.appender.file.logOnSuccess"),
-                           LOG_PREFIX_SUCCESS);
-        this._ignorableErrorCount = 0;
-        break;
       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._syncError = true;
           this._log.debug(data + " failed to apply some records.");
         }
         break;
-      case "weave:resource:status:401":
-        this._handleResource401(subject);
-        break;
       case "nsPref:changed":
         if (this._ignorePrefObserver)
           return;
         let engine = data.slice((PREFS_BRANCH + "engine.").length);
         this._handleEngineStatusChanged(engine);
         break;
     }
   },
@@ -598,17 +522,17 @@ WeaveSvc.prototype = {
 
     let fail;
     let res = new Resource(this.userAPI + this.username + "/node/weave");
     try {
       let node = res.get();
       switch (node.status) {
         case 400:
           Status.login = LOGIN_FAILED_LOGIN_REJECTED;
-          fail = "Find cluster denied: " + this._errorStr(node);
+          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")
             node = null;
@@ -661,21 +585,21 @@ WeaveSvc.prototype = {
   _fetchInfo: function _fetchInfo(url) {
     let infoURL = url || this.infoURL;
 
     this._log.trace("In _fetchInfo: " + infoURL);
     let info;
     try {
       info = new Resource(infoURL).get();
     } catch (ex) {
-      this._checkServerError(ex);
+      ErrorHandler.checkServerError(ex);
       throw ex;
     }
     if (!info.success) {
-      this._checkServerError(info);
+      ErrorHandler.checkServerError(info);
       throw "aborting sync, failed to get collections";
     }
     return info;
   },
 
   verifyAndFetchSymmetricKeys: function verifyAndFetchSymmetricKeys(infoResponse) {
 
     this._log.debug("Fetching and verifying -- or generating -- symmetric keys.");
@@ -880,17 +804,17 @@ WeaveSvc.prototype = {
               return this.verifyLogin();
 
             // We must have the right cluster, but the server doesn't expect us
             Status.login = LOGIN_FAILED_LOGIN_REJECTED;
             return false;
 
           default:
             // Server didn't respond with something that we expected
-            this._checkServerError(test);
+            ErrorHandler.checkServerError(test);
             Status.login = LOGIN_FAILED_SERVER_ERROR;
             return false;
         }
       }
       catch (ex) {
         // Must have failed on some network issue
         this._log.debug("verifyLogin failed: " + Utils.exceptionStr(ex));
         Status.login = LOGIN_FAILED_NETWORK_ERROR;
@@ -1094,41 +1018,16 @@ WeaveSvc.prototype = {
       return;
 
     this._log.info("Logging out");
     this._loggedIn = false;
 
     Svc.Obs.notify("weave:service:logout:finish");
   },
 
-  _errorStr: function WeaveSvc__errorStr(code) {
-    switch (code.toString()) {
-    case "1":
-      return "illegal-method";
-    case "2":
-      return "invalid-captcha";
-    case "3":
-      return "invalid-username";
-    case "4":
-      return "cannot-overwrite-resource";
-    case "5":
-      return "userid-mismatch";
-    case "6":
-      return "json-parse-failure";
-    case "7":
-      return "invalid-password";
-    case "8":
-      return "invalid-record";
-    case "9":
-      return "weak-password";
-    default:
-      return "generic-server-error";
-    }
-  },
-
   checkAccount: function checkAccount(account) {
     let username = this._usernameFromAccount(account);
     let url = this.userAPI + username;
     let res = new Resource(url);
     res.authenticator = new NoOpAuthenticator();
 
     let data = "";
     try {
@@ -1139,17 +1038,17 @@ WeaveSvc.prototype = {
         else if (data == "1")
           return "notAvailable";
       }
 
     }
     catch(ex) {}
 
     // Convert to the error string, or default to generic on exception.
-    return this._errorStr(data);
+    return ErrorHandler.errorStr(data);
   },
 
   createAccount: function createAccount(email, password,
                                         captchaChallenge, captchaResponse) {
     let username = this._usernameFromAccount(email);
     let payload = JSON.stringify({
       "password": Utils.encodeUTF8(password),
       "email": email,
@@ -1171,17 +1070,17 @@ WeaveSvc.prototype = {
       let register = res.put(payload);
       if (register.success) {
         this._log.info("Account created: " + register);
         return null;
       }
 
       // Must have failed, so figure out the reason
       if (register.status == 400)
-        error = this._errorStr(register);
+        error = ErrorHandler.errorStr(register);
     }
     catch(ex) {
       this._log.warn("Failed to create account: " + ex);
     }
 
     return error;
   },
 
@@ -1243,17 +1142,17 @@ WeaveSvc.prototype = {
     if (!meta || !meta.payload.storageVersion || !meta.payload.syncID ||
         STORAGE_VERSION > parseFloat(remoteVersion)) {
 
       this._log.info("One of: no meta, no meta storageVersion, or no meta syncID. Fresh start needed.");
 
       // abort the server wipe if the GET status was anything other than 404 or 200
       let status = Records.response.status;
       if (status != 200 && status != 404) {
-        this._checkServerError(Records.response);
+        ErrorHandler.checkServerError(Records.response);
         Status.sync = METARECORD_DOWNLOAD_FAIL;
         this._log.warn("Unknown error while downloading metadata record. " +
                        "Aborting sync.");
         return false;
       }
 
       if (!meta)
         this._log.info("No metadata record, server wipe needed");
@@ -1356,24 +1255,16 @@ WeaveSvc.prototype = {
       reason = kFirstSyncChoiceNotMade;
 
     if (ignore && ignore.indexOf(reason) != -1)
       return "";
 
     return reason;
   },
 
-  _ignorableErrorCount: 0,
-  shouldIgnoreError: function shouldIgnoreError() {
-    // Never show an error bar for a locked master password.
-    return (Status.login == MASTER_PASSWORD_LOCKED) ||
-           ([Status.login, Status.sync].indexOf(LOGIN_FAILED_NETWORK_ERROR) != -1
-            && this._ignorableErrorCount < MAX_IGNORE_ERROR_COUNT);
-  },
-
   sync: function sync() {
     let dateStr = new Date().toLocaleFormat(LOG_DATE_FORMAT);
     this._log.debug("User-Agent: " + SyncStorageRequest.prototype.userAgent);
     this._log.info("Starting sync at " + dateStr);
     this._catch(function () {
       // Make sure we're logged in.
       if (this._shouldLogin()) {
         this._log.debug("In sync: should login.");
@@ -1606,17 +1497,17 @@ WeaveSvc.prototype = {
         // 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;
       }
 
-      this._checkServerError(e);
+      ErrorHandler.checkServerError(e);
 
       Status.engines = [engine.name, e.failureCode || ENGINE_UNKNOWN_FAIL];
 
       this._syncError = true;
       this._log.debug(engine.name + " failed: " + Utils.exceptionStr(e));
       return true;
     }
   },
@@ -1696,59 +1587,16 @@ WeaveSvc.prototype = {
     this.wipeServer(collections);
 
     // Generate, upload, and download new keys. Do this last so we don't wipe
     // them...
     this.generateNewSymmetricKeys();
   },
 
   /**
-   * Handle HTTP response results or exceptions and set the appropriate
-   * Status.* bits.
-   */
-  _checkServerError: function WeaveSvc__checkServerError(resp) {
-    switch (resp.status) {
-      case 400:
-        if (resp == RESPONSE_OVER_QUOTA) {
-          Status.sync = OVER_QUOTA;
-        }
-        break;
-
-      case 401:
-        this.logout();
-        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"]) {
-          Svc.Obs.notify("weave:service:backoff:interval",
-                         parseInt(resp.headers["retry-after"], 10));
-        }
-        break;
-    }
-
-    switch (resp.result) {
-      case Cr.NS_ERROR_UNKNOWN_HOST:
-      case Cr.NS_ERROR_CONNECTION_REFUSED:
-      case Cr.NS_ERROR_NET_TIMEOUT:
-      case Cr.NS_ERROR_NET_RESET:
-      case Cr.NS_ERROR_NET_INTERRUPT:
-      case Cr.NS_ERROR_PROXY_CONNECTION_REFUSED:
-        // The constant says it's about login, but in fact it just
-        // indicates general network error.
-        Status.sync = LOGIN_FAILED_NETWORK_ERROR;
-        break;
-    }
-  },
-
-  /**
    * Wipe user data from the server.
    *
    * @param collections [optional]
    *        Array of collections to wipe. If not given, all collections are wiped.
    *
    * @param includeKeys [optional]
    *        If true, keys/pubkey and keys/privkey are deleted from the server.
    *        This is false by default, which will cause the usual upgrade paths
new file mode 100644
--- /dev/null
+++ b/services/sync/tests/unit/test_errorhandler.js
@@ -0,0 +1,126 @@
+/* Any copyright is dedicated to the Public Domain.
+   http://creativecommons.org/publicdomain/zero/1.0/ */
+
+Cu.import("resource://services-sync/engines/clients.js");
+Cu.import("resource://services-sync/constants.js");
+Cu.import("resource://services-sync/policies.js");
+Cu.import("resource://services-sync/status.js");
+
+Svc.DefaultPrefs.set("registerEngines", "");
+Cu.import("resource://services-sync/service.js");
+
+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();
+}
+
+function sync_httpd_setup() {
+  let global = new ServerWBO("global", {
+    syncID: Service.syncID,
+    storageVersion: STORAGE_VERSION,
+    engines: {clients: {version: Clients.version,
+                        syncID: Clients.syncID}}
+  });
+  let clientsColl = new ServerCollection({}, true);
+
+  // Tracking info/collections.
+  let collectionsHelper = track_collections_helper();
+  let upd = collectionsHelper.with_updated_collection;
+
+  let handler_401 = httpd_handler(401, "Unauthorized");
+  return httpd_setup({
+    "/1.1/johndoe/storage/meta/global": upd("meta", global.handler()),
+    "/1.1/johndoe/info/collections": collectionsHelper.handler,
+    "/1.1/johndoe/storage/crypto/keys":
+      upd("crypto", (new ServerWBO("keys")).handler()),
+    "/1.1/johndoe/storage/clients": upd("clients", clientsColl.handler()),
+
+    "/1.1/janedoe/storage/meta/global": handler_401,
+    "/1.1/janedoe/info/collections": handler_401,
+  });
+}
+
+function setUp() {
+  Service.username = "johndoe";
+  Service.password = "ilovejane";
+  Service.passphrase = "abcdeabcdeabcdeabcdeabcdea";
+  Service.clusterURL = "http://localhost:8080/";
+
+  generateNewKeys();
+  let serverKeys = CollectionKeys.asWBO("crypto", "keys");
+  serverKeys.encrypt(Service.syncKeyBundle);
+  return serverKeys.upload(Service.cryptoKeysURL);
+}
+
+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);
+
+  // Make sync fail due to login rejected.
+  Service.username = "janedoe";
+  Service.sync();
+  
+  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();
+  do_check_eq(Status.sync, SYNC_SUCCEEDED);
+  do_check_true(Service.isLoggedIn);
+
+  // Make sync fail due to changed credentials. We simply re-encrypt
+  // the keys with a different Sync Key, without changing the local one.
+  let newSyncKeyBundle = new SyncKeyBundle(PWDMGR_PASSPHRASE_REALM, Service.username);
+  newSyncKeyBundle.keyStr = "23456234562345623456234562";
+  let keys = CollectionKeys.asWBO();
+  keys.encrypt(newSyncKeyBundle);
+  keys.upload(Service.cryptoKeysURL);
+  Service.sync();
+  
+  do_check_eq(Status.sync, CREDENTIALS_CHANGED);
+  do_check_false(Service.isLoggedIn);
+
+  // Clean up.
+  Service.startOver();
+  server.stop(run_next_test);
+});
+
+add_test(function test_shouldIgnoreError() {
+  Status.login = MASTER_PASSWORD_LOCKED;
+  Status.sync = LOGIN_FAILED_NETWORK_ERROR;
+
+  // Error ignored since master password locked.
+  do_check_true(ErrorHandler.shouldIgnoreError());
+
+  Status.login = LOGIN_FAILED_LOGIN_REJECTED;
+  Status.sync = LOGIN_FAILED_NETWORK_ERROR
+
+  // Error ignored due to network error.
+  do_check_true(ErrorHandler.shouldIgnoreError());
+
+  Status.login = LOGIN_FAILED_LOGIN_REJECTED;
+  Status.sync = NO_SYNC_NODE_FOUND;
+
+  // Error not ignored.
+  do_check_false(ErrorHandler.shouldIgnoreError());
+
+  run_next_test();
+});
rename from services/sync/tests/unit/test_service_filelog.js
rename to services/sync/tests/unit/test_errorhandler_filelog.js
--- a/services/sync/tests/unit/test_service_filelog.js
+++ b/services/sync/tests/unit/test_errorhandler_filelog.js
@@ -95,17 +95,17 @@ add_test(function test_logOnSuccess_true
       run_next_test();
     });
   });
 
   // Fake a successful sync.
   Svc.Obs.notify("weave:service:sync:finish");
 });
 
-add_test(function test_logOnError_false() {
+add_test(function test_sync_error_logOnError_false() {
   Svc.Prefs.set("log.appender.file.logOnError", false);
 
   let log = Log4Moz.repository.getLogger("Sync.Test.FileLog");
   log.info("this won't show up");
 
   Svc.Obs.add("weave:service:reset-file-log", function onResetFileLog() {
     Svc.Obs.remove("weave:service:reset-file-log", onResetFileLog);
     // No log file was written.
@@ -114,17 +114,17 @@ add_test(function test_logOnError_false(
     Svc.Prefs.resetBranch("");
     run_next_test();
   });
 
   // Fake an unsuccessful sync.
   Svc.Obs.notify("weave:service:sync:error");
 });
 
-add_test(function test_logOnError_true() {
+add_test(function test_sync_error_logOnError_true() {
   Svc.Prefs.set("log.appender.file.logOnError", true);
 
   let log = Log4Moz.repository.getLogger("Sync.Test.FileLog");
   const MESSAGE = "this WILL show up";
   log.info(MESSAGE);
 
   Svc.Obs.add("weave:service:reset-file-log", function onResetFileLog() {
     Svc.Obs.remove("weave:service:reset-file-log", onResetFileLog);
@@ -154,8 +154,68 @@ add_test(function test_logOnError_true()
       Svc.Prefs.resetBranch("");
       run_next_test();
     });
   });
 
   // Fake an unsuccessful sync.
   Svc.Obs.notify("weave:service:sync:error");
 });
+
+add_test(function test_login_error_logOnError_false() {
+  Svc.Prefs.set("log.appender.file.logOnError", false);
+
+  let log = Log4Moz.repository.getLogger("Sync.Test.FileLog");
+  log.info("this won't show up");
+
+  Svc.Obs.add("weave:service:reset-file-log", function onResetFileLog() {
+    Svc.Obs.remove("weave:service:reset-file-log", onResetFileLog);
+    // No log file was written.
+    do_check_false(logsdir.directoryEntries.hasMoreElements());
+
+    Svc.Prefs.resetBranch("");
+    run_next_test();
+  });
+
+  // Fake an unsuccessful login.
+  Svc.Obs.notify("weave:service:login:error");
+});
+
+add_test(function test_login_error_logOnError_true() {
+  Svc.Prefs.set("log.appender.file.logOnError", true);
+
+  let log = Log4Moz.repository.getLogger("Sync.Test.FileLog");
+  const MESSAGE = "this WILL show up";
+  log.info(MESSAGE);
+
+  Svc.Obs.add("weave:service:reset-file-log", function onResetFileLog() {
+    Svc.Obs.remove("weave:service:reset-file-log", onResetFileLog);
+
+    // Exactly one log file was written.
+    let entries = logsdir.directoryEntries;
+    do_check_true(entries.hasMoreElements());
+    let logfile = entries.getNext().QueryInterface(Ci.nsILocalFile);
+    do_check_eq(logfile.leafName.slice(-4), ".txt");
+    do_check_eq(logfile.leafName.slice(0, LOG_PREFIX_ERROR.length),
+                LOG_PREFIX_ERROR);
+    do_check_false(entries.hasMoreElements());
+
+    // Ensure the log message was actually written to file.
+    readFile(logfile, function (error, data) {
+      do_check_true(Components.isSuccessCode(error));
+      do_check_neq(data.indexOf(MESSAGE), -1);
+
+      // Clean up.
+      try {
+        logfile.remove(false);
+      } catch(ex) {
+        dump("Couldn't delete file: " + ex + "\n");
+        // Stupid Windows box.
+      }
+
+      Svc.Prefs.resetBranch("");
+      run_next_test();
+    });
+  });
+
+  // Fake an unsuccessful sync.
+  Svc.Obs.notify("weave:service:login:error");
+});
rename from services/sync/tests/unit/test_service_sync_checkServerError.js
rename to services/sync/tests/unit/test_errorhandler_sync_checkServerError.js
--- a/services/sync/tests/unit/test_service_sync_checkServerError.js
+++ b/services/sync/tests/unit/test_errorhandler_sync_checkServerError.js
@@ -1,11 +1,12 @@
 Cu.import("resource://services-sync/engines.js");
 Cu.import("resource://services-sync/status.js");
 Cu.import("resource://services-sync/constants.js");
+Cu.import("resource://services-sync/policies.js");
 
 Cu.import("resource://services-sync/util.js");
 Svc.DefaultPrefs.set("registerEngines", "");
 Cu.import("resource://services-sync/service.js");
 Cu.import("resource://services-sync/record.js");
 
 initTestLogging();
 
@@ -146,103 +147,103 @@ add_test(function test_overQuota() {
   server.stop(run_next_test);
 });
 
 add_test(function test_service_networkError() {
   _("Test: Connection refused error from Service.sync() leads to the right status code.");
   setUp();
   // Provoke connection refused.
   Service.clusterURL = "http://localhost:12345/";
-  Service._ignorableErrorCount = 0;
+  ErrorHandler._ignorableErrorCount = 0;
 
   try {
     do_check_eq(Status.sync, SYNC_SUCCEEDED);
 
     Service._loggedIn = true;
     Service.sync();
 
     do_check_eq(Status.sync, LOGIN_FAILED_NETWORK_ERROR);
-    do_check_eq(Service._ignorableErrorCount, 1);
+    do_check_eq(ErrorHandler._ignorableErrorCount, 1);
   } finally {
     Status.resetSync();
     Service.startOver();
   }
   run_next_test();
 });
 
 add_test(function test_service_offline() {
   _("Test: Wanting to sync in offline mode leads to the right status code but does not increment the ignorable error count.");
   setUp();
   Services.io.offline = true;
-  Service._ignorableErrorCount = 0;
+  ErrorHandler._ignorableErrorCount = 0;
 
   try {
     do_check_eq(Status.sync, SYNC_SUCCEEDED);
 
     Service._loggedIn = true;
     Service.sync();
 
     do_check_eq(Status.sync, LOGIN_FAILED_NETWORK_ERROR);
-    do_check_eq(Service._ignorableErrorCount, 0);
+    do_check_eq(ErrorHandler._ignorableErrorCount, 0);
   } finally {
     Status.resetSync();
     Service.startOver();
   }
   Services.io.offline = false;
   run_next_test();
 });
 
 add_test(function test_service_reset_ignorableErrorCount() {
   _("Test: Successful sync resets the ignorable error count.");
   setUp();
   let server = sync_httpd_setup();
-  Service._ignorableErrorCount = 10;
+  ErrorHandler._ignorableErrorCount = 10;
 
   // Disable the engine so that sync completes.
   let engine = Engines.get("catapult");
   engine.enabled = false;
 
   try {
     do_check_eq(Status.sync, SYNC_SUCCEEDED);
 
     do_check_true(generateAndUploadKeys());
 
     Service.login();
     Service.sync();
 
     do_check_eq(Status.sync, SYNC_SUCCEEDED);
-    do_check_eq(Service._ignorableErrorCount, 0);
+    do_check_eq(ErrorHandler._ignorableErrorCount, 0);
   } finally {
     Status.resetSync();
     Service.startOver();
   }
   server.stop(run_next_test);
 });
 
 add_test(function test_engine_networkError() {
   _("Test: Network related exceptions from engine.sync() lead to the right status code.");
   setUp();
   let server = sync_httpd_setup();
-  Service._ignorableErrorCount = 0;
+  ErrorHandler._ignorableErrorCount = 0;
 
   let engine = Engines.get("catapult");
   engine.enabled = true;
   engine.exception = Components.Exception("NS_ERROR_UNKNOWN_HOST",
                                           Cr.NS_ERROR_UNKNOWN_HOST);
 
   try {
     do_check_eq(Status.sync, SYNC_SUCCEEDED);
 
     do_check_true(generateAndUploadKeys());
 
     Service.login();
     Service.sync();
 
     do_check_eq(Status.sync, LOGIN_FAILED_NETWORK_ERROR);
-    do_check_eq(Service._ignorableErrorCount, 1);
+    do_check_eq(ErrorHandler._ignorableErrorCount, 1);
   } finally {
     Status.resetSync();
     Service.startOver();
   }
   server.stop(run_next_test);
 });
 
 add_test(function test_resource_timeout() {
@@ -268,17 +269,17 @@ add_test(function test_resource_timeout(
     Status.resetSync();
     Service.startOver();
   }
   server.stop(run_next_test);
 });
 
 
 // Slightly misplaced test as it doesn't actually test checkServerError,
-// but the observer for "weave:engine:sync:apply-failed".
+// but the observer for "weave:engine:sync:applied".
 // This test should be the last one since it monkeypatches the engine object
 // and we should only have one engine object throughout the file (bug 629664).
 add_test(function test_engine_applyFailed() {
   setUp();
   let server = sync_httpd_setup();
 
   let engine = Engines.get("catapult");
   engine.enabled = true;
--- a/services/sync/tests/unit/test_service_login.js
+++ b/services/sync/tests/unit/test_service_login.js
@@ -24,20 +24,20 @@ function run_test() {
 
   run_next_test();
 }
 
 add_test(function test_offline() {
   try {
     _("The right bits are set when we're offline.");
     Services.io.offline = true;
-    do_check_eq(Service._ignorableErrorCount, 0);
+    do_check_eq(ErrorHandler._ignorableErrorCount, 0);
     do_check_false(!!Service.login());
     do_check_eq(Status.login, LOGIN_FAILED_NETWORK_ERROR);
-    do_check_eq(Service._ignorableErrorCount, 0);
+    do_check_eq(ErrorHandler._ignorableErrorCount, 0);
     Services.io.offline = false;
   } finally {
     Svc.Prefs.resetBranch("");
     run_next_test();
   }
 });
 
 function setup() {
--- a/services/sync/tests/unit/xpcshell.ini
+++ b/services/sync/tests/unit/xpcshell.ini
@@ -20,16 +20,19 @@ tail =
 [test_clients_engine.js]
 [test_clients_escape.js]
 [test_collection_inc_get.js]
 [test_collections_recovery.js]
 [test_corrupt_keys.js]
 [test_engine.js]
 [test_engine_abort.js]
 [test_enginemanager.js]
+[test_errorhandler.js]
+[test_errorhandler_filelog.js]
+[test_errorhandler_sync_checkServerError.js]
 [test_forms_store.js]
 [test_forms_tracker.js]
 [test_history_engine.js]
 [test_history_store.js]
 [test_history_tracker.js]
 [test_hmac_error.js]
 [test_interval_triggers.js]
 [test_jpakeclient.js]
@@ -54,29 +57,27 @@ skip-if = os == "win" || os == "android"
 [test_restrequest.js]
 [test_score_triggers.js]
 [test_service_attributes.js]
 [test_service_changePassword.js]
 [test_service_checkAccount.js]
 [test_service_cluster.js]
 [test_service_createAccount.js]
 [test_service_detect_upgrade.js]
-[test_service_filelog.js]
 # Bug 664090: this test persistently fails on Windows opt builds.
 # Bug 676978: test hangs on Android (see also testing/xpcshell/xpcshell.ini)
 skip-if = (os == "win" && !debug) || os == "android"
 [test_service_getStorageInfo.js]
 [test_service_login.js]
 [test_service_migratePrefs.js]
 [test_service_passwordUTF8.js]
 [test_service_persistLogin.js]
 [test_service_startOver.js]
 [test_service_startup.js]
 [test_service_sync_401.js]
-[test_service_sync_checkServerError.js]
 # Bug 604565: this test intermittently hangs on OS X debug builds.
 # Bug 676978: test hangs on Android (see also testing/xpcshell/xpcshell.ini)
 skip-if = (os == "mac" && debug) || os == "android"
 [test_service_sync_locked.js]
 [test_service_sync_remoteSetup.js]
 # Bug 604565: this test intermittently hangs on OS X debug builds.
 # Bug 676978: test hangs on Android (see also testing/xpcshell/xpcshell.ini)
 skip-if = (os == "mac" && debug) || os == "android"