mozilla
/
fxa-content-server
зеркало из https://github.com/mozilla/fxa-content-server.git
1
0
Форкнуть 0
Код Задачи Пакеты Проекты Релизы Вики Активность

chore(metrics): Namespace errors for metrics logging and other error logging updates. 

@kparlante asked for us to add a bit of context, like a screen name, to errors so that it is easier to decide where an error originated. This led down a bit of a rabbit hole of cleaning up the error code.

* The different error types overlap in their errno's, this adds a namespace so that different errors are able to be selected from the metrics logs.

Namespaces used:
* auth-errors.js: 'auth'
* oauth-errors.js: 'oauth'
* profile-client.js: 'profile'

* The `message` field of auth-errors.js.toError was never used, so this is repuprosed to pass in a `context`.
* The format of an error event has changed to accommodate a context: 'error.<context>.<namespace>.<errno>
* When a view's `logError` function is called, the screen name is set as the error's context.
* base.js called metrics.js to fetch ids for screens and errors, then called metrics.js->logEvent. Updated to just call `logError` and `logScreen`
* A couple of TestHelper functions were added: `isScreenLogged` and `isErrorLogged`

fixes #1500
This commit is contained in:
Shane Tomlinson 2014-09-08 13:27:04 +01:00
Родитель a18a86f877
Коммит e81aa06428
10 изменённых файлов: 145 добавлений и 42 удалений
Показать статистику Скачать Patch файл Скачать Diff файл Показать все файлы Свернуть все файлы

17
app/scripts/lib/auth-errors.js
Просмотреть файл

@ -97,6 +97,7 @@ function () {
return {
ERROR_TO_CODE: ERROR_TO_CODE,
CODE_TO_MESSAGES: CODE_TO_MESSAGES,
NAMESPACE: 'auth',
/**
* Convert an error, a numeric code or string type to a message
@ -161,11 +162,23 @@ function () {
},
/**
* Synthesize an error of the given type with the supplied message.
* Synthesize an error of the given type
*
* @param {String || Number || Object} type
* @param {String} [context]
*/
toError: function (type, message) {
toError: function (type, context) {
var message = this.toMessage(type);
var err = new Error(message);
err.errno = this.toCode(type);
err.namespace = this.NAMESPACE;
err.message = message;
if (context) {
err.context = context;
}
return err;
},

36
app/scripts/lib/metrics.js
Просмотреть файл

@ -20,8 +20,9 @@ define([
'jquery',
'speedTrap',
'lib/promise',
'lib/url'
], function (_, Backbone, $, speedTrap, p, Url) {
'lib/url',
'lib/strings'
], function (_, Backbone, $, speedTrap, p, Url, Strings) {
'use strict';
// Speed trap is a singleton, convert it
@ -205,17 +206,36 @@ define([
},
/**
* Convert an error to an identifier
* Log an error.
*/
errorToId: function (err, errors) {
return 'error.' + errors.toCode(err);
logError: function (error) {
this.logEvent(this.errorToId(error));
},
/**
* Convert a pathname from a URL to an identifier
* Convert an error to an identifier that can be used for logging.
*/
pathToId: function (path) {
return 'screen.' + Url.pathToScreenName(path);
errorToId: function (error) {
var id = Strings.interpolate('error.%s.%s.%s', [
error.context || 'unknown context',
error.namespace || 'unknown namespace',
error.errno || String(error)
]);
return id;
},
/**
* Log a screen
*/
logScreen: function (screenName) {
this.logEvent(this.screenToId(screenName));
},
/**
* Convert a screenName an identifier
*/
screenToId: function (screenName) {
return 'screen.' + screenName;
},
/**

3
app/scripts/lib/oauth-errors.js
Просмотреть файл

@ -44,6 +44,7 @@ function (_, AuthErrors) {
return _.extend({}, AuthErrors, {
ERROR_TO_CODE: ERROR_TO_CODE,
CODE_TO_MESSAGES: CODE_TO_MESSAGES
CODE_TO_MESSAGES: CODE_TO_MESSAGES,
NAMESPACE: 'oauth'
});
});

3
app/scripts/lib/profile-client.js
Просмотреть файл

@ -82,7 +82,8 @@ function ($, _, p, Session, ConfigLoader, OAuthClient, Assertion, AuthErrors) {
var ProfileErrors = ProfileClient.Errors = _.extend({}, AuthErrors, {
ERROR_TO_CODE: ERROR_TO_CODE,
CODE_TO_MESSAGES: CODE_TO_MESSAGES
CODE_TO_MESSAGES: CODE_TO_MESSAGES,
NAMESPACE: 'profile'
});
return ProfileClient;

20
app/scripts/views/base.js
Просмотреть файл

@ -162,7 +162,7 @@ function (_, Backbone, $, p, Session, AuthErrors, Url, Strings,
* authentication, but this could be extended to other types of
* authorization as well.
*/
isUserAuthorized: function() {
isUserAuthorized: function () {
if (this.mustAuth) {
return this.fxaClient.isSignedIn(Session.sessionToken);
}
@ -381,7 +381,16 @@ function (_, Backbone, $, p, Session, AuthErrors, Url, Strings,
}
err.logged = true;
this.logEvent(this.metrics.errorToId(err, errors));
if (typeof console !== 'undefined' && console) {
console.error(err.message || String(err));
}
this.metrics.logError(err);
},
getScreenName: function () {
var screenName = Url.pathToScreenName(this.window.location.pathname);
return screenName;
},
_normalizeError: function (err, errors) {
@ -395,6 +404,10 @@ function (_, Backbone, $, p, Session, AuthErrors, Url, Strings,
}
}
if (! err.context) {
err.context = this.getScreenName();
}
return err;
},
@ -402,8 +415,7 @@ function (_, Backbone, $, p, Session, AuthErrors, Url, Strings,
* Log the current screen
*/
logScreen: function () {
var path = this.window.location.pathname;
this.logEvent(this.metrics.pathToId(path));
this.metrics.logScreen(this.getScreenName());
},
/**

14
app/tests/lib/helpers.js
Просмотреть файл

@ -99,6 +99,16 @@ define([
return false;
}
function isErrorLogged(metrics, error) {
var eventName = metrics.errorToId(error);
return isEventLogged(metrics, eventName);
}
function isScreenLogged(metrics, screenName) {
var eventName = metrics.screenToId(screenName);
return isEventLogged(metrics, eventName);
}
return {
requiresFocus: requiresFocus,
addFxaClientSpy: addFxaClientSpy,
@ -107,6 +117,8 @@ define([
createRandomHexString: createRandomHexString,
createEmail: createEmail,
emailToUser: emailToUser,
isEventLogged: isEventLogged
isEventLogged: isEventLogged,
isErrorLogged: isErrorLogged,
isScreenLogged: isScreenLogged
};
});

18
app/tests/spec/lib/auth-errors.js
Просмотреть файл

@ -15,6 +15,24 @@ function (chai, AuthErrors) {
var assert = chai.assert;
describe('lib/auth-errors', function () {
describe('toError', function () {
it('converts a string to an Error object with expected fields', function () {
var err = AuthErrors.toError('INVALID_TOKEN');
assert.isTrue(err instanceof Error);
assert.equal(err.errno, 110);
assert.equal(err.namespace, 'auth');
assert.equal(err.message, AuthErrors.toMessage('INVALID_TOKEN'));
});
it('sets `context` field of error if given', function () {
var err = AuthErrors.toError('INVALID_TOKEN', 'the context');
assert.isTrue(err instanceof Error);
assert.equal(err.context, 'the context');
});
});
describe('toMessage', function () {
it('converts a code to a message', function () {
assert.equal(AuthErrors.toMessage(102), 'Unknown account');

31
app/tests/spec/lib/metrics.js
Просмотреть файл

@ -8,9 +8,11 @@ define([
'chai',
'jquery',
'lib/metrics',
'../../mocks/window'
'lib/auth-errors',
'../../mocks/window',
'../../lib/helpers'
],
function (chai, $, Metrics, WindowMock) {
function (chai, $, Metrics, AuthErrors, WindowMock, TestHelpers) {
'use strict';
/*global describe, it*/
@ -186,5 +188,30 @@ function (chai, $, Metrics, WindowMock) {
});
});
});
describe('errorToId', function () {
it('converts an error into an id that can be used for logging', function () {
var error = AuthErrors.toError('UNEXPECTED_ERROR', 'signup');
var id = metrics.errorToId(error);
assert.equal(id, 'error.signup.auth.999');
});
});
describe('logError', function () {
it('logs an error', function () {
var error = AuthErrors.toError('UNEXPECTED_ERROR', 'signup');
metrics.logError(error);
assert.isTrue(TestHelpers.isErrorLogged(metrics, error));
});
});
describe('logScreen', function () {
it('logs the screen', function () {
metrics.logScreen('signup');
assert.isTrue(TestHelpers.isScreenLogged(metrics, 'signup'));
});
});
});
});

39
app/tests/spec/views/base.js
Просмотреть файл

@ -39,6 +39,7 @@ function (chai, jQuery, sinon, BaseView, Translator, EphemeralMessages, Metrics,
var metrics;
var fxaClient;
var relier;
var screenName = 'screen';
beforeEach(function () {
translator = new Translator('en-US', ['en-US']);
@ -49,6 +50,7 @@ function (chai, jQuery, sinon, BaseView, Translator, EphemeralMessages, Metrics,
router = new RouterMock();
windowMock = new WindowMock();
windowMock.location.pathname = '/' + screenName;
ephemeralMessages = new EphemeralMessages();
metrics = new Metrics();
relier = new Relier();
@ -223,11 +225,10 @@ function (chai, jQuery, sinon, BaseView, Translator, EphemeralMessages, Metrics,
});
it('adds an entry into the event stream', function () {
var err = AuthErrors.toError('INVALID_TOKEN', 'bad token, man');
var err = AuthErrors.toError('INVALID_TOKEN', screenName);
view.displayError(err);
assert.isTrue(TestHelpers.isEventLogged(metrics,
metrics.errorToId('INVALID_TOKEN', AuthErrors)));
assert.isTrue(TestHelpers.isErrorLogged(metrics, err));
});
it('displays an `Unexpected Error` if no error passed in', function () {
@ -249,11 +250,11 @@ function (chai, jQuery, sinon, BaseView, Translator, EphemeralMessages, Metrics,
});
it('adds an entry into the event stream', function () {
var err = AuthErrors.toError('INVALID_TOKEN', 'bad token, man');
var err = AuthErrors.toError('INVALID_TOKEN', screenName);
view.displayError(err);
assert.isTrue(TestHelpers.isEventLogged(metrics,
metrics.errorToId('INVALID_TOKEN', AuthErrors)));
assert.isTrue(TestHelpers.isErrorLogged(metrics, err));
});
it('displays an `Unexpected Error` if no error passed in', function () {
@ -279,7 +280,7 @@ function (chai, jQuery, sinon, BaseView, Translator, EphemeralMessages, Metrics,
describe('navigate', function () {
it('navigates to a page', function (done) {
router.on('navigate', function (newPage) {
wrapAssertion(function() {
wrapAssertion(function () {
assert.equal(newPage, 'signin');
}, done);
});
@ -287,12 +288,12 @@ function (chai, jQuery, sinon, BaseView, Translator, EphemeralMessages, Metrics,
});
it('logs an error if an error is passed in the options', function () {
var err = AuthErrors.toError('SESSION_EXPIRED', screenName);
view.navigate('signin', {
error: AuthErrors.toError('SESSION_EXPIRED')
error: err
});
assert.isTrue(TestHelpers.isEventLogged(metrics,
metrics.errorToId('SESSION_EXPIRED', AuthErrors)));
assert.isTrue(TestHelpers.isErrorLogged(metrics, err));
});
});
@ -332,7 +333,7 @@ function (chai, jQuery, sinon, BaseView, Translator, EphemeralMessages, Metrics,
describe('BaseView.preventDefaultThen', function () {
it('can take the name of a function as the name of the event handler', function (done) {
view.eventHandler = function (event) {
wrapAssertion(function() {
wrapAssertion(function () {
assert.isTrue(event.isDefaultPrevented());
}, done);
};
@ -343,7 +344,7 @@ function (chai, jQuery, sinon, BaseView, Translator, EphemeralMessages, Metrics,
it('can take a function as the event handler', function (done) {
function eventHandler(event) {
wrapAssertion(function() {
wrapAssertion(function () {
assert.isTrue(event.isDefaultPrevented());
}, done);
}
@ -365,7 +366,7 @@ function (chai, jQuery, sinon, BaseView, Translator, EphemeralMessages, Metrics,
describe('BaseView.cancelEventThen', function () {
it('can take the name of a function as the name of the event handler', function (done) {
view.eventHandler = function (event) {
wrapAssertion(function() {
wrapAssertion(function () {
assert.isTrue(event.isDefaultPrevented());
assert.isTrue(event.isPropagationStopped());
}, done);
@ -377,7 +378,7 @@ function (chai, jQuery, sinon, BaseView, Translator, EphemeralMessages, Metrics,
it('can take a function as the event handler', function (done) {
function eventHandler(event) {
wrapAssertion(function() {
wrapAssertion(function () {
assert.isTrue(event.isDefaultPrevented());
assert.isTrue(event.isPropagationStopped());
}, done);
@ -440,9 +441,10 @@ function (chai, jQuery, sinon, BaseView, Translator, EphemeralMessages, Metrics,
describe('logError', function () {
it('logs an error to the event stream', function () {
view.logError(AuthErrors.toError('INVALID_TOKEN'));
assert.isTrue(TestHelpers.isEventLogged(metrics,
metrics.errorToId('INVALID_TOKEN', AuthErrors)));
var err = AuthErrors.toError('INVALID_TOKEN', screenName);
view.logError(err);
assert.isTrue(TestHelpers.isErrorLogged(metrics, err));
});
it('does not log already logged errors', function () {
@ -460,7 +462,8 @@ function (chai, jQuery, sinon, BaseView, Translator, EphemeralMessages, Metrics,
view.metrics.events.clear();
view.logError();
assert.isTrue(TestHelpers.isEventLogged(metrics, metrics.errorToId('UNEXPECTED_ERROR', AuthErrors)));
var err = AuthErrors.toError('UNEXPECTED_ERROR', screenName);
assert.isTrue(TestHelpers.isErrorLogged(metrics, err));
});
it('prints a stack trace via console.trace to facilitate debugging if no error object is passed in', function () {

6
app/tests/spec/views/form.js
Просмотреть файл

@ -70,10 +70,6 @@ function (chai, $, p, FormView, Template, Constants, Metrics, AuthErrors, TestHe
});
}
function testEventLogged(eventName) {
assert.isTrue(TestHelpers.isEventLogged(metrics, eventName));
}
beforeEach(function () {
metrics = new Metrics();
view = new View({
@ -265,7 +261,7 @@ function (chai, $, p, FormView, Template, Constants, Metrics, AuthErrors, TestHe
var err = AuthErrors.toError('EMAIL_REQUIRED');
view.on('validation_error', function () {
TestHelpers.wrapAssertion(function () {
testEventLogged(metrics.errorToId(err, AuthErrors));
assert.isTrue(TestHelpers.isErrorLogged(metrics, err));
}, done);
});
view.showValidationError('#focusMe', err);