Skip to content

Commit

Permalink
Refactor robust and robust.spec
Browse files Browse the repository at this point in the history
  • Loading branch information
arelra committed Oct 1, 2024
1 parent 91f286b commit 35dfddb
Show file tree
Hide file tree
Showing 5 changed files with 49 additions and 86 deletions.
1 change: 0 additions & 1 deletion src/core/messenger.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -250,7 +250,6 @@ describe('Cross-frame messenger', () => {
source: 'saucy',
})
.then(() => {
console.log(postMessage);
expect(postMessage).toHaveBeenCalledWith(
{
error: null,
Expand Down
7 changes: 3 additions & 4 deletions src/init/consented.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,15 +20,14 @@ 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';

type Modules = Array<[`${string}-${string}`, () => Promise<unknown>]>;

const tags: Record<string, string> = {
feature: 'commercial',
bundle: 'standalone',
};
// modules necessary to load the first ads on the page
Expand Down Expand Up @@ -72,7 +71,7 @@ const loadModules = (modules: Modules, eventName: string) => {
modules.forEach((module) => {
const [moduleName, moduleInit] = module;

catchErrorsWithContext(
catchErrorsAndReport(
[
[
moduleName,
Expand Down Expand Up @@ -120,7 +119,7 @@ const bootCommercial = async (): Promise<void> => {
// Init Commercial event timers
EventTimer.init();

catchErrorsWithContext(
catchErrorsAndReport(
[
[
'ga-user-timing-commercial-start',
Expand Down
2 changes: 1 addition & 1 deletion src/utils/report-error.spec.ts
Original file line number Diff line number Diff line change
@@ -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', () => {
Expand Down
64 changes: 26 additions & 38 deletions src/utils/robust.spec.ts
Original file line number Diff line number Diff line change
@@ -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',
});
});
});
61 changes: 19 additions & 42 deletions src/utils/robust.ts
Original file line number Diff line number Diff line change
@@ -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<string, string>,
): void => {
window.console.warn('Caught error.', error.stack);
reportError(error, 'commercial', { ...tags, module: moduleName });
};

const catchAndLogError = (
name: string,
fn: ModuleFunction,
tags?: Record<string, string>,
): void => {
const error = catchErrors(fn);

if (error) {
logError(name, error, tags);
}
};

const catchErrorsWithContext = (
const catchErrorsAndReport = (
modules: Modules,
tags?: Record<string, string>,
): 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 };

0 comments on commit 35dfddb

Please sign in to comment.