Bug 1333431 - Fixed ERROR_PARSE thrown on 304 response when fetching profile. r?eoger draft
authorMayank Srivastav <mayanksri18@yahoo.in>
Tue, 07 Feb 2017 23:25:01 +0530
changeset 480673 59144282cdb96270c07e3e3280f03c488ce4fa06
parent 479958 e677ba018b22558fef1d07b74d416fd3a28a5dc3
child 545024 dc2a0951f0c4bd74b3e1dccc6be7a807bc412c0b
push id44621
push userbmo:eoger@fastmail.com
push dateWed, 08 Feb 2017 19:42:47 +0000
reviewerseoger
bugs1333431
milestone54.0a1
Bug 1333431 - Fixed ERROR_PARSE thrown on 304 response when fetching profile. r?eoger MozReview-Commit-ID: KJPvZeU10xP
services/fxaccounts/FxAccountsProfile.jsm
services/fxaccounts/FxAccountsProfileClient.jsm
services/fxaccounts/tests/xpcshell/test_profile.js
services/fxaccounts/tests/xpcshell/test_profile_client.js
--- a/services/fxaccounts/FxAccountsProfile.jsm
+++ b/services/fxaccounts/FxAccountsProfile.jsm
@@ -94,26 +94,26 @@ this.FxAccountsProfile.prototype = {
       this._currentFetchPromise = null;
     }
     return this.fxa.getProfileCache()
       .then(profileCache => {
         const etag = profileCache ? profileCache.etag : null;
         return this.client.fetchProfile(etag);
       })
       .then(response => {
-        return this._cacheProfile(response);
+        // response may be null if the profile was not modified (same ETag).
+        return response ? this._cacheProfile(response) : null;
       })
       .then(body => { // finally block
         onFinally();
+        // body may be null if the profile was not modified
         return body;
       }, err => {
         onFinally();
-        if (err.code != 304) { // fetchProfile() throws when the profile wasn't modified
-          throw err;
-        }
+        throw err;
       });
   },
 
   _fetchAndCacheProfile() {
     if (!this._currentFetchPromise) {
       this._currentFetchPromise = this._fetchAndCacheProfileInternal();
     }
     return this._currentFetchPromise;
--- a/services/fxaccounts/FxAccountsProfileClient.jsm
+++ b/services/fxaccounts/FxAccountsProfileClient.jsm
@@ -122,17 +122,18 @@ this.FxAccountsProfileClient.prototype =
    *
    * @param {String} path
    *        Profile server path, i.e "/profile".
    * @param {String} method
    *        Type of request, i.e "GET".
    * @param {String} token
    * @param {String} etag
    * @return Promise
-   *         Resolves: {body: Object, etag: Object} Successful response from the Profile server.
+   *         Resolves: {body: Object, etag: Object} Successful response from the Profile server
+                        or null if 304 is hit (same ETag).
    *         Rejects: {FxAccountsProfileClientError} Profile client error.
    * @private
    */
   _rawRequest(path, method, token, etag) {
     return new Promise((resolve, reject) => {
       let profileDataUrl = this.serverURL + path;
       let request = new this._Request(profileDataUrl);
       method = method.toUpperCase();
@@ -149,16 +150,19 @@ this.FxAccountsProfileClient.prototype =
             error: ERROR_NETWORK,
             errno: ERRNO_NETWORK,
             message: error.toString(),
           }));
         }
 
         let body = null;
         try {
+          if (request.response.status == 304) {
+            return resolve(null);
+          }
           body = JSON.parse(request.response.body);
         } catch (e) {
           return reject(new FxAccountsProfileClientError({
             error: ERROR_PARSE,
             errno: ERRNO_PARSE,
             code: request.response.status,
             message: request.response.body,
           }));
--- a/services/fxaccounts/tests/xpcshell/test_profile.js
+++ b/services/fxaccounts/tests/xpcshell/test_profile.js
@@ -286,16 +286,39 @@ add_task(function* fetchAndCacheProfileO
   client.fetchProfile = function() {
     return Promise.resolve({body: { avatar: "myimg"}});
   };
 
   let got = yield profile._fetchAndCacheProfile();
   do_check_eq(got.avatar, "myimg");
 });
 
+add_test(function fetchAndCacheProfile_alreadyCached() {
+  let cachedUrl = "cachedurl";
+  let fxa = mockFxa();
+  fxa.profileCache = { profile: { avatar: cachedUrl }, etag: "bogusETag" };
+  let client = mockClient(fxa);
+  client.fetchProfile = function(etag) {
+    do_check_eq(etag, "bogusETag");
+    return Promise.resolve(null);
+  };
+
+  let profile = CreateFxAccountsProfile(fxa, client);
+  profile._cacheProfile = function(toCache) {
+    do_throw("This method should not be called.");
+  };
+
+  return profile._fetchAndCacheProfile()
+    .then(result => {
+      do_check_eq(result, null);
+      do_check_eq(fxa.profileCache.profile.avatar, cachedUrl);
+      run_next_test();
+    });
+});
+
 // Check that a new profile request within PROFILE_FRESHNESS_THRESHOLD of the
 // last one doesn't kick off a new request to check the cached copy is fresh.
 add_task(function* fetchAndCacheProfileAfterThreshold() {
   let numFetches = 0;
   let client = mockClient(mockFxa());
   client.fetchProfile = function() {
     numFetches += 1;
     return Promise.resolve({ avatar: "myimg"});
--- a/services/fxaccounts/tests/xpcshell/test_profile_client.js
+++ b/services/fxaccounts/tests/xpcshell/test_profile_client.js
@@ -67,17 +67,17 @@ let mockResponseError = function(error) 
   };
 };
 
 add_test(function successfulResponse() {
   let client = new FxAccountsProfileClient(PROFILE_OPTIONS);
   let response = {
     success: true,
     status: STATUS_SUCCESS,
-    headers: { etag:"bogusETag" },
+    headers: { etag: "bogusETag" },
     body: "{\"email\":\"someone@restmail.net\",\"uid\":\"0d5c1a89b8c54580b8e3e8adadae864a\"}",
   };
 
   client._Request = new mockResponse(response);
   client.fetchProfile()
     .then(
       function(result) {
         do_check_eq(client._Request._requestUri, "http://127.0.0.1:1111/v1/profile");
@@ -107,16 +107,34 @@ add_test(function setsIfNoneMatchETagHea
         do_check_eq(result.body.email, "someone@restmail.net");
         do_check_eq(result.body.uid, "0d5c1a89b8c54580b8e3e8adadae864a");
         do_check_true(req.ifNoneMatchSet);
         run_next_test();
       }
     );
 });
 
+add_test(function successful304Response() {
+  let client = new FxAccountsProfileClient(PROFILE_OPTIONS);
+  let response = {
+    success: true,
+    headers: { etag: "bogusETag" },
+    status: 304,
+  };
+
+  client._Request = new mockResponse(response);
+  client.fetchProfile()
+    .then(
+      function(result) {
+        do_check_eq(result, null);
+        run_next_test();
+      }
+    );
+});
+
 add_test(function parseErrorResponse() {
   let client = new FxAccountsProfileClient(PROFILE_OPTIONS);
   let response = {
     success: true,
     status: STATUS_SUCCESS,
     body: "unexpected",
   };