Merge eba158e into e01cf0d

.changeset/good-planets-smile.md

@@ -0,0 +1,5 @@
+---
+'@guardian/commercial': patch
+---
+
+remove raven client from commercial for error reporting

jest.setupTestFrameworkScriptFile.js

@@ -32,6 +32,11 @@ window.guardian = {
 		active: undefined,
 		onDetect: [],
 	},
+	modules: {
+		sentry: {
+			reportError: jest.fn(),
+		},
+	},
 };
 
 window.fetch = jest.fn().mockImplementation(() =>

package.json

@@ -108,7 +108,6 @@
 		"fastdom": "^1.0.11",
 		"lodash-es": "^4.17.21",
 		"process": "^0.11.10",
-		"raven-js": "^3.27.2",
 		"tslib": "^2.6.2",
 		"web-vitals": "^4.2.1"
 	},

src/core/messenger.spec.ts

@@ -250,7 +250,6 @@ describe('Cross-frame messenger', () => {
 			source: 'saucy',
 		})
 			.then(() => {
-				console.log(postMessage);
 				expect(postMessage).toHaveBeenCalledWith(
 					{
 						error: null,

src/core/messenger.ts

@@ -1,3 +1,4 @@
+import { reportError } from 'utils/report-error';
 import { postMessage } from './messenger/post-message';
 
 /**
@@ -117,11 +118,6 @@ type ListenerOptions = {
 	window?: WindowProxy;
 };
 
-type MessengerErrorHandler = (
-	err: Error,
-	features: Record<string, string>,
-) => void;
-
 /**
  * Types of functions to register a listener for a given type of iframe message
  */
@@ -163,9 +159,6 @@ type RespondCallback = (
 
 const LISTENERS: Listeners = {};
 let REGISTERED_LISTENERS = 0;
-let reportError: MessengerErrorHandler = () => {
-	// not set yet
-};
 
 const error405 = {
 	code: 405,
@@ -374,9 +367,7 @@ const onMessage = async (event: MessageEvent): Promise<void> => {
 				respond(message.id, event.source, null, response);
 			})
 			.catch((ex: Error) => {
-				reportError(ex, {
-					feature: 'native-ads',
-				});
+				reportError(ex, 'native-ads');
 				respond(
 					message.id,
 					event.source,
@@ -500,24 +491,12 @@ export const unregister: UnregisterListener = (type, callback, options) => {
  * @param persistentListeners The persistent listener registration functions
  */
 export const init = (
-	listeners: Array<
-		(
-			register: RegisterListener,
-			errorHandler: MessengerErrorHandler,
-		) => void
-	>,
-	persistentListeners: Array<
-		(
-			register: RegisterPersistentListener,
-			errorHandler: MessengerErrorHandler,
-		) => void
-	>,
-	errorHandler: MessengerErrorHandler,
+	listeners: Array<(register: RegisterListener) => void>,
+	persistentListeners: Array<(register: RegisterPersistentListener) => void>,
 ): void => {
-	reportError = errorHandler;
-	listeners.forEach((moduleInit) => moduleInit(register, errorHandler));
+	listeners.forEach((moduleInit) => moduleInit(register));
 	persistentListeners.forEach((moduleInit) =>
-		moduleInit(registerPersistentListener, errorHandler),
+		moduleInit(registerPersistentListener),
 	);
 };
 
@@ -529,5 +508,4 @@ export type {
 	RegisterPersistentListener,
 	UnregisterListener,
 	ListenerOptions,
-	MessengerErrorHandler,
 };

src/define/create-advert.ts

@@ -21,14 +21,7 @@ const createAdvert = (
 		log('commercial', errMsg);
 
 		if (!navigator.userAgent.includes('DuckDuckGo')) {
-			reportError(
-				new Error(errMsg),
-				{
-					feature: 'commercial',
-				},
-				false,
-				1 / 100,
-			);
+			reportError(new Error(errMsg), 'commercial');
 		}
 
 		return null;

src/events/on-slot-render.ts

@@ -20,16 +20,14 @@ const reportEmptyResponse = (
 			? event.slot.getTargeting('k')
 			: [];
 		const adKeywords = adTargetingKValues.join(', ');
-
 		reportError(
 			new Error('dfp returned an empty ad response'),
+			'commercial',
 			{
-				feature: 'commercial',
 				adUnit: adUnitPath,
 				adSlot: adSlotId,
 				adKeywords,
 			},
-			false,
 		);
 	}
 };

src/events/render-advert.ts

@@ -189,7 +189,6 @@ const renderAdvert = (
 	slotRenderEndedEvent: googletag.events.SlotRenderEndedEvent,
 ): Promise<boolean> => {
 	addContentClass(advert.node);
-
 	return getAdIframe(advert.node)
 		.then((isRendered) => {
 			const creativeTemplateId =
@@ -227,14 +226,7 @@ const renderAdvert = (
 				.then(() => isRendered);
 		})
 		.catch((err) => {
-			reportError(
-				err,
-				{
-					feature: 'commercial',
-				},
-				false,
-			);
-
+			reportError(err, 'commercial');
 			return Promise.resolve(false);
 		});
 };

src/init/consented.ts

@@ -20,18 +20,16 @@ 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
 const commercialBaseModules: Modules = [];
 
@@ -73,7 +71,7 @@ const loadModules = (modules: Modules, eventName: string) => {
 	modules.forEach((module) => {
 		const [moduleName, moduleInit] = module;
 
-		catchErrorsWithContext(
+		catchErrorsAndReport(
 			[
 				[
 					moduleName,
@@ -121,7 +119,7 @@ const bootCommercial = async (): Promise<void> => {
 	// Init Commercial event timers
 	EventTimer.init();
 
-	catchErrorsWithContext(
+	catchErrorsAndReport(
 		[
 			[
 				'ga-user-timing-commercial-start',
@@ -150,7 +148,7 @@ const bootCommercial = async (): Promise<void> => {
 		await Promise.all(promises).then(recordCommercialMetrics);
 	} catch (error) {
 		// report async errors in bootCommercial to Sentry with the commercial feature tag
-		reportError(error, tags, false);
+		reportError(error, 'commercial', tags);
 	}
 };
 

src/init/consented/dfp-listeners.ts

@@ -1,4 +1,4 @@
-import raven from 'lib/raven';
+import { wrapWithErrorReporting } from 'utils/report-error';
 import { onSlotLoad } from '../../events/on-slot-load';
 import { onSlotRender } from '../../events/on-slot-render';
 import { onSlotViewableFunction } from '../../events/on-slot-viewable';
@@ -9,11 +9,11 @@ const initDfpListeners = (): Promise<void> => {
 
 		pubads.addEventListener(
 			'slotRenderEnded',
-			raven.wrap<typeof onSlotRender>(onSlotRender),
+			wrapWithErrorReporting(onSlotRender),
 		);
 		pubads.addEventListener(
 			'slotOnload',
-			raven.wrap<typeof onSlotLoad>(onSlotLoad),
+			wrapWithErrorReporting(onSlotLoad),
 		);
 		pubads.addEventListener('impressionViewable', onSlotViewableFunction());
 	});

src/init/consented/dynamic-ad-slots.ts

@@ -27,10 +27,7 @@ export const initDynamicAdSlots = async (): Promise<void> => {
 			try {
 				await init();
 			} catch (error) {
-				reportError(error, {
-					feature: 'commercial',
-					tag: name,
-				});
+				reportError(error, 'commercial', { tag: name });
 			}
 		}),
 	);

src/init/consented/messenger.ts

@@ -12,7 +12,6 @@ import { init as resize } from 'core/messenger/resize';
 import { init as scroll } from 'core/messenger/scroll';
 import { init as type } from 'core/messenger/type';
 import { init as viewport } from 'core/messenger/viewport';
-import { reportError } from 'utils/report-error';
 
 /**
  * Messenger gets to skip the promise chain and run immediately.
@@ -32,7 +31,6 @@ initMessenger(
 		passback,
 	],
 	[scroll, viewport],
-	reportError,
 );
 
 const init = () => Promise.resolve();

src/init/consented/prepare-permutive.spec.ts

@@ -317,24 +317,26 @@ describe('Generating Permutive payload utils', () => {
 			page: { ...testPageConfig, section: 'uk' },
 		};
 
-		it('catches errors and calls the logger correctly when no global permutive', () => {
-			const logger = jest.fn();
+		it('catches errors and calls reportError correctly when no global permutive', () => {
+			const reportError = window.guardian.modules.sentry?.reportError;
 			_.runPermutive(
 				{
 					page: testPageConfig,
 				},
 				undefined,
-				logger,
 			);
-			const err = (logger.mock.calls[0] as Error[])[0];
+			const err = (
+				(reportError as jest.Mock).mock.calls[0] as Error[]
+			)[0];
 			expect(err).toBeInstanceOf(Error);
 			expect(err?.message).toBe('Global Permutive setup error');
 		});
+
 		it('calls the permutive addon method with the correct payload', () => {
 			const mockPermutive = { addon: jest.fn(), identify: jest.fn() };
 			const logger = jest.fn();
 
-			_.runPermutive(validConfigForPayload, mockPermutive, logger);
+			_.runPermutive(validConfigForPayload, mockPermutive);
 			expect(mockPermutive.addon).toHaveBeenCalledWith('web', {
 				page: {
 					content: expect.objectContaining({ section: 'uk' }) as {
@@ -356,22 +358,21 @@ describe('Generating Permutive payload utils', () => {
 				...validConfigForPayload,
 			};
 
-			_.runPermutive(config, mockPermutive, logger);
+			_.runPermutive(config, mockPermutive);
 			expect(mockPermutive.identify).toHaveBeenCalledWith([
 				{ tag: 'ophan', id: bwid },
 			]);
 			expect(logger).not.toHaveBeenCalled();
 		});
 		it("calls permutive's identify before it calls addon, if the browser ID is present", () => {
 			const mockPermutive = { addon: jest.fn(), identify: jest.fn() };
-			const logger = jest.fn();
 			const bwid = '1234567890abcdef';
 			const config = {
 				ophan: { browserId: bwid, pageViewId: 'pvid' },
 				...validConfigForPayload,
 			};
 
-			_.runPermutive(config, mockPermutive, logger);
+			_.runPermutive(config, mockPermutive);
 			const [identifyCallOrder] =
 				mockPermutive.identify.mock.invocationCallOrder;
 			const [addonCallOrder] = mockPermutive.addon.mock
@@ -382,7 +383,7 @@ describe('Generating Permutive payload utils', () => {
 			const mockPermutive = { addon: jest.fn(), identify: jest.fn() };
 			const logger = jest.fn();
 
-			_.runPermutive(validConfigForPayload, mockPermutive, logger);
+			_.runPermutive(validConfigForPayload, mockPermutive);
 			expect(mockPermutive.identify).not.toHaveBeenCalled();
 			expect(logger).not.toHaveBeenCalled();
 		});

src/init/consented/prepare-permutive.ts

@@ -140,7 +140,6 @@ const generatePermutiveIdentities = (
 const runPermutive = (
 	pageConfig: PermutivePageConfig,
 	permutiveGlobal: Permutive | undefined,
-	logger: typeof reportError,
 ): void => {
 	try {
 		if (!permutiveGlobal?.addon) {
@@ -157,7 +156,7 @@ const runPermutive = (
 			page: payload,
 		});
 	} catch (err) {
-		logger(err, { feature: 'commercial' }, false);
+		reportError(err, 'commercial');
 	}
 };
 
@@ -237,7 +236,7 @@ export const initPermutive = (): Promise<void> => {
 		page: window.guardian.config.page,
 		ophan: window.guardian.config.ophan,
 	};
-	runPermutive(permutiveConfig, window.permutive, reportError);
+	runPermutive(permutiveConfig, window.permutive);
 
 	return Promise.resolve();
 };

src/init/consentless.ts

@@ -10,11 +10,10 @@ import { initConsentless } from 'init/consentless/prepare-ootag';
 import { reloadPageOnConsentChange } from 'init/shared/reload-page-on-consent-change';
 import { init as setAdTestCookie } from 'init/shared/set-adtest-cookie';
 import { init as setAdTestInLabelsCookie } from 'init/shared/set-adtest-in-labels-cookie';
-import { reportError } from 'utils/report-error';
 
 const bootConsentless = async (consentState: ConsentState): Promise<void> => {
 	const consentlessModuleList = [
-		initMessenger([background, resize, type], [], reportError),
+		initMessenger([background, resize, type], []),
 		setAdTestCookie(),
 		setAdTestInLabelsCookie(),
 		initConsentless(consentState),