Bug 958447 - respect Retry-After header from token server. r=rnewman

services/common/tokenserverclient.js

@@ -17,6 +17,7 @@ Cu.import("resource://gre/modules/Preferences.jsm");
 Cu.import("resource://gre/modules/Log.jsm");
 Cu.import("resource://services-common/rest.js");
 Cu.import("resource://services-common/utils.js");
+Cu.import("resource://services-common/observers.js");
 
 const Prefs = new Preferences("services.common.tokenserverclient.");
 
@@ -328,6 +329,9 @@ TokenServerClient.prototype = {
       return;
     }
 
+    // Any response status can have an X-Backoff header.
+    this._maybeNotifyBackoff(response, "x-backoff");
+
     // The service shouldn't have any 3xx, so we don't need to handle those.
     if (response.status != 200) {
       // We /should/ have a Cornice error report in the JSON. We log that to
@@ -379,6 +383,10 @@ TokenServerClient.prototype = {
         error.cause = "unknown-service";
       }
 
+      // A Retry-After header should theoretically only appear on a 503, but
+      // we'll look for it on any error response.
+      this._maybeNotifyBackoff(response, "retry-after");
+
       cb(error, null);
       return;
     }
@@ -405,6 +413,23 @@ TokenServerClient.prototype = {
     });
   },
 
+  // Given an optional header value, notify that a backoff has been requested.
+  _maybeNotifyBackoff: function (response, headerName) {
+    let headerVal = response.headers[headerName];
+    if (!headerVal) {
+      return;
+    }
+    let backoffInterval;
+    try {
+      backoffInterval = parseInt(headerVal, 10);
+    } catch (ex) {
+      this._log.error("TokenServer response had invalid backoff value in '" +
+                      headerName + "' header: " + headerVal);
+      return;
+    }
+    Observers.notify("tokenserver:backoff:interval", backoffInterval);
+  },
+
   // override points for testing.
   newRESTRequest: function(url) {
     return new RESTRequest(url);

services/sync/modules/policies.js

@@ -92,6 +92,7 @@ SyncScheduler.prototype = {
     Svc.Obs.add("weave:engine:sync:applied", this);
     Svc.Obs.add("weave:service:setup-complete", this);
     Svc.Obs.add("weave:service:start-over", this);
+    Svc.Obs.add("tokenserver:backoff:interval", this);
 
     if (Status.checkSetup() == STATUS_OK) {
       Svc.Idle.addIdleObserver(this, Svc.Prefs.get("scheduler.idleTime"));
@@ -181,6 +182,7 @@ SyncScheduler.prototype = {
         this.nextSync = 0;
         this.handleSyncError();
         break;
+      case "tokenserver:backoff:interval":
       case "weave:service:backoff:interval":
         let requested_interval = subject * 1000;
         this._log.debug("Got backoff notification: " + requested_interval + "ms");

services/sync/tests/unit/test_browserid_identity.js

@@ -13,6 +13,7 @@ Cu.import("resource://gre/modules/FxAccounts.jsm");
 Cu.import("resource://gre/modules/FxAccountsClient.jsm");
 Cu.import("resource://gre/modules/FxAccountsCommon.js");
 Cu.import("resource://services-common/tokenserverclient.js");
+Cu.import("resource://services-sync/service.js");
 Cu.import("resource://services-sync/status.js");
 Cu.import("resource://services-sync/constants.js");
 
@@ -363,6 +364,36 @@ add_task(function test_getTokenErrors() {
   Assert.equal(Status.login, LOGIN_FAILED_NETWORK_ERROR, "login state is LOGIN_FAILED_NETWORK_ERROR");
 });
 
+add_task(function test_getTokenErrorWithRetry() {
+  _("tokenserver sends an observer notification on various backoff headers.");
+
+  // Set Sync's backoffInterval to zero - after we simulated the backoff header
+  // it should reflect the value we sent.
+  Status.backoffInterval = 0;
+  _("Arrange for a 503 with a Retry-After header.");
+  yield initializeIdentityWithTokenServerFailure({
+    status: 503,
+    headers: {"content-type": "application/json",
+              "retry-after": "100"},
+    body: JSON.stringify({}),
+  });
+  // The observer should have fired - check it got the value in the response.
+  Assert.equal(Status.login, LOGIN_FAILED_NETWORK_ERROR, "login was rejected");
+  // Sync will have the value in ms with some slop - so check it is at least that.
+  Assert.ok(Status.backoffInterval >= 100000);
+
+  _("Arrange for a 200 with an X-Backoff header.");
+  Status.backoffInterval = 0;
+  yield initializeIdentityWithTokenServerFailure({
+    status: 503,
+    headers: {"content-type": "application/json",
+              "x-backoff": "200"},
+    body: JSON.stringify({}),
+  });
+  // The observer should have fired - check it got the value in the response.
+  Assert.ok(Status.backoffInterval >= 200000);
+});
+
 add_task(function test_getHAWKErrors() {
   _("BrowserIDManager correctly handles various HAWK failures.");