From 1181a21daede24976ee9cc1591f405b11dcc4eb0 Mon Sep 17 00:00:00 2001 From: Marcos Gurgel Date: Mon, 23 Jun 2025 12:51:30 +0100 Subject: [PATCH 01/28] Report metric schema --- .../pages/duckplayer/types/duckplayer.ts | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/special-pages/pages/duckplayer/types/duckplayer.ts b/special-pages/pages/duckplayer/types/duckplayer.ts index 56a1321601..bec7358366 100644 --- a/special-pages/pages/duckplayer/types/duckplayer.ts +++ b/special-pages/pages/duckplayer/types/duckplayer.ts @@ -6,6 +6,7 @@ * @module Duckplayer Messages */ +export type ReportMetricEvent = ExceptionMetric; export type YouTubeError = "age-restricted" | "sign-in-required" | "no-embed" | "unknown"; export type PrivatePlayerMode = | { @@ -26,6 +27,7 @@ export interface DuckplayerMessages { | OpenInfoNotification | OpenSettingsNotification | ReportInitExceptionNotification + | ReportMetricNotification | ReportPageExceptionNotification | ReportYouTubeErrorNotification | TelemetryEventNotification; @@ -54,6 +56,19 @@ export interface ReportInitExceptionNotification { export interface ReportInitExceptionNotify { message: string; } +/** + * Generated from @see "../messages/reportMetric.notify.json" + */ +export interface ReportMetricNotification { + method: "reportMetric"; + params: ReportMetricEvent; +} +export interface ExceptionMetric { + metricName: "exception"; + params: { + message: string; + }; +} /** * Generated from @see "../messages/reportPageException.notify.json" */ From a6eff5e6b785a0c711f598a1c247bb3fc05f1f59 Mon Sep 17 00:00:00 2001 From: Marcos Gurgel Date: Mon, 23 Jun 2025 16:07:02 +0100 Subject: [PATCH 02/28] reportMetric method --- injected/src/features/duck-player.js | 29 ++++++++------ special-pages/pages/duckplayer/app/index.js | 2 + .../app/providers/UserValuesProvider.jsx | 2 + special-pages/pages/duckplayer/src/index.js | 13 ++++++ .../pages/duckplayer/types/duckplayer.ts | 15 ------- .../shared/messages/reportMetric.notify.json | 26 ++++++++++++ special-pages/shared/report-metric.js | 33 +++++++++++++++ special-pages/shared/types/shared.ts | 29 ++++++++++++++ special-pages/types.mjs | 11 +++++ special-pages/unit-test/report-metric.mjs | 40 +++++++++++++++++++ typedoc.js | 1 + types-generator/json-schema.mjs | 4 ++ 12 files changed, 178 insertions(+), 27 deletions(-) create mode 100644 special-pages/shared/messages/reportMetric.notify.json create mode 100644 special-pages/shared/report-metric.js create mode 100644 special-pages/shared/types/shared.ts create mode 100644 special-pages/unit-test/report-metric.mjs diff --git a/injected/src/features/duck-player.js b/injected/src/features/duck-player.js index db6d1cb8fa..d69a2f965c 100644 --- a/injected/src/features/duck-player.js +++ b/injected/src/features/duck-player.js @@ -37,6 +37,7 @@ import { DuckPlayerOverlayMessages, OpenInDuckPlayerMsg, Pixel } from './duckpla import { isBeingFramed } from '../utils.js'; import { initOverlays } from './duckplayer/overlays.js'; import { Environment } from './duckplayer/environment.js'; +import { reportException } from '../../../special-pages/shared/report-metric.js'; /** * @typedef UserValues - A way to communicate user settings @@ -93,19 +94,23 @@ export default class DuckPlayerFeature extends ContentFeature { throw new Error('cannot operate duck player without a messaging backend'); } - const locale = args?.locale || args?.language || 'en'; - const env = new Environment({ - debug: this.isDebug, - injectName: import.meta.injectName, - platform: this.platform, - locale, - }); - const comms = new DuckPlayerOverlayMessages(this.messaging, env); + try { + const locale = args?.locale || args?.language || 'en'; + const env = new Environment({ + debug: this.isDebug, + injectName: import.meta.injectName, + platform: this.platform, + locale, + }); + const comms = new DuckPlayerOverlayMessages(this.messaging, env); - if (overlaysEnabled) { - initOverlays(overlaySettings.youtube, env, comms); - } else if (serpProxyEnabled) { - comms.serpProxy(); + if (overlaysEnabled) { + initOverlays(overlaySettings.youtube, env, comms); + } else if (serpProxyEnabled) { + comms.serpProxy(); + } + } catch (e) { + reportException(this.messaging, { message: 'could not initialize duck player: ' + e.toString() }); } } } diff --git a/special-pages/pages/duckplayer/app/index.js b/special-pages/pages/duckplayer/app/index.js index 9b604739b1..97e70019ee 100644 --- a/special-pages/pages/duckplayer/app/index.js +++ b/special-pages/pages/duckplayer/app/index.js @@ -70,6 +70,8 @@ export async function init(messaging, telemetry, baseEnvironment) { const didCatch = (error) => { const message = error?.message || 'unknown'; + messaging.reportException({ message }); + // TODO: Remove the following event once all native platforms are responding to 'reportMetric: exception' messaging.reportPageException({ message }); }; diff --git a/special-pages/pages/duckplayer/app/providers/UserValuesProvider.jsx b/special-pages/pages/duckplayer/app/providers/UserValuesProvider.jsx index e0bb2b2850..8c40c28901 100644 --- a/special-pages/pages/duckplayer/app/providers/UserValuesProvider.jsx +++ b/special-pages/pages/duckplayer/app/providers/UserValuesProvider.jsx @@ -60,6 +60,8 @@ export function UserValuesProvider({ initial, children }) { }) .catch((err) => { console.error('could not set the enabled flag', err); + messaging.reportException({ message: 'could not set the enabled flag: ' + err.toString() }); + // TODO: Remove the following event once all native platforms are responding to 'reportMetric: exception' messaging.reportPageException({ message: 'could not set the enabled flag: ' + err.toString() }); }); } diff --git a/special-pages/pages/duckplayer/src/index.js b/special-pages/pages/duckplayer/src/index.js index c677b427ea..d7d2ac5296 100644 --- a/special-pages/pages/duckplayer/src/index.js +++ b/special-pages/pages/duckplayer/src/index.js @@ -4,6 +4,7 @@ import { createSpecialPageMessaging } from '../../../shared/create-special-page- import { init } from '../app/index.js'; import { initStorage } from './storage.js'; import '../../../shared/live-reload.js'; +import { reportException } from '../../../shared/report-metric.js'; export class DuckplayerPage { /** @@ -115,8 +116,18 @@ export class DuckplayerPage { reportInitException(params) { this.messaging.notify('reportInitException', params); } + + /** + * This will be sent to report metrics to the native layer + * @param {import('../../../shared/types/shared.ts').ReportMetricEvent['params']} params + */ + reportException(params) { + reportException(this.messaging, params); + } } +// TODO: Remove telemetry + /** * Events that occur in the client-side application */ @@ -181,6 +192,8 @@ init(duckplayerPage, telemetry, baseEnvironment).catch((e) => { // messages. console.error(e); const msg = typeof e?.message === 'string' ? e.message : 'unknown init error'; + duckplayerPage.reportException({ message: msg }); + // TODO: Remove this event once all native platforms are responding to 'reportMetric: exception' duckplayerPage.reportInitException({ message: msg }); }); diff --git a/special-pages/pages/duckplayer/types/duckplayer.ts b/special-pages/pages/duckplayer/types/duckplayer.ts index bec7358366..56a1321601 100644 --- a/special-pages/pages/duckplayer/types/duckplayer.ts +++ b/special-pages/pages/duckplayer/types/duckplayer.ts @@ -6,7 +6,6 @@ * @module Duckplayer Messages */ -export type ReportMetricEvent = ExceptionMetric; export type YouTubeError = "age-restricted" | "sign-in-required" | "no-embed" | "unknown"; export type PrivatePlayerMode = | { @@ -27,7 +26,6 @@ export interface DuckplayerMessages { | OpenInfoNotification | OpenSettingsNotification | ReportInitExceptionNotification - | ReportMetricNotification | ReportPageExceptionNotification | ReportYouTubeErrorNotification | TelemetryEventNotification; @@ -56,19 +54,6 @@ export interface ReportInitExceptionNotification { export interface ReportInitExceptionNotify { message: string; } -/** - * Generated from @see "../messages/reportMetric.notify.json" - */ -export interface ReportMetricNotification { - method: "reportMetric"; - params: ReportMetricEvent; -} -export interface ExceptionMetric { - metricName: "exception"; - params: { - message: string; - }; -} /** * Generated from @see "../messages/reportPageException.notify.json" */ diff --git a/special-pages/shared/messages/reportMetric.notify.json b/special-pages/shared/messages/reportMetric.notify.json new file mode 100644 index 0000000000..413ee6a491 --- /dev/null +++ b/special-pages/shared/messages/reportMetric.notify.json @@ -0,0 +1,26 @@ +{ + "$schema": "http://json-schema.org/draft-07/schema#", + "title": "Report Metric Event", + "type": "object", + "oneOf": [ + { + "type": "object", + "title": "Exception Metric", + "required": ["metricName", "params"], + "properties": { + "metricName": { + "const": "exception" + }, + "params": { + "type": "object", + "required": ["message"], + "properties": { + "message": { + "type": "string" + } + } + } + } + } + ] +} diff --git a/special-pages/shared/report-metric.js b/special-pages/shared/report-metric.js new file mode 100644 index 0000000000..373af2b944 --- /dev/null +++ b/special-pages/shared/report-metric.js @@ -0,0 +1,33 @@ +export const REPORT_METRIC_MESSAGE_ID = 'reportMetric'; +export const METRIC_NAME_EXCEPTION = 'exception'; + +/** + * @typedef {import('../shared/types/shared.ts').SharedMessages} SharedMessages + * @typedef {import('@duckduckgo/messaging/lib/shared-types.js').MessagingBase|import('@duckduckgo/messaging').Messaging} SharedMessaging + */ + +/** + * Sends a standard 'reportMetric' event to the native layer + * @param {SharedMessaging} messaging + * @param {import('../shared/types/shared.ts').ReportMetricEvent} metricEvent + */ +export function reportMetric(messaging, metricEvent) { + if (!messaging?.notify) { + throw new Error('messaging.notify is not defined'); + } + + if (!metricEvent?.metricName) { + throw new Error('metricName is required'); + } + + messaging.notify(REPORT_METRIC_MESSAGE_ID, metricEvent); +} + +/** + * Sends a 'reportMetric' event with metric name 'exception' + * @param {SharedMessaging} messaging + * @param {import('../shared/types/shared.ts').ExceptionMetric['params']} params + */ +export function reportException(messaging, params) { + reportMetric(messaging, { metricName: METRIC_NAME_EXCEPTION, params }); +} diff --git a/special-pages/shared/types/shared.ts b/special-pages/shared/types/shared.ts new file mode 100644 index 0000000000..9754dd1042 --- /dev/null +++ b/special-pages/shared/types/shared.ts @@ -0,0 +1,29 @@ +/** + * These types are auto-generated from schema files. + * scripts/build-types.mjs is responsible for type generation. + * **DO NOT** edit this file directly as your changes will be lost. + * + * @module Shared Messages + */ + +export type ReportMetricEvent = ExceptionMetric; + +/** + * Requests, Notifications and Subscriptions from the Shared feature + */ +export interface SharedMessages { + notifications: ReportMetricNotification; +} +/** + * Generated from @see "../messages/reportMetric.notify.json" + */ +export interface ReportMetricNotification { + method: 'reportMetric'; + params: ReportMetricEvent; +} +export interface ExceptionMetric { + metricName: 'exception'; + params: { + message: string; + }; +} diff --git a/special-pages/types.mjs b/special-pages/types.mjs index 95f2c9049d..d3ff91d1d7 100644 --- a/special-pages/types.mjs +++ b/special-pages/types.mjs @@ -29,6 +29,17 @@ for (const pageListElement of pageList) { }; } +/* Shared types */ +specialPagesTypes.shared = { + schemaDir: join(specialPagesRoot, 'shared', 'messages'), + typesDir: join(specialPagesRoot, 'shared', 'types'), + exclude: process.platform === 'win32', + kind: 'single', + resolve: (_dirname) => '../src/index.js', + className: () => null, + filename: `shared.ts`, +}; + if (isLaunchFile(import.meta.url)) { buildTypes(specialPagesTypes).catch((error) => { console.error(error); diff --git a/special-pages/unit-test/report-metric.mjs b/special-pages/unit-test/report-metric.mjs new file mode 100644 index 0000000000..65c15dbb78 --- /dev/null +++ b/special-pages/unit-test/report-metric.mjs @@ -0,0 +1,40 @@ +import { describe, it, mock } from 'node:test'; +import assert from 'node:assert'; +import { reportMetric } from '../shared/report-metric.js'; + +// @ts-expect-error - this is a mock +const messaging = /** @type {Messaging} */ ({ + notify: mock.fn((params) => { + console.log('Notify called with', params); + }), +}); + +describe('reportMetric', () => { + it('should throw an error if messaging is not provided', () => { + const eventParams = /** @type {any} */ ({ metricName: '', params: {} }); + // @ts-expect-error - this is a forced error + assert.throws(() => reportMetric(null, eventParams)); + assert.strictEqual(messaging.notify.mock.callCount(), 0); + }); + + it('should throw an error if metricName is not provided', () => { + const eventParams = /** @type {any} */ ({ metricName: '', params: {} }); + assert.throws(() => reportMetric(messaging, eventParams)); + assert.strictEqual(messaging.notify.mock.callCount(), 0); + }); + + it('should call messaging.notify with the correct parameters', () => { + const eventParams = /** @type {any} */ ({ metricName: 'exception', params: { message: 'This is a test' } }); + assert.strictEqual(messaging.notify.mock.callCount(), 0); + reportMetric(messaging, eventParams); + assert.strictEqual(messaging.notify.mock.callCount(), 1); + const call = messaging.notify.mock.calls[0]; + assert.deepEqual(call.arguments, [ + 'reportMetric', + { + metricName: 'exception', + params: { message: 'This is a test' }, + }, + ]); + }); +}); diff --git a/typedoc.js b/typedoc.js index efc432c7db..c403aefc7b 100644 --- a/typedoc.js +++ b/typedoc.js @@ -35,6 +35,7 @@ const config = { 'special-pages/pages/special-error/app/types.js', 'special-pages/pages/new-tab/app/favorites/constants.js', 'special-pages/pages/**/types/*.ts', + 'special-pages/shared/types/*.ts', ], categoryOrder: ['Special Pages', 'Content Scope Scripts Integrations', 'Other'], out: 'docs', diff --git a/types-generator/json-schema.mjs b/types-generator/json-schema.mjs index f5a6cd86c1..9cf5eb7652 100644 --- a/types-generator/json-schema.mjs +++ b/types-generator/json-schema.mjs @@ -173,6 +173,10 @@ export function generateSchema(featureName, fileList) { * @return {string} */ export function createMessagingTypes(job, { featurePath, className }) { + if (!className) { + return ''; + } + const json = job.schema; const notifications = (json.properties?.notifications?.oneOf?.length ?? 0) > 0; const requests = (json.properties?.requests?.oneOf?.length ?? 0) > 0; From 1f793a4a46a031886b957f46ede3d2b7b0dc205a Mon Sep 17 00:00:00 2001 From: Marcos Gurgel Date: Mon, 23 Jun 2025 16:15:59 +0100 Subject: [PATCH 03/28] Lint config --- eslint.config.js | 1 + special-pages/shared/types/shared.ts | 14 +++++++------- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/eslint.config.js b/eslint.config.js index 313092a5d0..4a10b284ec 100644 --- a/eslint.config.js +++ b/eslint.config.js @@ -17,6 +17,7 @@ export default tseslint.config( 'special-pages/pages/**/public', 'special-pages/pages/**/types', 'special-pages/pages/**/messages', + 'special-pages/shared/types', 'special-pages/playwright-report/', 'special-pages/test-results/', 'playwright-report', diff --git a/special-pages/shared/types/shared.ts b/special-pages/shared/types/shared.ts index 9754dd1042..3b57a3974f 100644 --- a/special-pages/shared/types/shared.ts +++ b/special-pages/shared/types/shared.ts @@ -12,18 +12,18 @@ export type ReportMetricEvent = ExceptionMetric; * Requests, Notifications and Subscriptions from the Shared feature */ export interface SharedMessages { - notifications: ReportMetricNotification; + notifications: ReportMetricNotification; } /** * Generated from @see "../messages/reportMetric.notify.json" */ export interface ReportMetricNotification { - method: 'reportMetric'; - params: ReportMetricEvent; + method: "reportMetric"; + params: ReportMetricEvent; } export interface ExceptionMetric { - metricName: 'exception'; - params: { - message: string; - }; + metricName: "exception"; + params: { + message: string; + }; } From 5091890b4ba416fa8a3faaeb7d853f861e92ca6e Mon Sep 17 00:00:00 2001 From: Marcos Gurgel Date: Mon, 23 Jun 2025 17:13:18 +0100 Subject: [PATCH 04/28] Prettier actually --- .prettierignore | 1 + eslint.config.js | 1 - 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/.prettierignore b/.prettierignore index 94b59dabd6..a71fd8c561 100644 --- a/.prettierignore +++ b/.prettierignore @@ -3,6 +3,7 @@ docs/**/* !injected/docs/**/* injected/src/types special-pages/pages/**/types +special-pages/shared/types injected/integration-test/extension/contentScope.js injected/src/features/Scriptlets **/*.json diff --git a/eslint.config.js b/eslint.config.js index 4a10b284ec..313092a5d0 100644 --- a/eslint.config.js +++ b/eslint.config.js @@ -17,7 +17,6 @@ export default tseslint.config( 'special-pages/pages/**/public', 'special-pages/pages/**/types', 'special-pages/pages/**/messages', - 'special-pages/shared/types', 'special-pages/playwright-report/', 'special-pages/test-results/', 'playwright-report', From fe3a3455aeb6bbcdb811bb58c4ba664909b48d4e Mon Sep 17 00:00:00 2001 From: Marcos Gurgel Date: Wed, 25 Jun 2025 17:37:51 +0100 Subject: [PATCH 05/28] Added kind param --- .../pages/duckplayer/app/embed-settings.js | 24 +++++++++++--- special-pages/pages/duckplayer/app/index.js | 16 ++++++--- .../app/providers/UserValuesProvider.jsx | 7 ++-- .../integration-tests/duck-player.js | 33 +++++++++++++++++++ .../integration-tests/duckplayer.spec.js | 8 +++++ special-pages/pages/duckplayer/src/index.js | 8 +++-- .../shared/messages/reportMetric.notify.json | 3 ++ special-pages/shared/report-metric.js | 19 +++++++++-- special-pages/shared/types/shared.ts | 1 + 9 files changed, 102 insertions(+), 17 deletions(-) diff --git a/special-pages/pages/duckplayer/app/embed-settings.js b/special-pages/pages/duckplayer/app/embed-settings.js index 59060aaf45..e485ce75af 100644 --- a/special-pages/pages/duckplayer/app/embed-settings.js +++ b/special-pages/pages/duckplayer/app/embed-settings.js @@ -103,9 +103,17 @@ class VideoId { * @throws {Error} */ constructor(input) { - if (typeof input !== 'string') throw new Error('string required, got: ' + input); + if (typeof input !== 'string') { + const error = new Error('string required, got: ' + input); + error.name = 'VideoIdError'; + throw error; + } const sanitized = sanitizeYoutubeId(input); - if (sanitized === null) throw new Error('invalid ID from: ' + input); + if (sanitized === null) { + const error = new Error('invalid ID from: ' + input); + error.name = 'VideoIdError'; + throw error; + } this.id = sanitized; } @@ -126,9 +134,17 @@ class Timestamp { * @throws {Error} */ constructor(input) { - if (typeof input !== 'string') throw new Error('string required for timestamp'); + if (typeof input !== 'string') { + const error = new Error('string required for timestamp, got: ' + input); + error.name = 'TimestampError'; + throw error; + } const seconds = timestampInSeconds(input); - if (seconds === null) throw new Error('invalid input for timestamp: ' + input); + if (seconds === null) { + const error = new Error('invalid input for timestamp: ' + input); + error.name = 'TimestampError'; + throw error; + } this.seconds = seconds; } diff --git a/special-pages/pages/duckplayer/app/index.js b/special-pages/pages/duckplayer/app/index.js index 97e70019ee..c97210cf85 100644 --- a/special-pages/pages/duckplayer/app/index.js +++ b/special-pages/pages/duckplayer/app/index.js @@ -27,11 +27,14 @@ import { YouTubeErrorProvider } from './providers/YouTubeErrorProvider'; export async function init(messaging, telemetry, baseEnvironment) { const result = await callWithRetry(() => messaging.initialSetup()); if ('error' in result) { - throw new Error(result.error); + console.error('INITIAL SETUP ERROR', result.error); + const error = new Error(result.error); + error.name = 'InitialSetupError'; + throw error; } const init = result.value; - console.log('INITIAL DATA', init); + console.log('INITIAL DATA'); // update the 'env' in case it was changed by native sides const environment = baseEnvironment @@ -69,10 +72,13 @@ export async function init(messaging, telemetry, baseEnvironment) { const embed = createEmbedSettings(window.location.href, settings); const didCatch = (error) => { - const message = error?.message || 'unknown'; - messaging.reportException({ message }); + const message = error?.message; + const kind = error?.error?.name; + messaging.reportException({ message, kind }); + console.log('reportException', message, kind); + // TODO: Remove the following event once all native platforms are responding to 'reportMetric: exception' - messaging.reportPageException({ message }); + messaging.reportPageException({ message: message || 'unknown error' }); }; document.body.dataset.layout = settings.layout; diff --git a/special-pages/pages/duckplayer/app/providers/UserValuesProvider.jsx b/special-pages/pages/duckplayer/app/providers/UserValuesProvider.jsx index 8c40c28901..98da5bb3f4 100644 --- a/special-pages/pages/duckplayer/app/providers/UserValuesProvider.jsx +++ b/special-pages/pages/duckplayer/app/providers/UserValuesProvider.jsx @@ -60,9 +60,12 @@ export function UserValuesProvider({ initial, children }) { }) .catch((err) => { console.error('could not set the enabled flag', err); - messaging.reportException({ message: 'could not set the enabled flag: ' + err.toString() }); + const message = 'could not set the enabled flag: ' + err.toString(); + const kind = 'MessagingError'; + messaging.reportException({ message, kind }); + // TODO: Remove the following event once all native platforms are responding to 'reportMetric: exception' - messaging.reportPageException({ message: 'could not set the enabled flag: ' + err.toString() }); + messaging.reportPageException({ message }); }); } diff --git a/special-pages/pages/duckplayer/integration-tests/duck-player.js b/special-pages/pages/duckplayer/integration-tests/duck-player.js index 51460d6f6a..936e4e80bf 100644 --- a/special-pages/pages/duckplayer/integration-tests/duck-player.js +++ b/special-pages/pages/duckplayer/integration-tests/duck-player.js @@ -145,6 +145,14 @@ export class DuckPlayerPage { this.mocks.defaultResponses(clone); } + initError() { + const clone = structuredClone(this.defaults); + // Force an init error by passing an empty response to initialSetup + // @ts-expect-error - this is a test + clone.initialSetup = undefined; + this.mocks.defaultResponses(clone); + } + /** * We don't need to actually load the content for these tests. * By mocking the response, we make the tests about 10x faster and also ensure they work offline. @@ -537,6 +545,31 @@ export class DuckPlayerPage { ]); } + /** + * @param {import('../../../shared/types/shared.ts').ReportMetricEvent} evt + */ + async didSendReportMetric(evt) { + const events = await this.mocks.waitForCallCount({ method: 'reportMetric', count: 1 }); + expect(events).toStrictEqual([ + { + payload: { + context: 'specialPages', + featureName: 'duckPlayerPage', + method: 'reportMetric', + params: evt, + }, + }, + ]); + } + + /** + * @param {string} kind + * @param {string} message + */ + didSendException(kind, message) { + return this.didSendReportMetric({ metricName: 'exception', params: { kind, message } }); + } + async withStorageValues() { await this.page.evaluate(() => { localStorage.setItem('foo', 'bar'); diff --git a/special-pages/pages/duckplayer/integration-tests/duckplayer.spec.js b/special-pages/pages/duckplayer/integration-tests/duckplayer.spec.js index 7d3a1155e1..7db0c8b4e3 100644 --- a/special-pages/pages/duckplayer/integration-tests/duckplayer.spec.js +++ b/special-pages/pages/duckplayer/integration-tests/duckplayer.spec.js @@ -333,6 +333,14 @@ test.describe('reporting exceptions', () => { // load as normal await duckplayer.openWithException(); await duckplayer.showsErrorMessage(); + await duckplayer.didSendException('Error', 'Simulated Exception'); + }); + test('initial setup error', async ({ page }, workerInfo) => { + const duckplayer = DuckPlayerPage.create(page, workerInfo); + // load as normal + duckplayer.initError(); + await duckplayer.openWithVideoID(); + await duckplayer.didSendException('InitError', 'Max attempts reached: Error: response not found for initialSetup'); }); }); diff --git a/special-pages/pages/duckplayer/src/index.js b/special-pages/pages/duckplayer/src/index.js index d7d2ac5296..6415486c2a 100644 --- a/special-pages/pages/duckplayer/src/index.js +++ b/special-pages/pages/duckplayer/src/index.js @@ -191,10 +191,12 @@ const telemetry = new Telemetry(messaging); init(duckplayerPage, telemetry, baseEnvironment).catch((e) => { // messages. console.error(e); - const msg = typeof e?.message === 'string' ? e.message : 'unknown init error'; - duckplayerPage.reportException({ message: msg }); + const message = typeof e?.message === 'string' ? e.message : 'unknown error'; + const kind = 'InitError'; + duckplayerPage.reportException({ message, kind }); + // TODO: Remove this event once all native platforms are responding to 'reportMetric: exception' - duckplayerPage.reportInitException({ message: msg }); + duckplayerPage.reportInitException({ message }); }); initStorage(); diff --git a/special-pages/shared/messages/reportMetric.notify.json b/special-pages/shared/messages/reportMetric.notify.json index 413ee6a491..111faccebd 100644 --- a/special-pages/shared/messages/reportMetric.notify.json +++ b/special-pages/shared/messages/reportMetric.notify.json @@ -17,6 +17,9 @@ "properties": { "message": { "type": "string" + }, + "kind": { + "type": "string" } } } diff --git a/special-pages/shared/report-metric.js b/special-pages/shared/report-metric.js index 373af2b944..cffa645912 100644 --- a/special-pages/shared/report-metric.js +++ b/special-pages/shared/report-metric.js @@ -2,6 +2,8 @@ export const REPORT_METRIC_MESSAGE_ID = 'reportMetric'; export const METRIC_NAME_EXCEPTION = 'exception'; /** + * @typedef {import('../shared/types/shared.ts').ExceptionMetric} ExceptionMetric + * @typedef {import('../shared/types/shared.ts').ReportMetricEvent} ReportMetricEvent * @typedef {import('../shared/types/shared.ts').SharedMessages} SharedMessages * @typedef {import('@duckduckgo/messaging/lib/shared-types.js').MessagingBase|import('@duckduckgo/messaging').Messaging} SharedMessaging */ @@ -9,7 +11,7 @@ export const METRIC_NAME_EXCEPTION = 'exception'; /** * Sends a standard 'reportMetric' event to the native layer * @param {SharedMessaging} messaging - * @param {import('../shared/types/shared.ts').ReportMetricEvent} metricEvent + * @param {ReportMetricEvent} metricEvent */ export function reportMetric(messaging, metricEvent) { if (!messaging?.notify) { @@ -26,8 +28,19 @@ export function reportMetric(messaging, metricEvent) { /** * Sends a 'reportMetric' event with metric name 'exception' * @param {SharedMessaging} messaging - * @param {import('../shared/types/shared.ts').ExceptionMetric['params']} params + * @param {ExceptionMetric['params']} params */ export function reportException(messaging, params) { - reportMetric(messaging, { metricName: METRIC_NAME_EXCEPTION, params }); + const message = typeof params?.message === 'string' ? params.message : 'unknown error'; + const kind = typeof params?.kind === 'string' ? params.kind : 'Error'; + + /** @type {ExceptionMetric} */ + const metric = { + metricName: METRIC_NAME_EXCEPTION, + params: { + message, + kind, + }, + }; + reportMetric(messaging, metric); } diff --git a/special-pages/shared/types/shared.ts b/special-pages/shared/types/shared.ts index 3b57a3974f..56943e8ee1 100644 --- a/special-pages/shared/types/shared.ts +++ b/special-pages/shared/types/shared.ts @@ -25,5 +25,6 @@ export interface ExceptionMetric { metricName: "exception"; params: { message: string; + kind?: string; }; } From e9a8facb5de3173194448a6df1082869d59f1b1e Mon Sep 17 00:00:00 2001 From: Marcos Gurgel Date: Wed, 25 Jun 2025 18:40:36 +0100 Subject: [PATCH 06/28] More exception coverage and testing --- .../duckplayer-mobile.spec.js | 10 +++++++ injected/integration-test/duckplayer.spec.js | 10 +++++++ .../page-objects/duckplayer-overlays.js | 15 ++++++++++ injected/src/features/duck-player.js | 8 +++-- .../components/ddg-video-overlay.js | 18 +++++++++-- .../features/duckplayer/overlay-messages.js | 18 +++++++++-- injected/src/features/duckplayer/overlays.js | 1 + .../src/features/duckplayer/video-overlay.js | 30 +++++++++++++++---- 8 files changed, 96 insertions(+), 14 deletions(-) diff --git a/injected/integration-test/duckplayer-mobile.spec.js b/injected/integration-test/duckplayer-mobile.spec.js index 375db575c1..b971607347 100644 --- a/injected/integration-test/duckplayer-mobile.spec.js +++ b/injected/integration-test/duckplayer-mobile.spec.js @@ -147,3 +147,13 @@ test.describe('Overlay screenshot @screenshots', () => { await expect(page.locator('.html5-video-player')).toHaveScreenshot('overlay.png', { maxDiffPixels: 20 }); }); }); + +test.describe('Reporting exceptions', () => { + test('initial setup error', async ({ page }, workerInfo) => { + const overlays = DuckplayerOverlays.create(page, workerInfo); + await overlays.withRemoteConfig({ locale: 'en' }); + await overlays.initialSetupError(); + await overlays.gotoPlayerPage(); + await overlays.didSendException('TypeError', "undefined is not an object (evaluating 'userValues.privatePlayerMode')"); + }); +}); diff --git a/injected/integration-test/duckplayer.spec.js b/injected/integration-test/duckplayer.spec.js index ca0aa628dd..3b7d673e04 100644 --- a/injected/integration-test/duckplayer.spec.js +++ b/injected/integration-test/duckplayer.spec.js @@ -369,3 +369,13 @@ test.describe('serp proxy', () => { await overlays.userValuesCallIsProxied(); }); }); + +test.describe('Reporting exceptions', () => { + test('initial setup error', async ({ page }, workerInfo) => { + const overlays = DuckplayerOverlays.create(page, workerInfo); + await overlays.withRemoteConfig({ locale: 'en' }); + await overlays.initialSetupError(); + await overlays.gotoPlayerPage(); + await overlays.didSendException('TypeError', "Cannot read properties of undefined (reading 'privatePlayerMode')"); + }); +}); diff --git a/injected/integration-test/page-objects/duckplayer-overlays.js b/injected/integration-test/page-objects/duckplayer-overlays.js index 1b0850a3ee..4fd069fb8b 100644 --- a/injected/integration-test/page-objects/duckplayer-overlays.js +++ b/injected/integration-test/page-objects/duckplayer-overlays.js @@ -305,6 +305,12 @@ export class DuckplayerOverlays { }); } + async initialSetupError() { + await this.collector.updateMockResponse({ + initialSetup: {}, + }); + } + /** * @param {keyof userValues} setting */ @@ -477,6 +483,15 @@ export class DuckplayerOverlays { ]); } + /** + * @param {string} kind + * @param {string} message + */ + async didSendException(kind, message) { + const messages = await this.collector.waitForMessage('reportMetric'); + expect(messages).toMatchObject([{ payload: { params: { metricName: 'exception', params: { kind, message } } } }]); + } + /** * @return {Promise} */ diff --git a/injected/src/features/duck-player.js b/injected/src/features/duck-player.js index d69a2f965c..1e8a88f660 100644 --- a/injected/src/features/duck-player.js +++ b/injected/src/features/duck-player.js @@ -61,7 +61,7 @@ import { reportException } from '../../../special-pages/shared/report-metric.js' * @internal */ export default class DuckPlayerFeature extends ContentFeature { - init(args) { + async init(args) { /** * This feature never operates in a frame */ @@ -105,12 +105,14 @@ export default class DuckPlayerFeature extends ContentFeature { const comms = new DuckPlayerOverlayMessages(this.messaging, env); if (overlaysEnabled) { - initOverlays(overlaySettings.youtube, env, comms); + await initOverlays(overlaySettings.youtube, env, comms); } else if (serpProxyEnabled) { comms.serpProxy(); } } catch (e) { - reportException(this.messaging, { message: 'could not initialize duck player: ' + e.toString() }); + const message = e.message || 'Could not initialize duck player: ' + e.toString(); + const kind = e.name; + reportException(this.messaging, { message, kind }); } } } diff --git a/injected/src/features/duckplayer/components/ddg-video-overlay.js b/injected/src/features/duckplayer/components/ddg-video-overlay.js index 8b797d5dbc..2f3b629644 100644 --- a/injected/src/features/duckplayer/components/ddg-video-overlay.js +++ b/injected/src/features/duckplayer/components/ddg-video-overlay.js @@ -22,7 +22,11 @@ export class DDGVideoOverlay extends HTMLElement { */ constructor({ environment, params, ui, manager }) { super(); - if (!(manager instanceof VideoOverlay)) throw new Error('invalid arguments'); + if (!(manager instanceof VideoOverlay)) { + const error = new Error('Invalid VideoOverlay manager'); + error.name = 'VideoOverlayError'; + throw error; + } this.environment = environment; this.ui = ui; this.params = params; @@ -121,7 +125,11 @@ export class DDGVideoOverlay extends HTMLElement { const optOutHandler = (e) => { if (e.isTrusted) { const remember = containerElement.querySelector('input[name="ddg-remember"]'); - if (!(remember instanceof HTMLInputElement)) throw new Error('cannot find our input'); + if (!(remember instanceof HTMLInputElement)) { + const error = new Error('Cannot find remember checkbox'); + error.name = 'VideoOverlayError'; + throw error; + } this.manager.userOptOut(remember.checked, params); } }; @@ -129,7 +137,11 @@ export class DDGVideoOverlay extends HTMLElement { if (e.isTrusted) { e.preventDefault(); const remember = containerElement.querySelector('input[name="ddg-remember"]'); - if (!(remember instanceof HTMLInputElement)) throw new Error('cannot find our input'); + if (!(remember instanceof HTMLInputElement)) { + const error = new Error('Cannot find remember checkbox'); + error.name = 'VideoOverlayError'; + throw error; + } this.manager.userOptIn(remember.checked, params); } }; diff --git a/injected/src/features/duckplayer/overlay-messages.js b/injected/src/features/duckplayer/overlay-messages.js index 3e61e5f471..0477215f5c 100644 --- a/injected/src/features/duckplayer/overlay-messages.js +++ b/injected/src/features/duckplayer/overlay-messages.js @@ -123,22 +123,36 @@ export class DuckPlayerOverlayMessages { if (evt.detail.kind === constants.MSG_NAME_SET_VALUES) { return this.setUserValues(evt.detail.data) .then((updated) => respond(constants.MSG_NAME_PUSH_DATA, updated)) - .catch(console.error); + .catch((e) => { + console.error(e); + this.reportException({ message: e.toString(), kind: 'MessagingError' }); + }); } if (evt.detail.kind === constants.MSG_NAME_READ_VALUES_SERP) { return this.getUserValues() .then((updated) => respond(constants.MSG_NAME_PUSH_DATA, updated)) - .catch(console.error); + .catch((e) => { + console.error(e); + this.reportException({ message: e.toString(), kind: 'MessagingError' }); + }); } if (evt.detail.kind === constants.MSG_NAME_OPEN_INFO) { return this.openInfo(); } console.warn('unhandled event', evt); } catch (e) { + this.reportException({ message: e.toString(), kind: 'MessagingError' }); console.warn('cannot handle this message', e); } }); } + + /** + * @param {import('../../../../special-pages/shared/types/shared.ts').ExceptionMetric['params']} params + */ + reportException(params) { + return this.messaging.notify('reportMetric', { metricName: 'exception', params }); + } } /** diff --git a/injected/src/features/duckplayer/overlays.js b/injected/src/features/duckplayer/overlays.js index 9de25b5442..42efc5c50d 100644 --- a/injected/src/features/duckplayer/overlays.js +++ b/injected/src/features/duckplayer/overlays.js @@ -26,6 +26,7 @@ export async function initOverlays(settings, environment, messages) { try { initialSetup = await messages.initialSetup(); } catch (e) { + console.log('INITIAL SETUP ERROR'); console.warn(e); return; } diff --git a/injected/src/features/duckplayer/video-overlay.js b/injected/src/features/duckplayer/video-overlay.js index 129de68800..f0d17f0fb9 100644 --- a/injected/src/features/duckplayer/video-overlay.js +++ b/injected/src/features/duckplayer/video-overlay.js @@ -254,10 +254,16 @@ export class VideoOverlay { elem.text = mobileStrings(this.environment.strings('overlays.json')); elem.addEventListener(DDGVideoOverlayMobile.OPEN_INFO, () => this.messages.openInfo()); elem.addEventListener(DDGVideoOverlayMobile.OPT_OUT, (/** @type {CustomEvent<{remember: boolean}>} */ e) => { - return this.mobileOptOut(e.detail.remember).catch(console.error); + return this.mobileOptOut(e.detail.remember).catch((e) => { + console.error(e); + this.messages.reportException({ message: e.toString(), kind: 'MessagingError' }); + }); }); elem.addEventListener(DDGVideoOverlayMobile.OPT_IN, (/** @type {CustomEvent<{remember: boolean}>} */ e) => { - return this.mobileOptIn(e.detail.remember, params).catch(console.error); + return this.mobileOptIn(e.detail.remember, params).catch((e) => { + console.error(e); + this.messages.reportException({ message: e.toString(), kind: 'MessagingError' }); + }); }); targetElement.appendChild(elem); @@ -289,7 +295,10 @@ export class VideoOverlay { drawer.text = mobileStrings(this.environment.strings('overlays.json')); drawer.addEventListener(DDGVideoDrawerMobile.OPEN_INFO, () => this.messages.openInfo()); drawer.addEventListener(DDGVideoDrawerMobile.OPT_OUT, (/** @type {CustomEvent<{remember: boolean}>} */ e) => { - return this.mobileOptOut(e.detail.remember).catch(console.error); + return this.mobileOptOut(e.detail.remember).catch((e) => { + console.error(e); + this.messages.reportException({ message: e.toString(), kind: 'MessagingError' }); + }); }); drawer.addEventListener(DDGVideoDrawerMobile.DISMISS, () => { return this.dismissOverlay(); @@ -298,7 +307,10 @@ export class VideoOverlay { return this.dismissOverlay(); }); drawer.addEventListener(DDGVideoDrawerMobile.OPT_IN, (/** @type {CustomEvent<{remember: boolean}>} */ e) => { - return this.mobileOptIn(e.detail.remember, params).catch(console.error); + return this.mobileOptIn(e.detail.remember, params).catch((e) => { + console.error(e); + this.messages.reportException({ message: e.toString(), kind: 'MessagingError' }); + }); }); drawerTargetElement.appendChild(drawer); @@ -412,7 +424,10 @@ export class VideoOverlay { } return this.environment.setHref(params.toPrivatePlayerUrl()); }) - .catch((e) => console.error('error setting user choice', e)); + .catch((e) => { + console.error('error setting user choice for opt-in', e); + this.messages.reportException({ message: e.toString(), kind: 'MessagingError' }); + }); } /** @@ -440,7 +455,10 @@ export class VideoOverlay { this.userValues = values; }) .then(() => this.watchForVideoBeingAdded({ ignoreCache: true, via: 'userOptOut' })) - .catch((e) => console.error('could not set userChoice for opt-out', e)); + .catch((e) => { + console.error('could not set userChoice for opt-out', e); + this.messages.reportException({ message: e.toString(), kind: 'MessagingError' }); + }); } else { this.messages.sendPixel(new Pixel({ name: 'play.do_not_use', remember: '0' })); this.destroy(); From 7206a3ad281799460145cfb65c778c35edff29bf Mon Sep 17 00:00:00 2001 From: Marcos Gurgel Date: Wed, 25 Jun 2025 19:05:20 +0100 Subject: [PATCH 07/28] Test fix --- .../duckplayer/integration-tests/duck-player.js | 7 +++++-- .../duckplayer/integration-tests/duckplayer.spec.js | 13 ++++++++++++- 2 files changed, 17 insertions(+), 3 deletions(-) diff --git a/special-pages/pages/duckplayer/integration-tests/duck-player.js b/special-pages/pages/duckplayer/integration-tests/duck-player.js index 936e4e80bf..a8937b9e69 100644 --- a/special-pages/pages/duckplayer/integration-tests/duck-player.js +++ b/special-pages/pages/duckplayer/integration-tests/duck-player.js @@ -147,9 +147,12 @@ export class DuckPlayerPage { initError() { const clone = structuredClone(this.defaults); - // Force an init error by passing an empty response to initialSetup // @ts-expect-error - this is a test - clone.initialSetup = undefined; + clone.initialSetup = { + locale: 'en', + env: 'development', + platform: this.platform.name === 'windows' ? undefined : { name: this.platform.name }, + }; this.mocks.defaultResponses(clone); } diff --git a/special-pages/pages/duckplayer/integration-tests/duckplayer.spec.js b/special-pages/pages/duckplayer/integration-tests/duckplayer.spec.js index 7db0c8b4e3..4b6049960a 100644 --- a/special-pages/pages/duckplayer/integration-tests/duckplayer.spec.js +++ b/special-pages/pages/duckplayer/integration-tests/duckplayer.spec.js @@ -340,10 +340,21 @@ test.describe('reporting exceptions', () => { // load as normal duckplayer.initError(); await duckplayer.openWithVideoID(); - await duckplayer.didSendException('InitError', 'Max attempts reached: Error: response not found for initialSetup'); + const message = isWindows(workerInfo) + ? 'Cannot read properties of undefined (reading \'pip\')' + : 'undefined is not an object (evaluating \'init2.settings.pip\')'; + await duckplayer.didSendException('InitError', message); }); }); +/** + * @param {import("@playwright/test").TestInfo} testInfo + */ +function isWindows(testInfo) { + const u = /** @type {any} */ (testInfo.project.use); + return u?.platform === 'windows'; +} + /** * @param {import("@playwright/test").TestInfo} testInfo */ From 5b50860276b0f6cf5c8a08b682d4ffe416a21062 Mon Sep 17 00:00:00 2001 From: Marcos Gurgel Date: Wed, 25 Jun 2025 19:07:14 +0100 Subject: [PATCH 08/28] Lint --- .../pages/duckplayer/integration-tests/duckplayer.spec.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/special-pages/pages/duckplayer/integration-tests/duckplayer.spec.js b/special-pages/pages/duckplayer/integration-tests/duckplayer.spec.js index 4b6049960a..fccbdc83c0 100644 --- a/special-pages/pages/duckplayer/integration-tests/duckplayer.spec.js +++ b/special-pages/pages/duckplayer/integration-tests/duckplayer.spec.js @@ -341,8 +341,8 @@ test.describe('reporting exceptions', () => { duckplayer.initError(); await duckplayer.openWithVideoID(); const message = isWindows(workerInfo) - ? 'Cannot read properties of undefined (reading \'pip\')' - : 'undefined is not an object (evaluating \'init2.settings.pip\')'; + ? "Cannot read properties of undefined (reading 'pip')" + : "undefined is not an object (evaluating 'init2.settings.pip')"; await duckplayer.didSendException('InitError', message); }); }); From 36b3c38f12f731bc6fea14d1e9bca2b08ebe7c53 Mon Sep 17 00:00:00 2001 From: Marcos Gurgel Date: Thu, 26 Jun 2025 12:14:47 +0100 Subject: [PATCH 09/28] Constants refactor --- .../page-objects/duckplayer-overlays.js | 2 ++ .../duckplayer/components/ddg-video-overlay.js | 7 ++++--- .../src/features/duckplayer/overlay-messages.js | 14 ++++---------- injected/src/features/duckplayer/video-overlay.js | 14 +++++++------- .../pages/duckplayer/app/embed-settings.js | 9 +++++---- special-pages/pages/duckplayer/app/index.js | 5 +++-- .../app/providers/UserValuesProvider.jsx | 4 ++-- .../duckplayer/integration-tests/duck-player.js | 14 ++++++++++++++ .../integration-tests/duckplayer.spec.js | 13 +------------ special-pages/pages/duckplayer/src/index.js | 13 ++----------- special-pages/shared/report-metric.js | 7 +++++++ 11 files changed, 51 insertions(+), 51 deletions(-) diff --git a/injected/integration-test/page-objects/duckplayer-overlays.js b/injected/integration-test/page-objects/duckplayer-overlays.js index 4fd069fb8b..8af983f269 100644 --- a/injected/integration-test/page-objects/duckplayer-overlays.js +++ b/injected/integration-test/page-objects/duckplayer-overlays.js @@ -488,6 +488,8 @@ export class DuckplayerOverlays { * @param {string} message */ async didSendException(kind, message) { + console.log('messages', await this.collector.outgoingMessages()); + const messages = await this.collector.waitForMessage('reportMetric'); expect(messages).toMatchObject([{ payload: { params: { metricName: 'exception', params: { kind, message } } } }]); } diff --git a/injected/src/features/duckplayer/components/ddg-video-overlay.js b/injected/src/features/duckplayer/components/ddg-video-overlay.js index 2f3b629644..8b64489d51 100644 --- a/injected/src/features/duckplayer/components/ddg-video-overlay.js +++ b/injected/src/features/duckplayer/components/ddg-video-overlay.js @@ -4,6 +4,7 @@ import { overlayCopyVariants } from '../text.js'; import { appendImageAsBackground } from '../util.js'; import { VideoOverlay } from '../video-overlay.js'; import { createPolicy, html, trustedUnsafe } from '../../../dom-utils.js'; +import { METRIC_NAME_VIDEO_OVERLAY_ERROR } from '../../../../../special-pages/shared/report-metric.js'; /** * The custom element that we use to present our UI elements @@ -24,7 +25,7 @@ export class DDGVideoOverlay extends HTMLElement { super(); if (!(manager instanceof VideoOverlay)) { const error = new Error('Invalid VideoOverlay manager'); - error.name = 'VideoOverlayError'; + error.name = METRIC_NAME_VIDEO_OVERLAY_ERROR; throw error; } this.environment = environment; @@ -127,7 +128,7 @@ export class DDGVideoOverlay extends HTMLElement { const remember = containerElement.querySelector('input[name="ddg-remember"]'); if (!(remember instanceof HTMLInputElement)) { const error = new Error('Cannot find remember checkbox'); - error.name = 'VideoOverlayError'; + error.name = METRIC_NAME_VIDEO_OVERLAY_ERROR; throw error; } this.manager.userOptOut(remember.checked, params); @@ -139,7 +140,7 @@ export class DDGVideoOverlay extends HTMLElement { const remember = containerElement.querySelector('input[name="ddg-remember"]'); if (!(remember instanceof HTMLInputElement)) { const error = new Error('Cannot find remember checkbox'); - error.name = 'VideoOverlayError'; + error.name = METRIC_NAME_VIDEO_OVERLAY_ERROR; throw error; } this.manager.userOptIn(remember.checked, params); diff --git a/injected/src/features/duckplayer/overlay-messages.js b/injected/src/features/duckplayer/overlay-messages.js index 0477215f5c..36ebaaec6e 100644 --- a/injected/src/features/duckplayer/overlay-messages.js +++ b/injected/src/features/duckplayer/overlay-messages.js @@ -1,5 +1,6 @@ /* eslint-disable promise/prefer-await-to-then */ import * as constants from './constants.js'; +import { reportException, METRIC_NAME_MESSAGING_ERROR } from '../../../../special-pages/shared/report-metric.js'; /** * @typedef {import("@duckduckgo/messaging").Messaging} Messaging @@ -125,7 +126,7 @@ export class DuckPlayerOverlayMessages { .then((updated) => respond(constants.MSG_NAME_PUSH_DATA, updated)) .catch((e) => { console.error(e); - this.reportException({ message: e.toString(), kind: 'MessagingError' }); + reportException(this.messaging, { message: e.toString(), kind: METRIC_NAME_MESSAGING_ERROR }); }); } if (evt.detail.kind === constants.MSG_NAME_READ_VALUES_SERP) { @@ -133,7 +134,7 @@ export class DuckPlayerOverlayMessages { .then((updated) => respond(constants.MSG_NAME_PUSH_DATA, updated)) .catch((e) => { console.error(e); - this.reportException({ message: e.toString(), kind: 'MessagingError' }); + reportException(this.messaging, { message: e.toString(), kind: METRIC_NAME_MESSAGING_ERROR }); }); } if (evt.detail.kind === constants.MSG_NAME_OPEN_INFO) { @@ -141,18 +142,11 @@ export class DuckPlayerOverlayMessages { } console.warn('unhandled event', evt); } catch (e) { - this.reportException({ message: e.toString(), kind: 'MessagingError' }); + reportException(this.messaging, { message: e.toString(), kind: METRIC_NAME_MESSAGING_ERROR }); console.warn('cannot handle this message', e); } }); } - - /** - * @param {import('../../../../special-pages/shared/types/shared.ts').ExceptionMetric['params']} params - */ - reportException(params) { - return this.messaging.notify('reportMetric', { metricName: 'exception', params }); - } } /** diff --git a/injected/src/features/duckplayer/video-overlay.js b/injected/src/features/duckplayer/video-overlay.js index f0d17f0fb9..b0caf5689c 100644 --- a/injected/src/features/duckplayer/video-overlay.js +++ b/injected/src/features/duckplayer/video-overlay.js @@ -33,7 +33,7 @@ import { mobileStrings } from './text.js'; import { DDGVideoOverlayMobile } from './components/ddg-video-overlay-mobile.js'; import { DDGVideoThumbnailOverlay } from './components/ddg-video-thumbnail-overlay-mobile.js'; import { DDGVideoDrawerMobile } from './components/ddg-video-drawer-mobile.js'; - +import { reportException, METRIC_NAME_MESSAGING_ERROR } from '../../../../special-pages/shared/report-metric.js'; /** * Handle the switch between small & large overlays * + conduct any communications @@ -256,13 +256,13 @@ export class VideoOverlay { elem.addEventListener(DDGVideoOverlayMobile.OPT_OUT, (/** @type {CustomEvent<{remember: boolean}>} */ e) => { return this.mobileOptOut(e.detail.remember).catch((e) => { console.error(e); - this.messages.reportException({ message: e.toString(), kind: 'MessagingError' }); + reportException(this.messages.messaging, { message: e.toString(), kind: METRIC_NAME_MESSAGING_ERROR }); }); }); elem.addEventListener(DDGVideoOverlayMobile.OPT_IN, (/** @type {CustomEvent<{remember: boolean}>} */ e) => { return this.mobileOptIn(e.detail.remember, params).catch((e) => { console.error(e); - this.messages.reportException({ message: e.toString(), kind: 'MessagingError' }); + reportException(this.messages.messaging, { message: e.toString(), kind: METRIC_NAME_MESSAGING_ERROR }); }); }); targetElement.appendChild(elem); @@ -297,7 +297,7 @@ export class VideoOverlay { drawer.addEventListener(DDGVideoDrawerMobile.OPT_OUT, (/** @type {CustomEvent<{remember: boolean}>} */ e) => { return this.mobileOptOut(e.detail.remember).catch((e) => { console.error(e); - this.messages.reportException({ message: e.toString(), kind: 'MessagingError' }); + reportException(this.messages.messaging, { message: e.toString(), kind: METRIC_NAME_MESSAGING_ERROR }); }); }); drawer.addEventListener(DDGVideoDrawerMobile.DISMISS, () => { @@ -309,7 +309,7 @@ export class VideoOverlay { drawer.addEventListener(DDGVideoDrawerMobile.OPT_IN, (/** @type {CustomEvent<{remember: boolean}>} */ e) => { return this.mobileOptIn(e.detail.remember, params).catch((e) => { console.error(e); - this.messages.reportException({ message: e.toString(), kind: 'MessagingError' }); + reportException(this.messages.messaging, { message: e.toString(), kind: METRIC_NAME_MESSAGING_ERROR }); }); }); drawerTargetElement.appendChild(drawer); @@ -426,7 +426,7 @@ export class VideoOverlay { }) .catch((e) => { console.error('error setting user choice for opt-in', e); - this.messages.reportException({ message: e.toString(), kind: 'MessagingError' }); + reportException(this.messages.messaging, { message: e.toString(), kind: METRIC_NAME_MESSAGING_ERROR }); }); } @@ -457,7 +457,7 @@ export class VideoOverlay { .then(() => this.watchForVideoBeingAdded({ ignoreCache: true, via: 'userOptOut' })) .catch((e) => { console.error('could not set userChoice for opt-out', e); - this.messages.reportException({ message: e.toString(), kind: 'MessagingError' }); + reportException(this.messages.messaging, { message: e.toString(), kind: METRIC_NAME_MESSAGING_ERROR }); }); } else { this.messages.sendPixel(new Pixel({ name: 'play.do_not_use', remember: '0' })); diff --git a/special-pages/pages/duckplayer/app/embed-settings.js b/special-pages/pages/duckplayer/app/embed-settings.js index e485ce75af..e01e04571a 100644 --- a/special-pages/pages/duckplayer/app/embed-settings.js +++ b/special-pages/pages/duckplayer/app/embed-settings.js @@ -1,3 +1,4 @@ +import { METRIC_NAME_VIDEO_ID_ERROR, METRIC_NAME_TIMESTAMP_ERROR } from '../../../shared/report-metric.js'; export class EmbedSettings { /** * @param {object} params @@ -105,13 +106,13 @@ class VideoId { constructor(input) { if (typeof input !== 'string') { const error = new Error('string required, got: ' + input); - error.name = 'VideoIdError'; + error.name = METRIC_NAME_VIDEO_ID_ERROR; throw error; } const sanitized = sanitizeYoutubeId(input); if (sanitized === null) { const error = new Error('invalid ID from: ' + input); - error.name = 'VideoIdError'; + error.name = METRIC_NAME_VIDEO_ID_ERROR; throw error; } this.id = sanitized; @@ -136,13 +137,13 @@ class Timestamp { constructor(input) { if (typeof input !== 'string') { const error = new Error('string required for timestamp, got: ' + input); - error.name = 'TimestampError'; + error.name = METRIC_NAME_TIMESTAMP_ERROR; throw error; } const seconds = timestampInSeconds(input); if (seconds === null) { const error = new Error('invalid input for timestamp: ' + input); - error.name = 'TimestampError'; + error.name = METRIC_NAME_TIMESTAMP_ERROR; throw error; } this.seconds = seconds; diff --git a/special-pages/pages/duckplayer/app/index.js b/special-pages/pages/duckplayer/app/index.js index c97210cf85..67875312eb 100644 --- a/special-pages/pages/duckplayer/app/index.js +++ b/special-pages/pages/duckplayer/app/index.js @@ -15,6 +15,7 @@ import { Components } from './components/Components.jsx'; import { MobileApp } from './components/MobileApp.jsx'; import { DesktopApp } from './components/DesktopApp.jsx'; import { YouTubeErrorProvider } from './providers/YouTubeErrorProvider'; +import { reportException, METRIC_NAME_INITIAL_SETUP_ERROR } from '../../../shared/report-metric.js'; /** @typedef {import('../types/duckplayer').YouTubeError} YouTubeError */ @@ -29,7 +30,7 @@ export async function init(messaging, telemetry, baseEnvironment) { if ('error' in result) { console.error('INITIAL SETUP ERROR', result.error); const error = new Error(result.error); - error.name = 'InitialSetupError'; + error.name = METRIC_NAME_INITIAL_SETUP_ERROR; throw error; } @@ -74,7 +75,7 @@ export async function init(messaging, telemetry, baseEnvironment) { const didCatch = (error) => { const message = error?.message; const kind = error?.error?.name; - messaging.reportException({ message, kind }); + reportException(messaging.messaging, { message, kind }); console.log('reportException', message, kind); // TODO: Remove the following event once all native platforms are responding to 'reportMetric: exception' diff --git a/special-pages/pages/duckplayer/app/providers/UserValuesProvider.jsx b/special-pages/pages/duckplayer/app/providers/UserValuesProvider.jsx index 98da5bb3f4..0d4a606500 100644 --- a/special-pages/pages/duckplayer/app/providers/UserValuesProvider.jsx +++ b/special-pages/pages/duckplayer/app/providers/UserValuesProvider.jsx @@ -2,6 +2,7 @@ import { useContext, useState } from 'preact/hooks'; import { h, createContext } from 'preact'; import { useMessaging } from '../types.js'; import { useEffect } from 'preact/hooks'; +import { reportException, METRIC_NAME_MESSAGING_ERROR } from '../../../../shared/report-metric.js'; /** * @typedef {import("../../types/duckplayer.js").UserValues} UserValues @@ -61,8 +62,7 @@ export function UserValuesProvider({ initial, children }) { .catch((err) => { console.error('could not set the enabled flag', err); const message = 'could not set the enabled flag: ' + err.toString(); - const kind = 'MessagingError'; - messaging.reportException({ message, kind }); + reportException(messaging.messaging, { message, kind: METRIC_NAME_MESSAGING_ERROR }); // TODO: Remove the following event once all native platforms are responding to 'reportMetric: exception' messaging.reportPageException({ message }); diff --git a/special-pages/pages/duckplayer/integration-tests/duck-player.js b/special-pages/pages/duckplayer/integration-tests/duck-player.js index a8937b9e69..d87d7c0e20 100644 --- a/special-pages/pages/duckplayer/integration-tests/duck-player.js +++ b/special-pages/pages/duckplayer/integration-tests/duck-player.js @@ -573,6 +573,20 @@ export class DuckPlayerPage { return this.didSendReportMetric({ metricName: 'exception', params: { kind, message } }); } + async didSendInitErrorException() { + await this.build.switch({ + android: async () => { + await this.didSendException('InitError', "undefined is not an object (evaluating 'init2.settings.pip')"); + }, + apple: async () => { + await this.didSendException('InitError', "undefined is not an object (evaluating 'init2.settings.pip')"); + }, + windows: async () => { + await this.didSendException('InitError', "Cannot read properties of undefined (reading 'pip')"); + }, + }); + } + async withStorageValues() { await this.page.evaluate(() => { localStorage.setItem('foo', 'bar'); diff --git a/special-pages/pages/duckplayer/integration-tests/duckplayer.spec.js b/special-pages/pages/duckplayer/integration-tests/duckplayer.spec.js index fccbdc83c0..970a41c13a 100644 --- a/special-pages/pages/duckplayer/integration-tests/duckplayer.spec.js +++ b/special-pages/pages/duckplayer/integration-tests/duckplayer.spec.js @@ -340,21 +340,10 @@ test.describe('reporting exceptions', () => { // load as normal duckplayer.initError(); await duckplayer.openWithVideoID(); - const message = isWindows(workerInfo) - ? "Cannot read properties of undefined (reading 'pip')" - : "undefined is not an object (evaluating 'init2.settings.pip')"; - await duckplayer.didSendException('InitError', message); + await duckplayer.didSendInitErrorException(); }); }); -/** - * @param {import("@playwright/test").TestInfo} testInfo - */ -function isWindows(testInfo) { - const u = /** @type {any} */ (testInfo.project.use); - return u?.platform === 'windows'; -} - /** * @param {import("@playwright/test").TestInfo} testInfo */ diff --git a/special-pages/pages/duckplayer/src/index.js b/special-pages/pages/duckplayer/src/index.js index 6415486c2a..86aa0c34d2 100644 --- a/special-pages/pages/duckplayer/src/index.js +++ b/special-pages/pages/duckplayer/src/index.js @@ -4,7 +4,7 @@ import { createSpecialPageMessaging } from '../../../shared/create-special-page- import { init } from '../app/index.js'; import { initStorage } from './storage.js'; import '../../../shared/live-reload.js'; -import { reportException } from '../../../shared/report-metric.js'; +import { reportException, METRIC_NAME_INIT_ERROR } from '../../../shared/report-metric.js'; export class DuckplayerPage { /** @@ -116,14 +116,6 @@ export class DuckplayerPage { reportInitException(params) { this.messaging.notify('reportInitException', params); } - - /** - * This will be sent to report metrics to the native layer - * @param {import('../../../shared/types/shared.ts').ReportMetricEvent['params']} params - */ - reportException(params) { - reportException(this.messaging, params); - } } // TODO: Remove telemetry @@ -192,8 +184,7 @@ init(duckplayerPage, telemetry, baseEnvironment).catch((e) => { // messages. console.error(e); const message = typeof e?.message === 'string' ? e.message : 'unknown error'; - const kind = 'InitError'; - duckplayerPage.reportException({ message, kind }); + reportException(duckplayerPage.messaging, { message, kind: METRIC_NAME_INIT_ERROR }); // TODO: Remove this event once all native platforms are responding to 'reportMetric: exception' duckplayerPage.reportInitException({ message }); diff --git a/special-pages/shared/report-metric.js b/special-pages/shared/report-metric.js index cffa645912..2bdd6b8605 100644 --- a/special-pages/shared/report-metric.js +++ b/special-pages/shared/report-metric.js @@ -1,5 +1,12 @@ export const REPORT_METRIC_MESSAGE_ID = 'reportMetric'; export const METRIC_NAME_EXCEPTION = 'exception'; +export const METRIC_NAME_GENERIC_ERROR = 'Error'; +export const METRIC_NAME_INIT_ERROR = 'InitError'; +export const METRIC_NAME_INITIAL_SETUP_ERROR = 'InitialSetupError'; +export const METRIC_NAME_MESSAGING_ERROR = 'MessagingError'; +export const METRIC_NAME_TIMESTAMP_ERROR = 'TimestampError'; +export const METRIC_NAME_VIDEO_ID_ERROR = 'VideoIdError'; +export const METRIC_NAME_VIDEO_OVERLAY_ERROR = 'VideoOverlayError'; /** * @typedef {import('../shared/types/shared.ts').ExceptionMetric} ExceptionMetric From c52819fd086da230d15a1728d60addd4793e8b23 Mon Sep 17 00:00:00 2001 From: Marcos Gurgel Date: Thu, 26 Jun 2025 13:03:01 +0100 Subject: [PATCH 10/28] Cleanup --- injected/src/features/duck-player.js | 6 ++--- .../features/duckplayer/overlay-messages.js | 6 ++--- .../src/features/duckplayer/video-overlay.js | 12 ++++----- .../pages/duckplayer/app/embed-settings.js | 25 +++---------------- special-pages/pages/duckplayer/app/index.js | 20 ++++++++++++--- special-pages/pages/duckplayer/src/index.js | 5 ++-- special-pages/shared/report-metric.js | 3 +-- 7 files changed, 36 insertions(+), 41 deletions(-) diff --git a/injected/src/features/duck-player.js b/injected/src/features/duck-player.js index 1e8a88f660..866d052572 100644 --- a/injected/src/features/duck-player.js +++ b/injected/src/features/duck-player.js @@ -37,7 +37,7 @@ import { DuckPlayerOverlayMessages, OpenInDuckPlayerMsg, Pixel } from './duckpla import { isBeingFramed } from '../utils.js'; import { initOverlays } from './duckplayer/overlays.js'; import { Environment } from './duckplayer/environment.js'; -import { reportException } from '../../../special-pages/shared/report-metric.js'; +import { METRIC_NAME_GENERIC_ERROR, reportException } from '../../../special-pages/shared/report-metric.js'; /** * @typedef UserValues - A way to communicate user settings @@ -110,8 +110,8 @@ export default class DuckPlayerFeature extends ContentFeature { comms.serpProxy(); } } catch (e) { - const message = e.message || 'Could not initialize duck player: ' + e.toString(); - const kind = e.name; + const message = typeof e?.message === 'string' ? e.message : 'Could not initialize duck player: ' + e?.toString(); + const kind = typeof e?.name === 'string' ? e.name : METRIC_NAME_GENERIC_ERROR; reportException(this.messaging, { message, kind }); } } diff --git a/injected/src/features/duckplayer/overlay-messages.js b/injected/src/features/duckplayer/overlay-messages.js index 36ebaaec6e..b0a716ee62 100644 --- a/injected/src/features/duckplayer/overlay-messages.js +++ b/injected/src/features/duckplayer/overlay-messages.js @@ -126,7 +126,7 @@ export class DuckPlayerOverlayMessages { .then((updated) => respond(constants.MSG_NAME_PUSH_DATA, updated)) .catch((e) => { console.error(e); - reportException(this.messaging, { message: e.toString(), kind: METRIC_NAME_MESSAGING_ERROR }); + reportException(this.messaging, { message: e?.toString(), kind: METRIC_NAME_MESSAGING_ERROR }); }); } if (evt.detail.kind === constants.MSG_NAME_READ_VALUES_SERP) { @@ -134,7 +134,7 @@ export class DuckPlayerOverlayMessages { .then((updated) => respond(constants.MSG_NAME_PUSH_DATA, updated)) .catch((e) => { console.error(e); - reportException(this.messaging, { message: e.toString(), kind: METRIC_NAME_MESSAGING_ERROR }); + reportException(this.messaging, { message: e?.toString(), kind: METRIC_NAME_MESSAGING_ERROR }); }); } if (evt.detail.kind === constants.MSG_NAME_OPEN_INFO) { @@ -142,7 +142,7 @@ export class DuckPlayerOverlayMessages { } console.warn('unhandled event', evt); } catch (e) { - reportException(this.messaging, { message: e.toString(), kind: METRIC_NAME_MESSAGING_ERROR }); + reportException(this.messaging, { message: e?.toString(), kind: METRIC_NAME_MESSAGING_ERROR }); console.warn('cannot handle this message', e); } }); diff --git a/injected/src/features/duckplayer/video-overlay.js b/injected/src/features/duckplayer/video-overlay.js index b0caf5689c..7a9d2ffc8f 100644 --- a/injected/src/features/duckplayer/video-overlay.js +++ b/injected/src/features/duckplayer/video-overlay.js @@ -256,13 +256,13 @@ export class VideoOverlay { elem.addEventListener(DDGVideoOverlayMobile.OPT_OUT, (/** @type {CustomEvent<{remember: boolean}>} */ e) => { return this.mobileOptOut(e.detail.remember).catch((e) => { console.error(e); - reportException(this.messages.messaging, { message: e.toString(), kind: METRIC_NAME_MESSAGING_ERROR }); + reportException(this.messages.messaging, { message: e?.toString(), kind: METRIC_NAME_MESSAGING_ERROR }); }); }); elem.addEventListener(DDGVideoOverlayMobile.OPT_IN, (/** @type {CustomEvent<{remember: boolean}>} */ e) => { return this.mobileOptIn(e.detail.remember, params).catch((e) => { console.error(e); - reportException(this.messages.messaging, { message: e.toString(), kind: METRIC_NAME_MESSAGING_ERROR }); + reportException(this.messages.messaging, { message: e?.toString(), kind: METRIC_NAME_MESSAGING_ERROR }); }); }); targetElement.appendChild(elem); @@ -297,7 +297,7 @@ export class VideoOverlay { drawer.addEventListener(DDGVideoDrawerMobile.OPT_OUT, (/** @type {CustomEvent<{remember: boolean}>} */ e) => { return this.mobileOptOut(e.detail.remember).catch((e) => { console.error(e); - reportException(this.messages.messaging, { message: e.toString(), kind: METRIC_NAME_MESSAGING_ERROR }); + reportException(this.messages.messaging, { message: e?.toString(), kind: METRIC_NAME_MESSAGING_ERROR }); }); }); drawer.addEventListener(DDGVideoDrawerMobile.DISMISS, () => { @@ -309,7 +309,7 @@ export class VideoOverlay { drawer.addEventListener(DDGVideoDrawerMobile.OPT_IN, (/** @type {CustomEvent<{remember: boolean}>} */ e) => { return this.mobileOptIn(e.detail.remember, params).catch((e) => { console.error(e); - reportException(this.messages.messaging, { message: e.toString(), kind: METRIC_NAME_MESSAGING_ERROR }); + reportException(this.messages.messaging, { message: e?.toString(), kind: METRIC_NAME_MESSAGING_ERROR }); }); }); drawerTargetElement.appendChild(drawer); @@ -426,7 +426,7 @@ export class VideoOverlay { }) .catch((e) => { console.error('error setting user choice for opt-in', e); - reportException(this.messages.messaging, { message: e.toString(), kind: METRIC_NAME_MESSAGING_ERROR }); + reportException(this.messages.messaging, { message: e?.toString(), kind: METRIC_NAME_MESSAGING_ERROR }); }); } @@ -457,7 +457,7 @@ export class VideoOverlay { .then(() => this.watchForVideoBeingAdded({ ignoreCache: true, via: 'userOptOut' })) .catch((e) => { console.error('could not set userChoice for opt-out', e); - reportException(this.messages.messaging, { message: e.toString(), kind: METRIC_NAME_MESSAGING_ERROR }); + reportException(this.messages.messaging, { message: e?.toString(), kind: METRIC_NAME_MESSAGING_ERROR }); }); } else { this.messages.sendPixel(new Pixel({ name: 'play.do_not_use', remember: '0' })); diff --git a/special-pages/pages/duckplayer/app/embed-settings.js b/special-pages/pages/duckplayer/app/embed-settings.js index e01e04571a..59060aaf45 100644 --- a/special-pages/pages/duckplayer/app/embed-settings.js +++ b/special-pages/pages/duckplayer/app/embed-settings.js @@ -1,4 +1,3 @@ -import { METRIC_NAME_VIDEO_ID_ERROR, METRIC_NAME_TIMESTAMP_ERROR } from '../../../shared/report-metric.js'; export class EmbedSettings { /** * @param {object} params @@ -104,17 +103,9 @@ class VideoId { * @throws {Error} */ constructor(input) { - if (typeof input !== 'string') { - const error = new Error('string required, got: ' + input); - error.name = METRIC_NAME_VIDEO_ID_ERROR; - throw error; - } + if (typeof input !== 'string') throw new Error('string required, got: ' + input); const sanitized = sanitizeYoutubeId(input); - if (sanitized === null) { - const error = new Error('invalid ID from: ' + input); - error.name = METRIC_NAME_VIDEO_ID_ERROR; - throw error; - } + if (sanitized === null) throw new Error('invalid ID from: ' + input); this.id = sanitized; } @@ -135,17 +126,9 @@ class Timestamp { * @throws {Error} */ constructor(input) { - if (typeof input !== 'string') { - const error = new Error('string required for timestamp, got: ' + input); - error.name = METRIC_NAME_TIMESTAMP_ERROR; - throw error; - } + if (typeof input !== 'string') throw new Error('string required for timestamp'); const seconds = timestampInSeconds(input); - if (seconds === null) { - const error = new Error('invalid input for timestamp: ' + input); - error.name = METRIC_NAME_TIMESTAMP_ERROR; - throw error; - } + if (seconds === null) throw new Error('invalid input for timestamp: ' + input); this.seconds = seconds; } diff --git a/special-pages/pages/duckplayer/app/index.js b/special-pages/pages/duckplayer/app/index.js index 67875312eb..279f5249e6 100644 --- a/special-pages/pages/duckplayer/app/index.js +++ b/special-pages/pages/duckplayer/app/index.js @@ -15,7 +15,7 @@ import { Components } from './components/Components.jsx'; import { MobileApp } from './components/MobileApp.jsx'; import { DesktopApp } from './components/DesktopApp.jsx'; import { YouTubeErrorProvider } from './providers/YouTubeErrorProvider'; -import { reportException, METRIC_NAME_INITIAL_SETUP_ERROR } from '../../../shared/report-metric.js'; +import { reportException, METRIC_NAME_INITIAL_SETUP_ERROR, METRIC_NAME_INIT_ERROR } from '../../../shared/report-metric.js'; /** @typedef {import('../types/duckplayer').YouTubeError} YouTubeError */ @@ -35,7 +35,11 @@ export async function init(messaging, telemetry, baseEnvironment) { } const init = result.value; - console.log('INITIAL DATA'); + if (!init) { + const error = new Error('missing initialSetup data'); + error.name = METRIC_NAME_INITIAL_SETUP_ERROR; + throw error; + } // update the 'env' in case it was changed by native sides const environment = baseEnvironment @@ -71,12 +75,16 @@ export async function init(messaging, telemetry, baseEnvironment) { console.log(settings); const embed = createEmbedSettings(window.location.href, settings); + if (!embed) { + // TODO: Should we continue to render the page if embed is not found? + reportException(messaging.messaging, { message: 'embed not found', kind: METRIC_NAME_INIT_ERROR }); + console.log('embed not found'); + } const didCatch = (error) => { const message = error?.message; const kind = error?.error?.name; reportException(messaging.messaging, { message, kind }); - console.log('reportException', message, kind); // TODO: Remove the following event once all native platforms are responding to 'reportMetric: exception' messaging.reportPageException({ message: message || 'unknown error' }); @@ -85,7 +93,11 @@ export async function init(messaging, telemetry, baseEnvironment) { document.body.dataset.layout = settings.layout; const root = document.querySelector('body'); - if (!root) throw new Error('could not render, root element missing'); + if (!root) { + const error = new Error('could not render, root element missing'); + error.name = METRIC_NAME_INIT_ERROR; + throw error; + } if (environment.display === 'app') { render( diff --git a/special-pages/pages/duckplayer/src/index.js b/special-pages/pages/duckplayer/src/index.js index 86aa0c34d2..92b63b8efe 100644 --- a/special-pages/pages/duckplayer/src/index.js +++ b/special-pages/pages/duckplayer/src/index.js @@ -4,7 +4,7 @@ import { createSpecialPageMessaging } from '../../../shared/create-special-page- import { init } from '../app/index.js'; import { initStorage } from './storage.js'; import '../../../shared/live-reload.js'; -import { reportException, METRIC_NAME_INIT_ERROR } from '../../../shared/report-metric.js'; +import { reportException, METRIC_NAME_GENERIC_ERROR } from '../../../shared/report-metric.js'; export class DuckplayerPage { /** @@ -184,7 +184,8 @@ init(duckplayerPage, telemetry, baseEnvironment).catch((e) => { // messages. console.error(e); const message = typeof e?.message === 'string' ? e.message : 'unknown error'; - reportException(duckplayerPage.messaging, { message, kind: METRIC_NAME_INIT_ERROR }); + const kind = typeof e?.name === 'string' ? e.name : METRIC_NAME_GENERIC_ERROR; + reportException(duckplayerPage.messaging, { message, kind }); // TODO: Remove this event once all native platforms are responding to 'reportMetric: exception' duckplayerPage.reportInitException({ message }); diff --git a/special-pages/shared/report-metric.js b/special-pages/shared/report-metric.js index 2bdd6b8605..9578dc6dde 100644 --- a/special-pages/shared/report-metric.js +++ b/special-pages/shared/report-metric.js @@ -4,8 +4,6 @@ export const METRIC_NAME_GENERIC_ERROR = 'Error'; export const METRIC_NAME_INIT_ERROR = 'InitError'; export const METRIC_NAME_INITIAL_SETUP_ERROR = 'InitialSetupError'; export const METRIC_NAME_MESSAGING_ERROR = 'MessagingError'; -export const METRIC_NAME_TIMESTAMP_ERROR = 'TimestampError'; -export const METRIC_NAME_VIDEO_ID_ERROR = 'VideoIdError'; export const METRIC_NAME_VIDEO_OVERLAY_ERROR = 'VideoOverlayError'; /** @@ -38,6 +36,7 @@ export function reportMetric(messaging, metricEvent) { * @param {ExceptionMetric['params']} params */ export function reportException(messaging, params) { + console.log('reportException', params); const message = typeof params?.message === 'string' ? params.message : 'unknown error'; const kind = typeof params?.kind === 'string' ? params.kind : 'Error'; From 4a38d1c994cb562caf24aad7fefbaefe959388b5 Mon Sep 17 00:00:00 2001 From: Marcos Gurgel Date: Thu, 26 Jun 2025 14:24:42 +0100 Subject: [PATCH 11/28] Test refactor --- .../duckplayer-mobile.spec.js | 2 +- injected/integration-test/duckplayer.spec.js | 2 +- .../page-objects/duckplayer-overlays.js | 58 +++++++++++++++-- injected/src/features/duckplayer/overlays.js | 8 ++- .../integration-tests/duck-player.js | 62 ++++++++++++------- .../integration-tests/duckplayer.spec.js | 10 ++- special-pages/shared/report-metric.js | 1 + 7 files changed, 113 insertions(+), 30 deletions(-) diff --git a/injected/integration-test/duckplayer-mobile.spec.js b/injected/integration-test/duckplayer-mobile.spec.js index b971607347..ecece7ac65 100644 --- a/injected/integration-test/duckplayer-mobile.spec.js +++ b/injected/integration-test/duckplayer-mobile.spec.js @@ -154,6 +154,6 @@ test.describe('Reporting exceptions', () => { await overlays.withRemoteConfig({ locale: 'en' }); await overlays.initialSetupError(); await overlays.gotoPlayerPage(); - await overlays.didSendException('TypeError', "undefined is not an object (evaluating 'userValues.privatePlayerMode')"); + await overlays.didSendInitialSetupErrorException(); }); }); diff --git a/injected/integration-test/duckplayer.spec.js b/injected/integration-test/duckplayer.spec.js index 3b7d673e04..25fdd88cd1 100644 --- a/injected/integration-test/duckplayer.spec.js +++ b/injected/integration-test/duckplayer.spec.js @@ -376,6 +376,6 @@ test.describe('Reporting exceptions', () => { await overlays.withRemoteConfig({ locale: 'en' }); await overlays.initialSetupError(); await overlays.gotoPlayerPage(); - await overlays.didSendException('TypeError', "Cannot read properties of undefined (reading 'privatePlayerMode')"); + await overlays.didSendInitialSetupErrorException(); }); }); diff --git a/injected/integration-test/page-objects/duckplayer-overlays.js b/injected/integration-test/page-objects/duckplayer-overlays.js index 8af983f269..265e4cf30a 100644 --- a/injected/integration-test/page-objects/duckplayer-overlays.js +++ b/injected/integration-test/page-objects/duckplayer-overlays.js @@ -306,8 +306,31 @@ export class DuckplayerOverlays { } async initialSetupError() { - await this.collector.updateMockResponse({ - initialSetup: {}, + await this.build.switch({ + android: async () => { + await this.collector.updateMockResponse({ + initialSetup: { + locale: 'en', + env: 'development', + platform: { name: 'android ' }, + }, + }); + }, + apple: async () => { + await this.collector.updateMockResponse({ + initialSetup: null, + }); + }, + 'apple-isolated': async () => { + await this.collector.updateMockResponse({ + initialSetup: null, + }); + }, + windows: async () => { + await this.collector.updateMockResponse({ + initialSetup: '', + }); + }, }); } @@ -487,11 +510,38 @@ export class DuckplayerOverlays { * @param {string} kind * @param {string} message */ - async didSendException(kind, message) { + async didSendException(kind, message, context = 'contentScopeScripts') { console.log('messages', await this.collector.outgoingMessages()); const messages = await this.collector.waitForMessage('reportMetric'); - expect(messages).toMatchObject([{ payload: { params: { metricName: 'exception', params: { kind, message } } } }]); + expect(messages).toMatchObject([ + { + payload: { + context, + featureName: 'duckPlayer', + method: 'reportMetric', + params: { metricName: 'exception', params: { kind, message } }, + }, + }, + ]); + } + + async didSendInitialSetupErrorException() { + await this.build.switch({ + android: async () => { + // Android produces a TypeError due to how its messaging lib is wired up + await this.didSendException('TypeError', "undefined is not an object (evaluating 'init2.settings.pip')"); + }, + apple: async () => { + await this.didSendException('InitialSetupError', 'Error: an unknown error occurred'); + }, + 'apple-isolated': async () => { + await this.didSendException('InitialSetupError', 'Error: an unknown error occurred', 'contentScopeScriptsIsolated'); + }, + windows: async () => { + await this.didSendException('InitialSetupError', 'Error: unknown error'); + }, + }); } /** diff --git a/injected/src/features/duckplayer/overlays.js b/injected/src/features/duckplayer/overlays.js index 42efc5c50d..0f0fad882d 100644 --- a/injected/src/features/duckplayer/overlays.js +++ b/injected/src/features/duckplayer/overlays.js @@ -2,6 +2,7 @@ import { DomState } from './util.js'; import { ClickInterception, Thumbnails } from './thumbnails.js'; import { VideoOverlay } from './video-overlay.js'; import { registerCustomElements } from './components/index.js'; +import { reportException, METRIC_NAME_INITIAL_SETUP_ERROR } from '../../../../special-pages/shared/report-metric.js'; /** * @typedef {object} OverlayOptions @@ -28,14 +29,19 @@ export async function initOverlays(settings, environment, messages) { } catch (e) { console.log('INITIAL SETUP ERROR'); console.warn(e); + reportException(messages.messaging, { message: e?.toString(), kind: METRIC_NAME_INITIAL_SETUP_ERROR }); return; } if (!initialSetup) { - console.warn('cannot continue without user settings'); + const message = 'cannot continue without user settings'; + console.warn(message); + reportException(messages.messaging, { message, kind: METRIC_NAME_INITIAL_SETUP_ERROR }); return; } + console.log('initialSetup Yo', JSON.stringify(initialSetup, null, 2)); + let { userValues, ui } = initialSetup; /** diff --git a/special-pages/pages/duckplayer/integration-tests/duck-player.js b/special-pages/pages/duckplayer/integration-tests/duck-player.js index d87d7c0e20..48c1344231 100644 --- a/special-pages/pages/duckplayer/integration-tests/duck-player.js +++ b/special-pages/pages/duckplayer/integration-tests/duck-player.js @@ -145,15 +145,30 @@ export class DuckPlayerPage { this.mocks.defaultResponses(clone); } - initError() { + initialSetupError() { const clone = structuredClone(this.defaults); - // @ts-expect-error - this is a test - clone.initialSetup = { - locale: 'en', - env: 'development', - platform: this.platform.name === 'windows' ? undefined : { name: this.platform.name }, - }; - this.mocks.defaultResponses(clone); + + this.build.switch({ + android: () => { + // @ts-expect-error - this is a test + clone.initialSetup = { + locale: 'en', + env: 'development', + platform: this.platform.name === 'windows' ? undefined : { name: this.platform.name }, + }; + this.mocks.defaultResponses(clone); + }, + apple: () => { + // @ts-expect-error - this is a test + clone.initialSetup = null; + this.mocks.defaultResponses(clone); + }, + windows: () => { + // @ts-expect-error - this is a test + clone.initialSetup = ''; + this.mocks.defaultResponses(clone); + }, + }); } /** @@ -270,6 +285,11 @@ export class DuckPlayerPage { await this.openPage(params); } + async openWithNoEmbed() { + const params = new URLSearchParams({ videoID: '' }); + await this.openPage(params); + } + /** * @param {string} [videoID] * @returns {Promise} @@ -553,16 +573,15 @@ export class DuckPlayerPage { */ async didSendReportMetric(evt) { const events = await this.mocks.waitForCallCount({ method: 'reportMetric', count: 1 }); - expect(events).toStrictEqual([ - { - payload: { - context: 'specialPages', - featureName: 'duckPlayerPage', - method: 'reportMetric', - params: evt, - }, + console.log('events', events); + expect(events).toContainEqual({ + payload: { + context: 'specialPages', + featureName: 'duckPlayerPage', + method: 'reportMetric', + params: evt, }, - ]); + }); } /** @@ -573,16 +592,17 @@ export class DuckPlayerPage { return this.didSendReportMetric({ metricName: 'exception', params: { kind, message } }); } - async didSendInitErrorException() { + async didSendInitialSetupErrorException() { await this.build.switch({ android: async () => { - await this.didSendException('InitError', "undefined is not an object (evaluating 'init2.settings.pip')"); + // Android produces a TypeError due to how its messaging lib is wired up + await this.didSendException('TypeError', "undefined is not an object (evaluating 'init2.settings.pip')"); }, apple: async () => { - await this.didSendException('InitError', "undefined is not an object (evaluating 'init2.settings.pip')"); + await this.didSendException('InitialSetupError', 'Max attempts reached: Error: an unknown error occurred'); }, windows: async () => { - await this.didSendException('InitError', "Cannot read properties of undefined (reading 'pip')"); + await this.didSendException('InitialSetupError', 'Max attempts reached: Error: unknown error'); }, }); } diff --git a/special-pages/pages/duckplayer/integration-tests/duckplayer.spec.js b/special-pages/pages/duckplayer/integration-tests/duckplayer.spec.js index 970a41c13a..27329fe1fa 100644 --- a/special-pages/pages/duckplayer/integration-tests/duckplayer.spec.js +++ b/special-pages/pages/duckplayer/integration-tests/duckplayer.spec.js @@ -335,12 +335,18 @@ test.describe('reporting exceptions', () => { await duckplayer.showsErrorMessage(); await duckplayer.didSendException('Error', 'Simulated Exception'); }); + test('no embed error', async ({ page }, workerInfo) => { + const duckplayer = DuckPlayerPage.create(page, workerInfo); + // load as normal + await duckplayer.openWithNoEmbed(); + await duckplayer.didSendException('InitError', 'embed not found'); + }); test('initial setup error', async ({ page }, workerInfo) => { const duckplayer = DuckPlayerPage.create(page, workerInfo); // load as normal - duckplayer.initError(); + duckplayer.initialSetupError(); await duckplayer.openWithVideoID(); - await duckplayer.didSendInitErrorException(); + await duckplayer.didSendInitialSetupErrorException(); }); }); diff --git a/special-pages/shared/report-metric.js b/special-pages/shared/report-metric.js index 9578dc6dde..f4e8a9bf29 100644 --- a/special-pages/shared/report-metric.js +++ b/special-pages/shared/report-metric.js @@ -1,5 +1,6 @@ export const REPORT_METRIC_MESSAGE_ID = 'reportMetric'; export const METRIC_NAME_EXCEPTION = 'exception'; +/* Error Types */ export const METRIC_NAME_GENERIC_ERROR = 'Error'; export const METRIC_NAME_INIT_ERROR = 'InitError'; export const METRIC_NAME_INITIAL_SETUP_ERROR = 'InitialSetupError'; From ca598f294ae1b1dd458dcca5f9add0519d80c167 Mon Sep 17 00:00:00 2001 From: Marcos Gurgel Date: Thu, 26 Jun 2025 15:02:30 +0100 Subject: [PATCH 12/28] Patched Android mock messaging --- .../integration-test/page-objects/duckplayer-overlays.js | 8 ++------ messaging/lib/test-utils.mjs | 3 +++ 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/injected/integration-test/page-objects/duckplayer-overlays.js b/injected/integration-test/page-objects/duckplayer-overlays.js index 265e4cf30a..41ae5ec8cc 100644 --- a/injected/integration-test/page-objects/duckplayer-overlays.js +++ b/injected/integration-test/page-objects/duckplayer-overlays.js @@ -309,11 +309,7 @@ export class DuckplayerOverlays { await this.build.switch({ android: async () => { await this.collector.updateMockResponse({ - initialSetup: { - locale: 'en', - env: 'development', - platform: { name: 'android ' }, - }, + initialSetup: {}, }); }, apple: async () => { @@ -530,7 +526,7 @@ export class DuckplayerOverlays { await this.build.switch({ android: async () => { // Android produces a TypeError due to how its messaging lib is wired up - await this.didSendException('TypeError', "undefined is not an object (evaluating 'init2.settings.pip')"); + await this.didSendException('TypeError', "Cannot read properties of undefined (reading 'privatePlayerMode')"); }, apple: async () => { await this.didSendException('InitialSetupError', 'Error: an unknown error occurred'); diff --git a/messaging/lib/test-utils.mjs b/messaging/lib/test-utils.mjs index 3efa56543f..9a4d3aa088 100644 --- a/messaging/lib/test-utils.mjs +++ b/messaging/lib/test-utils.mjs @@ -305,6 +305,9 @@ export function mockAndroidMessaging(params) { ), ); + // force a 'tick' to allow tests to reset mocks before reading responses + await new Promise((resolve) => setTimeout(resolve, 0)); + // if it's a notification, simulate the empty response and don't check for a response if (!('id' in msg)) { return; From dfefab2b3738c635f2b5bacc54382fa12cf441e5 Mon Sep 17 00:00:00 2001 From: Marcos Gurgel Date: Thu, 26 Jun 2025 16:30:13 +0100 Subject: [PATCH 13/28] Docs --- messaging/lib/test-utils.mjs | 2 +- special-pages/shared/report-metric.js | 85 ++++++++++++++++++++++++--- typedoc.js | 1 + 3 files changed, 80 insertions(+), 8 deletions(-) diff --git a/messaging/lib/test-utils.mjs b/messaging/lib/test-utils.mjs index 9a4d3aa088..cf3753d4a7 100644 --- a/messaging/lib/test-utils.mjs +++ b/messaging/lib/test-utils.mjs @@ -292,7 +292,7 @@ export function mockAndroidMessaging(params) { * @param {string} secret * @return {Promise} */ - // eslint-disable-next-line require-await + process: async (jsonString, secret) => { /** @type {RequestMessage | NotificationMessage} */ const msg = JSON.parse(jsonString); diff --git a/special-pages/shared/report-metric.js b/special-pages/shared/report-metric.js index f4e8a9bf29..c41498f437 100644 --- a/special-pages/shared/report-metric.js +++ b/special-pages/shared/report-metric.js @@ -1,10 +1,52 @@ +/** + * Utility module for reporting metrics and exceptions to the native layer. + * + * This module provides standardized functions for sending metric events and exception reports + * through the messaging system. It includes predefined metric names for common error types + * and helper functions to construct and send metric events. + * + * Please see https://duckduckgo.github.io/content-scope-scripts/interfaces/shared_messages.reportmetricnotification + * for the message schema + * + * @example + * ```javascript + * import { reportMetric, reportException } from './report-metric.js'; + * + * // Report a custom metric + * reportMetric(messaging, { + * metricName: 'userAction', + * params: { action: 'buttonClick', page: 'home' } + * }); + * + * // Report an exception + * reportException(messaging, { + * message: 'Failed to load user data', + * kind: 'NetworkError' + * }); + * ``` + * + * @module Report Metric + */ + +/** The message ID used for reporting metrics to the native layer */ export const REPORT_METRIC_MESSAGE_ID = 'reportMetric'; + +/** The metric name used for exception reporting */ export const METRIC_NAME_EXCEPTION = 'exception'; -/* Error Types */ + +/** Metric name for generic errors */ export const METRIC_NAME_GENERIC_ERROR = 'Error'; + +/** Metric name for initialization errors */ export const METRIC_NAME_INIT_ERROR = 'InitError'; + +/** Metric name for initial setup errors */ export const METRIC_NAME_INITIAL_SETUP_ERROR = 'InitialSetupError'; + +/** Metric name for messaging-related errors */ export const METRIC_NAME_MESSAGING_ERROR = 'MessagingError'; + +/** Metric name for video overlay errors */ export const METRIC_NAME_VIDEO_OVERLAY_ERROR = 'VideoOverlayError'; /** @@ -15,9 +57,20 @@ export const METRIC_NAME_VIDEO_OVERLAY_ERROR = 'VideoOverlayError'; */ /** - * Sends a standard 'reportMetric' event to the native layer - * @param {SharedMessaging} messaging - * @param {ReportMetricEvent} metricEvent + * Sends a standard 'reportMetric' event to the native layer. + * + * @param {SharedMessaging} messaging - The messaging instance used to communicate with the native layer + * @param {ReportMetricEvent} metricEvent - The metric event to report, must contain a metricName + * @throws {Error} When messaging.notify is not defined + * @throws {Error} When metricEvent.metricName is missing + * + * @example + * ```javascript + * reportMetric(messaging, { + * metricName: 'pageLoad', + * params: { duration: 1500, page: 'home' } + * }); + * ``` */ export function reportMetric(messaging, metricEvent) { if (!messaging?.notify) { @@ -32,9 +85,27 @@ export function reportMetric(messaging, metricEvent) { } /** - * Sends a 'reportMetric' event with metric name 'exception' - * @param {SharedMessaging} messaging - * @param {ExceptionMetric['params']} params + * Sends a 'reportMetric' event with metric name 'exception'. + * + * This is a convenience function for reporting exceptions. It constructs + * an exception metric with the provided parameters and sends it using the standard + * reportMetric function. If message or kind parameters are not provided, default values + * will be used. + * + * @param {SharedMessaging} messaging - The messaging instance used to communicate with the native layer + * @param {{message?: string, kind?: string}} params - The exception parameters containing message and kind (both optional) + * + * @example + * ```javascript + * // Report an exception with custom message and kind + * reportException(messaging, { + * message: 'Failed to fetch user data from API', + * kind: 'NetworkError' + * }); + * + * // Report an exception with default values (message: 'unknown error', kind: 'Error') + * reportException(messaging, {}); + * ``` */ export function reportException(messaging, params) { console.log('reportException', params); diff --git a/typedoc.js b/typedoc.js index c403aefc7b..00e5d0d1aa 100644 --- a/typedoc.js +++ b/typedoc.js @@ -36,6 +36,7 @@ const config = { 'special-pages/pages/new-tab/app/favorites/constants.js', 'special-pages/pages/**/types/*.ts', 'special-pages/shared/types/*.ts', + 'special-pages/shared/report-metric.js', ], categoryOrder: ['Special Pages', 'Content Scope Scripts Integrations', 'Other'], out: 'docs', From e7b24f718c183c82839a01d421a0da9ec566b774 Mon Sep 17 00:00:00 2001 From: Marcos Gurgel Date: Thu, 26 Jun 2025 16:46:19 +0100 Subject: [PATCH 14/28] Cleanup --- injected/integration-test/page-objects/duckplayer-overlays.js | 2 -- injected/src/features/duckplayer/overlays.js | 3 --- special-pages/pages/duckplayer/app/index.js | 1 - 3 files changed, 6 deletions(-) diff --git a/injected/integration-test/page-objects/duckplayer-overlays.js b/injected/integration-test/page-objects/duckplayer-overlays.js index 41ae5ec8cc..1f222587e6 100644 --- a/injected/integration-test/page-objects/duckplayer-overlays.js +++ b/injected/integration-test/page-objects/duckplayer-overlays.js @@ -507,8 +507,6 @@ export class DuckplayerOverlays { * @param {string} message */ async didSendException(kind, message, context = 'contentScopeScripts') { - console.log('messages', await this.collector.outgoingMessages()); - const messages = await this.collector.waitForMessage('reportMetric'); expect(messages).toMatchObject([ { diff --git a/injected/src/features/duckplayer/overlays.js b/injected/src/features/duckplayer/overlays.js index 0f0fad882d..5ad17b5bd8 100644 --- a/injected/src/features/duckplayer/overlays.js +++ b/injected/src/features/duckplayer/overlays.js @@ -27,7 +27,6 @@ export async function initOverlays(settings, environment, messages) { try { initialSetup = await messages.initialSetup(); } catch (e) { - console.log('INITIAL SETUP ERROR'); console.warn(e); reportException(messages.messaging, { message: e?.toString(), kind: METRIC_NAME_INITIAL_SETUP_ERROR }); return; @@ -40,8 +39,6 @@ export async function initOverlays(settings, environment, messages) { return; } - console.log('initialSetup Yo', JSON.stringify(initialSetup, null, 2)); - let { userValues, ui } = initialSetup; /** diff --git a/special-pages/pages/duckplayer/app/index.js b/special-pages/pages/duckplayer/app/index.js index 279f5249e6..d9221038cd 100644 --- a/special-pages/pages/duckplayer/app/index.js +++ b/special-pages/pages/duckplayer/app/index.js @@ -28,7 +28,6 @@ import { reportException, METRIC_NAME_INITIAL_SETUP_ERROR, METRIC_NAME_INIT_ERRO export async function init(messaging, telemetry, baseEnvironment) { const result = await callWithRetry(() => messaging.initialSetup()); if ('error' in result) { - console.error('INITIAL SETUP ERROR', result.error); const error = new Error(result.error); error.name = METRIC_NAME_INITIAL_SETUP_ERROR; throw error; From 72846ec11468d6c9cdb7467e643a0982653b109a Mon Sep 17 00:00:00 2001 From: Marcos Gurgel Date: Fri, 27 Jun 2025 15:25:23 +0100 Subject: [PATCH 15/28] Refactor --- injected/src/features/duck-player.js | 7 +- .../components/ddg-video-overlay.js | 8 +- .../features/duckplayer/overlay-messages.js | 33 ++- injected/src/features/duckplayer/overlays.js | 7 +- .../src/features/duckplayer/video-overlay.js | 15 +- special-pages/pages/duckplayer/app/index.js | 13 +- .../app/providers/UserValuesProvider.jsx | 6 - special-pages/pages/duckplayer/src/index.js | 30 ++- special-pages/shared/report-metric.js | 223 +++++++++++------- special-pages/unit-test/report-metric.mjs | 112 +++++++-- 10 files changed, 303 insertions(+), 151 deletions(-) diff --git a/injected/src/features/duck-player.js b/injected/src/features/duck-player.js index 866d052572..2cd9499ad7 100644 --- a/injected/src/features/duck-player.js +++ b/injected/src/features/duck-player.js @@ -37,7 +37,7 @@ import { DuckPlayerOverlayMessages, OpenInDuckPlayerMsg, Pixel } from './duckpla import { isBeingFramed } from '../utils.js'; import { initOverlays } from './duckplayer/overlays.js'; import { Environment } from './duckplayer/environment.js'; -import { METRIC_NAME_GENERIC_ERROR, reportException } from '../../../special-pages/shared/report-metric.js'; +import { ReportMetric } from '../../../special-pages/shared/report-metric.js'; /** * @typedef UserValues - A way to communicate user settings @@ -110,9 +110,8 @@ export default class DuckPlayerFeature extends ContentFeature { comms.serpProxy(); } } catch (e) { - const message = typeof e?.message === 'string' ? e.message : 'Could not initialize duck player: ' + e?.toString(); - const kind = typeof e?.name === 'string' ? e.name : METRIC_NAME_GENERIC_ERROR; - reportException(this.messaging, { message, kind }); + const metrics = new ReportMetric(this.messaging); + metrics.reportExceptionWithError(e); } } } diff --git a/injected/src/features/duckplayer/components/ddg-video-overlay.js b/injected/src/features/duckplayer/components/ddg-video-overlay.js index 8b64489d51..f69208f4ab 100644 --- a/injected/src/features/duckplayer/components/ddg-video-overlay.js +++ b/injected/src/features/duckplayer/components/ddg-video-overlay.js @@ -4,7 +4,8 @@ import { overlayCopyVariants } from '../text.js'; import { appendImageAsBackground } from '../util.js'; import { VideoOverlay } from '../video-overlay.js'; import { createPolicy, html, trustedUnsafe } from '../../../dom-utils.js'; -import { METRIC_NAME_VIDEO_OVERLAY_ERROR } from '../../../../../special-pages/shared/report-metric.js'; + +const EXCEPTION_KIND_VIDEO_OVERLAY_ERROR = 'VideoOverlayError'; /** * The custom element that we use to present our UI elements @@ -25,7 +26,6 @@ export class DDGVideoOverlay extends HTMLElement { super(); if (!(manager instanceof VideoOverlay)) { const error = new Error('Invalid VideoOverlay manager'); - error.name = METRIC_NAME_VIDEO_OVERLAY_ERROR; throw error; } this.environment = environment; @@ -128,7 +128,7 @@ export class DDGVideoOverlay extends HTMLElement { const remember = containerElement.querySelector('input[name="ddg-remember"]'); if (!(remember instanceof HTMLInputElement)) { const error = new Error('Cannot find remember checkbox'); - error.name = METRIC_NAME_VIDEO_OVERLAY_ERROR; + error.name = EXCEPTION_KIND_VIDEO_OVERLAY_ERROR; throw error; } this.manager.userOptOut(remember.checked, params); @@ -140,7 +140,7 @@ export class DDGVideoOverlay extends HTMLElement { const remember = containerElement.querySelector('input[name="ddg-remember"]'); if (!(remember instanceof HTMLInputElement)) { const error = new Error('Cannot find remember checkbox'); - error.name = METRIC_NAME_VIDEO_OVERLAY_ERROR; + error.name = EXCEPTION_KIND_VIDEO_OVERLAY_ERROR; throw error; } this.manager.userOptIn(remember.checked, params); diff --git a/injected/src/features/duckplayer/overlay-messages.js b/injected/src/features/duckplayer/overlay-messages.js index b0a716ee62..9a011fc9f5 100644 --- a/injected/src/features/duckplayer/overlay-messages.js +++ b/injected/src/features/duckplayer/overlay-messages.js @@ -1,6 +1,6 @@ /* eslint-disable promise/prefer-await-to-then */ import * as constants from './constants.js'; -import { reportException, METRIC_NAME_MESSAGING_ERROR } from '../../../../special-pages/shared/report-metric.js'; +import { ReportMetric, EXCEPTION_KIND_MESSAGING_ERROR } from '../../../../special-pages/shared/report-metric.js'; /** * @typedef {import("@duckduckgo/messaging").Messaging} Messaging @@ -22,12 +22,13 @@ export class DuckPlayerOverlayMessages { */ this.messaging = messaging; this.environment = environment; + this.metrics = new ReportMetric(messaging); } /** * @returns {Promise} */ - initialSetup() { + async initialSetup() { if (this.environment.isIntegrationMode()) { return Promise.resolve({ userValues: { @@ -37,7 +38,12 @@ export class DuckPlayerOverlayMessages { ui: {}, }); } - return this.messaging.request(constants.MSG_NAME_INITIAL_SETUP); + try { + return await this.messaging.request(constants.MSG_NAME_INITIAL_SETUP); + } catch (e) { + this.metrics.reportException({ message: e?.message, kind: EXCEPTION_KIND_MESSAGING_ERROR }); + throw e; + } } /** @@ -45,15 +51,25 @@ export class DuckPlayerOverlayMessages { * @param {import("../duck-player.js").UserValues} userValues * @returns {Promise} */ - setUserValues(userValues) { - return this.messaging.request(constants.MSG_NAME_SET_VALUES, userValues); + async setUserValues(userValues) { + try { + return await this.messaging.request(constants.MSG_NAME_SET_VALUES, userValues); + } catch (e) { + this.metrics.reportException({ message: e?.message, kind: EXCEPTION_KIND_MESSAGING_ERROR }); + throw e; + } } /** * @returns {Promise} */ - getUserValues() { - return this.messaging.request(constants.MSG_NAME_READ_VALUES, {}); + async getUserValues() { + try { + return await this.messaging.request(constants.MSG_NAME_READ_VALUES, {}); + } catch (e) { + this.metrics.reportException({ message: e?.message, kind: EXCEPTION_KIND_MESSAGING_ERROR }); + throw e; + } } /** @@ -126,7 +142,6 @@ export class DuckPlayerOverlayMessages { .then((updated) => respond(constants.MSG_NAME_PUSH_DATA, updated)) .catch((e) => { console.error(e); - reportException(this.messaging, { message: e?.toString(), kind: METRIC_NAME_MESSAGING_ERROR }); }); } if (evt.detail.kind === constants.MSG_NAME_READ_VALUES_SERP) { @@ -134,7 +149,6 @@ export class DuckPlayerOverlayMessages { .then((updated) => respond(constants.MSG_NAME_PUSH_DATA, updated)) .catch((e) => { console.error(e); - reportException(this.messaging, { message: e?.toString(), kind: METRIC_NAME_MESSAGING_ERROR }); }); } if (evt.detail.kind === constants.MSG_NAME_OPEN_INFO) { @@ -142,7 +156,6 @@ export class DuckPlayerOverlayMessages { } console.warn('unhandled event', evt); } catch (e) { - reportException(this.messaging, { message: e?.toString(), kind: METRIC_NAME_MESSAGING_ERROR }); console.warn('cannot handle this message', e); } }); diff --git a/injected/src/features/duckplayer/overlays.js b/injected/src/features/duckplayer/overlays.js index 5ad17b5bd8..c01772e572 100644 --- a/injected/src/features/duckplayer/overlays.js +++ b/injected/src/features/duckplayer/overlays.js @@ -2,7 +2,7 @@ import { DomState } from './util.js'; import { ClickInterception, Thumbnails } from './thumbnails.js'; import { VideoOverlay } from './video-overlay.js'; import { registerCustomElements } from './components/index.js'; -import { reportException, METRIC_NAME_INITIAL_SETUP_ERROR } from '../../../../special-pages/shared/report-metric.js'; +import { EXCEPTION_KIND_INITIAL_SETUP_ERROR } from '../../../../special-pages/shared/report-metric.js'; /** * @typedef {object} OverlayOptions @@ -28,14 +28,13 @@ export async function initOverlays(settings, environment, messages) { initialSetup = await messages.initialSetup(); } catch (e) { console.warn(e); - reportException(messages.messaging, { message: e?.toString(), kind: METRIC_NAME_INITIAL_SETUP_ERROR }); return; } if (!initialSetup) { - const message = 'cannot continue without user settings'; + const message = 'InitialSetup data is missing'; console.warn(message); - reportException(messages.messaging, { message, kind: METRIC_NAME_INITIAL_SETUP_ERROR }); + messages.metrics.reportException({ message, kind: EXCEPTION_KIND_INITIAL_SETUP_ERROR }); return; } diff --git a/injected/src/features/duckplayer/video-overlay.js b/injected/src/features/duckplayer/video-overlay.js index 7a9d2ffc8f..1b21c19de9 100644 --- a/injected/src/features/duckplayer/video-overlay.js +++ b/injected/src/features/duckplayer/video-overlay.js @@ -33,7 +33,7 @@ import { mobileStrings } from './text.js'; import { DDGVideoOverlayMobile } from './components/ddg-video-overlay-mobile.js'; import { DDGVideoThumbnailOverlay } from './components/ddg-video-thumbnail-overlay-mobile.js'; import { DDGVideoDrawerMobile } from './components/ddg-video-drawer-mobile.js'; -import { reportException, METRIC_NAME_MESSAGING_ERROR } from '../../../../special-pages/shared/report-metric.js'; + /** * Handle the switch between small & large overlays * + conduct any communications @@ -256,13 +256,13 @@ export class VideoOverlay { elem.addEventListener(DDGVideoOverlayMobile.OPT_OUT, (/** @type {CustomEvent<{remember: boolean}>} */ e) => { return this.mobileOptOut(e.detail.remember).catch((e) => { console.error(e); - reportException(this.messages.messaging, { message: e?.toString(), kind: METRIC_NAME_MESSAGING_ERROR }); + this.messages.metrics.reportExceptionWithError(e); }); }); elem.addEventListener(DDGVideoOverlayMobile.OPT_IN, (/** @type {CustomEvent<{remember: boolean}>} */ e) => { return this.mobileOptIn(e.detail.remember, params).catch((e) => { console.error(e); - reportException(this.messages.messaging, { message: e?.toString(), kind: METRIC_NAME_MESSAGING_ERROR }); + this.messages.metrics.reportExceptionWithError(e); }); }); targetElement.appendChild(elem); @@ -297,7 +297,7 @@ export class VideoOverlay { drawer.addEventListener(DDGVideoDrawerMobile.OPT_OUT, (/** @type {CustomEvent<{remember: boolean}>} */ e) => { return this.mobileOptOut(e.detail.remember).catch((e) => { console.error(e); - reportException(this.messages.messaging, { message: e?.toString(), kind: METRIC_NAME_MESSAGING_ERROR }); + this.messages.metrics.reportExceptionWithError(e); }); }); drawer.addEventListener(DDGVideoDrawerMobile.DISMISS, () => { @@ -309,7 +309,7 @@ export class VideoOverlay { drawer.addEventListener(DDGVideoDrawerMobile.OPT_IN, (/** @type {CustomEvent<{remember: boolean}>} */ e) => { return this.mobileOptIn(e.detail.remember, params).catch((e) => { console.error(e); - reportException(this.messages.messaging, { message: e?.toString(), kind: METRIC_NAME_MESSAGING_ERROR }); + this.messages.metrics.reportExceptionWithError(e); }); }); drawerTargetElement.appendChild(drawer); @@ -425,8 +425,7 @@ export class VideoOverlay { return this.environment.setHref(params.toPrivatePlayerUrl()); }) .catch((e) => { - console.error('error setting user choice for opt-in', e); - reportException(this.messages.messaging, { message: e?.toString(), kind: METRIC_NAME_MESSAGING_ERROR }); + this.messages.metrics.reportExceptionWithError(e); }); } @@ -457,7 +456,7 @@ export class VideoOverlay { .then(() => this.watchForVideoBeingAdded({ ignoreCache: true, via: 'userOptOut' })) .catch((e) => { console.error('could not set userChoice for opt-out', e); - reportException(this.messages.messaging, { message: e?.toString(), kind: METRIC_NAME_MESSAGING_ERROR }); + this.messages.metrics.reportExceptionWithError(e); }); } else { this.messages.sendPixel(new Pixel({ name: 'play.do_not_use', remember: '0' })); diff --git a/special-pages/pages/duckplayer/app/index.js b/special-pages/pages/duckplayer/app/index.js index d9221038cd..8d84c23445 100644 --- a/special-pages/pages/duckplayer/app/index.js +++ b/special-pages/pages/duckplayer/app/index.js @@ -15,7 +15,7 @@ import { Components } from './components/Components.jsx'; import { MobileApp } from './components/MobileApp.jsx'; import { DesktopApp } from './components/DesktopApp.jsx'; import { YouTubeErrorProvider } from './providers/YouTubeErrorProvider'; -import { reportException, METRIC_NAME_INITIAL_SETUP_ERROR, METRIC_NAME_INIT_ERROR } from '../../../shared/report-metric.js'; +import { EXCEPTION_KIND_INIT_ERROR, EXCEPTION_KIND_INITIAL_SETUP_ERROR } from '../../../shared/report-metric'; /** @typedef {import('../types/duckplayer').YouTubeError} YouTubeError */ @@ -29,14 +29,14 @@ export async function init(messaging, telemetry, baseEnvironment) { const result = await callWithRetry(() => messaging.initialSetup()); if ('error' in result) { const error = new Error(result.error); - error.name = METRIC_NAME_INITIAL_SETUP_ERROR; + error.name = EXCEPTION_KIND_INITIAL_SETUP_ERROR; throw error; } const init = result.value; if (!init) { const error = new Error('missing initialSetup data'); - error.name = METRIC_NAME_INITIAL_SETUP_ERROR; + error.name = EXCEPTION_KIND_INITIAL_SETUP_ERROR; throw error; } @@ -75,15 +75,14 @@ export async function init(messaging, telemetry, baseEnvironment) { const embed = createEmbedSettings(window.location.href, settings); if (!embed) { - // TODO: Should we continue to render the page if embed is not found? - reportException(messaging.messaging, { message: 'embed not found', kind: METRIC_NAME_INIT_ERROR }); + messaging.metrics.reportException({ message: 'embed not found', kind: EXCEPTION_KIND_INIT_ERROR }); console.log('embed not found'); } const didCatch = (error) => { const message = error?.message; const kind = error?.error?.name; - reportException(messaging.messaging, { message, kind }); + messaging.metrics.reportException({ message, kind }); // TODO: Remove the following event once all native platforms are responding to 'reportMetric: exception' messaging.reportPageException({ message: message || 'unknown error' }); @@ -94,7 +93,7 @@ export async function init(messaging, telemetry, baseEnvironment) { const root = document.querySelector('body'); if (!root) { const error = new Error('could not render, root element missing'); - error.name = METRIC_NAME_INIT_ERROR; + error.name = EXCEPTION_KIND_INIT_ERROR; throw error; } diff --git a/special-pages/pages/duckplayer/app/providers/UserValuesProvider.jsx b/special-pages/pages/duckplayer/app/providers/UserValuesProvider.jsx index 0d4a606500..0411105436 100644 --- a/special-pages/pages/duckplayer/app/providers/UserValuesProvider.jsx +++ b/special-pages/pages/duckplayer/app/providers/UserValuesProvider.jsx @@ -2,7 +2,6 @@ import { useContext, useState } from 'preact/hooks'; import { h, createContext } from 'preact'; import { useMessaging } from '../types.js'; import { useEffect } from 'preact/hooks'; -import { reportException, METRIC_NAME_MESSAGING_ERROR } from '../../../../shared/report-metric.js'; /** * @typedef {import("../../types/duckplayer.js").UserValues} UserValues @@ -61,11 +60,6 @@ export function UserValuesProvider({ initial, children }) { }) .catch((err) => { console.error('could not set the enabled flag', err); - const message = 'could not set the enabled flag: ' + err.toString(); - reportException(messaging.messaging, { message, kind: METRIC_NAME_MESSAGING_ERROR }); - - // TODO: Remove the following event once all native platforms are responding to 'reportMetric: exception' - messaging.reportPageException({ message }); }); } diff --git a/special-pages/pages/duckplayer/src/index.js b/special-pages/pages/duckplayer/src/index.js index 92b63b8efe..7fefe0a40a 100644 --- a/special-pages/pages/duckplayer/src/index.js +++ b/special-pages/pages/duckplayer/src/index.js @@ -4,7 +4,12 @@ import { createSpecialPageMessaging } from '../../../shared/create-special-page- import { init } from '../app/index.js'; import { initStorage } from './storage.js'; import '../../../shared/live-reload.js'; -import { reportException, METRIC_NAME_GENERIC_ERROR } from '../../../shared/report-metric.js'; +import { + ReportMetric, + EXCEPTION_KIND_GENERIC_ERROR, + EXCEPTION_KIND_INITIAL_SETUP_ERROR, + EXCEPTION_KIND_MESSAGING_ERROR, +} from '../../../shared/report-metric.js'; export class DuckplayerPage { /** @@ -13,6 +18,7 @@ export class DuckplayerPage { constructor(messaging, injectName) { this.messaging = createTypedMessages(this, messaging); this.injectName = injectName; + this.metrics = new ReportMetric(messaging); } /** @@ -20,7 +26,7 @@ export class DuckplayerPage { * has occurred that cannot be recovered from * @returns {Promise} */ - initialSetup() { + async initialSetup() { if (this.injectName === 'integration') { return Promise.resolve({ platform: { name: 'ios' }, @@ -37,7 +43,12 @@ export class DuckplayerPage { locale: 'en', }); } - return this.messaging.request('initialSetup'); + try { + return await this.messaging.request('initialSetup'); + } catch (e) { + this.metrics.reportException({ message: e?.message, kind: EXCEPTION_KIND_INITIAL_SETUP_ERROR }); + throw e; + } } /** @@ -45,8 +56,13 @@ export class DuckplayerPage { * * @param {import("../types/duckplayer.ts").UserValues} userValues */ - setUserValues(userValues) { - return this.messaging.request('setUserValues', userValues); + async setUserValues(userValues) { + try { + return await this.messaging.request('setUserValues', userValues); + } catch (e) { + this.metrics.reportException({ message: e?.message, kind: EXCEPTION_KIND_MESSAGING_ERROR }); + throw e; + } } /** @@ -184,8 +200,8 @@ init(duckplayerPage, telemetry, baseEnvironment).catch((e) => { // messages. console.error(e); const message = typeof e?.message === 'string' ? e.message : 'unknown error'; - const kind = typeof e?.name === 'string' ? e.name : METRIC_NAME_GENERIC_ERROR; - reportException(duckplayerPage.messaging, { message, kind }); + const kind = typeof e?.name === 'string' ? e.name : EXCEPTION_KIND_GENERIC_ERROR; + duckplayerPage.metrics.reportException({ message, kind }); // TODO: Remove this event once all native platforms are responding to 'reportMetric: exception' duckplayerPage.reportInitException({ message }); diff --git a/special-pages/shared/report-metric.js b/special-pages/shared/report-metric.js index c41498f437..3d18251018 100644 --- a/special-pages/shared/report-metric.js +++ b/special-pages/shared/report-metric.js @@ -1,25 +1,27 @@ /** - * Utility module for reporting metrics and exceptions to the native layer. + * Utility class for reporting metrics and exceptions to the native layer. * - * This module provides standardized functions for sending metric events and exception reports + * This class provides standardized methods for sending metric events and exception reports * through the messaging system. It includes predefined metric names for common error types - * and helper functions to construct and send metric events. + * and helper methods to construct and send metric events. * * Please see https://duckduckgo.github.io/content-scope-scripts/interfaces/shared_messages.reportmetricnotification * for the message schema * * @example * ```javascript - * import { reportMetric, reportException } from './report-metric.js'; + * import { ReportMetric } from './report-metric.js'; + * + * const metrics = new ReportMetric(messaging); * * // Report a custom metric - * reportMetric(messaging, { + * metrics.reportMetric({ * metricName: 'userAction', * params: { action: 'buttonClick', page: 'home' } * }); * * // Report an exception - * reportException(messaging, { + * metrics.reportException({ * message: 'Failed to load user data', * kind: 'NetworkError' * }); @@ -28,27 +30,6 @@ * @module Report Metric */ -/** The message ID used for reporting metrics to the native layer */ -export const REPORT_METRIC_MESSAGE_ID = 'reportMetric'; - -/** The metric name used for exception reporting */ -export const METRIC_NAME_EXCEPTION = 'exception'; - -/** Metric name for generic errors */ -export const METRIC_NAME_GENERIC_ERROR = 'Error'; - -/** Metric name for initialization errors */ -export const METRIC_NAME_INIT_ERROR = 'InitError'; - -/** Metric name for initial setup errors */ -export const METRIC_NAME_INITIAL_SETUP_ERROR = 'InitialSetupError'; - -/** Metric name for messaging-related errors */ -export const METRIC_NAME_MESSAGING_ERROR = 'MessagingError'; - -/** Metric name for video overlay errors */ -export const METRIC_NAME_VIDEO_OVERLAY_ERROR = 'VideoOverlayError'; - /** * @typedef {import('../shared/types/shared.ts').ExceptionMetric} ExceptionMetric * @typedef {import('../shared/types/shared.ts').ReportMetricEvent} ReportMetricEvent @@ -56,69 +37,141 @@ export const METRIC_NAME_VIDEO_OVERLAY_ERROR = 'VideoOverlayError'; * @typedef {import('@duckduckgo/messaging/lib/shared-types.js').MessagingBase|import('@duckduckgo/messaging').Messaging} SharedMessaging */ +/** Exception kind for generic errors */ +export const EXCEPTION_KIND_GENERIC_ERROR = 'Error'; + +/** Exception kind for initialization errors */ +export const EXCEPTION_KIND_INIT_ERROR = 'InitError'; + +/** Exception kind for initial setup errors */ +export const EXCEPTION_KIND_INITIAL_SETUP_ERROR = 'InitialSetupError'; + +/** Exception kind for messaging-related errors */ +export const EXCEPTION_KIND_MESSAGING_ERROR = 'MessagingError'; + /** - * Sends a standard 'reportMetric' event to the native layer. - * - * @param {SharedMessaging} messaging - The messaging instance used to communicate with the native layer - * @param {ReportMetricEvent} metricEvent - The metric event to report, must contain a metricName - * @throws {Error} When messaging.notify is not defined - * @throws {Error} When metricEvent.metricName is missing - * - * @example - * ```javascript - * reportMetric(messaging, { - * metricName: 'pageLoad', - * params: { duration: 1500, page: 'home' } - * }); - * ``` + * Class for reporting metrics and exceptions to the native layer. */ -export function reportMetric(messaging, metricEvent) { - if (!messaging?.notify) { - throw new Error('messaging.notify is not defined'); +export class ReportMetric { + /** The message ID used for reporting metrics to the native layer */ + static MESSAGE_ID = /** @type {const} */ ('reportMetric'); + + /** The metric name used for exception reporting */ + static METRIC_NAME_EXCEPTION = /** @type {const} */ ('exception'); + + /** The default exception message */ + static DEFAULT_EXCEPTION_MESSAGE = /** @type {const} */ ('Unknown error'); + + /** + * Creates a new ReportMetric instance. + * + * @param {SharedMessaging} messaging - The messaging instance used to communicate with the native layer + * @throws {Error} When messaging is not provided or messaging.notify is not defined + */ + constructor(messaging) { + if (!messaging) { + throw new Error('messaging is required'); + } + this.messaging = messaging; } - if (!metricEvent?.metricName) { - throw new Error('metricName is required'); + /** + * Send a metric event to the native layer + * + * @param {ReportMetricEvent} metricEvent + * + * @throws {Error} When metricEvent.metricName is missing + * @private + */ + _sendMetric(metricEvent) { + if (!metricEvent?.metricName) { + throw new Error('metricName is required'); + } + this.messaging.notify(ReportMetric.MESSAGE_ID, metricEvent); } - messaging.notify(REPORT_METRIC_MESSAGE_ID, metricEvent); -} + /** + * @typedef {Object} ExceptionMetricParams + * @property {string} [message] - The exception message + * @property {string} [kind] - The exception kind + */ -/** - * Sends a 'reportMetric' event with metric name 'exception'. - * - * This is a convenience function for reporting exceptions. It constructs - * an exception metric with the provided parameters and sends it using the standard - * reportMetric function. If message or kind parameters are not provided, default values - * will be used. - * - * @param {SharedMessaging} messaging - The messaging instance used to communicate with the native layer - * @param {{message?: string, kind?: string}} params - The exception parameters containing message and kind (both optional) - * - * @example - * ```javascript - * // Report an exception with custom message and kind - * reportException(messaging, { - * message: 'Failed to fetch user data from API', - * kind: 'NetworkError' - * }); - * - * // Report an exception with default values (message: 'unknown error', kind: 'Error') - * reportException(messaging, {}); - * ``` - */ -export function reportException(messaging, params) { - console.log('reportException', params); - const message = typeof params?.message === 'string' ? params.message : 'unknown error'; - const kind = typeof params?.kind === 'string' ? params.kind : 'Error'; - - /** @type {ExceptionMetric} */ - const metric = { - metricName: METRIC_NAME_EXCEPTION, - params: { - message, - kind, - }, - }; - reportMetric(messaging, metric); + /** + * Creates an exception metric object. If message or kind parameters are not provided, default values + * will be used. + * + * @param {ExceptionMetricParams} [params] - The exception parameters containing message and kind (both optional) + * @returns {ExceptionMetric} + * @private + */ + _createExceptionMetric(params) { + const message = params?.message && typeof params.message === 'string' ? params.message : ReportMetric.DEFAULT_EXCEPTION_MESSAGE; + const kind = params?.kind && typeof params.kind === 'string' ? params.kind : EXCEPTION_KIND_GENERIC_ERROR; + + return { + metricName: ReportMetric.METRIC_NAME_EXCEPTION, + params: { + message, + kind, + }, + }; + } + + /** + * Sends a standard 'reportMetric' event to the native layer. + * + * @param {ReportMetricEvent} metricEvent - The metric event to report, must contain a metricName + * @throws {Error} When metricEvent.metricName is missing + * + * @example + * ```javascript + * metrics.reportMetric({ + * metricName: 'pageLoad', + * params: { duration: 1500, page: 'home' } + * }); + * ``` + */ + reportMetric(metricEvent) { + this._sendMetric(metricEvent); + } + + /** + * Sends a `reportMetric` event with metric name `exception`, getting `message` and `kind` from the `params` object. If no params object is passed, defaults are used. + * + * + * @param {ExceptionMetricParams} [params] - The exception parameters containing message and kind (both optional) + * + * @example + * ```javascript + * // Report an exception with custom message and kind + * metrics.reportException({ + * message: 'Failed to fetch user data from API', + * kind: 'NetworkError' + * }); + * + * // Report an exception with default values (message: 'Unknown error', kind: 'Error') + * metrics.reportException(); + * ``` + */ + reportException(params) { + const metric = this._createExceptionMetric(params); + this._sendMetric(metric); + } + + /** + * Sends a 'reportMetric' event with metric name 'exception', getting message and kind from the error object. If no error object is passed, a default error is reported. + * + * If an invalid error object is passed, nothing is reported. + * + * @param {Error} [error] - The error to report + */ + reportExceptionWithError(error) { + console.log('reportExceptionWithError', error instanceof Error); + if (error && !(error instanceof Error)) { + console.warn('reportExceptionWithError: error is not an Error object', error); + return; + } + const metric = this._createExceptionMetric({ message: error?.message, kind: error?.name }); + this._sendMetric(metric); + } } diff --git a/special-pages/unit-test/report-metric.mjs b/special-pages/unit-test/report-metric.mjs index 65c15dbb78..6a95efa540 100644 --- a/special-pages/unit-test/report-metric.mjs +++ b/special-pages/unit-test/report-metric.mjs @@ -1,32 +1,37 @@ -import { describe, it, mock } from 'node:test'; +import { describe, it, mock, beforeEach } from 'node:test'; import assert from 'node:assert'; -import { reportMetric } from '../shared/report-metric.js'; - -// @ts-expect-error - this is a mock -const messaging = /** @type {Messaging} */ ({ - notify: mock.fn((params) => { - console.log('Notify called with', params); - }), -}); +import { ReportMetric } from '../shared/report-metric.js'; describe('reportMetric', () => { + let messaging; + + beforeEach(() => { + // Create the mock inside beforeEach to ensure it's fresh for each test + messaging = /** @type {any} */ ({ + notify: mock.fn((params) => { + console.log('Notify called with', params); + }), + }); + }); + it('should throw an error if messaging is not provided', () => { - const eventParams = /** @type {any} */ ({ metricName: '', params: {} }); // @ts-expect-error - this is a forced error - assert.throws(() => reportMetric(null, eventParams)); - assert.strictEqual(messaging.notify.mock.callCount(), 0); + assert.throws(() => new ReportMetric(null)); }); it('should throw an error if metricName is not provided', () => { - const eventParams = /** @type {any} */ ({ metricName: '', params: {} }); - assert.throws(() => reportMetric(messaging, eventParams)); + const metrics = new ReportMetric(messaging); + const metricParams = /** @type {any} */ ({ metricName: '', params: {} }); + assert.throws(() => metrics.reportMetric(metricParams)); assert.strictEqual(messaging.notify.mock.callCount(), 0); }); it('should call messaging.notify with the correct parameters', () => { - const eventParams = /** @type {any} */ ({ metricName: 'exception', params: { message: 'This is a test' } }); + const metrics = new ReportMetric(messaging); + const metricParams = /** @type {any} */ ({ metricName: 'exception', params: { message: 'This is a test' } }); assert.strictEqual(messaging.notify.mock.callCount(), 0); - reportMetric(messaging, eventParams); + + metrics.reportMetric(metricParams); assert.strictEqual(messaging.notify.mock.callCount(), 1); const call = messaging.notify.mock.calls[0]; assert.deepEqual(call.arguments, [ @@ -37,4 +42,79 @@ describe('reportMetric', () => { }, ]); }); + + it('should call messaging.notify when reportException is called', () => { + const metrics = new ReportMetric(messaging); + const eventParams = /** @type {any} */ ({ message: 'This is a test', kind: 'TestError' }); + assert.strictEqual(messaging.notify.mock.callCount(), 0); + + metrics.reportException(eventParams); + assert.strictEqual(messaging.notify.mock.callCount(), 1); + const call = messaging.notify.mock.calls[0]; + assert.deepEqual(call.arguments, [ + 'reportMetric', + { + metricName: 'exception', + params: { message: 'This is a test', kind: 'TestError' }, + }, + ]); + }); + + it('should send default values when reportException is called with empty params', () => { + const metrics = new ReportMetric(messaging); + const eventParams = /** @type {any} */ ({}); + assert.strictEqual(messaging.notify.mock.callCount(), 0); + + metrics.reportException(eventParams); + assert.strictEqual(messaging.notify.mock.callCount(), 1); + const call = messaging.notify.mock.calls[0]; + assert.deepEqual(call.arguments, [ + 'reportMetric', + { + metricName: 'exception', + params: { message: 'Unknown error', kind: 'Error' }, + }, + ]); + }); + + it('should not report anything when reportExceptionWithError is called with a non-Error object', () => { + const metrics = new ReportMetric(messaging); + const eventParams = /** @type {any} */ ({ message: 'This is a test', kind: 'TestError' }); + assert.strictEqual(messaging.notify.mock.callCount(), 0); + + metrics.reportExceptionWithError(eventParams); + assert.strictEqual(messaging.notify.mock.callCount(), 0); + }); + + it('should send the error message and kind when reportExceptionWithError is called with an Error object', () => { + const metrics = new ReportMetric(messaging); + const error = new Error('This is a test'); + error.name = 'TestError'; + assert.strictEqual(messaging.notify.mock.callCount(), 0); + + metrics.reportExceptionWithError(error); + assert.strictEqual(messaging.notify.mock.callCount(), 1); + const call = messaging.notify.mock.calls[0]; + assert.deepEqual(call.arguments, [ + 'reportMetric', + { + metricName: 'exception', + params: { message: 'This is a test', kind: 'TestError' }, + }, + ]); + }); + + it('should send default values when reportExceptionWithError is called with an empty error object', () => { + const metrics = new ReportMetric(messaging); + const error = new Error(); + assert.strictEqual(messaging.notify.mock.callCount(), 0); + + metrics.reportExceptionWithError(error); + assert.strictEqual(messaging.notify.mock.callCount(), 1); + const call = messaging.notify.mock.calls[0]; + assert.deepEqual(call.arguments, [ + 'reportMetric', + { metricName: 'exception', params: { message: 'Unknown error', kind: 'Error' } }, + ]); + }); }); From 03ee529cd31b958e477b18c8fe50a06bb536dd48 Mon Sep 17 00:00:00 2001 From: Marcos Gurgel Date: Fri, 27 Jun 2025 15:52:40 +0100 Subject: [PATCH 16/28] Test fix --- .../integration-test/duckplayer-mobile.spec.js | 4 ++-- injected/integration-test/duckplayer.spec.js | 4 ++-- .../page-objects/duckplayer-overlays.js | 13 ++++++++----- .../duckplayer/integration-tests/duck-player.js | 17 ++++++++--------- .../integration-tests/duckplayer.spec.js | 4 ++-- special-pages/pages/duckplayer/src/index.js | 10 +++------- 6 files changed, 25 insertions(+), 27 deletions(-) diff --git a/injected/integration-test/duckplayer-mobile.spec.js b/injected/integration-test/duckplayer-mobile.spec.js index ecece7ac65..0de39ac1a8 100644 --- a/injected/integration-test/duckplayer-mobile.spec.js +++ b/injected/integration-test/duckplayer-mobile.spec.js @@ -152,8 +152,8 @@ test.describe('Reporting exceptions', () => { test('initial setup error', async ({ page }, workerInfo) => { const overlays = DuckplayerOverlays.create(page, workerInfo); await overlays.withRemoteConfig({ locale: 'en' }); - await overlays.initialSetupError(); + await overlays.messagingError(); await overlays.gotoPlayerPage(); - await overlays.didSendInitialSetupErrorException(); + await overlays.didSendMessagingException(); }); }); diff --git a/injected/integration-test/duckplayer.spec.js b/injected/integration-test/duckplayer.spec.js index 25fdd88cd1..0f4de706fa 100644 --- a/injected/integration-test/duckplayer.spec.js +++ b/injected/integration-test/duckplayer.spec.js @@ -374,8 +374,8 @@ test.describe('Reporting exceptions', () => { test('initial setup error', async ({ page }, workerInfo) => { const overlays = DuckplayerOverlays.create(page, workerInfo); await overlays.withRemoteConfig({ locale: 'en' }); - await overlays.initialSetupError(); + await overlays.messagingError(); await overlays.gotoPlayerPage(); - await overlays.didSendInitialSetupErrorException(); + await overlays.didSendMessagingException(); }); }); diff --git a/injected/integration-test/page-objects/duckplayer-overlays.js b/injected/integration-test/page-objects/duckplayer-overlays.js index 1f222587e6..73b5122c1f 100644 --- a/injected/integration-test/page-objects/duckplayer-overlays.js +++ b/injected/integration-test/page-objects/duckplayer-overlays.js @@ -305,7 +305,10 @@ export class DuckplayerOverlays { }); } - async initialSetupError() { + /** + * Simulates a messaging error by passing an empty initialSetup object + */ + async messagingError() { await this.build.switch({ android: async () => { await this.collector.updateMockResponse({ @@ -520,20 +523,20 @@ export class DuckplayerOverlays { ]); } - async didSendInitialSetupErrorException() { + async didSendMessagingException() { await this.build.switch({ android: async () => { // Android produces a TypeError due to how its messaging lib is wired up await this.didSendException('TypeError', "Cannot read properties of undefined (reading 'privatePlayerMode')"); }, apple: async () => { - await this.didSendException('InitialSetupError', 'Error: an unknown error occurred'); + await this.didSendException('MessagingError', 'an unknown error occurred'); }, 'apple-isolated': async () => { - await this.didSendException('InitialSetupError', 'Error: an unknown error occurred', 'contentScopeScriptsIsolated'); + await this.didSendException('MessagingError', 'an unknown error occurred', 'contentScopeScriptsIsolated'); }, windows: async () => { - await this.didSendException('InitialSetupError', 'Error: unknown error'); + await this.didSendException('MessagingError', 'unknown error'); }, }); } diff --git a/special-pages/pages/duckplayer/integration-tests/duck-player.js b/special-pages/pages/duckplayer/integration-tests/duck-player.js index 48c1344231..3ef7284e00 100644 --- a/special-pages/pages/duckplayer/integration-tests/duck-player.js +++ b/special-pages/pages/duckplayer/integration-tests/duck-player.js @@ -145,17 +145,16 @@ export class DuckPlayerPage { this.mocks.defaultResponses(clone); } - initialSetupError() { + /** + * Simulates a messaging error by passing an empty initialSetup object + */ + messagingError() { const clone = structuredClone(this.defaults); this.build.switch({ android: () => { // @ts-expect-error - this is a test - clone.initialSetup = { - locale: 'en', - env: 'development', - platform: this.platform.name === 'windows' ? undefined : { name: this.platform.name }, - }; + clone.initialSetup = {}; this.mocks.defaultResponses(clone); }, apple: () => { @@ -592,17 +591,17 @@ export class DuckPlayerPage { return this.didSendReportMetric({ metricName: 'exception', params: { kind, message } }); } - async didSendInitialSetupErrorException() { + async didSendMessagingException() { await this.build.switch({ android: async () => { // Android produces a TypeError due to how its messaging lib is wired up await this.didSendException('TypeError', "undefined is not an object (evaluating 'init2.settings.pip')"); }, apple: async () => { - await this.didSendException('InitialSetupError', 'Max attempts reached: Error: an unknown error occurred'); + await this.didSendException('MessagingError', 'an unknown error occurred'); }, windows: async () => { - await this.didSendException('InitialSetupError', 'Max attempts reached: Error: unknown error'); + await this.didSendException('MessagingError', 'unknown error'); }, }); } diff --git a/special-pages/pages/duckplayer/integration-tests/duckplayer.spec.js b/special-pages/pages/duckplayer/integration-tests/duckplayer.spec.js index 27329fe1fa..e3f29e2acc 100644 --- a/special-pages/pages/duckplayer/integration-tests/duckplayer.spec.js +++ b/special-pages/pages/duckplayer/integration-tests/duckplayer.spec.js @@ -344,9 +344,9 @@ test.describe('reporting exceptions', () => { test('initial setup error', async ({ page }, workerInfo) => { const duckplayer = DuckPlayerPage.create(page, workerInfo); // load as normal - duckplayer.initialSetupError(); + duckplayer.messagingError(); await duckplayer.openWithVideoID(); - await duckplayer.didSendInitialSetupErrorException(); + await duckplayer.didSendMessagingException(); }); }); diff --git a/special-pages/pages/duckplayer/src/index.js b/special-pages/pages/duckplayer/src/index.js index 7fefe0a40a..3b94686237 100644 --- a/special-pages/pages/duckplayer/src/index.js +++ b/special-pages/pages/duckplayer/src/index.js @@ -6,8 +6,6 @@ import { initStorage } from './storage.js'; import '../../../shared/live-reload.js'; import { ReportMetric, - EXCEPTION_KIND_GENERIC_ERROR, - EXCEPTION_KIND_INITIAL_SETUP_ERROR, EXCEPTION_KIND_MESSAGING_ERROR, } from '../../../shared/report-metric.js'; @@ -46,7 +44,7 @@ export class DuckplayerPage { try { return await this.messaging.request('initialSetup'); } catch (e) { - this.metrics.reportException({ message: e?.message, kind: EXCEPTION_KIND_INITIAL_SETUP_ERROR }); + this.metrics.reportException({ message: e?.message, kind: EXCEPTION_KIND_MESSAGING_ERROR }); throw e; } } @@ -197,13 +195,11 @@ const duckplayerPage = new DuckplayerPage(messaging, import.meta.injectName); const telemetry = new Telemetry(messaging); init(duckplayerPage, telemetry, baseEnvironment).catch((e) => { - // messages. console.error(e); - const message = typeof e?.message === 'string' ? e.message : 'unknown error'; - const kind = typeof e?.name === 'string' ? e.name : EXCEPTION_KIND_GENERIC_ERROR; - duckplayerPage.metrics.reportException({ message, kind }); + duckplayerPage.metrics.reportExceptionWithError(e); // TODO: Remove this event once all native platforms are responding to 'reportMetric: exception' + const message = typeof e?.message === 'string' ? e.message : 'unknown error'; duckplayerPage.reportInitException({ message }); }); From 316597a5a01e5da9d5705c86863e438f46024e27 Mon Sep 17 00:00:00 2001 From: Marcos Gurgel Date: Fri, 27 Jun 2025 16:06:35 +0100 Subject: [PATCH 17/28] Lint --- special-pages/pages/duckplayer/src/index.js | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/special-pages/pages/duckplayer/src/index.js b/special-pages/pages/duckplayer/src/index.js index 3b94686237..45b988aed8 100644 --- a/special-pages/pages/duckplayer/src/index.js +++ b/special-pages/pages/duckplayer/src/index.js @@ -4,10 +4,7 @@ import { createSpecialPageMessaging } from '../../../shared/create-special-page- import { init } from '../app/index.js'; import { initStorage } from './storage.js'; import '../../../shared/live-reload.js'; -import { - ReportMetric, - EXCEPTION_KIND_MESSAGING_ERROR, -} from '../../../shared/report-metric.js'; +import { ReportMetric, EXCEPTION_KIND_MESSAGING_ERROR } from '../../../shared/report-metric.js'; export class DuckplayerPage { /** From f210e25bd29c48e149ccc51a048947be461b63a2 Mon Sep 17 00:00:00 2001 From: Marcos Gurgel Date: Fri, 27 Jun 2025 17:39:14 +0100 Subject: [PATCH 18/28] Cleanup --- .../duckplayer/components/ddg-video-overlay.js | 5 +---- .../src/features/duckplayer/overlay-messages.js | 8 ++------ .../src/features/duckplayer/video-overlay.js | 3 ++- special-pages/pages/duckplayer/app/index.js | 8 ++++---- .../app/providers/UserValuesProvider.jsx | 3 +++ .../integration-tests/duckplayer.spec.js | 2 +- special-pages/shared/report-metric.js | 17 ++++++++++++++--- 7 files changed, 27 insertions(+), 19 deletions(-) diff --git a/injected/src/features/duckplayer/components/ddg-video-overlay.js b/injected/src/features/duckplayer/components/ddg-video-overlay.js index f69208f4ab..f2fe32a4ed 100644 --- a/injected/src/features/duckplayer/components/ddg-video-overlay.js +++ b/injected/src/features/duckplayer/components/ddg-video-overlay.js @@ -24,10 +24,7 @@ export class DDGVideoOverlay extends HTMLElement { */ constructor({ environment, params, ui, manager }) { super(); - if (!(manager instanceof VideoOverlay)) { - const error = new Error('Invalid VideoOverlay manager'); - throw error; - } + if (!(manager instanceof VideoOverlay)) throw new Error('Invalid VideoOverlay manager'); this.environment = environment; this.ui = ui; this.params = params; diff --git a/injected/src/features/duckplayer/overlay-messages.js b/injected/src/features/duckplayer/overlay-messages.js index 9a011fc9f5..98fbec39d6 100644 --- a/injected/src/features/duckplayer/overlay-messages.js +++ b/injected/src/features/duckplayer/overlay-messages.js @@ -140,16 +140,12 @@ export class DuckPlayerOverlayMessages { if (evt.detail.kind === constants.MSG_NAME_SET_VALUES) { return this.setUserValues(evt.detail.data) .then((updated) => respond(constants.MSG_NAME_PUSH_DATA, updated)) - .catch((e) => { - console.error(e); - }); + .catch(console.error); } if (evt.detail.kind === constants.MSG_NAME_READ_VALUES_SERP) { return this.getUserValues() .then((updated) => respond(constants.MSG_NAME_PUSH_DATA, updated)) - .catch((e) => { - console.error(e); - }); + .catch(console.error); } if (evt.detail.kind === constants.MSG_NAME_OPEN_INFO) { return this.openInfo(); diff --git a/injected/src/features/duckplayer/video-overlay.js b/injected/src/features/duckplayer/video-overlay.js index 1b21c19de9..dd2f963dd1 100644 --- a/injected/src/features/duckplayer/video-overlay.js +++ b/injected/src/features/duckplayer/video-overlay.js @@ -425,6 +425,7 @@ export class VideoOverlay { return this.environment.setHref(params.toPrivatePlayerUrl()); }) .catch((e) => { + console.error(e); this.messages.metrics.reportExceptionWithError(e); }); } @@ -455,7 +456,7 @@ export class VideoOverlay { }) .then(() => this.watchForVideoBeingAdded({ ignoreCache: true, via: 'userOptOut' })) .catch((e) => { - console.error('could not set userChoice for opt-out', e); + console.error(e); this.messages.metrics.reportExceptionWithError(e); }); } else { diff --git a/special-pages/pages/duckplayer/app/index.js b/special-pages/pages/duckplayer/app/index.js index 8d84c23445..ae57060ac2 100644 --- a/special-pages/pages/duckplayer/app/index.js +++ b/special-pages/pages/duckplayer/app/index.js @@ -75,14 +75,14 @@ export async function init(messaging, telemetry, baseEnvironment) { const embed = createEmbedSettings(window.location.href, settings); if (!embed) { - messaging.metrics.reportException({ message: 'embed not found', kind: EXCEPTION_KIND_INIT_ERROR }); - console.log('embed not found'); + const message = 'Embed not found'; + messaging.metrics.reportException({ message, kind: EXCEPTION_KIND_INIT_ERROR }); + console.log(message); } const didCatch = (error) => { const message = error?.message; - const kind = error?.error?.name; - messaging.metrics.reportException({ message, kind }); + messaging.metrics.reportExceptionWithError(error?.error); // TODO: Remove the following event once all native platforms are responding to 'reportMetric: exception' messaging.reportPageException({ message: message || 'unknown error' }); diff --git a/special-pages/pages/duckplayer/app/providers/UserValuesProvider.jsx b/special-pages/pages/duckplayer/app/providers/UserValuesProvider.jsx index 0411105436..73cf9fbbf0 100644 --- a/special-pages/pages/duckplayer/app/providers/UserValuesProvider.jsx +++ b/special-pages/pages/duckplayer/app/providers/UserValuesProvider.jsx @@ -60,6 +60,9 @@ export function UserValuesProvider({ initial, children }) { }) .catch((err) => { console.error('could not set the enabled flag', err); + + // TODO: Remove the following event once all native platforms are responding to 'reportMetric: exception' + messaging.reportPageException({ message: 'could not set the enabled flag: ' + err.toString() }); }); } diff --git a/special-pages/pages/duckplayer/integration-tests/duckplayer.spec.js b/special-pages/pages/duckplayer/integration-tests/duckplayer.spec.js index e3f29e2acc..da58715f8e 100644 --- a/special-pages/pages/duckplayer/integration-tests/duckplayer.spec.js +++ b/special-pages/pages/duckplayer/integration-tests/duckplayer.spec.js @@ -339,7 +339,7 @@ test.describe('reporting exceptions', () => { const duckplayer = DuckPlayerPage.create(page, workerInfo); // load as normal await duckplayer.openWithNoEmbed(); - await duckplayer.didSendException('InitError', 'embed not found'); + await duckplayer.didSendException('InitError', 'Embed not found'); }); test('initial setup error', async ({ page }, workerInfo) => { const duckplayer = DuckPlayerPage.create(page, workerInfo); diff --git a/special-pages/shared/report-metric.js b/special-pages/shared/report-metric.js index 3d18251018..c2bbb0fdd0 100644 --- a/special-pages/shared/report-metric.js +++ b/special-pages/shared/report-metric.js @@ -25,6 +25,9 @@ * message: 'Failed to load user data', * kind: 'NetworkError' * }); + * + * // Report an exception by passing an Error object + * metrics.reportExceptionWithError(new Error('Missing params')); * ``` * * @module Report Metric @@ -118,7 +121,7 @@ export class ReportMetric { } /** - * Sends a standard 'reportMetric' event to the native layer. + * Sends a standard `reportMetric` event to the native layer. * * @param {ReportMetricEvent} metricEvent - The metric event to report, must contain a metricName * @throws {Error} When metricEvent.metricName is missing @@ -159,14 +162,22 @@ export class ReportMetric { } /** - * Sends a 'reportMetric' event with metric name 'exception', getting message and kind from the error object. If no error object is passed, a default error is reported. + * Sends a `reportMetric` event with metric name `exception`, getting message and kind from the error object. The `kind` property is inferred from `error.name`. + * + * If no error object is passed, a default error is reported. * * If an invalid error object is passed, nothing is reported. * * @param {Error} [error] - The error to report + * + * @example + * ```javascript + * const error = new Error('Failed to fetch user data from API'); + * error.name = 'NetworkError'; + * metrics.reportExceptionWithError(error); + * ``` */ reportExceptionWithError(error) { - console.log('reportExceptionWithError', error instanceof Error); if (error && !(error instanceof Error)) { console.warn('reportExceptionWithError: error is not an Error object', error); return; From 64c5295e1d7cd9c4bbd4a8808a1307ca6a198ccb Mon Sep 17 00:00:00 2001 From: Marcos Gurgel Date: Sun, 29 Jun 2025 13:08:35 +0100 Subject: [PATCH 19/28] Renamed ReportMetric to MetricReporter --- injected/src/features/duck-player.js | 4 ++-- .../features/duckplayer/overlay-messages.js | 4 ++-- injected/src/features/duckplayer/overlays.js | 2 +- special-pages/pages/duckplayer/app/index.js | 2 +- .../integration-tests/duck-player.js | 1 - special-pages/pages/duckplayer/src/index.js | 4 ++-- .../{report-metric.js => metrics-reporter.js} | 16 ++++++++-------- ...{report-metric.mjs => metrics-reporter.mjs} | 18 +++++++++--------- typedoc.js | 2 +- 9 files changed, 26 insertions(+), 27 deletions(-) rename special-pages/shared/{report-metric.js => metrics-reporter.js} (93%) rename special-pages/unit-test/{report-metric.mjs => metrics-reporter.mjs} (89%) diff --git a/injected/src/features/duck-player.js b/injected/src/features/duck-player.js index 2cd9499ad7..2750307b01 100644 --- a/injected/src/features/duck-player.js +++ b/injected/src/features/duck-player.js @@ -37,7 +37,7 @@ import { DuckPlayerOverlayMessages, OpenInDuckPlayerMsg, Pixel } from './duckpla import { isBeingFramed } from '../utils.js'; import { initOverlays } from './duckplayer/overlays.js'; import { Environment } from './duckplayer/environment.js'; -import { ReportMetric } from '../../../special-pages/shared/report-metric.js'; +import { MetricsReporter } from '../../../special-pages/shared/metrics-reporter.js'; /** * @typedef UserValues - A way to communicate user settings @@ -110,7 +110,7 @@ export default class DuckPlayerFeature extends ContentFeature { comms.serpProxy(); } } catch (e) { - const metrics = new ReportMetric(this.messaging); + const metrics = new MetricsReporter(this.messaging); metrics.reportExceptionWithError(e); } } diff --git a/injected/src/features/duckplayer/overlay-messages.js b/injected/src/features/duckplayer/overlay-messages.js index 98fbec39d6..6d970f39db 100644 --- a/injected/src/features/duckplayer/overlay-messages.js +++ b/injected/src/features/duckplayer/overlay-messages.js @@ -1,6 +1,6 @@ /* eslint-disable promise/prefer-await-to-then */ import * as constants from './constants.js'; -import { ReportMetric, EXCEPTION_KIND_MESSAGING_ERROR } from '../../../../special-pages/shared/report-metric.js'; +import { MetricsReporter, EXCEPTION_KIND_MESSAGING_ERROR } from '../../../../special-pages/shared/metrics-reporter.js'; /** * @typedef {import("@duckduckgo/messaging").Messaging} Messaging @@ -22,7 +22,7 @@ export class DuckPlayerOverlayMessages { */ this.messaging = messaging; this.environment = environment; - this.metrics = new ReportMetric(messaging); + this.metrics = new MetricsReporter(messaging); } /** diff --git a/injected/src/features/duckplayer/overlays.js b/injected/src/features/duckplayer/overlays.js index c01772e572..6e8f039179 100644 --- a/injected/src/features/duckplayer/overlays.js +++ b/injected/src/features/duckplayer/overlays.js @@ -2,7 +2,7 @@ import { DomState } from './util.js'; import { ClickInterception, Thumbnails } from './thumbnails.js'; import { VideoOverlay } from './video-overlay.js'; import { registerCustomElements } from './components/index.js'; -import { EXCEPTION_KIND_INITIAL_SETUP_ERROR } from '../../../../special-pages/shared/report-metric.js'; +import { EXCEPTION_KIND_INITIAL_SETUP_ERROR } from '../../../../special-pages/shared/metrics-reporter.js'; /** * @typedef {object} OverlayOptions diff --git a/special-pages/pages/duckplayer/app/index.js b/special-pages/pages/duckplayer/app/index.js index ae57060ac2..321a17ecea 100644 --- a/special-pages/pages/duckplayer/app/index.js +++ b/special-pages/pages/duckplayer/app/index.js @@ -15,7 +15,7 @@ import { Components } from './components/Components.jsx'; import { MobileApp } from './components/MobileApp.jsx'; import { DesktopApp } from './components/DesktopApp.jsx'; import { YouTubeErrorProvider } from './providers/YouTubeErrorProvider'; -import { EXCEPTION_KIND_INIT_ERROR, EXCEPTION_KIND_INITIAL_SETUP_ERROR } from '../../../shared/report-metric'; +import { EXCEPTION_KIND_INIT_ERROR, EXCEPTION_KIND_INITIAL_SETUP_ERROR } from '../../../shared/metrics-reporter.js'; /** @typedef {import('../types/duckplayer').YouTubeError} YouTubeError */ diff --git a/special-pages/pages/duckplayer/integration-tests/duck-player.js b/special-pages/pages/duckplayer/integration-tests/duck-player.js index 3ef7284e00..9036d9e872 100644 --- a/special-pages/pages/duckplayer/integration-tests/duck-player.js +++ b/special-pages/pages/duckplayer/integration-tests/duck-player.js @@ -572,7 +572,6 @@ export class DuckPlayerPage { */ async didSendReportMetric(evt) { const events = await this.mocks.waitForCallCount({ method: 'reportMetric', count: 1 }); - console.log('events', events); expect(events).toContainEqual({ payload: { context: 'specialPages', diff --git a/special-pages/pages/duckplayer/src/index.js b/special-pages/pages/duckplayer/src/index.js index 45b988aed8..0a765aecca 100644 --- a/special-pages/pages/duckplayer/src/index.js +++ b/special-pages/pages/duckplayer/src/index.js @@ -4,7 +4,7 @@ import { createSpecialPageMessaging } from '../../../shared/create-special-page- import { init } from '../app/index.js'; import { initStorage } from './storage.js'; import '../../../shared/live-reload.js'; -import { ReportMetric, EXCEPTION_KIND_MESSAGING_ERROR } from '../../../shared/report-metric.js'; +import { MetricsReporter, EXCEPTION_KIND_MESSAGING_ERROR } from '../../../shared/metrics-reporter.js'; export class DuckplayerPage { /** @@ -13,7 +13,7 @@ export class DuckplayerPage { constructor(messaging, injectName) { this.messaging = createTypedMessages(this, messaging); this.injectName = injectName; - this.metrics = new ReportMetric(messaging); + this.metrics = new MetricsReporter(messaging); } /** diff --git a/special-pages/shared/report-metric.js b/special-pages/shared/metrics-reporter.js similarity index 93% rename from special-pages/shared/report-metric.js rename to special-pages/shared/metrics-reporter.js index c2bbb0fdd0..c3b8adeaf1 100644 --- a/special-pages/shared/report-metric.js +++ b/special-pages/shared/metrics-reporter.js @@ -10,9 +10,9 @@ * * @example * ```javascript - * import { ReportMetric } from './report-metric.js'; + * import { MetricsReporter } from './metrics-reporter.js'; * - * const metrics = new ReportMetric(messaging); + * const metrics = new MetricsReporter(messaging); * * // Report a custom metric * metrics.reportMetric({ @@ -30,7 +30,7 @@ * metrics.reportExceptionWithError(new Error('Missing params')); * ``` * - * @module Report Metric + * @module Metrics Reporter */ /** @@ -55,7 +55,7 @@ export const EXCEPTION_KIND_MESSAGING_ERROR = 'MessagingError'; /** * Class for reporting metrics and exceptions to the native layer. */ -export class ReportMetric { +export class MetricsReporter { /** The message ID used for reporting metrics to the native layer */ static MESSAGE_ID = /** @type {const} */ ('reportMetric'); @@ -66,7 +66,7 @@ export class ReportMetric { static DEFAULT_EXCEPTION_MESSAGE = /** @type {const} */ ('Unknown error'); /** - * Creates a new ReportMetric instance. + * Creates a new MetricsReporter instance. * * @param {SharedMessaging} messaging - The messaging instance used to communicate with the native layer * @throws {Error} When messaging is not provided or messaging.notify is not defined @@ -90,7 +90,7 @@ export class ReportMetric { if (!metricEvent?.metricName) { throw new Error('metricName is required'); } - this.messaging.notify(ReportMetric.MESSAGE_ID, metricEvent); + this.messaging.notify(MetricsReporter.MESSAGE_ID, metricEvent); } /** @@ -108,11 +108,11 @@ export class ReportMetric { * @private */ _createExceptionMetric(params) { - const message = params?.message && typeof params.message === 'string' ? params.message : ReportMetric.DEFAULT_EXCEPTION_MESSAGE; + const message = params?.message && typeof params.message === 'string' ? params.message : MetricsReporter.DEFAULT_EXCEPTION_MESSAGE; const kind = params?.kind && typeof params.kind === 'string' ? params.kind : EXCEPTION_KIND_GENERIC_ERROR; return { - metricName: ReportMetric.METRIC_NAME_EXCEPTION, + metricName: MetricsReporter.METRIC_NAME_EXCEPTION, params: { message, kind, diff --git a/special-pages/unit-test/report-metric.mjs b/special-pages/unit-test/metrics-reporter.mjs similarity index 89% rename from special-pages/unit-test/report-metric.mjs rename to special-pages/unit-test/metrics-reporter.mjs index 6a95efa540..9004488544 100644 --- a/special-pages/unit-test/report-metric.mjs +++ b/special-pages/unit-test/metrics-reporter.mjs @@ -1,6 +1,6 @@ import { describe, it, mock, beforeEach } from 'node:test'; import assert from 'node:assert'; -import { ReportMetric } from '../shared/report-metric.js'; +import { MetricsReporter } from '../shared/metrics-reporter.js'; describe('reportMetric', () => { let messaging; @@ -16,18 +16,18 @@ describe('reportMetric', () => { it('should throw an error if messaging is not provided', () => { // @ts-expect-error - this is a forced error - assert.throws(() => new ReportMetric(null)); + assert.throws(() => new MetricsReporter(null)); }); it('should throw an error if metricName is not provided', () => { - const metrics = new ReportMetric(messaging); + const metrics = new MetricsReporter(messaging); const metricParams = /** @type {any} */ ({ metricName: '', params: {} }); assert.throws(() => metrics.reportMetric(metricParams)); assert.strictEqual(messaging.notify.mock.callCount(), 0); }); it('should call messaging.notify with the correct parameters', () => { - const metrics = new ReportMetric(messaging); + const metrics = new MetricsReporter(messaging); const metricParams = /** @type {any} */ ({ metricName: 'exception', params: { message: 'This is a test' } }); assert.strictEqual(messaging.notify.mock.callCount(), 0); @@ -44,7 +44,7 @@ describe('reportMetric', () => { }); it('should call messaging.notify when reportException is called', () => { - const metrics = new ReportMetric(messaging); + const metrics = new MetricsReporter(messaging); const eventParams = /** @type {any} */ ({ message: 'This is a test', kind: 'TestError' }); assert.strictEqual(messaging.notify.mock.callCount(), 0); @@ -61,7 +61,7 @@ describe('reportMetric', () => { }); it('should send default values when reportException is called with empty params', () => { - const metrics = new ReportMetric(messaging); + const metrics = new MetricsReporter(messaging); const eventParams = /** @type {any} */ ({}); assert.strictEqual(messaging.notify.mock.callCount(), 0); @@ -78,7 +78,7 @@ describe('reportMetric', () => { }); it('should not report anything when reportExceptionWithError is called with a non-Error object', () => { - const metrics = new ReportMetric(messaging); + const metrics = new MetricsReporter(messaging); const eventParams = /** @type {any} */ ({ message: 'This is a test', kind: 'TestError' }); assert.strictEqual(messaging.notify.mock.callCount(), 0); @@ -87,7 +87,7 @@ describe('reportMetric', () => { }); it('should send the error message and kind when reportExceptionWithError is called with an Error object', () => { - const metrics = new ReportMetric(messaging); + const metrics = new MetricsReporter(messaging); const error = new Error('This is a test'); error.name = 'TestError'; assert.strictEqual(messaging.notify.mock.callCount(), 0); @@ -105,7 +105,7 @@ describe('reportMetric', () => { }); it('should send default values when reportExceptionWithError is called with an empty error object', () => { - const metrics = new ReportMetric(messaging); + const metrics = new MetricsReporter(messaging); const error = new Error(); assert.strictEqual(messaging.notify.mock.callCount(), 0); diff --git a/typedoc.js b/typedoc.js index 00e5d0d1aa..38dbab4770 100644 --- a/typedoc.js +++ b/typedoc.js @@ -36,7 +36,7 @@ const config = { 'special-pages/pages/new-tab/app/favorites/constants.js', 'special-pages/pages/**/types/*.ts', 'special-pages/shared/types/*.ts', - 'special-pages/shared/report-metric.js', + 'special-pages/shared/metrics-reporter.js', ], categoryOrder: ['Special Pages', 'Content Scope Scripts Integrations', 'Other'], out: 'docs', From ec9156a19d897ba85be883f47d5116129b59275b Mon Sep 17 00:00:00 2001 From: Marcos Gurgel Date: Mon, 30 Jun 2025 12:28:00 +0100 Subject: [PATCH 20/28] PR feedback --- .../duckplayer-native.spec.js | 10 ++ .../page-objects/duckplayer-native.js | 95 +++++++++++++++++-- injected/src/features/duck-player-native.js | 52 +++++++--- injected/src/features/duck-player.js | 13 ++- .../features/duckplayer-native/messages.js | 12 ++- special-pages/shared/metrics-reporter.js | 1 + 6 files changed, 154 insertions(+), 29 deletions(-) diff --git a/injected/integration-test/duckplayer-native.spec.js b/injected/integration-test/duckplayer-native.spec.js index 35053543d2..8e5beaf06d 100644 --- a/injected/integration-test/duckplayer-native.spec.js +++ b/injected/integration-test/duckplayer-native.spec.js @@ -170,3 +170,13 @@ test.describe('Duck Player Native custom error view', () => { await duckPlayer.didShowUnknownError(); }); }); + +test.describe('Reporting exceptions', () => { + test('initial setup error', async ({ page }, workerInfo) => { + const duckPlayer = DuckPlayerNative.create(page, workerInfo); + await duckPlayer.withRemoteConfig(); + await duckPlayer.messagingError(); + await duckPlayer.gotoBlankPage(); + await duckPlayer.didSendMessagingException(); + }); +}); diff --git a/injected/integration-test/page-objects/duckplayer-native.js b/injected/integration-test/page-objects/duckplayer-native.js index 34127dce81..b1d7c75a43 100644 --- a/injected/integration-test/page-objects/duckplayer-native.js +++ b/injected/integration-test/page-objects/duckplayer-native.js @@ -11,12 +11,16 @@ import { ResultsCollector } from './results-collector.js'; // eslint-disable-next-line @typescript-eslint/no-unused-vars const configFiles = /** @type {const} */ (['native.json']); -const defaultInitialSetup = { +const featureName = 'duckPlayerNative'; + +/** @type {PageType} */ +const DEFAULT_PAGE_TYPE = 'YOUTUBE'; + +/** @type {import('../../src/features/duck-player-native.js').InitialSettings} */ +const DEFAULT_INITIAL_SETUP = { locale: 'en', }; -const featureName = 'duckPlayerNative'; - export class DuckPlayerNative { /** @type {Partial>} */ pages = { @@ -36,7 +40,7 @@ export class DuckPlayerNative { this.isMobile = platform.name === 'android' || platform.name === 'ios'; this.collector = new ResultsCollector(page, build, platform); this.collector.withMockResponse({ - initialSetup: defaultInitialSetup, + initialSetup: DEFAULT_INITIAL_SETUP, onCurrentTimestamp: {}, }); this.collector.withUserPreferences({ @@ -86,13 +90,22 @@ export class DuckPlayerNative { } /** - * @param {PageType} pageType + * Goes to the default page (YOUTUBE) without overriding initialSetup + * Useful for testing messaging errors + **/ + async gotoBlankPage() { + await this.gotoPage(); + } + + /** + * @param {PageType} [pageType] * @param {object} [params] * @param {PlayerPageVariants} [params.variant] * @param {string} [params.videoID] */ async gotoPage(pageType, params = {}) { - await this.withPageType(pageType); + if (pageType) await this.withPageType(pageType); + const page = this.pages[pageType || DEFAULT_PAGE_TYPE]; const defaultVariant = this.isMobile ? 'mobile' : 'default'; const { variant = defaultVariant, videoID = '123' } = params; @@ -101,8 +114,6 @@ export class DuckPlayerNative { ['variant', variant], ]); - const page = this.pages[pageType]; - await this.page.goto(page + '?' + urlParams.toString()); } @@ -122,7 +133,7 @@ export class DuckPlayerNative { * @return {Promise} */ async withPageType(pageType) { - const initialSetup = this.collector.mockResponses?.initialSetup || defaultInitialSetup; + const initialSetup = this.collector.mockResponses?.initialSetup || DEFAULT_INITIAL_SETUP; await this.collector.updateMockResponse({ initialSetup: { pageType, ...initialSetup }, @@ -134,7 +145,7 @@ export class DuckPlayerNative { * @return {Promise} */ async withPlaybackPaused(playbackPaused = true) { - const initialSetup = this.collector.mockResponses.initialSetup || defaultInitialSetup; + const initialSetup = this.collector.mockResponses.initialSetup || DEFAULT_INITIAL_SETUP; await this.collector.updateMockResponse({ initialSetup: { playbackPaused, ...initialSetup }, @@ -206,6 +217,34 @@ export class DuckPlayerNative { await this.simulateSubscriptionMessage('onUrlChanged', { pageType }); } + /** + * Simulates a messaging error by passing an empty initialSetup object + */ + async messagingError() { + await this.build.switch({ + android: async () => { + await this.collector.updateMockResponse({ + initialSetup: {}, + }); + }, + apple: async () => { + await this.collector.updateMockResponse({ + initialSetup: null, + }); + }, + 'apple-isolated': async () => { + await this.collector.updateMockResponse({ + initialSetup: null, + }); + }, + windows: async () => { + await this.collector.updateMockResponse({ + initialSetup: '', + }); + }, + }); + } + /* Messaging assertions */ async didSendInitialHandshake() { @@ -236,6 +275,42 @@ export class DuckPlayerNative { ]); } + /** + * @param {string} kind + * @param {string} message + */ + async didSendException(kind, message, context = 'contentScopeScripts') { + const messages = await this.collector.waitForMessage('reportMetric'); + expect(messages).toMatchObject([ + { + payload: { + context, + featureName: 'duckPlayerNative', + method: 'reportMetric', + params: { metricName: 'exception', params: { kind, message } }, + }, + }, + ]); + } + + async didSendMessagingException() { + await this.build.switch({ + android: async () => { + // Android produces a TypeError due to how its messaging lib is wired up + await this.didSendException('TypeError', "Cannot read properties of undefined (reading 'privatePlayerMode')"); + }, + apple: async () => { + await this.didSendException('MessagingError', 'an unknown error occurred'); + }, + 'apple-isolated': async () => { + await this.didSendException('MessagingError', 'an unknown error occurred', 'contentScopeScriptsIsolated'); + }, + windows: async () => { + await this.didSendException('MessagingError', 'unknown error'); + }, + }); + } + /* Thumbnail Overlay assertions */ async didShowOverlay() { diff --git a/injected/src/features/duck-player-native.js b/injected/src/features/duck-player-native.js index 70e7f60ccf..bf4fc6544c 100644 --- a/injected/src/features/duck-player-native.js +++ b/injected/src/features/duck-player-native.js @@ -4,6 +4,7 @@ import { DuckPlayerNativeMessages } from './duckplayer-native/messages.js'; import { setupDuckPlayerForNoCookie, setupDuckPlayerForSerp, setupDuckPlayerForYouTube } from './duckplayer-native/sub-feature.js'; import { Environment } from './duckplayer/environment.js'; import { Logger } from './duckplayer/util.js'; +import { MetricsReporter, EXCEPTION_KIND_INITIAL_SETUP_ERROR } from '../../../special-pages/shared/metrics-reporter.js'; /** * @import {DuckPlayerNativeSubFeature} from './duckplayer-native/sub-feature.js' @@ -14,8 +15,8 @@ import { Logger } from './duckplayer/util.js'; /** * @typedef InitialSettings - The initial payload used to communicate render-blocking information * @property {string} locale - UI locale - * @property {UrlChangeSettings['pageType']} pageType - The type of the current page - * @property {boolean} playbackPaused - Should video start playing or paused + * @property {UrlChangeSettings['pageType']} [pageType] - The type of the current page + * @property {boolean} [playbackPaused] - Should video start playing or paused */ /** @@ -29,18 +30,12 @@ export class DuckPlayerNativeFeature extends ContentFeature { /** @type {TranslationFn} */ t; - async init(args) { + init(args) { /** * This feature never operates in a frame */ if (isBeingFramed()) return; - const selectors = this.getFeatureSetting('selectors'); - if (!selectors) { - console.warn('No selectors found. Check remote config. Feature will not be initialized.'); - return; - } - const locale = args?.locale || args?.language || 'en'; const env = new Environment({ debug: this.isDebug, @@ -48,11 +43,33 @@ export class DuckPlayerNativeFeature extends ContentFeature { platform: this.platform, locale, }); + const metrics = new MetricsReporter(this.messaging); + + try { + const selectors = this.getFeatureSetting('selectors'); + if (!selectors) { + console.warn('No selectors found. Check remote config. Feature will not be initialized.'); + return; + } - // Translation function to be used by view components - this.t = (key) => env.strings('native.json')[key]; + // Translation function to be used by view components + this.t = (key) => env.strings('native.json')[key]; + + const messages = new DuckPlayerNativeMessages(this.messaging, env); + this.initDuckPlayerNative(messages, selectors, env) + // Using then instead of await because this is the public interface of the parent, which doesn't explicitly wait for promises to be resolved. + // eslint-disable-next-line promise/prefer-await-to-then + .catch((e) => { + console.error(e); + metrics.reportExceptionWithError(e); + }); + } catch (e) { + console.error(e); + metrics.reportExceptionWithError(e); + } + } - const messages = new DuckPlayerNativeMessages(this.messaging, env); + async initDuckPlayerNative(messages, selectors, env) { messages.subscribeToURLChange(({ pageType }) => { const playbackPaused = false; // This can be added to the event data in the future if needed this.urlDidChange(pageType, selectors, playbackPaused, env, messages); @@ -64,7 +81,14 @@ export class DuckPlayerNativeFeature extends ContentFeature { try { initialSetup = await messages.initialSetup(); } catch (e) { - console.warn('Failed to get initial setup', e); + console.warn(e); + return; + } + + if (!initialSetup) { + const message = 'InitialSetup data is missing'; + console.warn(message); + messages.metrics.reportException({ message, kind: EXCEPTION_KIND_INITIAL_SETUP_ERROR }); return; } @@ -118,7 +142,7 @@ export class DuckPlayerNativeFeature extends ContentFeature { if (document.readyState === 'loading') { const loadHandler = () => { logger.log('Running deferred load handlers'); - nextPage.onLoad(); + nextPage?.onLoad(); messages.notifyScriptIsReady(); }; document.addEventListener('DOMContentLoaded', loadHandler, { once: true }); diff --git a/injected/src/features/duck-player.js b/injected/src/features/duck-player.js index 2750307b01..049c948651 100644 --- a/injected/src/features/duck-player.js +++ b/injected/src/features/duck-player.js @@ -61,7 +61,7 @@ import { MetricsReporter } from '../../../special-pages/shared/metrics-reporter. * @internal */ export default class DuckPlayerFeature extends ContentFeature { - async init(args) { + init(args) { /** * This feature never operates in a frame */ @@ -93,6 +93,7 @@ export default class DuckPlayerFeature extends ContentFeature { if (!this.messaging) { throw new Error('cannot operate duck player without a messaging backend'); } + const metrics = new MetricsReporter(this.messaging); try { const locale = args?.locale || args?.language || 'en'; @@ -105,12 +106,18 @@ export default class DuckPlayerFeature extends ContentFeature { const comms = new DuckPlayerOverlayMessages(this.messaging, env); if (overlaysEnabled) { - await initOverlays(overlaySettings.youtube, env, comms); + initOverlays(overlaySettings.youtube, env, comms) + // Using then instead of await because this is the public interface of the parent, which doesn't explicitly wait for promises to be resolved. + // eslint-disable-next-line promise/prefer-await-to-then + .catch((e) => { + console.error(e); + metrics.reportExceptionWithError(e); + }); } else if (serpProxyEnabled) { comms.serpProxy(); } } catch (e) { - const metrics = new MetricsReporter(this.messaging); + console.error(e); metrics.reportExceptionWithError(e); } } diff --git a/injected/src/features/duckplayer-native/messages.js b/injected/src/features/duckplayer-native/messages.js index 2cfecf7f8c..d480392113 100644 --- a/injected/src/features/duckplayer-native/messages.js +++ b/injected/src/features/duckplayer-native/messages.js @@ -1,4 +1,5 @@ import * as constants from './constants.js'; +import { MetricsReporter, EXCEPTION_KIND_MESSAGING_ERROR } from '../../../../special-pages/shared/metrics-reporter.js'; /** @import {YouTubeError} from './error-detection.js' */ /** @import {Environment} from '../duckplayer/environment.js' */ @@ -42,13 +43,20 @@ export class DuckPlayerNativeMessages { */ this.messaging = messaging; this.environment = environment; + this.metrics = new MetricsReporter(messaging); } /** * @returns {Promise} */ - initialSetup() { - return this.messaging.request(constants.MSG_NAME_INITIAL_SETUP); + async initialSetup() { + try { + console.log('initialSetup'); + return await this.messaging.request(constants.MSG_NAME_INITIAL_SETUP); + } catch (e) { + this.metrics.reportException({ message: e?.message, kind: EXCEPTION_KIND_MESSAGING_ERROR }); + throw e; + } } /** diff --git a/special-pages/shared/metrics-reporter.js b/special-pages/shared/metrics-reporter.js index c3b8adeaf1..c114d79734 100644 --- a/special-pages/shared/metrics-reporter.js +++ b/special-pages/shared/metrics-reporter.js @@ -157,6 +157,7 @@ export class MetricsReporter { * ``` */ reportException(params) { + console.log('reportException', params); const metric = this._createExceptionMetric(params); this._sendMetric(metric); } From 6408d32f8f976d148909eb4ef047740ad7006533 Mon Sep 17 00:00:00 2001 From: Marcos Gurgel Date: Mon, 30 Jun 2025 12:47:25 +0100 Subject: [PATCH 21/28] Cleanup --- injected/src/features/duckplayer-native/messages.js | 1 - special-pages/shared/metrics-reporter.js | 1 - 2 files changed, 2 deletions(-) diff --git a/injected/src/features/duckplayer-native/messages.js b/injected/src/features/duckplayer-native/messages.js index d480392113..18c8b26642 100644 --- a/injected/src/features/duckplayer-native/messages.js +++ b/injected/src/features/duckplayer-native/messages.js @@ -51,7 +51,6 @@ export class DuckPlayerNativeMessages { */ async initialSetup() { try { - console.log('initialSetup'); return await this.messaging.request(constants.MSG_NAME_INITIAL_SETUP); } catch (e) { this.metrics.reportException({ message: e?.message, kind: EXCEPTION_KIND_MESSAGING_ERROR }); diff --git a/special-pages/shared/metrics-reporter.js b/special-pages/shared/metrics-reporter.js index c114d79734..c3b8adeaf1 100644 --- a/special-pages/shared/metrics-reporter.js +++ b/special-pages/shared/metrics-reporter.js @@ -157,7 +157,6 @@ export class MetricsReporter { * ``` */ reportException(params) { - console.log('reportException', params); const metric = this._createExceptionMetric(params); this._sendMetric(metric); } From 40e91a6928950cdd413705a1f0fc1d081f02e79b Mon Sep 17 00:00:00 2001 From: Marcos Gurgel Date: Fri, 4 Jul 2025 16:24:08 +0100 Subject: [PATCH 22/28] Catch embed settings errors --- special-pages/pages/duckplayer/app/index.js | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/special-pages/pages/duckplayer/app/index.js b/special-pages/pages/duckplayer/app/index.js index 321a17ecea..9d74272072 100644 --- a/special-pages/pages/duckplayer/app/index.js +++ b/special-pages/pages/duckplayer/app/index.js @@ -73,11 +73,14 @@ export async function init(messaging, telemetry, baseEnvironment) { console.log(settings); - const embed = createEmbedSettings(window.location.href, settings); - if (!embed) { - const message = 'Embed not found'; - messaging.metrics.reportException({ message, kind: EXCEPTION_KIND_INIT_ERROR }); - console.log(message); + let embed; + try { + embed = createEmbedSettings(window.location.href, settings); + if (!embed) { + throw new Error('Embed not found'); + } + } catch (e) { + messaging.metrics.reportException({ message: e.message, kind: EXCEPTION_KIND_INIT_ERROR }); } const didCatch = (error) => { From c4d0035abb218a53e24f4102ba24686cd0f538e9 Mon Sep 17 00:00:00 2001 From: Marcos Gurgel Date: Fri, 18 Jul 2025 14:47:10 +0100 Subject: [PATCH 23/28] Type fix --- special-pages/pages/duckplayer/app/index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/special-pages/pages/duckplayer/app/index.js b/special-pages/pages/duckplayer/app/index.js index 9d74272072..1a0c93f423 100644 --- a/special-pages/pages/duckplayer/app/index.js +++ b/special-pages/pages/duckplayer/app/index.js @@ -73,7 +73,7 @@ export async function init(messaging, telemetry, baseEnvironment) { console.log(settings); - let embed; + let embed = null; try { embed = createEmbedSettings(window.location.href, settings); if (!embed) { From ddab5087d718899bf3152d647360ba7501fbd495 Mon Sep 17 00:00:00 2001 From: Marcos Gurgel Date: Wed, 30 Jul 2025 15:59:43 +0100 Subject: [PATCH 24/28] Avoiding double throws in error reporting --- injected/src/features/duckplayer-native/messages.js | 4 ++-- injected/src/features/duckplayer/overlay-messages.js | 12 ++++++------ injected/src/features/duckplayer/overlays.js | 5 ++--- injected/src/features/duckplayer/video-overlay.js | 10 +++++++--- special-pages/pages/duckplayer/src/index.js | 6 +++--- 5 files changed, 20 insertions(+), 17 deletions(-) diff --git a/injected/src/features/duckplayer-native/messages.js b/injected/src/features/duckplayer-native/messages.js index 18c8b26642..69c9c3a8e6 100644 --- a/injected/src/features/duckplayer-native/messages.js +++ b/injected/src/features/duckplayer-native/messages.js @@ -47,14 +47,14 @@ export class DuckPlayerNativeMessages { } /** - * @returns {Promise} + * @returns {Promise} */ async initialSetup() { try { return await this.messaging.request(constants.MSG_NAME_INITIAL_SETUP); } catch (e) { this.metrics.reportException({ message: e?.message, kind: EXCEPTION_KIND_MESSAGING_ERROR }); - throw e; + return null; } } diff --git a/injected/src/features/duckplayer/overlay-messages.js b/injected/src/features/duckplayer/overlay-messages.js index 6d970f39db..83d4c8e2b6 100644 --- a/injected/src/features/duckplayer/overlay-messages.js +++ b/injected/src/features/duckplayer/overlay-messages.js @@ -26,7 +26,7 @@ export class DuckPlayerOverlayMessages { } /** - * @returns {Promise} + * @returns {Promise} */ async initialSetup() { if (this.environment.isIntegrationMode()) { @@ -42,33 +42,33 @@ export class DuckPlayerOverlayMessages { return await this.messaging.request(constants.MSG_NAME_INITIAL_SETUP); } catch (e) { this.metrics.reportException({ message: e?.message, kind: EXCEPTION_KIND_MESSAGING_ERROR }); - throw e; + return null; } } /** * Inform the native layer that an interaction occurred * @param {import("../duck-player.js").UserValues} userValues - * @returns {Promise} + * @returns {Promise} */ async setUserValues(userValues) { try { return await this.messaging.request(constants.MSG_NAME_SET_VALUES, userValues); } catch (e) { this.metrics.reportException({ message: e?.message, kind: EXCEPTION_KIND_MESSAGING_ERROR }); - throw e; + return null; } } /** - * @returns {Promise} + * @returns {Promise} */ async getUserValues() { try { return await this.messaging.request(constants.MSG_NAME_READ_VALUES, {}); } catch (e) { this.metrics.reportException({ message: e?.message, kind: EXCEPTION_KIND_MESSAGING_ERROR }); - throw e; + return null; } } diff --git a/injected/src/features/duckplayer/overlays.js b/injected/src/features/duckplayer/overlays.js index 6e8f039179..99b0d40ff8 100644 --- a/injected/src/features/duckplayer/overlays.js +++ b/injected/src/features/duckplayer/overlays.js @@ -22,18 +22,17 @@ export async function initOverlays(settings, environment, messages) { // bind early to attach all listeners const domState = new DomState(); - /** @type {import("../duck-player.js").OverlaysInitialSettings} */ + /** @type {import("../duck-player.js").OverlaysInitialSettings|null} */ let initialSetup; try { initialSetup = await messages.initialSetup(); } catch (e) { - console.warn(e); + // console.warn(e); return; } if (!initialSetup) { const message = 'InitialSetup data is missing'; - console.warn(message); messages.metrics.reportException({ message, kind: EXCEPTION_KIND_INITIAL_SETUP_ERROR }); return; } diff --git a/injected/src/features/duckplayer/video-overlay.js b/injected/src/features/duckplayer/video-overlay.js index dd2f963dd1..07a50951be 100644 --- a/injected/src/features/duckplayer/video-overlay.js +++ b/injected/src/features/duckplayer/video-overlay.js @@ -452,9 +452,11 @@ export class VideoOverlay { overlayInteracted: true, }) .then((values) => { - this.userValues = values; + if (values) { + this.userValues = values; + this.watchForVideoBeingAdded({ ignoreCache: true, via: 'userOptOut' }); + } }) - .then(() => this.watchForVideoBeingAdded({ ignoreCache: true, via: 'userOptOut' })) .catch((e) => { console.error(e); this.messages.metrics.reportExceptionWithError(e); @@ -517,7 +519,9 @@ export class VideoOverlay { const updatedValues = await this.messages.setUserValues(next); // this is needed to ensure any future page navigations respect the new settings - this.userValues = updatedValues; + if (updatedValues) { + this.userValues = updatedValues; + } if (this.environment.debug) { console.log('user values response:', updatedValues); diff --git a/special-pages/pages/duckplayer/src/index.js b/special-pages/pages/duckplayer/src/index.js index 0a765aecca..cb60086786 100644 --- a/special-pages/pages/duckplayer/src/index.js +++ b/special-pages/pages/duckplayer/src/index.js @@ -19,7 +19,7 @@ export class DuckplayerPage { /** * This will be sent if the application has loaded, but a client-side error * has occurred that cannot be recovered from - * @returns {Promise} + * @returns {Promise} */ async initialSetup() { if (this.injectName === 'integration') { @@ -42,7 +42,7 @@ export class DuckplayerPage { return await this.messaging.request('initialSetup'); } catch (e) { this.metrics.reportException({ message: e?.message, kind: EXCEPTION_KIND_MESSAGING_ERROR }); - throw e; + return null; } } @@ -56,7 +56,7 @@ export class DuckplayerPage { return await this.messaging.request('setUserValues', userValues); } catch (e) { this.metrics.reportException({ message: e?.message, kind: EXCEPTION_KIND_MESSAGING_ERROR }); - throw e; + return null; } } From 1dd52307d9ea3f8d4220d088ad8bfdb1a5f72dea Mon Sep 17 00:00:00 2001 From: Shane Osbourne Date: Wed, 30 Jul 2025 16:08:12 +0100 Subject: [PATCH 25/28] co-locate --- .prettierignore | 2 +- injected/src/features/duck-player-native.js | 2 +- injected/src/features/duck-player.js | 2 +- injected/src/features/duckplayer-native/messages.js | 2 +- injected/src/features/duckplayer/overlay-messages.js | 2 +- injected/src/features/duckplayer/overlays.js | 2 +- special-pages/pages/duckplayer/app/index.js | 2 +- .../duckplayer/integration-tests/duck-player.js | 2 +- special-pages/pages/duckplayer/src/index.js | 2 +- .../{ => metrics}/messages/reportMetric.notify.json | 0 .../shared/{ => metrics}/metrics-reporter.js | 12 ++++++------ .../{types/shared.ts => metrics/types/metrics.ts} | 12 +++++++++--- special-pages/types.mjs | 12 ++++++------ special-pages/unit-test/metrics-reporter.mjs | 2 +- types-generator/json-schema.mjs | 4 ---- 15 files changed, 31 insertions(+), 29 deletions(-) rename special-pages/shared/{ => metrics}/messages/reportMetric.notify.json (100%) rename special-pages/shared/{ => metrics}/metrics-reporter.js (91%) rename special-pages/shared/{types/shared.ts => metrics/types/metrics.ts} (64%) diff --git a/.prettierignore b/.prettierignore index a71fd8c561..007b36ef7a 100644 --- a/.prettierignore +++ b/.prettierignore @@ -3,7 +3,7 @@ docs/**/* !injected/docs/**/* injected/src/types special-pages/pages/**/types -special-pages/shared/types +special-pages/shared/**/types injected/integration-test/extension/contentScope.js injected/src/features/Scriptlets **/*.json diff --git a/injected/src/features/duck-player-native.js b/injected/src/features/duck-player-native.js index bf4fc6544c..7ba6a9b841 100644 --- a/injected/src/features/duck-player-native.js +++ b/injected/src/features/duck-player-native.js @@ -4,7 +4,7 @@ import { DuckPlayerNativeMessages } from './duckplayer-native/messages.js'; import { setupDuckPlayerForNoCookie, setupDuckPlayerForSerp, setupDuckPlayerForYouTube } from './duckplayer-native/sub-feature.js'; import { Environment } from './duckplayer/environment.js'; import { Logger } from './duckplayer/util.js'; -import { MetricsReporter, EXCEPTION_KIND_INITIAL_SETUP_ERROR } from '../../../special-pages/shared/metrics-reporter.js'; +import { MetricsReporter, EXCEPTION_KIND_INITIAL_SETUP_ERROR } from '../../../special-pages/shared/metrics/metrics-reporter.js'; /** * @import {DuckPlayerNativeSubFeature} from './duckplayer-native/sub-feature.js' diff --git a/injected/src/features/duck-player.js b/injected/src/features/duck-player.js index 049c948651..d8a7e7c74e 100644 --- a/injected/src/features/duck-player.js +++ b/injected/src/features/duck-player.js @@ -37,7 +37,7 @@ import { DuckPlayerOverlayMessages, OpenInDuckPlayerMsg, Pixel } from './duckpla import { isBeingFramed } from '../utils.js'; import { initOverlays } from './duckplayer/overlays.js'; import { Environment } from './duckplayer/environment.js'; -import { MetricsReporter } from '../../../special-pages/shared/metrics-reporter.js'; +import { MetricsReporter } from '../../../special-pages/shared/metrics/metrics-reporter.js'; /** * @typedef UserValues - A way to communicate user settings diff --git a/injected/src/features/duckplayer-native/messages.js b/injected/src/features/duckplayer-native/messages.js index 69c9c3a8e6..44ebd787b3 100644 --- a/injected/src/features/duckplayer-native/messages.js +++ b/injected/src/features/duckplayer-native/messages.js @@ -1,5 +1,5 @@ import * as constants from './constants.js'; -import { MetricsReporter, EXCEPTION_KIND_MESSAGING_ERROR } from '../../../../special-pages/shared/metrics-reporter.js'; +import { MetricsReporter, EXCEPTION_KIND_MESSAGING_ERROR } from 'special-pages/shared/metrics/metrics-reporter.js'; /** @import {YouTubeError} from './error-detection.js' */ /** @import {Environment} from '../duckplayer/environment.js' */ diff --git a/injected/src/features/duckplayer/overlay-messages.js b/injected/src/features/duckplayer/overlay-messages.js index 83d4c8e2b6..236856025d 100644 --- a/injected/src/features/duckplayer/overlay-messages.js +++ b/injected/src/features/duckplayer/overlay-messages.js @@ -1,6 +1,6 @@ /* eslint-disable promise/prefer-await-to-then */ import * as constants from './constants.js'; -import { MetricsReporter, EXCEPTION_KIND_MESSAGING_ERROR } from '../../../../special-pages/shared/metrics-reporter.js'; +import { MetricsReporter, EXCEPTION_KIND_MESSAGING_ERROR } from 'special-pages/shared/metrics/metrics-reporter.js'; /** * @typedef {import("@duckduckgo/messaging").Messaging} Messaging diff --git a/injected/src/features/duckplayer/overlays.js b/injected/src/features/duckplayer/overlays.js index 99b0d40ff8..b38b3bd4c9 100644 --- a/injected/src/features/duckplayer/overlays.js +++ b/injected/src/features/duckplayer/overlays.js @@ -2,7 +2,7 @@ import { DomState } from './util.js'; import { ClickInterception, Thumbnails } from './thumbnails.js'; import { VideoOverlay } from './video-overlay.js'; import { registerCustomElements } from './components/index.js'; -import { EXCEPTION_KIND_INITIAL_SETUP_ERROR } from '../../../../special-pages/shared/metrics-reporter.js'; +import { EXCEPTION_KIND_INITIAL_SETUP_ERROR } from 'special-pages/shared/metrics/metrics-reporter.js'; /** * @typedef {object} OverlayOptions diff --git a/special-pages/pages/duckplayer/app/index.js b/special-pages/pages/duckplayer/app/index.js index 1a0c93f423..c62bc625d4 100644 --- a/special-pages/pages/duckplayer/app/index.js +++ b/special-pages/pages/duckplayer/app/index.js @@ -15,7 +15,7 @@ import { Components } from './components/Components.jsx'; import { MobileApp } from './components/MobileApp.jsx'; import { DesktopApp } from './components/DesktopApp.jsx'; import { YouTubeErrorProvider } from './providers/YouTubeErrorProvider'; -import { EXCEPTION_KIND_INIT_ERROR, EXCEPTION_KIND_INITIAL_SETUP_ERROR } from '../../../shared/metrics-reporter.js'; +import { EXCEPTION_KIND_INIT_ERROR, EXCEPTION_KIND_INITIAL_SETUP_ERROR } from '../../../shared/metrics/metrics-reporter.js'; /** @typedef {import('../types/duckplayer').YouTubeError} YouTubeError */ diff --git a/special-pages/pages/duckplayer/integration-tests/duck-player.js b/special-pages/pages/duckplayer/integration-tests/duck-player.js index 9036d9e872..043cab52bb 100644 --- a/special-pages/pages/duckplayer/integration-tests/duck-player.js +++ b/special-pages/pages/duckplayer/integration-tests/duck-player.js @@ -568,7 +568,7 @@ export class DuckPlayerPage { } /** - * @param {import('../../../shared/types/shared.ts').ReportMetricEvent} evt + * @param {import('../../../shared/metrics/types/metrics.js').ReportMetricEvent} evt */ async didSendReportMetric(evt) { const events = await this.mocks.waitForCallCount({ method: 'reportMetric', count: 1 }); diff --git a/special-pages/pages/duckplayer/src/index.js b/special-pages/pages/duckplayer/src/index.js index cb60086786..72e055fd14 100644 --- a/special-pages/pages/duckplayer/src/index.js +++ b/special-pages/pages/duckplayer/src/index.js @@ -4,7 +4,7 @@ import { createSpecialPageMessaging } from '../../../shared/create-special-page- import { init } from '../app/index.js'; import { initStorage } from './storage.js'; import '../../../shared/live-reload.js'; -import { MetricsReporter, EXCEPTION_KIND_MESSAGING_ERROR } from '../../../shared/metrics-reporter.js'; +import { MetricsReporter, EXCEPTION_KIND_MESSAGING_ERROR } from '../../../shared/metrics/metrics-reporter.js'; export class DuckplayerPage { /** diff --git a/special-pages/shared/messages/reportMetric.notify.json b/special-pages/shared/metrics/messages/reportMetric.notify.json similarity index 100% rename from special-pages/shared/messages/reportMetric.notify.json rename to special-pages/shared/metrics/messages/reportMetric.notify.json diff --git a/special-pages/shared/metrics-reporter.js b/special-pages/shared/metrics/metrics-reporter.js similarity index 91% rename from special-pages/shared/metrics-reporter.js rename to special-pages/shared/metrics/metrics-reporter.js index c3b8adeaf1..86163e5ca9 100644 --- a/special-pages/shared/metrics-reporter.js +++ b/special-pages/shared/metrics/metrics-reporter.js @@ -34,12 +34,12 @@ */ /** - * @typedef {import('../shared/types/shared.ts').ExceptionMetric} ExceptionMetric - * @typedef {import('../shared/types/shared.ts').ReportMetricEvent} ReportMetricEvent - * @typedef {import('../shared/types/shared.ts').SharedMessages} SharedMessages - * @typedef {import('@duckduckgo/messaging/lib/shared-types.js').MessagingBase|import('@duckduckgo/messaging').Messaging} SharedMessaging + * @typedef {import('./types/metrics.ts').ExceptionMetric} ExceptionMetric + * @typedef {import('./types/metrics.ts').ReportMetricEvent} ReportMetricEvent */ +import { createTypedMessages } from '@duckduckgo/messaging/lib/typed-messages.js'; + /** Exception kind for generic errors */ export const EXCEPTION_KIND_GENERIC_ERROR = 'Error'; @@ -68,14 +68,14 @@ export class MetricsReporter { /** * Creates a new MetricsReporter instance. * - * @param {SharedMessaging} messaging - The messaging instance used to communicate with the native layer + * @param {import('@duckduckgo/messaging').Messaging} messaging - The messaging instance used to communicate with the native layer * @throws {Error} When messaging is not provided or messaging.notify is not defined */ constructor(messaging) { if (!messaging) { throw new Error('messaging is required'); } - this.messaging = messaging; + this.messaging = createTypedMessages(this, messaging); } /** diff --git a/special-pages/shared/types/shared.ts b/special-pages/shared/metrics/types/metrics.ts similarity index 64% rename from special-pages/shared/types/shared.ts rename to special-pages/shared/metrics/types/metrics.ts index 56943e8ee1..42eb8fe191 100644 --- a/special-pages/shared/types/shared.ts +++ b/special-pages/shared/metrics/types/metrics.ts @@ -3,15 +3,15 @@ * scripts/build-types.mjs is responsible for type generation. * **DO NOT** edit this file directly as your changes will be lost. * - * @module Shared Messages + * @module Metrics Messages */ export type ReportMetricEvent = ExceptionMetric; /** - * Requests, Notifications and Subscriptions from the Shared feature + * Requests, Notifications and Subscriptions from the Metrics feature */ -export interface SharedMessages { +export interface MetricsMessages { notifications: ReportMetricNotification; } /** @@ -28,3 +28,9 @@ export interface ExceptionMetric { kind?: string; }; } + +declare module "../metrics-reporter.js" { + export interface MetricsReporter { + notify: import("@duckduckgo/messaging/lib/shared-types").MessagingBase['notify'] + } +} \ No newline at end of file diff --git a/special-pages/types.mjs b/special-pages/types.mjs index d3ff91d1d7..367e492cac 100644 --- a/special-pages/types.mjs +++ b/special-pages/types.mjs @@ -30,14 +30,14 @@ for (const pageListElement of pageList) { } /* Shared types */ -specialPagesTypes.shared = { - schemaDir: join(specialPagesRoot, 'shared', 'messages'), - typesDir: join(specialPagesRoot, 'shared', 'types'), +specialPagesTypes.metrics = { + schemaDir: join(specialPagesRoot, 'shared', 'metrics', 'messages'), + typesDir: join(specialPagesRoot, 'shared', 'metrics', 'types'), exclude: process.platform === 'win32', kind: 'single', - resolve: (_dirname) => '../src/index.js', - className: () => null, - filename: `shared.ts`, + resolve: (_dirname) => '../metrics-reporter.js', + className: (_top) => 'MetricsReporter', + filename: `metrics.ts`, }; if (isLaunchFile(import.meta.url)) { diff --git a/special-pages/unit-test/metrics-reporter.mjs b/special-pages/unit-test/metrics-reporter.mjs index 9004488544..159c44242e 100644 --- a/special-pages/unit-test/metrics-reporter.mjs +++ b/special-pages/unit-test/metrics-reporter.mjs @@ -1,6 +1,6 @@ import { describe, it, mock, beforeEach } from 'node:test'; import assert from 'node:assert'; -import { MetricsReporter } from '../shared/metrics-reporter.js'; +import { MetricsReporter } from '../shared/metrics/metrics-reporter.js'; describe('reportMetric', () => { let messaging; diff --git a/types-generator/json-schema.mjs b/types-generator/json-schema.mjs index 9cf5eb7652..f5a6cd86c1 100644 --- a/types-generator/json-schema.mjs +++ b/types-generator/json-schema.mjs @@ -173,10 +173,6 @@ export function generateSchema(featureName, fileList) { * @return {string} */ export function createMessagingTypes(job, { featurePath, className }) { - if (!className) { - return ''; - } - const json = job.schema; const notifications = (json.properties?.notifications?.oneOf?.length ?? 0) > 0; const requests = (json.properties?.requests?.oneOf?.length ?? 0) > 0; From e0fc9c2565d2c7c5afe17aefe68981d4c54205cf Mon Sep 17 00:00:00 2001 From: Shane Osbourne Date: Wed, 30 Jul 2025 16:42:49 +0100 Subject: [PATCH 26/28] top-level metrics class --- injected/src/features/duck-player-native.js | 2 +- injected/src/features/duck-player.js | 2 +- .../features/duckplayer-native/messages.js | 2 +- .../features/duckplayer/overlay-messages.js | 2 +- injected/src/features/duckplayer/overlays.js | 2 +- injected/unit-test/verify-artifacts.js | 2 +- metrics/docs/readme.md | 16 ++++++++ metrics/examples/metrics.js | 39 +++++++++++++++++++ .../messages/reportMetric.notify.json | 0 .../metrics => metrics}/metrics-reporter.js | 36 ----------------- metrics/package.json | 12 ++++++ metrics/scripts/types.mjs | 32 +++++++++++++++ metrics/types/metrics.ts | 36 +++++++++++++++++ .../unit-test/metrics-reporter.mjs | 2 +- package-lock.json | 12 +++++- package.json | 3 +- special-pages/pages/duckplayer/app/index.js | 2 +- .../integration-tests/duck-player.js | 2 +- special-pages/pages/duckplayer/src/index.js | 2 +- special-pages/shared/metrics/types/metrics.ts | 36 ----------------- special-pages/types.mjs | 11 ------ tsconfig.json | 1 + typedoc.js | 4 +- 23 files changed, 161 insertions(+), 97 deletions(-) create mode 100644 metrics/docs/readme.md create mode 100644 metrics/examples/metrics.js rename {special-pages/shared/metrics => metrics}/messages/reportMetric.notify.json (100%) rename {special-pages/shared/metrics => metrics}/metrics-reporter.js (83%) create mode 100644 metrics/package.json create mode 100644 metrics/scripts/types.mjs create mode 100644 metrics/types/metrics.ts rename {special-pages => metrics}/unit-test/metrics-reporter.mjs (98%) delete mode 100644 special-pages/shared/metrics/types/metrics.ts diff --git a/injected/src/features/duck-player-native.js b/injected/src/features/duck-player-native.js index 7ba6a9b841..a293923408 100644 --- a/injected/src/features/duck-player-native.js +++ b/injected/src/features/duck-player-native.js @@ -4,7 +4,7 @@ import { DuckPlayerNativeMessages } from './duckplayer-native/messages.js'; import { setupDuckPlayerForNoCookie, setupDuckPlayerForSerp, setupDuckPlayerForYouTube } from './duckplayer-native/sub-feature.js'; import { Environment } from './duckplayer/environment.js'; import { Logger } from './duckplayer/util.js'; -import { MetricsReporter, EXCEPTION_KIND_INITIAL_SETUP_ERROR } from '../../../special-pages/shared/metrics/metrics-reporter.js'; +import { MetricsReporter, EXCEPTION_KIND_INITIAL_SETUP_ERROR } from '../../../metrics/metrics-reporter.js'; /** * @import {DuckPlayerNativeSubFeature} from './duckplayer-native/sub-feature.js' diff --git a/injected/src/features/duck-player.js b/injected/src/features/duck-player.js index d8a7e7c74e..cdebf5be44 100644 --- a/injected/src/features/duck-player.js +++ b/injected/src/features/duck-player.js @@ -37,7 +37,7 @@ import { DuckPlayerOverlayMessages, OpenInDuckPlayerMsg, Pixel } from './duckpla import { isBeingFramed } from '../utils.js'; import { initOverlays } from './duckplayer/overlays.js'; import { Environment } from './duckplayer/environment.js'; -import { MetricsReporter } from '../../../special-pages/shared/metrics/metrics-reporter.js'; +import { MetricsReporter } from '../../../metrics/metrics-reporter.js'; /** * @typedef UserValues - A way to communicate user settings diff --git a/injected/src/features/duckplayer-native/messages.js b/injected/src/features/duckplayer-native/messages.js index 44ebd787b3..4a4d5365e0 100644 --- a/injected/src/features/duckplayer-native/messages.js +++ b/injected/src/features/duckplayer-native/messages.js @@ -1,5 +1,5 @@ import * as constants from './constants.js'; -import { MetricsReporter, EXCEPTION_KIND_MESSAGING_ERROR } from 'special-pages/shared/metrics/metrics-reporter.js'; +import { MetricsReporter, EXCEPTION_KIND_MESSAGING_ERROR } from '../../../..//metrics/metrics-reporter.js'; /** @import {YouTubeError} from './error-detection.js' */ /** @import {Environment} from '../duckplayer/environment.js' */ diff --git a/injected/src/features/duckplayer/overlay-messages.js b/injected/src/features/duckplayer/overlay-messages.js index 236856025d..a09c8ddd65 100644 --- a/injected/src/features/duckplayer/overlay-messages.js +++ b/injected/src/features/duckplayer/overlay-messages.js @@ -1,6 +1,6 @@ /* eslint-disable promise/prefer-await-to-then */ import * as constants from './constants.js'; -import { MetricsReporter, EXCEPTION_KIND_MESSAGING_ERROR } from 'special-pages/shared/metrics/metrics-reporter.js'; +import { MetricsReporter, EXCEPTION_KIND_MESSAGING_ERROR } from '../../../../metrics/metrics-reporter.js'; /** * @typedef {import("@duckduckgo/messaging").Messaging} Messaging diff --git a/injected/src/features/duckplayer/overlays.js b/injected/src/features/duckplayer/overlays.js index b38b3bd4c9..3e7dd39de2 100644 --- a/injected/src/features/duckplayer/overlays.js +++ b/injected/src/features/duckplayer/overlays.js @@ -2,7 +2,7 @@ import { DomState } from './util.js'; import { ClickInterception, Thumbnails } from './thumbnails.js'; import { VideoOverlay } from './video-overlay.js'; import { registerCustomElements } from './components/index.js'; -import { EXCEPTION_KIND_INITIAL_SETUP_ERROR } from 'special-pages/shared/metrics/metrics-reporter.js'; +import { EXCEPTION_KIND_INITIAL_SETUP_ERROR } from '../../../../metrics/metrics-reporter.js'; /** * @typedef {object} OverlayOptions diff --git a/injected/unit-test/verify-artifacts.js b/injected/unit-test/verify-artifacts.js index 484228c184..586555d3e3 100644 --- a/injected/unit-test/verify-artifacts.js +++ b/injected/unit-test/verify-artifacts.js @@ -7,7 +7,7 @@ const ROOT = join(cwd(import.meta.url), '..', '..'); console.log(ROOT); const BUILD = join(ROOT, 'build'); -let CSS_OUTPUT_SIZE = 770_000; +let CSS_OUTPUT_SIZE = 780_000; if (process.platform === 'win32') { CSS_OUTPUT_SIZE = CSS_OUTPUT_SIZE * 1.1; // 10% larger for Windows due to line endings } diff --git a/metrics/docs/readme.md b/metrics/docs/readme.md new file mode 100644 index 0000000000..9b8544c33b --- /dev/null +++ b/metrics/docs/readme.md @@ -0,0 +1,16 @@ +--- +title: Metrics +--- + +# Metrics + +Utility class for reporting metrics and exceptions to the native layer. + +This class provides standardized methods for sending metric events and exception reports +through the messaging system. It includes predefined metric names for common error types +and helper methods to construct and send metric events. + +Please see [metrics-reporter.js](../metrics-reporter.js) +for the message schema + +{@includeCode ../examples/metrics.js} \ No newline at end of file diff --git a/metrics/examples/metrics.js b/metrics/examples/metrics.js new file mode 100644 index 0000000000..f847ba0e65 --- /dev/null +++ b/metrics/examples/metrics.js @@ -0,0 +1,39 @@ +import { MetricsReporter } from '../metrics-reporter.js'; +import { Messaging, MessagingContext, TestTransportConfig } from '@duckduckgo/messaging'; + +const messaging = createMessaging(); +const metrics = new MetricsReporter(messaging); + +// Report a custom metric +metrics.reportMetric({ + metricName: 'exception', + params: { kind: 'abc', message: 'something went' }, +}); + +// Report an exception +metrics.reportException({ + message: 'Failed to load user data', + kind: 'NetworkError', +}); + +// Report an exception by passing an Error object +metrics.reportExceptionWithError(new Error('Missing params')); + +// test messaging example +function createMessaging() { + const context = new MessagingContext({ + context: 'test', + env: 'development', + featureName: 'testFeature', + }); + const config = new TestTransportConfig({ + notify() {}, + request() { + return Promise.resolve(null); + }, + subscribe() { + return () => {}; + }, + }); + return new Messaging(context, config); +} diff --git a/special-pages/shared/metrics/messages/reportMetric.notify.json b/metrics/messages/reportMetric.notify.json similarity index 100% rename from special-pages/shared/metrics/messages/reportMetric.notify.json rename to metrics/messages/reportMetric.notify.json diff --git a/special-pages/shared/metrics/metrics-reporter.js b/metrics/metrics-reporter.js similarity index 83% rename from special-pages/shared/metrics/metrics-reporter.js rename to metrics/metrics-reporter.js index 86163e5ca9..65dad034eb 100644 --- a/special-pages/shared/metrics/metrics-reporter.js +++ b/metrics/metrics-reporter.js @@ -1,43 +1,7 @@ -/** - * Utility class for reporting metrics and exceptions to the native layer. - * - * This class provides standardized methods for sending metric events and exception reports - * through the messaging system. It includes predefined metric names for common error types - * and helper methods to construct and send metric events. - * - * Please see https://duckduckgo.github.io/content-scope-scripts/interfaces/shared_messages.reportmetricnotification - * for the message schema - * - * @example - * ```javascript - * import { MetricsReporter } from './metrics-reporter.js'; - * - * const metrics = new MetricsReporter(messaging); - * - * // Report a custom metric - * metrics.reportMetric({ - * metricName: 'userAction', - * params: { action: 'buttonClick', page: 'home' } - * }); - * - * // Report an exception - * metrics.reportException({ - * message: 'Failed to load user data', - * kind: 'NetworkError' - * }); - * - * // Report an exception by passing an Error object - * metrics.reportExceptionWithError(new Error('Missing params')); - * ``` - * - * @module Metrics Reporter - */ - /** * @typedef {import('./types/metrics.ts').ExceptionMetric} ExceptionMetric * @typedef {import('./types/metrics.ts').ReportMetricEvent} ReportMetricEvent */ - import { createTypedMessages } from '@duckduckgo/messaging/lib/typed-messages.js'; /** Exception kind for generic errors */ diff --git a/metrics/package.json b/metrics/package.json new file mode 100644 index 0000000000..8ee31c5b5d --- /dev/null +++ b/metrics/package.json @@ -0,0 +1,12 @@ +{ + "private": true, + "name": "metrics", + "type": "module", + "scripts": { + "build": "node scripts/types.mjs", + "test-unit": "node --test unit-test/*" + }, + "dependencies": { + "@duckduckgo/messaging": "*" + } +} \ No newline at end of file diff --git a/metrics/scripts/types.mjs b/metrics/scripts/types.mjs new file mode 100644 index 0000000000..d8eeb9aef5 --- /dev/null +++ b/metrics/scripts/types.mjs @@ -0,0 +1,32 @@ +import { cwd, isLaunchFile } from '../../scripts/script-utils.js'; +import { join } from 'node:path'; +import { buildTypes } from '../../types-generator/build-types.mjs'; + +const root = join(cwd(import.meta.url), '..'); + +/** @type {Record} */ +const injectSchemaMapping = { + /** + * Add more mappings here + * - `schema` should be an absolute path to a valid JSON Schema document + * - `types` should be an absolute path to the output file + */ + 'Metrics Messages': { + schemaDir: join(root, 'messages'), + typesDir: join(root, 'types'), + exclude: process.platform === 'win32', + kind: 'single', + resolve: (_dirname) => '../metrics-reporter.js', + className: (_top) => 'MetricsReporter', + filename: `metrics.ts`, + }, +}; + +if (isLaunchFile(import.meta.url)) { + buildTypes(injectSchemaMapping) + // eslint-disable-next-line promise/prefer-await-to-then + .catch((error) => { + console.error(error); + process.exit(1); + }); +} diff --git a/metrics/types/metrics.ts b/metrics/types/metrics.ts new file mode 100644 index 0000000000..ab4429481d --- /dev/null +++ b/metrics/types/metrics.ts @@ -0,0 +1,36 @@ +/** + * These types are auto-generated from schema files. + * scripts/build-types.mjs is responsible for type generation. + * **DO NOT** edit this file directly as your changes will be lost. + * + * @module MetricsMessages Messages + */ + +export type ReportMetricEvent = ExceptionMetric; + +/** + * Requests, Notifications and Subscriptions from the MetricsMessages feature + */ +export interface MetricsMessagesMessages { + notifications: ReportMetricNotification; +} +/** + * Generated from @see "../messages/reportMetric.notify.json" + */ +export interface ReportMetricNotification { + method: 'reportMetric'; + params: ReportMetricEvent; +} +export interface ExceptionMetric { + metricName: 'exception'; + params: { + message: string; + kind?: string; + }; +} + +declare module '../metrics-reporter.js' { + export interface MetricsReporter { + notify: import('@duckduckgo/messaging/lib/shared-types').MessagingBase['notify']; + } +} diff --git a/special-pages/unit-test/metrics-reporter.mjs b/metrics/unit-test/metrics-reporter.mjs similarity index 98% rename from special-pages/unit-test/metrics-reporter.mjs rename to metrics/unit-test/metrics-reporter.mjs index 159c44242e..5fd941a4d0 100644 --- a/special-pages/unit-test/metrics-reporter.mjs +++ b/metrics/unit-test/metrics-reporter.mjs @@ -1,6 +1,6 @@ import { describe, it, mock, beforeEach } from 'node:test'; import assert from 'node:assert'; -import { MetricsReporter } from '../shared/metrics/metrics-reporter.js'; +import { MetricsReporter } from '../metrics-reporter.js'; describe('reportMetric', () => { let messaging; diff --git a/package-lock.json b/package-lock.json index a94083ec60..910808d1b8 100644 --- a/package-lock.json +++ b/package-lock.json @@ -10,7 +10,8 @@ "injected", "special-pages", "messaging", - "types-generator" + "types-generator", + "metrics" ], "dependencies": { "immutable-json-patch": "^6.0.1", @@ -62,6 +63,11 @@ "version": "1.0.0", "license": "ISC" }, + "metrics": { + "dependencies": { + "@duckduckgo/messaging": "*" + } + }, "node_modules/@apidevtools/json-schema-ref-parser": { "version": "11.7.2", "resolved": "https://registry.npmjs.org/@apidevtools/json-schema-ref-parser/-/json-schema-ref-parser-11.7.2.tgz", @@ -7148,6 +7154,10 @@ "node": ">= 8" } }, + "node_modules/metrics": { + "resolved": "metrics", + "link": true + }, "node_modules/micromatch": { "version": "4.0.8", "resolved": "https://registry.npmjs.org/micromatch/-/micromatch-4.0.8.tgz", diff --git a/package.json b/package.json index c40707272e..f009248859 100644 --- a/package.json +++ b/package.json @@ -34,7 +34,8 @@ "injected", "special-pages", "messaging", - "types-generator" + "types-generator", + "metrics" ], "devDependencies": { "@duckduckgo/eslint-config": "github:duckduckgo/eslint-config#v0.1.0", diff --git a/special-pages/pages/duckplayer/app/index.js b/special-pages/pages/duckplayer/app/index.js index c62bc625d4..8495e9de73 100644 --- a/special-pages/pages/duckplayer/app/index.js +++ b/special-pages/pages/duckplayer/app/index.js @@ -15,7 +15,7 @@ import { Components } from './components/Components.jsx'; import { MobileApp } from './components/MobileApp.jsx'; import { DesktopApp } from './components/DesktopApp.jsx'; import { YouTubeErrorProvider } from './providers/YouTubeErrorProvider'; -import { EXCEPTION_KIND_INIT_ERROR, EXCEPTION_KIND_INITIAL_SETUP_ERROR } from '../../../shared/metrics/metrics-reporter.js'; +import { EXCEPTION_KIND_INIT_ERROR, EXCEPTION_KIND_INITIAL_SETUP_ERROR } from '../../../../metrics/metrics-reporter.js'; /** @typedef {import('../types/duckplayer').YouTubeError} YouTubeError */ diff --git a/special-pages/pages/duckplayer/integration-tests/duck-player.js b/special-pages/pages/duckplayer/integration-tests/duck-player.js index 043cab52bb..c8032fa1e3 100644 --- a/special-pages/pages/duckplayer/integration-tests/duck-player.js +++ b/special-pages/pages/duckplayer/integration-tests/duck-player.js @@ -568,7 +568,7 @@ export class DuckPlayerPage { } /** - * @param {import('../../../shared/metrics/types/metrics.js').ReportMetricEvent} evt + * @param {import('../../../../metrics/types/metrics.js').ReportMetricEvent} evt */ async didSendReportMetric(evt) { const events = await this.mocks.waitForCallCount({ method: 'reportMetric', count: 1 }); diff --git a/special-pages/pages/duckplayer/src/index.js b/special-pages/pages/duckplayer/src/index.js index 72e055fd14..69087cc856 100644 --- a/special-pages/pages/duckplayer/src/index.js +++ b/special-pages/pages/duckplayer/src/index.js @@ -4,7 +4,7 @@ import { createSpecialPageMessaging } from '../../../shared/create-special-page- import { init } from '../app/index.js'; import { initStorage } from './storage.js'; import '../../../shared/live-reload.js'; -import { MetricsReporter, EXCEPTION_KIND_MESSAGING_ERROR } from '../../../shared/metrics/metrics-reporter.js'; +import { MetricsReporter, EXCEPTION_KIND_MESSAGING_ERROR } from '../../../../metrics/metrics-reporter.js'; export class DuckplayerPage { /** diff --git a/special-pages/shared/metrics/types/metrics.ts b/special-pages/shared/metrics/types/metrics.ts deleted file mode 100644 index 42eb8fe191..0000000000 --- a/special-pages/shared/metrics/types/metrics.ts +++ /dev/null @@ -1,36 +0,0 @@ -/** - * These types are auto-generated from schema files. - * scripts/build-types.mjs is responsible for type generation. - * **DO NOT** edit this file directly as your changes will be lost. - * - * @module Metrics Messages - */ - -export type ReportMetricEvent = ExceptionMetric; - -/** - * Requests, Notifications and Subscriptions from the Metrics feature - */ -export interface MetricsMessages { - notifications: ReportMetricNotification; -} -/** - * Generated from @see "../messages/reportMetric.notify.json" - */ -export interface ReportMetricNotification { - method: "reportMetric"; - params: ReportMetricEvent; -} -export interface ExceptionMetric { - metricName: "exception"; - params: { - message: string; - kind?: string; - }; -} - -declare module "../metrics-reporter.js" { - export interface MetricsReporter { - notify: import("@duckduckgo/messaging/lib/shared-types").MessagingBase['notify'] - } -} \ No newline at end of file diff --git a/special-pages/types.mjs b/special-pages/types.mjs index 367e492cac..95f2c9049d 100644 --- a/special-pages/types.mjs +++ b/special-pages/types.mjs @@ -29,17 +29,6 @@ for (const pageListElement of pageList) { }; } -/* Shared types */ -specialPagesTypes.metrics = { - schemaDir: join(specialPagesRoot, 'shared', 'metrics', 'messages'), - typesDir: join(specialPagesRoot, 'shared', 'metrics', 'types'), - exclude: process.platform === 'win32', - kind: 'single', - resolve: (_dirname) => '../metrics-reporter.js', - className: (_top) => 'MetricsReporter', - filename: `metrics.ts`, -}; - if (isLaunchFile(import.meta.url)) { buildTypes(specialPagesTypes).catch((error) => { console.error(error); diff --git a/tsconfig.json b/tsconfig.json index 6a96819b17..ddc9fddbc4 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -27,6 +27,7 @@ "types-generator", "special-pages", "messaging", + "metrics", "playwright.config.js", "injected", "injected/src/globals.d.ts", diff --git a/typedoc.js b/typedoc.js index 38dbab4770..1f2a20a36f 100644 --- a/typedoc.js +++ b/typedoc.js @@ -7,6 +7,7 @@ const config = { 'special-pages/pages/new-tab/app/new-tab.md', 'special-pages/pages/history/app/history.md', 'injected/docs/*.md', + 'metrics/docs/*.md', 'messaging/docs/messaging.md', ], @@ -35,8 +36,7 @@ const config = { 'special-pages/pages/special-error/app/types.js', 'special-pages/pages/new-tab/app/favorites/constants.js', 'special-pages/pages/**/types/*.ts', - 'special-pages/shared/types/*.ts', - 'special-pages/shared/metrics-reporter.js', + 'metrics/metrics-reporter.js', ], categoryOrder: ['Special Pages', 'Content Scope Scripts Integrations', 'Other'], out: 'docs', From f5c7191ee34036084a5cecb45e42c4a7e83eac1d Mon Sep 17 00:00:00 2001 From: Marcos Gurgel Date: Fri, 1 Aug 2025 15:33:07 +0100 Subject: [PATCH 27/28] Metrics types --- metrics/types/metrics.ts | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/metrics/types/metrics.ts b/metrics/types/metrics.ts index ab4429481d..0e8877b524 100644 --- a/metrics/types/metrics.ts +++ b/metrics/types/metrics.ts @@ -12,25 +12,25 @@ export type ReportMetricEvent = ExceptionMetric; * Requests, Notifications and Subscriptions from the MetricsMessages feature */ export interface MetricsMessagesMessages { - notifications: ReportMetricNotification; + notifications: ReportMetricNotification; } /** * Generated from @see "../messages/reportMetric.notify.json" */ export interface ReportMetricNotification { - method: 'reportMetric'; - params: ReportMetricEvent; + method: "reportMetric"; + params: ReportMetricEvent; } export interface ExceptionMetric { - metricName: 'exception'; - params: { - message: string; - kind?: string; - }; + metricName: "exception"; + params: { + message: string; + kind?: string; + }; } -declare module '../metrics-reporter.js' { - export interface MetricsReporter { - notify: import('@duckduckgo/messaging/lib/shared-types').MessagingBase['notify']; - } -} +declare module "../metrics-reporter.js" { + export interface MetricsReporter { + notify: import("@duckduckgo/messaging/lib/shared-types").MessagingBase['notify'] + } +} \ No newline at end of file From 8338afb94f513e007a1f543c040e7e81b5cb7e61 Mon Sep 17 00:00:00 2001 From: Marcos Gurgel Date: Fri, 1 Aug 2025 15:41:14 +0100 Subject: [PATCH 28/28] Exclude metrics/types from prettier --- .prettierignore | 1 + 1 file changed, 1 insertion(+) diff --git a/.prettierignore b/.prettierignore index 007b36ef7a..a65a70802d 100644 --- a/.prettierignore +++ b/.prettierignore @@ -4,6 +4,7 @@ docs/**/* injected/src/types special-pages/pages/**/types special-pages/shared/**/types +metrics/types injected/integration-test/extension/contentScope.js injected/src/features/Scriptlets **/*.json