Bug 1332993 - Do not write logs on FxA profile 304 response. r?markh draft
authorEdouard Oger <eoger@fastmail.com>
Mon, 23 Jan 2017 11:25:04 -0500
changeset 465701 8b89395924f3fb7a130a6587b2acdb14312d7be7
parent 464990 5a4412474c63e1d9e66036d603ac42e9cb2b9150
child 543219 27e58e9e90cc00388d17cda3d04b1886266d72c9
push id42676
push userbmo:eoger@fastmail.com
push dateTue, 24 Jan 2017 17:50:12 +0000
reviewersmarkh
bugs1332993
milestone54.0a1
Bug 1332993 - Do not write logs on FxA profile 304 response. r?markh MozReview-Commit-ID: Hf5d6rz8HNQ
services/fxaccounts/FxAccountsProfile.jsm
services/fxaccounts/tests/xpcshell/test_profile.js
--- a/services/fxaccounts/FxAccountsProfile.jsm
+++ b/services/fxaccounts/FxAccountsProfile.jsm
@@ -67,17 +67,16 @@ this.FxAccountsProfile.prototype = {
   _notifyProfileChange(uid) {
     this._isNotifying = true;
     Services.obs.notifyObservers(null, ON_PROFILE_CHANGE_NOTIFICATION, uid);
     this._isNotifying = false;
   },
 
   // Cache fetched data and send out a notification so that UI can update.
   _cacheProfile(response) {
-    this._cachedAt = Date.now();
     let profileCache = {
       profile: response.body,
       etag: response.etag
     };
 
     return this.fxa.setProfileCache(profileCache)
       .then(() => {
         return this.fxa.getSignedInUser();
@@ -85,30 +84,36 @@ this.FxAccountsProfile.prototype = {
       .then(userData => {
         log.debug("notifying profile changed for user ${uid}", userData);
         this._notifyProfileChange(userData.uid);
         return response.body;
       });
   },
 
   _fetchAndCacheProfileInternal() {
+    let onFinally = () => {
+      this._cachedAt = Date.now();
+      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);
       })
       .then(body => { // finally block
-        this._currentFetchPromise = null;
+        onFinally();
         return body;
-      }, e => {
-        this._currentFetchPromise = null;
-        throw e;
+      }, err => {
+        onFinally();
+        if (err.code != 304) { // fetchProfile() throws when the profile wasn't modified
+          throw err;
+        }
       });
   },
 
   _fetchAndCacheProfile() {
     if (!this._currentFetchPromise) {
       this._currentFetchPromise = this._fetchAndCacheProfileInternal();
     }
     return this._currentFetchPromise;
@@ -120,18 +125,17 @@ this.FxAccountsProfile.prototype = {
   getProfile() {
     return this.fxa.getProfileCache()
       .then(profileCache => {
         if (profileCache) {
           if (Date.now() > this._cachedAt + this.PROFILE_FRESHNESS_THRESHOLD) {
             // Note that _fetchAndCacheProfile isn't returned, so continues
             // in the background.
             this._fetchAndCacheProfile().catch(err => {
-              log.error("Background refresh of profile failed, bumping _cachedAt", err);
-              this._cachedAt = Date.now();
+              log.error("Background refresh of profile failed", err);
             });
           } else {
             log.trace("not checking freshness of profile as it remains recent");
           }
           return profileCache.profile;
         }
         return this._fetchAndCacheProfile();
       });
--- a/services/fxaccounts/tests/xpcshell/test_profile.js
+++ b/services/fxaccounts/tests/xpcshell/test_profile.js
@@ -111,43 +111,60 @@ add_test(function cacheProfile_change() 
   let fxa = mockFxa();
   fxa.setProfileCache = (data) => {
     setProfileCacheCalled = true;
     do_check_eq(data.profile.avatar, "myurl");
     do_check_eq(data.etag, "bogusetag");
     return Promise.resolve();
   }
   let profile = CreateFxAccountsProfile(fxa);
-  profile._cachedAt = 12345;
 
   makeObserver(ON_PROFILE_CHANGE_NOTIFICATION, function(subject, topic, data) {
     do_check_eq(data, ACCOUNT_DATA.uid);
-    do_check_neq(profile._cachedAt, 12345, "cachedAt has been bumped");
     do_check_true(setProfileCacheCalled);
     run_next_test();
   });
 
   return profile._cacheProfile({ body: { avatar: "myurl" }, etag: "bogusetag" });
 });
 
 add_test(function fetchAndCacheProfile_ok() {
   let client = mockClient(mockFxa());
   client.fetchProfile = function() {
     return Promise.resolve({ body: { avatar: "myimg"} });
   };
   let profile = CreateFxAccountsProfile(null, client);
+  profile._cachedAt = 12345;
 
   profile._cacheProfile = function(toCache) {
     do_check_eq(toCache.body.avatar, "myimg");
     return Promise.resolve(toCache.body);
   };
 
   return profile._fetchAndCacheProfile()
     .then(result => {
       do_check_eq(result.avatar, "myimg");
+      do_check_neq(profile._cachedAt, 12345, "cachedAt has been bumped");
+      run_next_test();
+    });
+});
+
+add_test(function fetchAndCacheProfile_always_bumps_cachedAt() {
+  let client = mockClient(mockFxa());
+  client.fetchProfile = function() {
+    return Promise.reject(new Error("oops"));
+  };
+  let profile = CreateFxAccountsProfile(null, client);
+  profile._cachedAt = 12345;
+
+  return profile._fetchAndCacheProfile()
+    .then(result => {
+      do_throw("Should not succeed");
+    }, err => {
+      do_check_neq(profile._cachedAt, 12345, "cachedAt has been bumped");
       run_next_test();
     });
 });
 
 add_test(function fetchAndCacheProfile_sendsETag() {
   let fxa = mockFxa();
   fxa.profileCache = { profile: {}, etag: "bogusETag" };
   let client = mockClient(fxa);
@@ -401,22 +418,20 @@ add_test(function getProfile_has_cached_
     });
 });
 
 add_test(function getProfile_fetchAndCacheProfile_throws() {
   let fxa = mockFxa();
   fxa.profileCache = { profile: { avatar: "myimg" } };
   let profile = CreateFxAccountsProfile(fxa);
 
-  profile._cachedAt = 12345;
   profile._fetchAndCacheProfile = () => Promise.reject(new Error());
 
   return profile.getProfile()
     .then(result => {
-      do_check_neq(profile._cachedAt, 12345);
       do_check_eq(result.avatar, "myimg");
       run_next_test();
     });
 });
 
 function run_test() {
   run_next_test();
 }