From 5a352931689fe0a1e500991ec9ed7c1762ded7c8 Mon Sep 17 00:00:00 2001 From: David Chin Date: Wed, 18 Sep 2019 13:09:49 +1000 Subject: [PATCH] fix(common): CHECKOUT-4418 Log error in console --- .../common/error/ConsoleErrorLogger.spec.ts | 77 +++++++++++++++++++ src/app/common/error/ConsoleErrorLogger.ts | 51 ++++++++++++ .../common/error/SentryErrorLogger.spec.ts | 21 ++++- src/app/common/error/SentryErrorLogger.ts | 33 ++++---- src/app/common/error/createErrorLogger.ts | 9 ++- src/app/common/error/defaultErrorTypes.ts | 11 +++ 6 files changed, 186 insertions(+), 16 deletions(-) create mode 100644 src/app/common/error/ConsoleErrorLogger.spec.ts create mode 100644 src/app/common/error/ConsoleErrorLogger.ts create mode 100644 src/app/common/error/defaultErrorTypes.ts diff --git a/src/app/common/error/ConsoleErrorLogger.spec.ts b/src/app/common/error/ConsoleErrorLogger.spec.ts new file mode 100644 index 0000000000..e308d5a1ef --- /dev/null +++ b/src/app/common/error/ConsoleErrorLogger.spec.ts @@ -0,0 +1,77 @@ +import ConsoleErrorLogger from './ConsoleErrorLogger'; +import { ErrorLevelType } from './ErrorLogger'; + +describe('ConsoleErrorLogger', () => { + let mockConsole: Console; + + beforeEach(() => { + mockConsole = { + error: jest.fn(), + info: jest.fn(), + warn: jest.fn(), + } as unknown as Console; + }); + + it('logs error as error to console by default', () => { + const logger = new ConsoleErrorLogger({ console: mockConsole }); + const error = new Error('Testing 123'); + const tags = { errorCode: 'abc' }; + + logger.log(error, tags); + + expect(mockConsole.error) + .toHaveBeenCalledWith(error, tags); + }); + + it('logs error as warning to console', () => { + const logger = new ConsoleErrorLogger({ console: mockConsole }); + const error = new Error('Testing 123'); + const tags = { errorCode: 'abc' }; + + logger.log(error, tags, ErrorLevelType.Warning); + + expect(mockConsole.warn) + .toHaveBeenCalledWith(error, tags); + }); + + it('logs error as info to console', () => { + const logger = new ConsoleErrorLogger({ console: mockConsole }); + const error = new Error('Testing 123'); + const tags = { errorCode: 'abc' }; + + logger.log(error, tags, ErrorLevelType.Info); + + expect(mockConsole.info) + .toHaveBeenCalledWith(error, tags); + }); + + it('allows additional error types to be logged', () => { + const logger = new ConsoleErrorLogger({ + console: mockConsole, + errorTypes: ['Foo'], + }); + + const error = new Error('Foo'); + error.name = 'Foo'; + + logger.log(error); + + expect(mockConsole.error) + .toHaveBeenCalledWith(error, undefined); + }); + + it('does not log custom errors unless they are listed as additional error types', () => { + const logger = new ConsoleErrorLogger({ + console: mockConsole, + errorTypes: ['Foo'], + }); + + const error = new Error('Bar'); + error.name = 'Bar'; + + logger.log(error); + + expect(mockConsole.error) + .not.toHaveBeenCalledWith(error, undefined); + }); +}); diff --git a/src/app/common/error/ConsoleErrorLogger.ts b/src/app/common/error/ConsoleErrorLogger.ts new file mode 100644 index 0000000000..162b489dbd --- /dev/null +++ b/src/app/common/error/ConsoleErrorLogger.ts @@ -0,0 +1,51 @@ +import { includes } from 'lodash'; + +import DEFAULT_ERROR_TYPES from './defaultErrorTypes'; +import ErrorLogger, { ErrorLevelType, ErrorTags } from './ErrorLogger'; + +export interface ConsoleErrorLoggerOptions { + console?: Console; + errorTypes?: string[]; +} + +// tslint:disable:no-console +export default class ConsoleErrorLogger implements ErrorLogger { + private console: Console; + private errorTypes: string[]; + + constructor( + options?: ConsoleErrorLoggerOptions + ) { + const { + console: customConsole = console, + errorTypes = [], + } = options || {}; + + this.console = customConsole; + this.errorTypes = [ + ...DEFAULT_ERROR_TYPES, + ...errorTypes, + ]; + } + + log( + error: Error, + tags?: ErrorTags, + level: ErrorLevelType = ErrorLevelType.Error + ): void { + if (!includes(this.errorTypes, error.name)) { + return; + } + + switch (level) { + case ErrorLevelType.Error: + return this.console.error(error, tags); + + case ErrorLevelType.Info: + return this.console.info(error, tags); + + case ErrorLevelType.Warning: + return this.console.warn(error, tags); + } + } +} diff --git a/src/app/common/error/SentryErrorLogger.spec.ts b/src/app/common/error/SentryErrorLogger.spec.ts index e47b798268..e51cd4f5a1 100644 --- a/src/app/common/error/SentryErrorLogger.spec.ts +++ b/src/app/common/error/SentryErrorLogger.spec.ts @@ -3,8 +3,10 @@ import { RewriteFrames } from '@sentry/integrations'; import { Integration } from '@sentry/types'; import computeErrorCode from './computeErrorCode'; +import DEFAULT_ERROR_TYPES from './defaultErrorTypes'; +import ConsoleErrorLogger from './ConsoleErrorLogger'; import { ErrorLevelType } from './ErrorLogger'; -import SentryErrorLogger, { DEFAULT_ERROR_TYPES } from './SentryErrorLogger'; +import SentryErrorLogger from './SentryErrorLogger'; jest.mock('@sentry/browser', () => { return { @@ -254,5 +256,22 @@ describe('SentryErrorLogger', () => { expect(scope.setLevel) .toHaveBeenNthCalledWith(3, Severity.Info); }); + + it('logs error in console if console logger is provided', () => { + const consoleLogger = new ConsoleErrorLogger(); + + jest.spyOn(consoleLogger, 'log') + .mockImplementation(); + + const logger = new SentryErrorLogger(config, { consoleLogger }); + const error = new Error('Testing 123'); + const tags = { errorCode: 'abc' }; + const level = ErrorLevelType.Error; + + logger.log(error, tags, level); + + expect(consoleLogger.log) + .toHaveBeenCalledWith(error, tags, level); + }); }); }); diff --git a/src/app/common/error/SentryErrorLogger.ts b/src/app/common/error/SentryErrorLogger.ts index a98415c4aa..4fc31e0de7 100644 --- a/src/app/common/error/SentryErrorLogger.ts +++ b/src/app/common/error/SentryErrorLogger.ts @@ -4,33 +4,38 @@ import { EventHint } from '@sentry/types'; import { includes, isEmpty } from 'lodash'; import computeErrorCode from './computeErrorCode'; -import ErrorLogger, { ErrorLevelType, ErrorLoggerOptions, ErrorTags } from './ErrorLogger'; - -export const DEFAULT_ERROR_TYPES = [ - 'Error', - 'EvalError', - 'RangeError', - 'ReferenceError', - 'SyntaxError', - 'TypeError', - 'URIError', -]; +import DEFAULT_ERROR_TYPES from './defaultErrorTypes'; +import ConsoleErrorLogger from './ConsoleErrorLogger'; +import ErrorLogger, { ErrorLevelType, ErrorTags } from './ErrorLogger'; +import NoopErrorLogger from './NoopErrorLogger'; + +export interface SentryErrorLoggerOptions { + consoleLogger?: ConsoleErrorLogger; + errorTypes?: string[]; + publicPath?: string; +} export default class SentryErrorLogger implements ErrorLogger { + private consoleLogger: ErrorLogger; private errorTypes: string[]; private publicPath: string; constructor( config: BrowserOptions, - options?: Omit + options?: SentryErrorLoggerOptions ) { - const { errorTypes = [], publicPath = '' } = options || {}; + const { + consoleLogger = new NoopErrorLogger(), + errorTypes = [], + publicPath = '', + } = options || {}; this.errorTypes = [ ...DEFAULT_ERROR_TYPES, ...errorTypes, ]; + this.consoleLogger = consoleLogger; this.publicPath = publicPath; init({ @@ -49,6 +54,8 @@ export default class SentryErrorLogger implements ErrorLogger { tags?: ErrorTags, level: ErrorLevelType = ErrorLevelType.Error ): void { + this.consoleLogger.log(error, tags, level); + withScope(scope => { const { errorCode = computeErrorCode(error) } = tags || {}; const fingerprint = ['{{ default }}', errorCode]; diff --git a/src/app/common/error/createErrorLogger.ts b/src/app/common/error/createErrorLogger.ts index 65d8608244..a085823fd3 100644 --- a/src/app/common/error/createErrorLogger.ts +++ b/src/app/common/error/createErrorLogger.ts @@ -1,3 +1,4 @@ +import ConsoleErrorLogger from './ConsoleErrorLogger'; import ErrorLogger, { ErrorLoggerOptions, ErrorLoggerServiceConfig } from './ErrorLogger'; import NoopErrorLogger from './NoopErrorLogger'; import SentryErrorLogger from './SentryErrorLogger'; @@ -9,9 +10,13 @@ export default function createErrorLogger( if (serviceConfig && serviceConfig.sentry) { return new SentryErrorLogger( serviceConfig.sentry, - options + { ...options, consoleLogger: new ConsoleErrorLogger(options) } ); } - return new NoopErrorLogger(); + if (process.env.NODE_ENV === 'test') { + return new NoopErrorLogger(); + } + + return new ConsoleErrorLogger(options); } diff --git a/src/app/common/error/defaultErrorTypes.ts b/src/app/common/error/defaultErrorTypes.ts new file mode 100644 index 0000000000..503020ca91 --- /dev/null +++ b/src/app/common/error/defaultErrorTypes.ts @@ -0,0 +1,11 @@ +const DEFAULT_ERROR_TYPES = [ + 'Error', + 'EvalError', + 'RangeError', + 'ReferenceError', + 'SyntaxError', + 'TypeError', + 'URIError', +]; + +export default DEFAULT_ERROR_TYPES;