Merge branch 'issue-2644-basket-500-errors'

Well done @TDA! Thank you for working with me on this.

r=@shane-tomlinson

app/scripts/lib/constants.js

@@ -56,7 +56,9 @@ define([], function () {
     MARKETING_EMAIL_NEWSLETTER_ID: 'firefox-accounts-journey',
 
     ACCESS_TYPE_ONLINE: 'online',
-    ACCESS_TYPE_OFFLINE: 'offline'
+    ACCESS_TYPE_OFFLINE: 'offline',
+
+    DEFAULT_XHR_TIMEOUT_MS: 2500
   };
 });
 

app/scripts/lib/errors.js

@@ -132,13 +132,18 @@ define([
         return this.toError('SERVICE_UNAVAILABLE');
       }
 
-      var errObj = xhr.responseJSON;
+      var serverError = xhr.responseJSON;
 
-      if (! errObj) {
+      if (! serverError) {
         return this.toError('UNEXPECTED_ERROR');
       }
+      // We need the error to be normalized before being returned.
+      // We also need to add a code to the normalized error, that
+      // contains the status code
+      var normalizedError = this.toError(serverError.errno);
+      normalizedError.code = serverError.code;
 
-      return this.toError(errObj.errno);
+      return normalizedError;
     }
   };
 });

app/scripts/lib/marketing-email-client.js

@@ -7,16 +7,17 @@
  */
 
 define([
+  'lib/constants',
   'lib/xhr',
   'lib/marketing-email-errors'
-], function (xhr, MarketingEmailErrors) {
+], function (Constants, xhr, MarketingEmailErrors) {
   'use strict';
 
   function MarketingEmailClient(options) {
     options = options || {};
 
     var self = this;
-
+    self._xhrTimeout = options.timeout || Constants.DEFAULT_XHR_TIMEOUT_MS;
     self._xhr = options.xhr || xhr;
     self._baseUrl = options.baseUrl;
     self._preferencesUrl = options.preferencesUrl;
@@ -25,20 +26,14 @@ define([
   MarketingEmailClient.prototype = {
     _request: function (method, endpoint, accessToken, data) {
       var url = this._baseUrl + endpoint;
-
       return this._xhr.oauthAjax({
         url: url,
         type: method,
         accessToken: accessToken,
-        data: data
+        data: data,
+        timeout: this._xhrTimeout
       })
       .fail(function (xhr) {
-        var respJSON = xhr.responseJSON;
-        if (respJSON && ! ('errno' in respJSON) && respJSON.code) {
-          // the basket API returns code instead of errno. Convert.
-          respJSON.errno = respJSON.code;
-        }
-
         throw MarketingEmailErrors.normalizeXHRError(xhr);
       });
     },

app/scripts/lib/marketing-email-errors.js

@@ -86,6 +86,16 @@ function (_, Errors) {
 
   return _.extend({}, Errors, {
     ERRORS: ERRORS,
-    NAMESPACE: 'basket-errors'
+    NAMESPACE: 'basket-errors',
+    normalizeXHRError: function (xhr) {
+      var respJSON = xhr.responseJSON;
+      if (respJSON && ! ('errno' in respJSON) && respJSON.code) {
+        // the basket API returns code instead of errno. Convert.
+        respJSON.errno = respJSON.code;
+        // we want to set the code equal to the http status code
+        respJSON.code = xhr.status;
+      }
+      return Errors.normalizeXHRError.call(this, xhr);
+    }
   });
 });

app/scripts/lib/xhr.js

@@ -67,6 +67,7 @@ define([
         // make sure to set the dataType for Firefox <21. See issue #1930
         dataType: 'json',
         type: options.type,
+        timeout: options.timeout,
         headers: {
           Authorization: 'Bearer ' + options.accessToken,
           Accept: 'application/json'

app/scripts/templates/settings/communication_preferences.mustache

@@ -4,6 +4,17 @@
   </header>
 
   <section>
+    {{#error}}
+      <div class="error visible nudge">
+        {{error}}
+      </div>
+      <div class="links email-optin-links">
+        <a href="#" id="back" class="back">{{#t}}Back{{/t}}</a>
+      </div>
+    </div>
+
+    {{/error}}
+    {{^error}}
     <div class="success"></div>
     <div class="error"></div>
 
@@ -35,5 +46,6 @@
         <a href="#" id="back" class="back">{{#t}}Back{{/t}}</a>
       {{/preferencesUrl}}
     </div>
+    {{/error}}
   </section>
 </div>

app/scripts/views/settings/communication_preferences.js

@@ -7,6 +7,7 @@ define([
   'lib/xss',
   'lib/constants',
   'lib/marketing-email-errors',
+  'lib/metrics',
   'views/base',
   'views/form',
   'views/mixins/back-mixin',
@@ -15,7 +16,7 @@ define([
   'views/mixins/loading-mixin',
   'stache!templates/settings/communication_preferences'
 ],
-function (Cocktail, Xss, Constants, MarketingEmailErrors, BaseView, FormView,
+function (Cocktail, Xss, Constants, MarketingEmailErrors, Metrics, BaseView, FormView,
   BackMixin, SettingsMixin, CheckboxMixin, LoadingMixin, Template) {
   'use strict';
 
@@ -39,34 +40,39 @@ function (Cocktail, Xss, Constants, MarketingEmailErrors, BaseView, FormView,
     beforeRender: function () {
       var self = this;
       var emailPrefs = self.getMarketingEmailPrefs();
-
       return emailPrefs.fetch()
         .fail(function (err) {
           if (MarketingEmailErrors.is(err, 'UNKNOWN_EMAIL')) {
             // user has not yet opted in to Basket yet. Ignore.
             return;
           }
-
-          // error fetching the email preferences, show the
-          // error and hide the spinner.
-          self.$('.spinner').hide();
-          self.displayError(err);
-          return false;
+          if (MarketingEmailErrors.is(err, 'UNKNOWN_ERROR')) {
+            self._error = self.translateError(MarketingEmailErrors.toError('SERVICE_UNAVAILABLE'));
+          } else {
+            self._error = self.translateError(err);
+          }
+          err = self._normalizeError(err);
+          var errorString = Metrics.prototype.errorToId(err);
+          err.code = err.code || 'unknown';
+          // Add status code to metrics data, to differentiate between
+          // 400 and 500
+          errorString = errorString + '.' + err.code;
+          self.logEvent(errorString);
         });
     },
 
     context: function () {
       var self = this;
       var emailPrefs = this.getMarketingEmailPrefs();
       var isOptedIn = emailPrefs.isOptedIn(NEWSLETTER_ID);
-
       self.logScreenEvent('newsletter.optin.' + String(isOptedIn));
 
       return {
         isOptedIn: isOptedIn,
         // preferencesURL is only available if the user is already
         // registered with basket.
-        preferencesUrl: Xss.href(emailPrefs.get('preferencesUrl'))
+        preferencesUrl: Xss.href(emailPrefs.get('preferencesUrl')),
+        error: self._error
       };
     },
 

app/styles/_state.scss

@@ -16,6 +16,10 @@
   a {
     color: $message-text-color;
   }
+  /*Push the error message down to avoid being too close to the header*/
+  &.nudge {
+    top: -5px;
+  }
 }
 
 .error {

app/tests/spec/views/settings/communication_preferences.js

@@ -13,22 +13,25 @@ define([
   'models/reliers/relier',
   'lib/promise',
   'lib/constants',
-  'lib/marketing-email-errors'
+  'lib/marketing-email-errors',
+  'lib/metrics',
+  '../../../lib/helpers'
 ],
 function (chai, $, sinon, View, User, Account, MarketingEmailPrefs, Relier,
-  p, Constants, MarketingEmailErrors) {
+  p, Constants, MarketingEmailErrors, Metrics, TestHelpers) {
   'use strict';
 
   var assert = chai.assert;
   var NEWSLETTER_ID = Constants.MARKETING_EMAIL_NEWSLETTER_ID;
 
   describe('views/settings/communication_preferences', function () {
-    var user;
     var account;
-    var view;
     var emailPrefsModel;
-    var relier;
+    var metrics;
     var preferencesUrl = 'https://marketing.preferences.com/user/user-token';
+    var relier;
+    var user;
+    var view;
 
     function render() {
       return view.render()
@@ -41,6 +44,7 @@ function (chai, $, sinon, View, User, Account, MarketingEmailPrefs, Relier,
       user = new User();
       relier = new Relier();
       account = new Account();
+      metrics = new Metrics();
 
       emailPrefsModel = new MarketingEmailPrefs({
         newsletters: [ NEWSLETTER_ID ],
@@ -66,7 +70,9 @@ function (chai, $, sinon, View, User, Account, MarketingEmailPrefs, Relier,
       });
 
       view = new View({
+        metrics: metrics,
         relier: relier,
+        screenName: 'settings.communication-preferences',
         user: user
       });
 
@@ -136,7 +142,41 @@ function (chai, $, sinon, View, User, Account, MarketingEmailPrefs, Relier,
 
         return render()
           .then(function () {
-            assert.isTrue(view.isErrorVisible());
+            assert.ok(view.$('.error').text().length);
+          });
+      });
+
+      it('correctly handles http 4xx errors as service unavailable', function () {
+        emailPrefsModel.fetch.restore();
+        sinon.stub(emailPrefsModel, 'fetch', function () {
+          var err = MarketingEmailErrors.toError('UNKNOWN_ERROR');
+          err.code = 400;
+          return p.reject(err);
+        });
+
+        return render()
+          .then(function () {
+            assert.equal(view.$('.error').text().trim(),
+              MarketingEmailErrors.toMessage('SERVICE_UNAVAILABLE'));
+            assert.isTrue(TestHelpers.isEventLogged(metrics,
+                'error.settings.communication-preferences.basket-errors.99.400'));
+          });
+      });
+
+      it('correctly handles http 5xx errors as service unavailable', function () {
+        emailPrefsModel.fetch.restore();
+        sinon.stub(emailPrefsModel, 'fetch', function () {
+          var err = MarketingEmailErrors.toError('UNKNOWN_ERROR');
+          err.code = 500;
+          return p.reject(err);
+        });
+
+        return render()
+          .then(function () {
+            assert.equal(view.$('.error').text().trim(),
+              MarketingEmailErrors.toMessage('SERVICE_UNAVAILABLE'));
+            assert.isTrue(TestHelpers.isEventLogged(metrics,
+                'error.settings.communication-preferences.basket-errors.99.500'));
           });
       });
     });