[O11Y-201] Prevent log spam for storage methods and check if storage is defined. (#75)

## Summary

Adding the ability to log once to prevent log spam.
Adding persistent storage availability checks.

## 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?
-->

## Does this work require review from our design team?

<!--
 Request review from julian-highlight / our design team
-->

Author: kinyoklion

.changeset/thirty-jeans-exist.md

@@ -0,0 +1,5 @@
+---
+'highlight.run': patch
+---
+
+Improve logging for persistent storage issues.

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

@@ -3,6 +3,9 @@ import {
 	shouldNetworkRequestBeRecorded,
 	shouldNetworkRequestBeTraced,
 } from '../client/listeners/network-listener/utils/utils'
+import { internalLogOnce } from '../sdk/util'
+
+type TestLogLevel = 'debug' | 'info' | 'error' | 'warn'
 
 describe('normalizeUrl', () => {
 	vi.stubGlobal('location', {
@@ -264,3 +267,123 @@ describe('shouldNetworkRequestBeRecorded', () => {
 		).toBe(false)
 	})
 })
+
+describe('internalLogOnce', () => {
+	let consoleWarnSpy: any
+	let consoleInfoSpy: any
+	let consoleErrorSpy: any
+	let consoleDebugSpy: any
+	let spyByLevel: Record<TestLogLevel, any>
+
+	beforeEach(() => {
+		consoleDebugSpy = vi
+			.spyOn(console, 'debug')
+			.mockImplementation(() => {})
+		consoleInfoSpy = vi.spyOn(console, 'info').mockImplementation(() => {})
+		consoleWarnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {})
+		consoleErrorSpy = vi
+			.spyOn(console, 'error')
+			.mockImplementation(() => {})
+		spyByLevel = {
+			debug: consoleDebugSpy,
+			info: consoleInfoSpy,
+			warn: consoleWarnSpy,
+			error: consoleErrorSpy,
+		}
+	})
+
+	afterEach(() => {
+		vi.restoreAllMocks()
+		vi.clearAllMocks()
+	})
+
+	it('logs a message only once for the same context and logOnceId', () => {
+		const context = 'test-context-1'
+		const logOnceId = 'test-id'
+		const message = 'test message'
+
+		// First call should log
+		internalLogOnce(context, logOnceId, 'warn', message)
+		expect(consoleWarnSpy).toHaveBeenCalledTimes(1)
+		expect(consoleWarnSpy).toHaveBeenCalledWith(
+			'[@launchdarkly plugins]: (test-context-1): ',
+			message,
+		)
+
+		// Second call with same context and logOnceId should not log
+		internalLogOnce(context, logOnceId, 'warn', message)
+		expect(consoleWarnSpy).toHaveBeenCalledTimes(1) // Still only called once
+
+		// Third call with same context and logOnceId should not log
+		internalLogOnce(context, logOnceId, 'warn', 'different message')
+		expect(consoleWarnSpy).toHaveBeenCalledTimes(1) // Still only called once
+	})
+
+	it('allows logging the same logOnceId in different contexts', () => {
+		const logOnceId = 'shared-id'
+		const context1 = 'test-context-2'
+		const context2 = 'test-context-3'
+		const message = 'test message'
+
+		// First call with context-1 should log
+		internalLogOnce(context1, logOnceId, 'warn', message)
+		expect(consoleWarnSpy).toHaveBeenCalledTimes(1)
+		expect(consoleWarnSpy).toHaveBeenLastCalledWith(
+			'[@launchdarkly plugins]: (test-context-2): ',
+			message,
+		)
+
+		// Call with context-2 and same logOnceId should also log
+		internalLogOnce(context2, logOnceId, 'warn', message)
+		expect(consoleWarnSpy).toHaveBeenCalledTimes(2)
+		expect(consoleWarnSpy).toHaveBeenLastCalledWith(
+			'[@launchdarkly plugins]: (test-context-3): ',
+			message,
+		)
+
+		// Second call with context-1 should not log
+		internalLogOnce(context1, logOnceId, 'warn', message)
+		expect(consoleWarnSpy).toHaveBeenCalledTimes(2) // Still only 2 calls
+
+		// Second call with context-2 should not log
+		internalLogOnce(context2, logOnceId, 'warn', message)
+		expect(consoleWarnSpy).toHaveBeenCalledTimes(2) // Still only 2 calls
+	})
+
+	it.each(['debug', 'info', 'error', 'warn'])(
+		'works with different log levels',
+		(level) => {
+			const context = 'test-context-4'
+			const logOnceId = `test-id-${level}`
+			const message = 'test message'
+
+			// Test with 'info' level
+			internalLogOnce(context, logOnceId, level as keyof Console, message)
+			expect(spyByLevel[level as TestLogLevel]).toHaveBeenCalledTimes(1)
+			expect(spyByLevel[level as TestLogLevel]).toHaveBeenCalledWith(
+				'[@launchdarkly plugins]: (test-context-4): ',
+				message,
+			)
+		},
+	)
+
+	it('handles multiple arguments correctly', () => {
+		const context = 'test-context-5'
+		const logOnceId = 'test-id'
+
+		internalLogOnce(context, logOnceId, 'warn', 'message1', 'message2', {
+			key: 'value',
+		})
+		expect(consoleWarnSpy).toHaveBeenCalledTimes(1)
+		expect(consoleWarnSpy).toHaveBeenCalledWith(
+			'[@launchdarkly plugins]: (test-context-5): ',
+			'message1',
+			'message2',
+			{ key: 'value' },
+		)
+
+		// Second call should not log
+		internalLogOnce(context, logOnceId, 'warn', 'different', 'args')
+		expect(consoleWarnSpy).toHaveBeenCalledTimes(1) // Still only called once
+	})
+})

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

@@ -1,6 +1,6 @@
 import Cookies from 'js-cookie'
 import { SESSION_PUSH_THRESHOLD } from '../constants/sessions'
-import { internalLog } from '../../sdk/util'
+import { internalLogOnce } from '../../sdk/util'
 
 export enum LocalStorageKeys {
 	CLIENT_ID = 'highlightClientID',
@@ -50,16 +50,41 @@ export const globalStorage = new Storage()
 export const cookieStorage = new CookieStorage()
 
 const getPersistentStorage = () => {
+	let storage:
+		| Storage
+		| typeof window.localStorage
+		| typeof window.sessionStorage
+		| null = null
+
 	try {
 		switch (mode) {
 			case 'localStorage':
-				return window.localStorage
+				storage = window.localStorage
+				break
 			case 'sessionStorage':
-				return window.sessionStorage
+				storage = window.sessionStorage
+				break
+		}
+		if (!storage) {
+			internalLogOnce(
+				'storage',
+				'getPersistentStorage',
+				'debug',
+				`persistent storage was not found for mode ${mode}; using global storage`,
+			)
+			storage = globalStorage
 		}
 	} catch (e) {
-		return globalStorage
+		internalLogOnce(
+			'storage',
+			'getPersistentStorage',
+			'debug',
+			`failed to get persistent storage; using global storage`,
+			e,
+		)
+		storage = globalStorage
 	}
+	return storage
 }
 
 export const setStorageMode = (m: Mode) => {
@@ -74,8 +99,9 @@ export const getItem = (key: string) => {
 	try {
 		return getPersistentStorage().getItem(key)
 	} catch (e) {
-		internalLog(
+		internalLogOnce(
 			`storage`,
+			'getItem',
 			'debug',
 			`failed to get item ${key}; using global storage`,
 			e,
@@ -89,8 +115,9 @@ export const setItem = (key: string, value: string) => {
 	try {
 		return getPersistentStorage().setItem(key, value)
 	} catch (e) {
-		internalLog(
+		internalLogOnce(
 			`storage`,
+			'setItem',
 			'debug',
 			`failed to set item ${key}; using global storage`,
 			e,
@@ -104,8 +131,9 @@ export const removeItem = (key: string) => {
 	try {
 		return getPersistentStorage().removeItem(key)
 	} catch (e) {
-		internalLog(
+		internalLogOnce(
 			`storage`,
+			'removeItem',
 			'debug',
 			`failed to remove item ${key}; using global storage`,
 			e,

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

@@ -1,3 +1,6 @@
+// Object for tracking previously logged messages.
+const loggedIds = Object.create(null)
+
 export const internalLog = (
 	context: string,
 	level: keyof Console,
@@ -8,6 +11,31 @@ export const internalLog = (
 	void reportLog(prefix, ...msg)
 }
 
+/**
+ * Utility to help avoid logging the same message multiple times.
+ *
+ * This is helpful to prevent spamming the logs with sticky error conditions.
+ * For example if local storage cannot be written to, then each attempt to
+ * write would fail, and logging each time would spam the logs.
+ *
+ * @param logOnceId - A message with this ID will only be logged one time for the given context.
+ * @param context - The context of the message.
+ * @param level - The level of the message.
+ * @param msg - The message to log.
+ */
+export const internalLogOnce = (
+	context: string,
+	logOnceId: string,
+	level: keyof Console,
+	...msg: any
+) => {
+	if (loggedIds[`${context}-${logOnceId}`]) {
+		return
+	}
+	loggedIds[`${context}-${logOnceId}`] = true
+	internalLog(context, level, ...msg)
+}
+
 const reportLog = async (prefix: string, ...msg: any) => {
 	try {
 		const { LDObserve } = await import('./LDObserve')