From b3d43f3f91d4a7bf0443d65a3ab2f15b3a3af673 Mon Sep 17 00:00:00 2001 From: Douglas Hall Date: Fri, 5 Apr 2019 14:36:29 -0400 Subject: [PATCH] Move LoggingService to npm package --- package-lock.json | 46 +++++++--- package.json | 1 + src/analytics/analytics.js | 3 +- src/analytics/analytics.test.js | 5 +- src/sagas/RootSaga.js | 2 +- src/services/LoggingService.js | 74 ---------------- src/services/LoggingService.test.js | 128 ---------------------------- src/services/ProfileApiService.js | 3 +- 8 files changed, 44 insertions(+), 218 deletions(-) delete mode 100644 src/services/LoggingService.js delete mode 100644 src/services/LoggingService.test.js diff --git a/package-lock.json b/package-lock.json index 2243c3f..0d53ccb 100644 --- a/package-lock.json +++ b/package-lock.json @@ -2600,6 +2600,11 @@ } } }, + "@edx/frontend-logging": { + "version": "1.0.0", + "resolved": "https://registry.npmjs.org/@edx/frontend-logging/-/frontend-logging-1.0.0.tgz", + "integrity": "sha512-Vy5SovHx3H21m7pdqTFeke/Ec23Vk3Es65bhgCBXU1Y7Vrp81Nkwq9EUofjoy8oxp9t9J7NtdjXoaDva2KjvUA==" + }, "@edx/paragon": { "version": "3.8.3", "resolved": "https://registry.npmjs.org/@edx/paragon/-/paragon-3.8.3.tgz", @@ -10336,7 +10341,8 @@ "ansi-regex": { "version": "2.1.1", "bundled": true, - "dev": true + "dev": true, + "optional": true }, "aproba": { "version": "1.2.0", @@ -10357,12 +10363,14 @@ "balanced-match": { "version": "1.0.0", "bundled": true, - "dev": true + "dev": true, + "optional": true }, "brace-expansion": { "version": "1.1.11", "bundled": true, "dev": true, + "optional": true, "requires": { "balanced-match": "^1.0.0", "concat-map": "0.0.1" @@ -10377,17 +10385,20 @@ "code-point-at": { "version": "1.1.0", "bundled": true, - "dev": true + "dev": true, + "optional": true }, "concat-map": { "version": "0.0.1", "bundled": true, - "dev": true + "dev": true, + "optional": true }, "console-control-strings": { "version": "1.1.0", "bundled": true, - "dev": true + "dev": true, + "optional": true }, "core-util-is": { "version": "1.0.2", @@ -10504,7 +10515,8 @@ "inherits": { "version": "2.0.3", "bundled": true, - "dev": true + "dev": true, + "optional": true }, "ini": { "version": "1.3.5", @@ -10516,6 +10528,7 @@ "version": "1.0.0", "bundled": true, "dev": true, + "optional": true, "requires": { "number-is-nan": "^1.0.0" } @@ -10530,6 +10543,7 @@ "version": "3.0.4", "bundled": true, "dev": true, + "optional": true, "requires": { "brace-expansion": "^1.1.7" } @@ -10537,12 +10551,14 @@ "minimist": { "version": "0.0.8", "bundled": true, - "dev": true + "dev": true, + "optional": true }, "minipass": { "version": "2.3.5", "bundled": true, "dev": true, + "optional": true, "requires": { "safe-buffer": "^5.1.2", "yallist": "^3.0.0" @@ -10561,6 +10577,7 @@ "version": "0.5.1", "bundled": true, "dev": true, + "optional": true, "requires": { "minimist": "0.0.8" } @@ -10641,7 +10658,8 @@ "number-is-nan": { "version": "1.0.1", "bundled": true, - "dev": true + "dev": true, + "optional": true }, "object-assign": { "version": "4.1.1", @@ -10653,6 +10671,7 @@ "version": "1.4.0", "bundled": true, "dev": true, + "optional": true, "requires": { "wrappy": "1" } @@ -10738,7 +10757,8 @@ "safe-buffer": { "version": "5.1.2", "bundled": true, - "dev": true + "dev": true, + "optional": true }, "safer-buffer": { "version": "2.1.2", @@ -10774,6 +10794,7 @@ "version": "1.0.2", "bundled": true, "dev": true, + "optional": true, "requires": { "code-point-at": "^1.0.0", "is-fullwidth-code-point": "^1.0.0", @@ -10793,6 +10814,7 @@ "version": "3.0.1", "bundled": true, "dev": true, + "optional": true, "requires": { "ansi-regex": "^2.0.0" } @@ -10836,12 +10858,14 @@ "wrappy": { "version": "1.0.2", "bundled": true, - "dev": true + "dev": true, + "optional": true }, "yallist": { "version": "3.0.3", "bundled": true, - "dev": true + "dev": true, + "optional": true } } }, diff --git a/package.json b/package.json index 481ef4c..c3f9669 100755 --- a/package.json +++ b/package.json @@ -29,6 +29,7 @@ "@edx/frontend-auth": "^3.0.3", "@edx/frontend-component-footer": "^2.0.3", "@edx/frontend-component-site-header": "^2.1.4", + "@edx/frontend-logging": "^1.0.0", "@edx/paragon": "^3.8.3", "@fortawesome/fontawesome-svg-core": "^1.2.14", "@fortawesome/free-brands-svg-icons": "^5.7.2", diff --git a/src/analytics/analytics.js b/src/analytics/analytics.js index be5cfe3..d6d09c6 100755 --- a/src/analytics/analytics.js +++ b/src/analytics/analytics.js @@ -1,8 +1,9 @@ import formurlencoded from 'form-urlencoded'; +import LoggingService from '@edx/frontend-logging'; + import apiClient from '../config/apiClient'; import { configuration } from '../config/environment'; import { snakeCaseObject } from '../services/utils'; -import LoggingService from '../services/LoggingService'; const eventLogApiBaseUrl = `${configuration.LMS_BASE_URL}/event`; diff --git a/src/analytics/analytics.test.js b/src/analytics/analytics.test.js index f514070..5d596bd 100644 --- a/src/analytics/analytics.test.js +++ b/src/analytics/analytics.test.js @@ -1,9 +1,10 @@ +import LoggingService from '@edx/frontend-logging'; + import { logEvent } from './analytics'; import { configuration } from '../config/environment'; -import LoggingService from '../services/LoggingService'; import apiClient from '../config/apiClient'; -jest.mock('../services/LoggingService'); +jest.mock('@edx/frontend-logging'); jest.mock('../config/apiClient'); const eventType = 'test.event'; diff --git a/src/sagas/RootSaga.js b/src/sagas/RootSaga.js index 94d59d5..838584a 100644 --- a/src/sagas/RootSaga.js +++ b/src/sagas/RootSaga.js @@ -1,5 +1,6 @@ import { push } from 'connected-react-router'; import { all, call, delay, put, select, takeEvery } from 'redux-saga/effects'; +import LoggingService from '@edx/frontend-logging'; // Actions import { @@ -31,7 +32,6 @@ import { handleSaveProfileSelector } from '../selectors/ProfilePageSelector'; // Services import * as ProfileApiService from '../services/ProfileApiService'; -import LoggingService from '../services/LoggingService'; // Utils import { keepKeys } from '../services/utils'; diff --git a/src/services/LoggingService.js b/src/services/LoggingService.js deleted file mode 100644 index 4cf4f91..0000000 --- a/src/services/LoggingService.js +++ /dev/null @@ -1,74 +0,0 @@ -/** - * Logs info and errors to NewRelic and console. - * - * Requires the NewRelic Browser JavaScript snippet. - */ - -// NewRelic will not log an error if it is too long. -const MAX_ERROR_LENGTH = 4000; - -function fixErrorLength(error) { - if (error.message && error.message.length > MAX_ERROR_LENGTH) { - const processedError = Object.create(error); - processedError.message = processedError.message.substring(0, MAX_ERROR_LENGTH); - return processedError; - } else if (typeof error === 'string' && error.length > MAX_ERROR_LENGTH) { - return error.substring(0, MAX_ERROR_LENGTH); - } - return error; -} - -class LoggingService { - static logInfo(message) { - if (typeof newrelic !== 'undefined') { - newrelic.addPageAction('INFO', { message }); - } - } - - static logError(error, customAttributes) { - if (typeof newrelic !== 'undefined') { - // Note: customProperties are not sent. Presumably High-Security Mode is being used. - newrelic.noticeError(fixErrorLength(error), customAttributes); - } - } - - // API errors look for axios API error format. - // Note: function will simply log errors that don't seem to be API error responses. - static logAPIErrorResponse(error, customAttributes) { - let processedError = error; - let updatedCustomAttributes = customAttributes; - - if (error.response) { - let data = !error.response.data ? '' : JSON.stringify(error.response.data); - // Don't include data if it is just an HTML document, like a 500 error page. - data = data.includes('') ? '' : data; - const responseAttributes = { - errorType: 'api-response-error', - errorStatus: error.response.status, - errorUrl: error.response.config ? error.response.config.url : '', - errorData: data, - }; - updatedCustomAttributes = Object.assign({}, responseAttributes, customAttributes); - processedError = new Error(`API request failed: ${error.response.status} ${responseAttributes.errorUrl} ${data}`); - } else if (error.request) { - const { config, request } = error; - const errorMessage = request.responseText || error.message; - const requestMethod = config && config.method; - const requestUrl = request.responseURL || (config && config.url); - const requestAttributes = { - errorType: 'api-request-error', - errorStatus: request.status, - errorMethod: requestMethod, - errorUrl: requestUrl, - errorData: errorMessage, - }; - updatedCustomAttributes = Object.assign({}, requestAttributes, customAttributes); - processedError = new Error(`API request failed: ${request.status} ${requestMethod} ${requestUrl} ${errorMessage}`); - } - - this.logError(processedError, updatedCustomAttributes); - } -} - -export default LoggingService; -export { MAX_ERROR_LENGTH }; diff --git a/src/services/LoggingService.test.js b/src/services/LoggingService.test.js deleted file mode 100644 index ef66db7..0000000 --- a/src/services/LoggingService.test.js +++ /dev/null @@ -1,128 +0,0 @@ -import LoggingService, { MAX_ERROR_LENGTH } from './LoggingService'; - -global.newrelic = { - addPageAction: jest.fn(), - noticeError: jest.fn(), -}; - -describe('logInfo', () => { - beforeEach(() => { - global.newrelic.addPageAction.mockReset(); - }); - - it('calls New Relic client to log message if the client is available', () => { - const message = 'Test log'; - LoggingService.logInfo(message); - expect(global.newrelic.addPageAction).toHaveBeenCalledWith('INFO', { message }); - }); -}); - -describe('logError', () => { - beforeEach(() => { - global.newrelic.noticeError.mockReset(); - }); - - it('calls New Relic client to log error if the client is available', () => { - const error = new Error('Failed!'); - LoggingService.logError(error); - expect(global.newrelic.noticeError).toHaveBeenCalledWith(error, undefined); - }); - - it('calls New Relic client to log error if the client is available', () => { - const error = new Error('Failed!'); - LoggingService.logError(error); - expect(global.newrelic.noticeError).toHaveBeenCalledWith(error, undefined); - }); - - it('calls New Relic client with truncated error string', () => { - const error = new Array(MAX_ERROR_LENGTH + 500 + 1).join('0'); - const expectedError = new Array(MAX_ERROR_LENGTH + 1).join('0'); - LoggingService.logError(error); - expect(global.newrelic.noticeError).toHaveBeenCalledWith(expectedError, undefined); - }); - - it('calls New Relic client with truncated error', () => { - const error = { - message: new Array(MAX_ERROR_LENGTH + 500 + 1).join('0'), - }; - const expectedError = { - message: new Array(MAX_ERROR_LENGTH + 1).join('0'), - }; - LoggingService.logError(error); - expect(global.newrelic.noticeError).toHaveBeenCalledWith(expectedError, undefined); - }); -}); - -describe('logAPIErrorResponse', () => { - beforeEach(() => { - global.newrelic.noticeError.mockReset(); - }); - - it('calls New Relic client to log error when error has request object', () => { - const error = { - request: { - status: 400, - responseURL: 'http://example.com', - responseText: 'Very bad request', - }, - config: { - method: 'get', - }, - }; - const message = `${error.request.status} ${error.config.method} ${error.request.responseURL} ${error.request.responseText}`; - const expectedError = new Error(`API request failed: ${message}`); - const expectedAttributes = { - errorType: 'api-request-error', - errorStatus: error.request.status, - errorMethod: error.config.method, - errorUrl: error.request.responseURL, - errorData: error.request.responseText, - }; - LoggingService.logAPIErrorResponse(error); - expect(global.newrelic.noticeError).toHaveBeenCalledWith(expectedError, expectedAttributes); - }); - - it('calls New Relic client to log error when error has response object', () => { - const error = { - response: { - status: 500, - config: { - url: 'http://example.com', - }, - data: { - detail: 'Very bad request', - }, - }, - }; - const message = `${error.response.status} ${error.response.config.url} ${JSON.stringify(error.response.data)}`; - const expectedError = new Error(`API request failed: ${message}`); - const expectedAttributes = { - errorType: 'api-response-error', - errorStatus: error.response.status, - errorUrl: error.response.config.url, - errorData: JSON.stringify(error.response.data), - test: 'custom', - }; - LoggingService.logAPIErrorResponse(error, { test: 'custom' }); - expect(global.newrelic.noticeError).toHaveBeenCalledWith(expectedError, expectedAttributes); - }); - - it('calls New Relic client to log error and ignores html content in response data.', () => { - const error = { - response: { - status: 500, - data: '', - }, - }; - const message = `${error.response.status} `; - const expectedError = new Error(`API request failed: ${message}`); - const expectedAttributes = { - errorType: 'api-response-error', - errorStatus: error.response.status, - errorUrl: '', - errorData: '', - }; - LoggingService.logAPIErrorResponse(error); - expect(global.newrelic.noticeError).toHaveBeenCalledWith(expectedError, expectedAttributes); - }); -}); diff --git a/src/services/ProfileApiService.js b/src/services/ProfileApiService.js index f470499..36aaf4b 100644 --- a/src/services/ProfileApiService.js +++ b/src/services/ProfileApiService.js @@ -1,3 +1,5 @@ +import LoggingService from '@edx/frontend-logging'; + import apiClient from '../config/apiClient'; import { configuration } from '../config/environment'; import { @@ -5,7 +7,6 @@ import { convertKeyNames, snakeCaseObject, } from './utils'; -import LoggingService from '../services/LoggingService'; function processAccountData(data) { const result = camelCaseObject(data);