diff --git a/src/core/messenger.spec.ts b/src/core/messenger.spec.ts index f0f5f7fa4..09a756ce9 100644 --- a/src/core/messenger.spec.ts +++ b/src/core/messenger.spec.ts @@ -250,7 +250,6 @@ describe('Cross-frame messenger', () => { source: 'saucy', }) .then(() => { - console.log(postMessage); expect(postMessage).toHaveBeenCalledWith( { error: null, diff --git a/src/init/consented.ts b/src/init/consented.ts index feb4c6855..21d7f874c 100644 --- a/src/init/consented.ts +++ b/src/init/consented.ts @@ -20,7 +20,7 @@ import { init as prepareAdVerification } from 'lib/ad-verification/prepare-ad-ve import { commercialFeatures } from 'lib/commercial-features'; import { adSlotIdPrefix } from 'lib/dfp/dfp-env-globals'; import { reportError } from 'utils/report-error'; -import { catchErrorsWithContext } from 'utils/robust'; +import { catchErrorsAndReport } from 'utils/robust'; import { initDfpListeners } from './consented/dfp-listeners'; import { initDynamicAdSlots } from './consented/dynamic-ad-slots'; import { init as initMessenger } from './consented/messenger'; @@ -28,7 +28,6 @@ import { init as initMessenger } from './consented/messenger'; type Modules = Array<[`${string}-${string}`, () => Promise]>; const tags: Record = { - feature: 'commercial', bundle: 'standalone', }; // modules necessary to load the first ads on the page @@ -72,7 +71,7 @@ const loadModules = (modules: Modules, eventName: string) => { modules.forEach((module) => { const [moduleName, moduleInit] = module; - catchErrorsWithContext( + catchErrorsAndReport( [ [ moduleName, @@ -120,7 +119,7 @@ const bootCommercial = async (): Promise => { // Init Commercial event timers EventTimer.init(); - catchErrorsWithContext( + catchErrorsAndReport( [ [ 'ga-user-timing-commercial-start', diff --git a/src/utils/report-error.spec.ts b/src/utils/report-error.spec.ts index 95be0f49c..be92af6d1 100644 --- a/src/utils/report-error.spec.ts +++ b/src/utils/report-error.spec.ts @@ -1,7 +1,7 @@ import { reportError } from './report-error'; describe('report-error', () => { - const error = new Error('Something broke'); + const error = new Error('Deliberate test error'); const tags = { test: 'testValue' }; test('Calls window.guardian.modules.sentry.reportError', () => { diff --git a/src/utils/robust.spec.ts b/src/utils/robust.spec.ts index 3d25aa97b..acda25057 100644 --- a/src/utils/robust.spec.ts +++ b/src/utils/robust.spec.ts @@ -1,70 +1,58 @@ import { reportError } from 'utils/report-error'; -import { _, catchErrorsWithContext } from './robust'; +import { catchErrorsAndReport } from './robust'; import type { Modules } from './robust'; -const { catchAndLogError } = _; - jest.mock('utils/report-error', () => ({ reportError: jest.fn(), })); -let origConsoleWarn: typeof window.console.warn; - beforeEach(() => { - jest.clearAllMocks(); - origConsoleWarn = window.console.warn; - window.console.warn = jest.fn(); + jest.spyOn(global.console, 'warn'); }); afterEach(() => { - window.console.warn = origConsoleWarn; + jest.spyOn(global.console, 'warn').mockRestore(); + jest.resetAllMocks(); }); describe('robust', () => { - const ERROR = new Error('Something broke.'); - const META = 'commercial'; - const TAGS = { - module: 'test', - }; - - const noError = () => true; + const ERROR = new Error('Deliberate test error'); + const tags = { tag: 'test' }; const throwError = () => { throw ERROR; }; - test('catchAndLogError()', () => { - expect(() => { - catchAndLogError('test', noError); - }).not.toThrowError(); - - expect(() => { - catchAndLogError('test', throwError); - }).not.toThrowError(ERROR); + test('catchErrorsAndReport with no errors', () => { + const runner = jest.fn(); - expect(window.console.warn).toHaveBeenCalledTimes(1); - }); + const modules = [ + ['test-1', runner], + ['test-2', runner], + ['test-3', runner], + ] as Modules; - test('catchAndLogError() - default reporter with no error', () => { - catchAndLogError('test', noError); + catchErrorsAndReport(modules); + expect(runner).toHaveBeenCalledTimes(modules.length); expect(reportError).not.toHaveBeenCalled(); }); - test('catchAndLogError() - default reporter with error', () => { - catchAndLogError('test', throwError); - expect(reportError).toHaveBeenCalledWith(ERROR, META, TAGS); - }); - - test('catchErrorsWithContext()', () => { + test('catchErrorsAndReport with one error', () => { const runner = jest.fn(); - const MODULES = [ + const modules = [ ['test-1', runner], - ['test-2', runner], + ['test-2', throwError], ['test-3', runner], ] as Modules; - catchErrorsWithContext(MODULES); - expect(runner).toHaveBeenCalledTimes(MODULES.length); + catchErrorsAndReport(modules, tags); + expect(runner).toHaveBeenCalledTimes(2); + expect(reportError).toHaveBeenCalledTimes(1); + expect(window.console.warn).toHaveBeenCalledTimes(1); + expect(reportError).toHaveBeenCalledWith(ERROR, 'commercial', { + tag: 'test', + module: 'test-2', + }); }); }); diff --git a/src/utils/robust.ts b/src/utils/robust.ts index 951db6c57..ff6c932f4 100644 --- a/src/utils/robust.ts +++ b/src/utils/robust.ts @@ -1,54 +1,31 @@ -/* -Swallows (and reports) exceptions. Designed to wrap around modules at the "bootstrap" level. -For example "comments throwing an exception should not stop auto refresh" -*/ - +/** + * Swallows and reports exceptions. Designed to wrap around modules at the "bootstrap" level. + */ import { reportError } from 'utils/report-error'; type ModuleFunction = () => void; type Module = [string, ModuleFunction]; type Modules = Module[]; -const catchErrors = (fn: ModuleFunction): Error | undefined => { - let error: Error | undefined; - - try { - fn(); - } catch (e) { - error = e instanceof Error ? e : new Error(String(e)); - } - - return error; -}; - -const logError = ( - moduleName: string, - error: Error, - tags?: Record, -): void => { - window.console.warn('Caught error.', error.stack); - reportError(error, 'commercial', { ...tags, module: moduleName }); -}; - -const catchAndLogError = ( - name: string, - fn: ModuleFunction, - tags?: Record, -): void => { - const error = catchErrors(fn); - - if (error) { - logError(name, error, tags); - } -}; - -const catchErrorsWithContext = ( +const catchErrorsAndReport = ( modules: Modules, tags?: Record, ): void => { - modules.forEach(([name, fn]) => catchAndLogError(name, fn, tags)); + modules.forEach(([name, fn]) => { + let error: Error | undefined; + + try { + fn(); + } catch (e) { + error = e instanceof Error ? e : new Error(String(e)); + } + + if (error) { + window.console.warn('Caught error.', error.stack); + reportError(error, 'commercial', { ...tags, module: name }); + } + }); }; -export { catchErrorsWithContext }; +export { catchErrorsAndReport }; export type { Modules }; -export const _ = { catchAndLogError };