diff --git a/app/scripts/lib/constants.js b/app/scripts/lib/constants.js index 4c82f70aec..789160ffc3 100644 --- a/app/scripts/lib/constants.js +++ b/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 }; }); diff --git a/app/scripts/lib/errors.js b/app/scripts/lib/errors.js index 7daac52087..f6988f5580 100644 --- a/app/scripts/lib/errors.js +++ b/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; } }; }); diff --git a/app/scripts/lib/marketing-email-client.js b/app/scripts/lib/marketing-email-client.js index 2687570429..b30f3f1f79 100644 --- a/app/scripts/lib/marketing-email-client.js +++ b/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); }); }, diff --git a/app/scripts/lib/marketing-email-errors.js b/app/scripts/lib/marketing-email-errors.js index d542a6ace9..3cd2a38ba3 100644 --- a/app/scripts/lib/marketing-email-errors.js +++ b/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); + } }); }); diff --git a/app/scripts/lib/xhr.js b/app/scripts/lib/xhr.js index e166f212ec..1f37b97b85 100644 --- a/app/scripts/lib/xhr.js +++ b/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' diff --git a/app/scripts/templates/settings/communication_preferences.mustache b/app/scripts/templates/settings/communication_preferences.mustache index 46fc1c129e..ab84f83695 100644 --- a/app/scripts/templates/settings/communication_preferences.mustache +++ b/app/scripts/templates/settings/communication_preferences.mustache @@ -4,6 +4,17 @@
+ {{#error}} +
+ {{error}} +
+ + + + {{/error}} + {{^error}}
@@ -35,5 +46,6 @@ {{#t}}Back{{/t}} {{/preferencesUrl}} + {{/error}}
diff --git a/app/scripts/views/settings/communication_preferences.js b/app/scripts/views/settings/communication_preferences.js index 188061ab5b..f90eb86e76 100644 --- a/app/scripts/views/settings/communication_preferences.js +++ b/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,19 +40,24 @@ 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); }); }, @@ -59,14 +65,14 @@ function (Cocktail, Xss, Constants, MarketingEmailErrors, BaseView, FormView, 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 }; }, diff --git a/app/styles/_state.scss b/app/styles/_state.scss index a5faeda0b3..a594709e3e 100644 --- a/app/styles/_state.scss +++ b/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 { diff --git a/app/tests/spec/views/settings/communication_preferences.js b/app/tests/spec/views/settings/communication_preferences.js index fd8d7f9c20..1aeb674caf 100644 --- a/app/tests/spec/views/settings/communication_preferences.js +++ b/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')); }); }); });