Bug 1148701 - React to Backoff and Retry-After headers from Reading List server. r=adw, a=sledru
authorMark Hammond <mhammond@skippinet.com.au>
Tue, 14 Apr 2015 11:14:00 +1000
changeset 258477 91df81e2edac
parent 258476 1412c445ff0d
child 258478 4f36d5aff5cf
push id4676
push userryanvm@gmail.com
push date2015-04-15 02:06 +0000
treeherdermozilla-beta@91df81e2edac [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersadw, sledru
bugs1148701
milestone38.0
Bug 1148701 - React to Backoff and Retry-After headers from Reading List server. r=adw, a=sledru
browser/components/readinglist/ServerClient.jsm
browser/components/readinglist/test/xpcshell/test_ServerClient.js
--- a/browser/components/readinglist/ServerClient.jsm
+++ b/browser/components/readinglist/ServerClient.jsm
@@ -128,21 +128,32 @@ ServerClient.prototype = {
       if (headers) {
         for (let [headerName, headerValue] in Iterator(headers)) {
           log.trace("Caller specified header: ${headerName}=${headerValue}", {headerName, headerValue});
           request.setHeader(headerName, headerValue);
         }
       }
 
       request.onComplete = error => {
+        // Although the server API docs say the "Backoff" header is on
+        // successful responses while "Retry-After" is on error responses, we
+        // just look for them both in both cases (as the scheduler makes no
+        // distinction)
+        let response = request.response;
+        if (response && response.headers) {
+          let backoff = response.headers["backoff"] || response.headers["retry-after"];
+          if (backoff) {
+            log.info("Server requested backoff", backoff);
+            Services.obs.notifyObservers(null, "readinglist:backoff-requested", backoff);
+          }
+        }
         if (error) {
           return reject(this._convertRestError(error));
         }
 
-        let response = request.response;
         log.debug("received response status: ${status} ${statusText}", response);
         // Handle response status codes we know about
         let result = {
           status: response.status,
           headers: response.headers
         };
         try {
           if (response.body) {
--- a/browser/components/readinglist/test/xpcshell/test_ServerClient.js
+++ b/browser/components/readinglist/test/xpcshell/test_ServerClient.js
@@ -76,16 +76,26 @@ function OAuthTokenServer() {
     },
   }
   server = new Server(handlers);
   server.numTokenFetches = 0;
   server.activeTokens = new Set();
   return server;
 }
 
+function promiseObserver(topic) {
+  return new Promise(resolve => {
+    function observe(subject, topic, data) {
+      Services.obs.removeObserver(observe, topic);
+      resolve(data);
+    }
+    Services.obs.addObserver(observe, topic, false);
+  });
+}
+
 // The tests.
 function run_test() {
   run_next_test();
 }
 
 // Arrange for the first token we hand out to be rejected - the client should
 // notice the 401 and silently get a new token and retry the request.
 add_task(function testAuthRetry() {
@@ -160,16 +170,82 @@ add_task(function testHeaders() {
       body: {foo: "bar"}});
     equal(response.status, 200, "got the 200 we expected");
     equal(response.headers["server-sent-header"], "hello", "got the server header");
   } finally {
     yield rlserver.stop();
   }
 });
 
+// Check that a "backoff" header causes the correct notification.
+add_task(function testBackoffHeader() {
+  let handlers = {
+    "/v1/batch": (request, response) => {
+      response.setHeader("Backoff", "123");
+      response.setStatusLine("1.1", 200, "OK");
+      response.write("{}");
+    }
+  };
+  let rlserver = new Server(handlers);
+  rlserver.start();
+
+  let observerPromise = promiseObserver("readinglist:backoff-requested");
+  try {
+    Services.prefs.setCharPref("readinglist.server", rlserver.host + "/v1");
+
+    let fxa = yield createMockFxA();
+    let sc = new ServerClient(fxa);
+    sc._getToken = () => Promise.resolve();
+
+    let response = yield sc.request({
+      path: "/batch",
+      method: "post",
+      headers: {"X-Foo": "bar"},
+      body: {foo: "bar"}});
+    equal(response.status, 200, "got the 200 we expected");
+    let data = yield observerPromise;
+    equal(data, "123", "got the expected header value.")
+  } finally {
+    yield rlserver.stop();
+  }
+});
+
+// Check that a "backoff" header causes the correct notification.
+add_task(function testRetryAfterHeader() {
+  let handlers = {
+    "/v1/batch": (request, response) => {
+      response.setHeader("Retry-After", "456");
+      response.setStatusLine("1.1", 500, "Not OK");
+      response.write("{}");
+    }
+  };
+  let rlserver = new Server(handlers);
+  rlserver.start();
+
+  let observerPromise = promiseObserver("readinglist:backoff-requested");
+  try {
+    Services.prefs.setCharPref("readinglist.server", rlserver.host + "/v1");
+
+    let fxa = yield createMockFxA();
+    let sc = new ServerClient(fxa);
+    sc._getToken = () => Promise.resolve();
+
+    let response = yield sc.request({
+      path: "/batch",
+      method: "post",
+      headers: {"X-Foo": "bar"},
+      body: {foo: "bar"}});
+    equal(response.status, 500, "got the 500 we expected");
+    let data = yield observerPromise;
+    equal(data, "456", "got the expected header value.")
+  } finally {
+    yield rlserver.stop();
+  }
+});
+
 // Check that unicode ends up as utf-8 in requests, and vice-versa in responses.
 // (Note the ServerClient assumes all strings in and out are UCS, and thus have
 // already been encoded/decoded (ie, it never expects to receive stuff already
 // utf-8 encoded, and never returns utf-8 encoded responses.)
 add_task(function testUTF8() {
   let handlers = {
     "/v1/hello": (request, response) => {
       // Get the body as bytes.