Skip to content

Commit 19a125d

Browse files
mgurgelmgurgel
committed
More exception coverage and testing
1 parent 4b568b6 commit 19a125d

File tree

8 files changed

+96
-14
lines changed

8 files changed

+96
-14
lines changed

‎injected/integration-test/duckplayer-mobile.spec.js

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -147,3 +147,13 @@ test.describe('Overlay screenshot @screenshots', () => {
147147
await expect(page.locator('.html5-video-player')).toHaveScreenshot('overlay.png', { maxDiffPixels: 20 });
148148
});
149149
});
150+
151+
test.describe('Reporting exceptions', () => {
152+
test('initial setup error', async ({ page }, workerInfo) => {
153+
const overlays = DuckplayerOverlays.create(page, workerInfo);
154+
await overlays.withRemoteConfig({ locale: 'en' });
155+
await overlays.initialSetupError();
156+
await overlays.gotoPlayerPage();
157+
await overlays.didSendException('TypeError', "undefined is not an object (evaluating 'userValues.privatePlayerMode')");
158+
});
159+
});

‎injected/integration-test/duckplayer.spec.js

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -369,3 +369,13 @@ test.describe('serp proxy', () => {
369369
await overlays.userValuesCallIsProxied();
370370
});
371371
});
372+
373+
test.describe('Reporting exceptions', () => {
374+
test('initial setup error', async ({ page }, workerInfo) => {
375+
const overlays = DuckplayerOverlays.create(page, workerInfo);
376+
await overlays.withRemoteConfig({ locale: 'en' });
377+
await overlays.initialSetupError();
378+
await overlays.gotoPlayerPage();
379+
await overlays.didSendException('TypeError', "Cannot read properties of undefined (reading 'privatePlayerMode')");
380+
});
381+
});

‎injected/integration-test/page-objects/duckplayer-overlays.js

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -305,6 +305,12 @@ export class DuckplayerOverlays {
305305
});
306306
}
307307

308+
async initialSetupError() {
309+
await this.collector.updateMockResponse({
310+
initialSetup: {},
311+
});
312+
}
313+
308314
/**
309315
* @param {keyof userValues} setting
310316
*/
@@ -477,6 +483,15 @@ export class DuckplayerOverlays {
477483
]);
478484
}
479485

486+
/**
487+
* @param {string} kind
488+
* @param {string} message
489+
*/
490+
async didSendException(kind, message) {
491+
const messages = await this.collector.waitForMessage('reportMetric');
492+
expect(messages).toMatchObject([{ payload: { params: { metricName: 'exception', params: { kind, message } } } }]);
493+
}
494+
480495
/**
481496
* @return {Promise<void>}
482497
*/

‎injected/src/features/duck-player.js

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ import { reportException } from '../../../special-pages/shared/report-metric.js'
6161
* @internal
6262
*/
6363
export default class DuckPlayerFeature extends ContentFeature {
64-
init(args) {
64+
async init(args) {
6565
/**
6666
* This feature never operates in a frame
6767
*/
@@ -105,12 +105,14 @@ export default class DuckPlayerFeature extends ContentFeature {
105105
const comms = new DuckPlayerOverlayMessages(this.messaging, env);
106106

107107
if (overlaysEnabled) {
108-
initOverlays(overlaySettings.youtube, env, comms);
108+
await initOverlays(overlaySettings.youtube, env, comms);
109109
} else if (serpProxyEnabled) {
110110
comms.serpProxy();
111111
}
112112
} catch (e) {
113-
reportException(this.messaging, { message: 'could not initialize duck player: ' + e.toString() });
113+
const message = e.message || 'Could not initialize duck player: ' + e.toString();
114+
const kind = e.name;
115+
reportException(this.messaging, { message, kind });
114116
}
115117
}
116118
}

‎injected/src/features/duckplayer/components/ddg-video-overlay.js

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,11 @@ export class DDGVideoOverlay extends HTMLElement {
2222
*/
2323
constructor({ environment, params, ui, manager }) {
2424
super();
25-
if (!(manager instanceof VideoOverlay)) throw new Error('invalid arguments');
25+
if (!(manager instanceof VideoOverlay)) {
26+
const error = new Error('Invalid VideoOverlay manager');
27+
error.name = 'VideoOverlayError';
28+
throw error;
29+
}
2630
this.environment = environment;
2731
this.ui = ui;
2832
this.params = params;
@@ -121,15 +125,23 @@ export class DDGVideoOverlay extends HTMLElement {
121125
const optOutHandler = (e) => {
122126
if (e.isTrusted) {
123127
const remember = containerElement.querySelector('input[name="ddg-remember"]');
124-
if (!(remember instanceof HTMLInputElement)) throw new Error('cannot find our input');
128+
if (!(remember instanceof HTMLInputElement)) {
129+
const error = new Error('Cannot find remember checkbox');
130+
error.name = 'VideoOverlayError';
131+
throw error;
132+
}
125133
this.manager.userOptOut(remember.checked, params);
126134
}
127135
};
128136
const watchInPlayerHandler = (e) => {
129137
if (e.isTrusted) {
130138
e.preventDefault();
131139
const remember = containerElement.querySelector('input[name="ddg-remember"]');
132-
if (!(remember instanceof HTMLInputElement)) throw new Error('cannot find our input');
140+
if (!(remember instanceof HTMLInputElement)) {
141+
const error = new Error('Cannot find remember checkbox');
142+
error.name = 'VideoOverlayError';
143+
throw error;
144+
}
133145
this.manager.userOptIn(remember.checked, params);
134146
}
135147
};

‎injected/src/features/duckplayer/overlay-messages.js

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -123,22 +123,36 @@ export class DuckPlayerOverlayMessages {
123123
if (evt.detail.kind === constants.MSG_NAME_SET_VALUES) {
124124
return this.setUserValues(evt.detail.data)
125125
.then((updated) => respond(constants.MSG_NAME_PUSH_DATA, updated))
126-
.catch(console.error);
126+
.catch((e) => {
127+
console.error(e);
128+
this.reportException({ message: e.toString(), kind: 'MessagingError' });
129+
});
127130
}
128131
if (evt.detail.kind === constants.MSG_NAME_READ_VALUES_SERP) {
129132
return this.getUserValues()
130133
.then((updated) => respond(constants.MSG_NAME_PUSH_DATA, updated))
131-
.catch(console.error);
134+
.catch((e) => {
135+
console.error(e);
136+
this.reportException({ message: e.toString(), kind: 'MessagingError' });
137+
});
132138
}
133139
if (evt.detail.kind === constants.MSG_NAME_OPEN_INFO) {
134140
return this.openInfo();
135141
}
136142
console.warn('unhandled event', evt);
137143
} catch (e) {
144+
this.reportException({ message: e.toString(), kind: 'MessagingError' });
138145
console.warn('cannot handle this message', e);
139146
}
140147
});
141148
}
149+
150+
/**
151+
* @param {import('../../../../special-pages/shared/types/shared.ts').ExceptionMetric['params']} params
152+
*/
153+
reportException(params) {
154+
return this.messaging.notify('reportMetric', { metricName: 'exception', params });
155+
}
142156
}
143157

144158
/**

‎injected/src/features/duckplayer/overlays.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ export async function initOverlays(settings, environment, messages) {
2626
try {
2727
initialSetup = await messages.initialSetup();
2828
} catch (e) {
29+
console.log('INITIAL SETUP ERROR');
2930
console.warn(e);
3031
return;
3132
}

‎injected/src/features/duckplayer/video-overlay.js

Lines changed: 24 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -254,10 +254,16 @@ export class VideoOverlay {
254254
elem.text = mobileStrings(this.environment.strings('overlays.json'));
255255
elem.addEventListener(DDGVideoOverlayMobile.OPEN_INFO, () => this.messages.openInfo());
256256
elem.addEventListener(DDGVideoOverlayMobile.OPT_OUT, (/** @type {CustomEvent<{remember: boolean}>} */ e) => {
257-
return this.mobileOptOut(e.detail.remember).catch(console.error);
257+
return this.mobileOptOut(e.detail.remember).catch((e) => {
258+
console.error(e);
259+
this.messages.reportException({ message: e.toString(), kind: 'MessagingError' });
260+
});
258261
});
259262
elem.addEventListener(DDGVideoOverlayMobile.OPT_IN, (/** @type {CustomEvent<{remember: boolean}>} */ e) => {
260-
return this.mobileOptIn(e.detail.remember, params).catch(console.error);
263+
return this.mobileOptIn(e.detail.remember, params).catch((e) => {
264+
console.error(e);
265+
this.messages.reportException({ message: e.toString(), kind: 'MessagingError' });
266+
});
261267
});
262268
targetElement.appendChild(elem);
263269

@@ -289,7 +295,10 @@ export class VideoOverlay {
289295
drawer.text = mobileStrings(this.environment.strings('overlays.json'));
290296
drawer.addEventListener(DDGVideoDrawerMobile.OPEN_INFO, () => this.messages.openInfo());
291297
drawer.addEventListener(DDGVideoDrawerMobile.OPT_OUT, (/** @type {CustomEvent<{remember: boolean}>} */ e) => {
292-
return this.mobileOptOut(e.detail.remember).catch(console.error);
298+
return this.mobileOptOut(e.detail.remember).catch((e) => {
299+
console.error(e);
300+
this.messages.reportException({ message: e.toString(), kind: 'MessagingError' });
301+
});
293302
});
294303
drawer.addEventListener(DDGVideoDrawerMobile.DISMISS, () => {
295304
return this.dismissOverlay();
@@ -298,7 +307,10 @@ export class VideoOverlay {
298307
return this.dismissOverlay();
299308
});
300309
drawer.addEventListener(DDGVideoDrawerMobile.OPT_IN, (/** @type {CustomEvent<{remember: boolean}>} */ e) => {
301-
return this.mobileOptIn(e.detail.remember, params).catch(console.error);
310+
return this.mobileOptIn(e.detail.remember, params).catch((e) => {
311+
console.error(e);
312+
this.messages.reportException({ message: e.toString(), kind: 'MessagingError' });
313+
});
302314
});
303315
drawerTargetElement.appendChild(drawer);
304316

@@ -412,7 +424,10 @@ export class VideoOverlay {
412424
}
413425
return this.environment.setHref(params.toPrivatePlayerUrl());
414426
})
415-
.catch((e) => console.error('error setting user choice', e));
427+
.catch((e) => {
428+
console.error('error setting user choice for opt-in', e);
429+
this.messages.reportException({ message: e.toString(), kind: 'MessagingError' });
430+
});
416431
}
417432

418433
/**
@@ -440,7 +455,10 @@ export class VideoOverlay {
440455
this.userValues = values;
441456
})
442457
.then(() => this.watchForVideoBeingAdded({ ignoreCache: true, via: 'userOptOut' }))
443-
.catch((e) => console.error('could not set userChoice for opt-out', e));
458+
.catch((e) => {
459+
console.error('could not set userChoice for opt-out', e);
460+
this.messages.reportException({ message: e.toString(), kind: 'MessagingError' });
461+
});
444462
} else {
445463
this.messages.sendPixel(new Pixel({ name: 'play.do_not_use', remember: '0' }));
446464
this.destroy();

0 commit comments

Comments
 (0)