fix: Fix an issue where metrics could have NaN or infinite values. (#220)

## Summary

Fix an issue where the `downlinkMax` metric could be infinite and
prevent successful metric exports.

Infinite/NaN would become `null` in JSON, and the collector will treat
the entire request payload as invalid if there is a null metric payload.

## How did you test this change?

<!--
Frontend - Leave a screencast or a screenshot to visually describe the
changes.
-->

## Are there any deployment considerations?

<!--
 Backend - Do we need to consider migrations or backfilling data?
-->

Author: kinyoklion

sdk/highlight-run/src/client/index.tsx

@@ -104,7 +104,7 @@ import {
 	setItem,
 	setStorageMode,
 } from './utils/storage'
-import { getDefaultDataURLOptions } from './utils/utils'
+import { getDefaultDataURLOptions, isMetricSafeNumber } from './utils/utils'
 import { type HighlightClientRequestWorker } from './workers/highlight-client-worker'
 import { payloadToBase64 } from './utils/payload'
 import HighlightClientWorker from './workers/highlight-client-worker?worker&inline'
@@ -1107,18 +1107,17 @@ SessionSecureID: ${this.sessionData.sessionSecureID}`,
 								value: payload.type.toString(),
 							})
 						}
-						Object.entries(payload).forEach(
-							([name, value]) =>
-								value &&
-								typeof value === 'number' &&
+						Object.entries(payload).forEach(([name, value]) => {
+							if (isMetricSafeNumber(value)) {
 								this.recordGauge({
 									name,
 									value: value as number,
 									category: MetricCategory.Performance,
 									group: window.location.href,
 									tags,
-								}),
-						)
+								})
+							}
+						})
 					},
 					this._recordingStartTime,
 				),

sdk/highlight-run/src/client/utils/utils.test.ts

@@ -0,0 +1,86 @@
+import { isMetricSafeNumber } from './utils'
+
+describe('isMetricSafeNumber', () => {
+	describe('valid numbers', () => {
+		const validNumbers = [
+			{ value: 42, description: 'positive integer' },
+			{ value: 0, description: 'zero' },
+			{ value: 1, description: 'one' },
+			{ value: -42, description: 'negative integer' },
+			{ value: -1, description: 'negative one' },
+			{ value: 3.14, description: 'positive float' },
+			{ value: 0.5, description: 'positive decimal' },
+			{ value: 1.0, description: 'float one' },
+			{ value: -3.14, description: 'negative float' },
+			{ value: -0.5, description: 'negative decimal' },
+			{ value: Number.MIN_VALUE, description: 'MIN_VALUE' },
+			{ value: Number.EPSILON, description: 'EPSILON' },
+			{ value: Number.MAX_SAFE_INTEGER, description: 'MAX_SAFE_INTEGER' },
+			{ value: Number.MAX_VALUE, description: 'MAX_VALUE' },
+		]
+
+		test.each(validNumbers)(
+			'should return true for $description ($value)',
+			({ value }) => {
+				expect(isMetricSafeNumber(value)).toBe(true)
+			},
+		)
+	})
+
+	describe('invalid numbers', () => {
+		const invalidNumbers = [
+			{ value: NaN, description: 'NaN' },
+			{ value: Number.NaN, description: 'Number.NaN' },
+			{ value: Infinity, description: 'Infinity' },
+			{
+				value: Number.POSITIVE_INFINITY,
+				description: 'Number.POSITIVE_INFINITY',
+			},
+			{ value: -Infinity, description: 'negative Infinity' },
+			{
+				value: Number.NEGATIVE_INFINITY,
+				description: 'Number.NEGATIVE_INFINITY',
+			},
+		]
+
+		test.each(invalidNumbers)(
+			'should return false for $description',
+			({ value }) => {
+				expect(isMetricSafeNumber(value)).toBe(false)
+			},
+		)
+	})
+
+	describe('non-number types', () => {
+		const nonNumbers = [
+			{ value: '42', description: 'numeric string' },
+			{ value: '3.14', description: 'decimal string' },
+			{ value: 'not a number', description: 'text string' },
+			{ value: '', description: 'empty string' },
+			{ value: true, description: 'boolean true' },
+			{ value: false, description: 'boolean false' },
+			{ value: {}, description: 'empty object' },
+			{ value: { value: 42 }, description: 'object with properties' },
+			{ value: [], description: 'empty array' },
+			{ value: [1, 2, 3], description: 'array with numbers' },
+			{ value: null, description: 'null' },
+			{ value: undefined, description: 'undefined' },
+			{ value: () => {}, description: 'arrow function' },
+			{ value: function () {}, description: 'function declaration' },
+			{ value: Symbol('test'), description: 'symbol' },
+			{ value: BigInt(42), description: 'BigInt constructor' },
+			{ value: 42n, description: 'BigInt literal' },
+			{ value: new Date(), description: 'Date object' },
+			{ value: new Date('2023-01-01'), description: 'Date with string' },
+			{ value: /test/, description: 'RegExp literal' },
+			{ value: new RegExp('test'), description: 'RegExp constructor' },
+		]
+
+		test.each(nonNumbers)(
+			'should return false for $description',
+			({ value }) => {
+				expect(isMetricSafeNumber(value)).toBe(false)
+			},
+		)
+	})
+})

sdk/highlight-run/src/client/utils/utils.ts

@@ -250,3 +250,16 @@ export function getDefaultDataURLOptions(): {
 		quality: 0.6,
 	}
 }
+
+/**
+ * Check that a value is safe to use as a metric.
+ *
+ * This means the number must be a number, be finite, and not NaN.
+ * NaN and Infinity serialize to null, which produces an invalid metric
+ * payload.
+ * @param value The value to check.
+ * @returns True if the value is safe to use as a metric, false otherwise.
+ */
+export function isMetricSafeNumber(value: unknown): boolean {
+	return typeof value === 'number' && !isNaN(value) && isFinite(value)
+}

sdk/highlight-run/src/sdk/observe.ts

@@ -79,6 +79,7 @@ import { recordException } from '../client/otel/recordException'
 import { ObserveOptions } from '../client/types/observe'
 import { WebTracerProvider } from '@opentelemetry/sdk-trace-web'
 import { MeterProvider } from '@opentelemetry/sdk-metrics'
+import { isMetricSafeNumber } from '../client/utils/utils'
 
 export class ObserveSDK implements Observe {
 	/** Verbose project ID that is exposed to users. Legacy users may still be using ints. */
@@ -602,16 +603,15 @@ export class ObserveSDK implements Observe {
 			}
 			Object.entries(payload)
 				.filter(([name]) => name !== 'relativeTimestamp')
-				.forEach(
-					([name, value]) =>
-						value &&
-						typeof value === 'number' &&
+				.forEach(([name, value]) => {
+					if (isMetricSafeNumber(value)) {
 						this.recordGauge({
 							name,
 							value: value as number,
 							attributes,
-						}),
-				)
+						})
+					}
+				})
 		}, new Date().getTime())
 
 		const { getDeviceDetails } = getPerformanceMethods()