Bug 591102 - Correctly identify network errors. r=mconnor
authorPhilipp von Weitershausen <philipp@weitershausen.de>
Mon, 31 Jan 2011 20:55:48 -0800
changeset 62002 c4ae1660dee9285c0176f82366b7c1b8cc8103f9
parent 62001 c6fafc22853b50388e5eb6612042cd0e9f3e2c46
child 62003 aa9d8ceefe9afa1640923e2a26a355cbdde28554
push idunknown
push userunknown
push dateunknown
reviewersmconnor
bugs591102
Bug 591102 - Correctly identify network errors. r=mconnor
services/sync/modules/service.js
services/sync/tests/unit/test_service_sync_checkServerError.js
services/sync/tests/unit/test_service_verifyLogin.js
--- a/services/sync/modules/service.js
+++ b/services/sync/modules/service.js
@@ -689,28 +689,29 @@ WeaveSvc.prototype = {
       }
     }
     return false;
   },
 
   /**
    * Perform the info fetch as part of a login or key fetch.
    */
-  _fetchInfo: function _fetchInfo(url, logout) {
+  _fetchInfo: function _fetchInfo(url) {
     let infoURL = url || this.infoURL;
     
     this._log.trace("In _fetchInfo: " + infoURL);
-    let info = new Resource(infoURL).get();
+    let info;
+    try {
+      info = new Resource(infoURL).get();
+    } catch (ex) {
+      this._checkServerError(ex);
+      throw ex;
+    }
     if (!info.success) {
-      if (info.status == 401) {
-        if (logout) {
-          this.logout();
-          Status.login = LOGIN_FAILED_LOGIN_REJECTED;
-        }
-      }
+      this._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.");
@@ -825,47 +826,47 @@ WeaveSvc.prototype = {
     } catch (e) {
       // This means no keys are present, or there's a network error.
       return false;
     }
   },
   
   verifyLogin: function verifyLogin()
     this._notify("verify-login", "", function() {
-      // 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;
-      }
-
       if (!this.username) {
         this._log.warn("No username in verifyLogin.");
         Status.login = LOGIN_FAILED_NO_USERNAME;
         return false;
       }
 
       // Unlock master password, or return.
+      // Attaching auth credentials to a request requires access to
+      // passwords, which means that Resource.get can throw MP-related
+      // exceptions!
+      // Try to fetch the passphrase first, while we still have control.
       try {
-        // Fetch collection info on every startup.
-        // Attaching auth credentials to a request requires access to
-        // passwords, which means that Resource.get can throw MP-related
-        // exceptions!
-        // Try to fetch the passphrase first, while we still have control.
-        try {
-          this.passphrase;
-        } catch (ex) {
-          this._log.debug("Fetching passphrase threw " + ex +
-                          "; assuming master password locked.");
-          Status.login = MASTER_PASSWORD_LOCKED;
-          return false;
+        this.passphrase;
+      } 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
+        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();
         
         switch (test.status) {
           case 200:
             // The user is authenticated.
 
             // We have no way of verifying the passphrase right now,
             // so wait until remoteSetup to do so.
@@ -1701,17 +1702,17 @@ WeaveSvc.prototype = {
     let now = Math.floor(Date.now() / 1000);
     let lastPing = Svc.Prefs.get("lastPing", 0);
     if (now - lastPing > 86400) { // 60 * 60 * 24
       infoURL += "?v=" + WEAVE_VERSION;
       Svc.Prefs.set("lastPing", now);
     }
 
     // Figure out what the last modified time is for each collection
-    let info = this._fetchInfo(infoURL, true);
+    let info = this._fetchInfo(infoURL);
     this.globalScore = 0;
 
     // Convert the response to an object and read out the modified times
     for each (let engine in [Clients].concat(Engines.getAll()))
       engine.lastModified = info.obj[engine.name] || 0;
 
     if (!(this._remoteSetup(info)))
       throw "aborting sync, remote setup failed";
@@ -1969,28 +1970,56 @@ WeaveSvc.prototype = {
     });
     this.wipeServer(collections);
     
     // Generate and upload new keys. Do this last so we don't wipe them...
     this.generateNewSymmetricKeys();
   },
 
   /**
-   * Check to see if this is a failure
-   *
+   * Handle HTTP response results or exceptions and set the appropriate
+   * Status.* bits.
    */
   _checkServerError: function WeaveSvc__checkServerError(resp) {
-    if (Utils.checkStatus(resp.status, null, [500, [502, 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));
+    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;
     }
-    if (resp.status == 400 && resp == RESPONSE_OVER_QUOTA)
-      Status.sync = OVER_QUOTA;
+
+    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;
+    }
   },
   /**
    * Return a value for a backoff interval.  Maximum is eight hours, unless
    * Status.backoffInterval is higher.
    *
    */
   _calculateBackoff: function WeaveSvc__calculateBackoff(attempts, base_interval) {
     const MAXIMUM_BACKOFF_INTERVAL = 8 * 60 * 60 * 1000; // 8 hours
--- a/services/sync/tests/unit/test_service_sync_checkServerError.js
+++ b/services/sync/tests/unit/test_service_sync_checkServerError.js
@@ -64,18 +64,18 @@ function test_backoff500(next) {
     
     Service.login();
     Service.sync();
     do_check_true(Status.enforceBackoff);
   } finally {
     Engines.unregister("catapult");
     Status.resetBackoff();
     Service.startOver();
-    server.stop(next);
   }
+  server.stop(next);
 }
 
 function test_backoff503(next) {
   _("Test: HTTP 503 with Retry-After header leads to backoff notification and sets backoff status.");
   let server = sync_httpd_setup();
   setUp();
 
   const BACKOFF = 42;
@@ -97,18 +97,18 @@ function test_backoff503(next) {
     Service.sync();
 
     do_check_true(Status.enforceBackoff);
     do_check_eq(backoffInterval, BACKOFF);
   } finally {
     Engines.unregister("catapult");
     Status.resetBackoff();
     Service.startOver();
-    server.stop(next);
   }
+  server.stop(next);
 }
 
 function test_overQuota(next) {
   _("Test: HTTP 400 with body error code 14 means over quota.");
   let server = sync_httpd_setup();
   setUp();
 
   Engines.register(CatapultEngine);
@@ -123,18 +123,62 @@ function test_overQuota(next) {
     Service.login();
     Service.sync();
 
     do_check_eq(Status.sync, OVER_QUOTA);
   } finally {
     Engines.unregister("catapult");
     Status.resetSync();
     Service.startOver();
-    server.stop(next);
+  }
+  server.stop(next);
+}
+
+function test_service_networkError(next) {
+  setUp();
+  // Provoke connection refused.
+  Service.clusterURL = "http://localhost:12345/";
+  try {
+    do_check_eq(Status.sync, SYNC_SUCCEEDED);
+
+    Service._loggedIn = true;
+    Service.sync();
+
+    do_check_eq(Status.sync, LOGIN_FAILED_NETWORK_ERROR);
+  } finally {
+    Status.resetSync();
+    Service.startOver();
   }
+  next();
+}
+
+function test_engine_networkError(next) {
+  _("Test: Network related exceptions from engine.sync() lead to the right status code.");
+  let server = sync_httpd_setup();
+  setUp();
+
+  Engines.register(CatapultEngine);
+  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);
+
+    Service.login();
+    Service.sync();
+
+    do_check_eq(Status.sync, LOGIN_FAILED_NETWORK_ERROR);
+  } finally {
+    Engines.unregister("catapult");
+    Status.resetSync();
+    Service.startOver();
+  }
+  server.stop(next);
 }
 
 // Slightly misplaced test as it doesn't actually test checkServerError,
 // but the observer for "weave:engine:sync:apply-failed".
 function test_engine_applyFailed(next) {
   let server = sync_httpd_setup();
   setUp();
 
@@ -151,23 +195,25 @@ function test_engine_applyFailed(next) {
     Service.login();
     Service.sync();
 
     do_check_eq(Status.engines["steam"], ENGINE_APPLY_FAIL);
   } finally {
     Engines.unregister("catapult");
     Status.resetSync();
     Service.startOver();
-    server.stop(next);
   }
+  server.stop(next);
 }
 
 function run_test() {
   if (DISABLE_TESTS_BUG_604565)
     return;
 
   do_test_pending();
   asyncChainTests(test_backoff500,
                   test_backoff503,
                   test_overQuota,
+                  test_service_networkError,
+                  test_engine_networkError,
                   test_engine_applyFailed,
                   do_test_finished)();
 }
--- a/services/sync/tests/unit/test_service_verifyLogin.js
+++ b/services/sync/tests/unit/test_service_verifyLogin.js
@@ -44,46 +44,64 @@ function run_test() {
   try {
     Service.serverURL = "http://localhost:8080/";
 
     _("Force the initial state.");
     Status.service = STATUS_OK;
     do_check_eq(Status.service, STATUS_OK);
 
     _("Credentials won't check out because we're not configured yet.");
+    Status.resetSync();
     do_check_false(Service.verifyLogin());
     do_check_eq(Status.service, CLIENT_NOT_CONFIGURED);
     do_check_eq(Status.login, LOGIN_FAILED_NO_USERNAME);
 
     _("Try again with username and password set.");
+    Status.resetSync();
     Service.username = "johndoe";
     Service.password = "ilovejane";
     do_check_false(Service.verifyLogin());
     do_check_eq(Status.service, CLIENT_NOT_CONFIGURED);
     do_check_eq(Status.login, LOGIN_FAILED_NO_PASSPHRASE);
 
     _("verifyLogin() has found out the user's cluster URL, though.");
     do_check_eq(Service.clusterURL, "http://localhost:8080/api/");
 
     _("Success if passphrase is set.");
+    Status.resetSync();
     Service.passphrase = "foo";
     do_check_true(Service.verifyLogin());
     do_check_eq(Status.service, STATUS_OK);
     do_check_eq(Status.login, LOGIN_SUCCEEDED);
 
     _("If verifyLogin() encounters a server error, it flips on the backoff flag and notifies observers on a 503 with Retry-After.");
+    Status.resetSync();
     Service.username = "janedoe";
     do_check_false(Status.enforceBackoff);
     let backoffInterval;    
     Svc.Obs.add("weave:service:backoff:interval", function(subject, data) {
       backoffInterval = subject;
     });
     do_check_false(Service.verifyLogin());
     do_check_true(Status.enforceBackoff);
     do_check_eq(backoffInterval, 42);
     do_check_eq(Status.service, LOGIN_FAILED);
     do_check_eq(Status.login, LOGIN_FAILED_SERVER_ERROR);
 
+    _("Ensure a network error when finding the cluster sets the right Status bits.");
+    Status.resetSync();
+    Service.serverURL = "http://localhost:12345/";
+    do_check_false(Service.verifyLogin());
+    do_check_eq(Status.service, LOGIN_FAILED);
+    do_check_eq(Status.login, LOGIN_FAILED_NETWORK_ERROR);
+
+    _("Ensure a network error when getting the collection info sets the right Status bits.");
+    Status.resetSync();
+    Service.clusterURL = "http://localhost:12345/";
+    do_check_false(Service.verifyLogin());
+    do_check_eq(Status.service, LOGIN_FAILED);
+    do_check_eq(Status.login, LOGIN_FAILED_NETWORK_ERROR);
+
   } finally {
     Svc.Prefs.resetBranch("");
     server.stop(do_test_finished);
   }
 }