feat(metrics): Add three new auth-errors

* INVALID_RESULT - oauth.1001 - if OAuth result is missing. * INVALID_RESULT_REDIRECT - oauth.1002 - if OAuth result is available, but is missing redirect field. * INVALID_RESULT_CODE - oauth.1003 - OAuth code in result's redirect is missing or invalid.
mozilla · Oct 28, 2014 · ca2e1c4 · ckarlof · Oct 28, 2014 · ca2e1c4
1 parent a8c63fd
commit ca2e1c4
Show file tree

Hide file tree

Showing 3 changed files with 23 additions and 15 deletions.
diff --git a/app/scripts/lib/oauth-errors.js b/app/scripts/lib/oauth-errors.js
@@ -25,7 +25,10 @@ function (_, AuthErrors) {
     // local only errors.
     SERVICE_UNAVAILABLE: 998,
     UNEXPECTED_ERROR: 999,
-    TRY_AGAIN: 1000
+    TRY_AGAIN: 1000,
+    INVALID_RESULT: 1001,
+    INVALID_RESULT_REDIRECT: 1002,
+    INVALID_RESULT_CODE: 1003
   };
 
   var CODE_TO_MESSAGES = {
@@ -39,7 +42,10 @@ function (_, AuthErrors) {
     // local only errors.
     998: t('System unavailable, try again soon'),
     999: t('Unexpected error'),
-    1000: t('Something went wrong. Please close this tab and try again.')
+    1000: t('Something went wrong. Please close this tab and try again.'),
+    1001: t('Unexpected error'),
+    1002: t('Unexpected error'),
+    1003: t('Unexpected error')
   };
 
   return _.extend({}, AuthErrors, {

diff --git a/app/scripts/views/mixins/service-mixin.js b/app/scripts/views/mixins/service-mixin.js
@@ -86,20 +86,22 @@ define([
    */
   function _formatOAuthResult(result) {
     // get code and state from redirect params
-    if (result && result.redirect) {
-      var redirectParams = result.redirect.split('?')[1];
+    if (! result) {
+      return p.reject(OAuthErrors.toError('INVALID_RESULT'));
+    } else if (! result.redirect) {
+      return p.reject(OAuthErrors.toError('INVALID_RESULT_REDIRECT'));
+    }
 
-      result.state = Url.searchParam('state', redirectParams);
-      result.code = Url.searchParam('code', redirectParams);
+    var redirectParams = result.redirect.split('?')[1];
 
-      if (! Validate.isOAuthCodeValid(result.code)) {
-        return p.reject(OAuthErrors.toError('UNEXPECTED_ERROR'));
-      }
+    result.state = Url.searchParam('state', redirectParams);
+    result.code = Url.searchParam('code', redirectParams);
 
-      return p(result);
-    } else {
-      return p.reject(OAuthErrors.toError('UNEXPECTED_ERROR'));
+    if (! Validate.isOAuthCodeValid(result.code)) {
+      return p.reject(OAuthErrors.toError('INVALID_RESULT_CODE'));
     }
+
+    return p(result);
   }
 
   return {

diff --git a/app/tests/spec/views/mixins/service-mixin.js b/app/tests/spec/views/mixins/service-mixin.js
@@ -93,14 +93,14 @@ define([
       it('throws on null', function () {
         relier.set('state', STATE);
         return view._formatOAuthResult().then(assert.fail, function (err) {
-          assert.isTrue(OAuthErrors.is(err, 'UNEXPECTED_ERROR'));
+          assert.isTrue(OAuthErrors.is(err, 'INVALID_RESULT'));
         });
       });
 
       it('throws on an empty object', function () {
         relier.set('state', STATE);
         return view._formatOAuthResult({}).then(assert.fail, function (err) {
-          assert.isTrue(OAuthErrors.is(err, 'UNEXPECTED_ERROR'));
+          assert.isTrue(OAuthErrors.is(err, 'INVALID_RESULT_REDIRECT'));
         });
       });
 
@@ -113,7 +113,7 @@ define([
         return view._formatOAuthResult({
           redirect: redirectBadCode
         }).then(assert.fail, function (err) {
-          assert.isTrue(OAuthErrors.is(err, 'UNEXPECTED_ERROR'));
+          assert.isTrue(OAuthErrors.is(err, 'INVALID_RESULT_CODE'));
         });
       });