Track frequency of unhandled JS errors with MC Stats (#50155)

* Add bumpStat and fix tests

* Add changelog

* chore: Update dependencies and add @woocommerce/tracks for remote logging

* feat: Track frequency of unhandled JS errors with bumpStat

* chore: Update error boundary to log unhandled JS errors with bumpStat

* Add changelog

* Fix lint

* Check if tracks is enabled before bumping stats

* Fix test

* Fix lint

* chore: Refactor buildQuerystring to buildQueryParams for clarity and consistency

* Add bumpStat to wc tracks mock

packages/js/internal-js-tests/changelog/add-bump-stats-error

@@ -0,0 +1,4 @@
+Significance: patch
+Type: add
+
+Add bumpStat to woocommerce-tracks mock

packages/js/internal-js-tests/src/mocks/woocommerce-tracks.js

@@ -3,4 +3,5 @@
 module.exports = {
 	recordEvent: jest.fn(),
 	recordPageView: jest.fn(),
+	bumpStat: jest.fn(),
 };

packages/js/remote-logging/changelog/add-bump-stats-error

@@ -0,0 +1,4 @@
+Significance: patch
+Type: add
+
+Track frequency of unhandled JS errors with MC Stats

packages/js/remote-logging/package.json

@@ -53,6 +53,7 @@
 		]
 	},
 	"dependencies": {
+		"@woocommerce/tracks": "workspace:*",
 		"@wordpress/hooks": "wp-6.0",
 		"debug": "^4.3.4",
 		"tracekit": "^0.4.6"
@@ -152,6 +153,9 @@
 				"node_modules/@woocommerce/eslint-plugin/configs",
 				"node_modules/@woocommerce/eslint-plugin/rules",
 				"node_modules/@woocommerce/eslint-plugin/index.js",
+				"node_modules/@woocommerce/tracks/build",
+				"node_modules/@woocommerce/tracks/build-module",
+				"node_modules/@woocommerce/tracks/build-types",
 				"package.json"
 			]
 		}

packages/js/remote-logging/src/remote-logger.ts

@@ -5,6 +5,7 @@ import debugFactory from 'debug';
 import { getSetting } from '@woocommerce/settings';
 import TraceKit from 'tracekit';
 import { applyFilters } from '@wordpress/hooks';
+import { bumpStat } from '@woocommerce/tracks';
 
 /**
  * Internal dependencies
@@ -187,6 +188,9 @@ export class RemoteLogger {
 			return;
 		}
 
+		// Bump the stat for unhandled JS errors to track the frequency of these errors.
+		bumpStat( 'error', 'unhandled-js-errors' );
+
 		if ( this.isRateLimited() ) {
 			return;
 		}

packages/js/tracks/README.md

@@ -27,8 +27,8 @@ recordEvent( 'page_view', { path } )
 
 | Param | Type | Description |
 | --- | --- | --- |
-| eventName | <code>String</code> | The name of the event to record, don't include the `wcadmin_` prefix |
-| eventProperties | <code>Object</code> | Event properties to include in the event |
+| eventName | `String` | The name of the event to record, don't include the `wcadmin_` prefix |
+| eventProperties | `Object` | Event properties to include in the event |
 
 ### queueRecordEvent( eventName, eventProperties )
 
@@ -38,22 +38,45 @@ This allows you to delay tracks events that would otherwise cause a race conditi
 For example, when we trigger `wcadmin_tasklist_appearance_continue_setup` we're simultaneously moving the user to a new page via
 `window.location`. This is an example of a race condition that should be avoided by enqueueing the event,
 and therefore running it on the next pageview.
-
 | Param | Type | Description |
 | --- | --- | --- |
-| eventName | <code>String</code> | The name of the event to record, don't include the `wcadmin_` prefix |
-| eventProperties | <code>Object</code> | Event properties to include in the event |
+| eventName | `String` | The name of the event to record, don't include the `wcadmin_` prefix |
+| eventProperties | `Object` | Event properties to include in the event |
 
-### recordPageView( eventName, eventProperties )
+### recordPageView( path, extraProperties )
 
 Record a page view to Tracks.
 
 | Param | Type | Description |
 | --- | --- | --- |
-| path | <code>String</code> | Path the page/path to record a page view for |
-| extraProperties | <code>Object</code> | Extra event properties to include in the event |
+| path | `String` | Path the page/path to record a page view for |
+| extraProperties | `Object` | Extra event properties to include in the event |
 
-# Debugging
+### bumpStat( statName, statValue )
+
+Bump a stat or group of stats.
+
+```typescript
+import { bumpStat } from '@woocommerce/tracks';
+
+// Bump a single stat
+bumpStat( 'stat_name', 'stat_value' );
+
+// Bump multiple stats
+bumpStat( {
+  stat1: 'value1',
+  stat2: 'value2'
+} );
+```
+
+| Param | Type | Description |
+| --- | --- | --- |
+| statName | `String` or `Object` | The name of the stat to bump, or an object of stat names and values |
+| statValue | `String` | The value for the stat (only used when statName is a string) |
+
+Note: Stat names are automatically prefixed with `x_woocommerce-`. Stat tracking is disabled in development mode.
+
+## Debugging
 
 When debugging is activated info for each recorded Tracks event is logged to the browser console.
 

packages/js/tracks/changelog/add-bump-stats-error

@@ -0,0 +1,4 @@
+Significance: minor
+Type: add
+
+Add bumpStats and fix unit test tooling

packages/js/tracks/jest.config.json

@@ -0,0 +1,7 @@
+{
+	"rootDir": "./",
+	"roots": [
+		"<rootDir>/src"
+	],
+	"preset": "./node_modules/@woocommerce/internal-js-tests/jest-preset.js"
+}

packages/js/tracks/package.json

@@ -47,6 +47,7 @@
 		"lint:fix:lang:js": "eslint src --fix",
 		"lint:lang:js": "eslint src",
 		"prepack": "pnpm build",
+		"test:js": "jest --config ./jest.config.json --passWithNoTests",
 		"watch:build": "pnpm --if-present --workspace-concurrency=Infinity --filter=\"$npm_package_name...\" --parallel '/^watch:build:project:.*$/'",
 		"watch:build:project": "pnpm --if-present run '/^watch:build:project:.*$/'",
 		"watch:build:project:cjs": "wireit",
@@ -55,7 +56,10 @@
 	"devDependencies": {
 		"@babel/core": "^7.23.5",
 		"@types/debug": "^4.1.12",
+		"@types/node": "^16.18.68",
+		"@types/jest": "^27.5.2",
 		"@woocommerce/eslint-plugin": "workspace:*",
+		"@woocommerce/internal-js-tests": "workspace:*",
 		"concurrently": "^7.6.0",
 		"eslint": "^8.55.0",
 		"jest": "~27.5.1",
@@ -121,6 +125,9 @@
 		"dependencyOutputs": {
 			"allowUsuallyExcludedPaths": true,
 			"files": [
+				"node_modules/@woocommerce/internal-js-tests/build",
+				"node_modules/@woocommerce/internal-js-tests/build-module",
+				"node_modules/@woocommerce/internal-js-tests/jest-preset.js",
 				"node_modules/@woocommerce/eslint-plugin/configs",
 				"node_modules/@woocommerce/eslint-plugin/rules",
 				"node_modules/@woocommerce/eslint-plugin/index.js",

packages/js/tracks/src/index.ts

@@ -7,6 +7,7 @@ import debug from 'debug';
  * Internal dependencies
  */
 import { isDevelopmentMode } from './utils';
+export { bumpStat } from './stats';
 
 /**
  * Module variables

packages/js/tracks/src/stats.ts

@@ -0,0 +1,87 @@
+/**
+ * External dependencies
+ */
+import debug from 'debug';
+
+/**
+ * Internal dependencies
+ */
+import { isDevelopmentMode } from './utils';
+
+/**
+ * Module variables
+ */
+const tracksDebug = debug( 'wc-admin:tracks:stats' );
+const GROUP_PREFIX = 'x_woocommerce-';
+
+/**
+ * Builds a query parameters from the given group and name parameters.
+ *
+ * This will automatically add the prefix `x_woocommerce-` to the group name.
+ *
+ * @param {Record<string, string> | string} group  - The group of stats or a single stat name.
+ * @param {string}                          [name] - The name of the stat if group is a string.
+ *
+ * @return {URLSearchParams} The constructed querys.
+ */
+function buildQueryParams(
+	group: Record< string, string > | string,
+	name: string
+): URLSearchParams {
+	const params = new URLSearchParams();
+	params.append( 'v', 'wpcom-no-pv' );
+
+	if ( typeof group !== 'object' ) {
+		params.append( `${ GROUP_PREFIX }${ group }`, name );
+	} else {
+		Object.entries( group as Record< string, string > ).forEach(
+			( [ key, value ] ) => {
+				params.append( `${ GROUP_PREFIX }${ key }`, value );
+			}
+		);
+	}
+
+	// Add a random number to the query string to avoid caching.
+	params.append( 't', Math.random().toString() );
+
+	return params;
+}
+
+/**
+ * Bumps a stat or group of stats.
+ *
+ * @param {Record<string, string> | string} group  - The group of stats or a single stat name.
+ * @param {string}                          [name] - The name of the stat if group is a string.
+ * @return {boolean} True if the stat was successfully bumped, false otherwise.
+ */
+export function bumpStat(
+	group: Record< string, string > | string,
+	name = ''
+): boolean {
+	if ( typeof group === 'object' ) {
+		tracksDebug( 'Bumping stats %o', group );
+	} else {
+		tracksDebug( 'Bumping stat %s:%s', group, name );
+
+		if ( ! name ) {
+			tracksDebug( 'No stat name provided for group %s', group );
+			return false;
+		}
+	}
+
+	const shouldBumpStat =
+		! isDevelopmentMode &&
+		!! window.wcTracks &&
+		!! window.wcTracks.isEnabled;
+
+	if ( ! shouldBumpStat ) {
+		return false;
+	}
+
+	const params = buildQueryParams( group, name );
+	new window.Image().src = `${
+		document.location.protocol
+	}//pixel.wp.com/g.gif?${ params.toString() }`;
+
+	return true;
+}

packages/js/tracks/src/test/stats.ts

@@ -0,0 +1,82 @@
+/**
+ * Internal dependencies
+ */
+import { bumpStat } from '../stats';
+
+jest.mock( '../utils', () => ( {
+	isDevelopmentMode: false,
+} ) );
+
+declare global {
+	interface Window {
+		Image: typeof Image;
+	}
+}
+
+describe( 'bumpStat', () => {
+	let originalImage: typeof Image;
+	let mockImage: { src: string };
+
+	beforeEach( () => {
+		originalImage = window.Image;
+		mockImage = { src: '' };
+		window.Image = jest.fn( () => mockImage ) as unknown as typeof Image;
+		window.wcTracks = {
+			isEnabled: true,
+			validateEvent: jest.fn(),
+			recordEvent: jest.fn(),
+		};
+	} );
+
+	afterEach( () => {
+		window.Image = originalImage;
+		jest.resetAllMocks();
+	} );
+
+	it( 'should not bump stats when wcTracks is not enabled', () => {
+		window.wcTracks.isEnabled = false;
+		const result = bumpStat( 'group', 'name' );
+		expect( result ).toBe( false );
+		expect( window.Image ).not.toHaveBeenCalled();
+	} );
+
+	it( 'should not bump stats in development mode', () => {
+		jest.resetModules();
+		jest.doMock( '../utils', () => ( {
+			isDevelopmentMode: true,
+		} ) );
+		// eslint-disable-next-line @typescript-eslint/no-var-requires
+		const { bumpStat: bumpStatDev } = require( '../stats' );
+
+		const result = bumpStatDev( 'group', 'name' );
+		expect( result ).toBe( false );
+		expect( window.Image ).not.toHaveBeenCalled();
+	} );
+
+	it( 'should not bump stats when name is empty given group is a string', () => {
+		const result = bumpStat( 'group', '' );
+
+		expect( result ).toBe( false );
+		expect( window.Image ).not.toHaveBeenCalled();
+	} );
+
+	it( 'should bump a single stat', () => {
+		const result = bumpStat( 'group', 'name' );
+
+		expect( result ).toBe( true );
+		expect( window.Image ).toHaveBeenCalledTimes( 1 );
+		expect( mockImage.src ).toMatch(
+			/^https?:\/\/pixel\.wp\.com\/g\.gif\?v=wpcom-no-pv&x_woocommerce-group=name&t=/
+		);
+	} );
+
+	it( 'should bump multiple stats', () => {
+		const result = bumpStat( { stat1: 'value1', stat2: 'value2' } );
+
+		expect( result ).toBe( true );
+		expect( window.Image ).toHaveBeenCalledTimes( 1 );
+		expect( mockImage.src ).toMatch(
+			/^https?:\/\/pixel\.wp\.com\/g\.gif\?v=wpcom-no-pv&x_woocommerce-stat1=value1&x_woocommerce-stat2=value2&t=/
+		);
+	} );
+} );

plugins/woocommerce-admin/client/error-boundary/index.tsx

@@ -5,6 +5,7 @@ import { Component, ReactNode, ErrorInfo } from 'react';
 import { __ } from '@wordpress/i18n';
 import { Button } from '@wordpress/components';
 import { captureException } from '@woocommerce/remote-logging';
+import { bumpStat } from '@woocommerce/tracks';
 /**
  * Internal dependencies
  */
@@ -38,6 +39,8 @@ export class ErrorBoundary extends Component<
 	componentDidCatch( error: Error, errorInfo: ErrorInfo ) {
 		this.setState( { errorInfo } );
 
+		bumpStat( 'error', 'unhandled-js-error-during-render' );
+
 		// Limit the component stack to 10 calls so we don't send too much data.
 		const componentStack = errorInfo.componentStack
 			.trim()

plugins/woocommerce/changelog/add-bump-stats-error

@@ -0,0 +1,4 @@
+Significance: patch
+Type: add
+
+Track frequency of unhandled JS errors with MC Stats

pnpm-lock.yaml

@@ -2824,6 +2824,9 @@ importers:
 
   packages/js/remote-logging:
     dependencies:
+      '@woocommerce/tracks':
+        specifier: workspace:*
+        version: link:../tracks
       '@wordpress/hooks':
         specifier: wp-6.0
         version: 3.6.1
@@ -2892,9 +2895,18 @@ importers:
       '@types/debug':
         specifier: ^4.1.12
         version: 4.1.12
+      '@types/jest':
+        specifier: ^27.5.2
+        version: 27.5.2
+      '@types/node':
+        specifier: ^16.18.68
+        version: 16.18.68
       '@woocommerce/eslint-plugin':
         specifier: workspace:*
         version: link:../eslint-plugin
+      '@woocommerce/internal-js-tests':
+        specifier: workspace:*
+        version: link:../internal-js-tests
       concurrently:
         specifier: ^7.6.0
         version: 7.6.0