From 66a16ebd98b26c8697437fa21e29babe694edf2b Mon Sep 17 00:00:00 2001 From: Daniel Graham Date: Wed, 16 Jul 2025 15:44:42 -0700 Subject: [PATCH 1/3] refactor: use global networkObserver in session replay --- .../session-replay-browser/src/observers.ts | 92 ------------------- .../src/session-replay.ts | 23 +++-- .../test/session-replay.test.ts | 64 ++++--------- 3 files changed, 28 insertions(+), 151 deletions(-) delete mode 100644 packages/session-replay-browser/src/observers.ts diff --git a/packages/session-replay-browser/src/observers.ts b/packages/session-replay-browser/src/observers.ts deleted file mode 100644 index 663d35a0d..000000000 --- a/packages/session-replay-browser/src/observers.ts +++ /dev/null @@ -1,92 +0,0 @@ -import { getGlobalScope } from '@amplitude/analytics-core'; - -export interface NetworkRequestEvent { - timestamp: number; - type: 'fetch'; - method: string; - url: string; - status?: number; - duration?: number; - requestHeaders?: Record; - responseHeaders?: Record; - error?: { - name: string; - message: string; - }; -} - -export type NetworkEventCallback = (event: NetworkRequestEvent) => void; - -export class NetworkObservers { - private fetchObserver: (() => void) | null = null; - private eventCallback?: NetworkEventCallback; - - start(eventCallback: NetworkEventCallback) { - this.eventCallback = eventCallback; - this.observeFetch(); - } - - stop() { - this.fetchObserver?.(); - this.fetchObserver = null; - this.eventCallback = undefined; - } - - protected notifyEvent(event: NetworkRequestEvent) { - this.eventCallback?.(event); - } - - private observeFetch() { - const globalScope = getGlobalScope(); - if (!globalScope) return; - - const originalFetch = globalScope.fetch; - if (!originalFetch) return; - - globalScope.fetch = async (input: RequestInfo | URL, init?: RequestInit) => { - const startTime = Date.now(); - const requestEvent: NetworkRequestEvent = { - timestamp: startTime, - type: 'fetch', - method: init?.method || 'GET', // Fetch API defaulted to GET when no method is provided - url: input.toString(), - requestHeaders: init?.headers as Record, - }; - - try { - const response = await originalFetch(input, init); - const endTime = Date.now(); - - requestEvent.status = response.status; - requestEvent.duration = endTime - startTime; - - // Convert Headers - const headers: Record = {}; - response.headers.forEach((value, key) => { - headers[key] = value; - }); - requestEvent.responseHeaders = headers; - - this.notifyEvent(requestEvent); - return response; - } catch (error) { - const endTime = Date.now(); - requestEvent.duration = endTime - startTime; - - // Capture error information - const typedError = error as Error; - requestEvent.error = { - name: typedError.name || 'UnknownError', - message: typedError.message || 'An unknown error occurred', - }; - - this.notifyEvent(requestEvent); - throw error; - } - }; - - this.fetchObserver = () => { - globalScope.fetch = originalFetch; - }; - } -} diff --git a/packages/session-replay-browser/src/session-replay.ts b/packages/session-replay-browser/src/session-replay.ts index 0edaeaac7..380b0c8d6 100644 --- a/packages/session-replay-browser/src/session-replay.ts +++ b/packages/session-replay-browser/src/session-replay.ts @@ -5,6 +5,9 @@ import { getGlobalScope, ILogger, LogLevel, + networkObserver, + NetworkRequestEvent, + NetworkEventCallback, } from '@amplitude/analytics-core'; // Import only specific types to avoid pulling in the entire rrweb-types package @@ -46,8 +49,6 @@ import { VERSION } from './version'; import { EventCompressor } from './events/event-compressor'; import { SafeLoggerProvider } from './logger'; -// Import only the type for NetworkRequestEvent to keep type safety -import type { NetworkRequestEvent, NetworkObservers } from './observers'; import type { RecordFunction } from './utils/rrweb'; type PageLeaveFn = (e: PageTransitionEvent | Event) => void; @@ -66,7 +67,8 @@ export class SessionReplay implements AmplitudeSessionReplay { // Visible for testing only pageLeaveFns: PageLeaveFn[] = []; private scrollHook?: scrollCallback; - private networkObservers?: NetworkObservers; + private networkObservers?: typeof networkObserver; + private networkEventCallback?: NetworkEventCallback; private metadata: SessionReplayMetadata | undefined; // Cache the dynamically imported record function @@ -416,9 +418,11 @@ export class SessionReplay implements AmplitudeSessionReplay { await this.initializeNetworkObservers(); - this.networkObservers?.start((event: NetworkRequestEvent) => { - void this.addCustomRRWebEvent(CustomRRwebEvent.FETCH_REQUEST, event); + const networkEventCallback = new NetworkEventCallback((event: NetworkRequestEvent) => { + void this.addCustomRRWebEvent(CustomRRwebEvent.FETCH_REQUEST, event.toSerializable()); }); + this.networkEventCallback = networkEventCallback; + this.networkObservers?.subscribe(networkEventCallback); const { privacyConfig, interactionConfig, loggingConfig } = config; const hooks = interactionConfig?.enabled @@ -549,7 +553,7 @@ export class SessionReplay implements AmplitudeSessionReplay { this.loggerProvider.log('Session Replay capture stopping.'); this.recordCancelCallback && this.recordCancelCallback(); this.recordCancelCallback = null; - this.networkObservers?.stop(); + this.networkEventCallback && this.networkObservers?.unsubscribe(this.networkEventCallback); } catch (error) { const typedError = error as Error; this.loggerProvider.warn(`Error occurred while stopping replay capture: ${typedError.toString()}`); @@ -613,12 +617,7 @@ export class SessionReplay implements AmplitudeSessionReplay { private async initializeNetworkObservers(): Promise { if (this.config?.loggingConfig?.network?.enabled && !this.networkObservers) { - try { - const { NetworkObservers: NetworkObserversClass } = await import('./observers'); - this.networkObservers = new NetworkObserversClass(); - } catch (error) { - this.loggerProvider.warn('Failed to import or instantiate NetworkObservers:', error); - } + this.networkObservers = networkObserver; } } } diff --git a/packages/session-replay-browser/test/session-replay.test.ts b/packages/session-replay-browser/test/session-replay.test.ts index 2e430470b..01e703fce 100644 --- a/packages/session-replay-browser/test/session-replay.test.ts +++ b/packages/session-replay-browser/test/session-replay.test.ts @@ -194,7 +194,7 @@ describe('SessionReplay', () => { }); await sessionReplay.init(apiKey, mockOptions).promise; - const startSpy = jest.spyOn(NetworkObservers.prototype, 'start'); + const startSpy = jest.spyOn(AnalyticsCore.networkObserver, 'subscribe'); await sessionReplay.recordEvents(); expect(startSpy).toHaveBeenCalled(); }); @@ -219,40 +219,6 @@ describe('SessionReplay', () => { expect((sessionReplayWithoutConfig as any).networkObservers).toBeUndefined(); }); - test('should log warning when NetworkObservers import fails', async () => { - __setNamespaceConfig({ - sr_sampling_config: samplingConfig, - sr_privacy_config: {}, - sr_interaction_config: { - enabled: true, - }, - sr_logging_config: { - network: { - enabled: true, - }, - }, - }); - - // Mock the dynamic import to throw an error - jest.doMock('../src/observers', () => { - throw new Error('Import failed'); - }); - - await sessionReplay.init(apiKey, mockOptions).promise; - - // Call initializeNetworkObservers directly to test the catch block - await (sessionReplay as any).initializeNetworkObservers(); - - expect(mockLoggerProvider.warn).toHaveBeenCalledWith( - 'Failed to import or instantiate NetworkObservers:', - expect.any(Error), - ); - expect((sessionReplay as any).networkObservers).toBeUndefined(); - - // Clean up the mock - jest.dontMock('../src/observers'); - }); - test('should catch error and log a warn when initializing', async () => { // enable interaction config __setNamespaceConfig({ @@ -1045,6 +1011,8 @@ describe('SessionReplay', () => { }); describe('stopRecordingEvents', () => { test('it should catch errors as warnings', async () => { + const mockUnsubscribe = jest.fn(); + jest.spyOn(AnalyticsCore.networkObserver, 'unsubscribe').mockImplementation(mockUnsubscribe); await sessionReplay.init(apiKey, mockOptions).promise; const mockStopRecordingEvents = jest.fn().mockImplementation(() => { throw new Error('test error'); @@ -1934,16 +1902,8 @@ describe('SessionReplay', () => { describe('should call addCustomRRWebEvent with network request events', () => { test('should call addCustomRRWebEvent with network request events', async () => { // Mock the observers module before initialization - const mockStart = jest.fn(); - const mockNetworkObserversClass = jest.fn().mockImplementation(() => ({ - start: mockStart, - stop: jest.fn(), - })); - - jest.doMock('../src/observers', () => ({ - NetworkObservers: mockNetworkObserversClass, - NetworkRequestEvent: {} as any, - })); + const mockSubscribe = jest.fn(); + jest.spyOn(AnalyticsCore.networkObserver, 'subscribe').mockImplementation(mockSubscribe); __setNamespaceConfig({ sr_sampling_config: samplingConfig, @@ -1968,17 +1928,27 @@ describe('SessionReplay', () => { responseHeaders: {}, requestBody: '', responseBody: '', + toSerializable() { + return { + ...this, + }; + }, }; await sessionReplay.recordEvents(); - expect(mockStart).toHaveBeenCalled(); - const startCallback = mockStart.mock.calls[0][0] as (event: NetworkRequestEvent) => void; + expect(mockSubscribe).toHaveBeenCalled(); + const startCallback = mockSubscribe.mock.calls[0][0].callback as (event: NetworkRequestEvent) => void; startCallback(mockNetworkEvent); expect(addCustomRRWebEventSpy).toHaveBeenCalledWith(CustomRRwebEvent.FETCH_REQUEST, mockNetworkEvent); + const mockUnsubscribe = jest.fn(); + jest.spyOn(AnalyticsCore.networkObserver, 'unsubscribe').mockImplementation(mockUnsubscribe); + await sessionReplay.shutdown(); + expect(mockUnsubscribe).toHaveBeenCalled(); + jest.dontMock('../src/observers'); }); }); From 45e029d4d47ba3fb14dc5c25b7822c70fe0a8e8b Mon Sep 17 00:00:00 2001 From: Daniel Graham Date: Wed, 16 Jul 2025 16:22:35 -0700 Subject: [PATCH 2/3] again --- .../test/observers.test.ts | 212 ------------------ .../test/session-replay.test.ts | 15 +- 2 files changed, 4 insertions(+), 223 deletions(-) delete mode 100644 packages/session-replay-browser/test/observers.test.ts diff --git a/packages/session-replay-browser/test/observers.test.ts b/packages/session-replay-browser/test/observers.test.ts deleted file mode 100644 index 3116abe6c..000000000 --- a/packages/session-replay-browser/test/observers.test.ts +++ /dev/null @@ -1,212 +0,0 @@ -import { NetworkObservers, NetworkRequestEvent } from '../src/observers'; -import * as AnalyticsCore from '@amplitude/analytics-core'; -type PartialGlobal = Pick; - -// Test subclass to access protected methods -class TestNetworkObservers extends NetworkObservers { - public testNotifyEvent(event: NetworkRequestEvent) { - this.notifyEvent(event); - } -} - -describe('NetworkObservers', () => { - let networkObservers: TestNetworkObservers; - let mockFetch: jest.Mock; - let events: NetworkRequestEvent[] = []; - let globalScope: PartialGlobal; - - beforeEach(() => { - jest.useFakeTimers(); - events = []; - mockFetch = jest.fn(); - globalScope = { fetch: mockFetch }; - - jest.spyOn(AnalyticsCore, 'getGlobalScope').mockReturnValue(globalScope as typeof globalThis); - - networkObservers = new TestNetworkObservers(); - }); - - afterEach(() => { - jest.useRealTimers(); - jest.restoreAllMocks(); - }); - - const callback = (event: NetworkRequestEvent) => { - events.push(event); - }; - - describe('successful requests', () => { - it('should track successful fetch requests with headers', async () => { - // Create a simple mock response - const mockResponse = { - status: 200, - headers: { - forEach: (callback: (value: string, key: string) => void) => { - callback('application/json', 'content-type'); - callback('test-server', 'server'); - }, - }, - }; - mockFetch.mockResolvedValue(mockResponse); - - networkObservers.start(callback); - - const requestHeaders = { - 'Content-Type': 'application/json', - Authorization: 'Bearer token123', - }; - - await globalScope.fetch('https://api.example.com/data', { - method: 'POST', - headers: requestHeaders, - }); - - expect(events).toHaveLength(1); - expect(events[0]).toMatchObject({ - type: 'fetch', - method: 'POST', - url: 'https://api.example.com/data', - status: 200, - requestHeaders, - responseHeaders: { - 'content-type': 'application/json', - server: 'test-server', - }, - }); - expect(events[0].duration).toBeGreaterThanOrEqual(0); - }); - - it('should track successful fetch requests without headers', async () => { - const mockResponse = { - status: 200, - headers: { - forEach: jest.fn(), // Mock function that does nothing - }, - }; - mockFetch.mockResolvedValue(mockResponse); - - networkObservers.start(callback); - - await globalScope.fetch('https://api.example.com/data'); - - expect(events).toHaveLength(1); - expect(events[0]).toMatchObject({ - type: 'fetch', - method: 'GET', - url: 'https://api.example.com/data', - status: 200, - requestHeaders: undefined, - responseHeaders: {}, - }); - expect(events[0].duration).toBeGreaterThanOrEqual(0); - }); - }); - - describe('failed requests', () => { - it('should track network errors', async () => { - const networkError = new TypeError('Failed to fetch'); - mockFetch.mockRejectedValue(networkError); - - networkObservers.start(callback); - - await expect(globalScope.fetch('https://api.example.com/data')).rejects.toThrow('Failed to fetch'); - - expect(events).toHaveLength(1); - expect(events[0]).toMatchObject({ - type: 'fetch', - method: 'GET', - url: 'https://api.example.com/data', - error: { - name: 'TypeError', - message: 'Failed to fetch', - }, - }); - expect(events[0].duration).toBeGreaterThanOrEqual(0); - }); - - it('should handle non-Error throws', async () => { - mockFetch.mockRejectedValue('string error'); - - networkObservers.start(callback); - - await expect(globalScope.fetch('https://api.example.com/data')).rejects.toBe('string error'); - - expect(events).toHaveLength(1); - expect(events[0].error).toEqual({ - name: 'UnknownError', - message: 'An unknown error occurred', - }); - }); - }); - - describe('observer lifecycle', () => { - it('should stop tracking when stopped', async () => { - networkObservers.start(callback); - networkObservers.stop(); - - expect(globalScope.fetch).toBe(mockFetch); - - await mockFetch('https://api.example.com/data'); - expect(events).toHaveLength(0); - }); - - it('should handle missing global scope', () => { - jest.spyOn(AnalyticsCore, 'getGlobalScope').mockReturnValue(undefined); - - networkObservers.start(callback); - - expect(() => networkObservers.stop()).not.toThrow(); - }); - - it('should handle missing fetch', () => { - const scopeWithoutFetch = {} as typeof globalThis; - jest.spyOn(AnalyticsCore, 'getGlobalScope').mockReturnValue(scopeWithoutFetch); - - networkObservers.start(callback); - - expect(() => networkObservers.stop()).not.toThrow(); - }); - - it('should call eventCallback with request event data', async () => { - const mockCallback = jest.fn(); - const mockResponse = { - status: 200, - headers: { - forEach: (callback: (value: string, key: string) => void) => { - callback('application/json', 'content-type'); - }, - }, - }; - mockFetch.mockResolvedValue(mockResponse); - - networkObservers.start(mockCallback); - - await globalScope.fetch('https://api.example.com/data', { - method: 'POST', - headers: { 'Content-Type': 'application/json' }, - }); - - expect(mockCallback).toHaveBeenCalledTimes(1); - }); - - it('should handle notifyEvent with optional chaining', async () => { - const mockEvent = { - timestamp: Date.now(), - type: 'fetch' as const, - method: 'GET', - url: 'https://api.example.com/data', - }; - - // Test with callback - const mockCallback = jest.fn(); - networkObservers.start(mockCallback); - networkObservers.testNotifyEvent(mockEvent); - expect(mockCallback).toHaveBeenCalledWith(mockEvent); - - // Test without callback - networkObservers.stop(); - networkObservers.testNotifyEvent(mockEvent); - expect(mockCallback).toHaveBeenCalledTimes(1); // Still only called once - }); - }); -}); diff --git a/packages/session-replay-browser/test/session-replay.test.ts b/packages/session-replay-browser/test/session-replay.test.ts index 01e703fce..330d08861 100644 --- a/packages/session-replay-browser/test/session-replay.test.ts +++ b/packages/session-replay-browser/test/session-replay.test.ts @@ -4,9 +4,8 @@ /* eslint-disable @typescript-eslint/no-unsafe-member-access */ /* eslint-disable @typescript-eslint/no-unsafe-assignment */ import * as AnalyticsCore from '@amplitude/analytics-core'; -import { LogLevel, ILogger, ServerZone } from '@amplitude/analytics-core'; +import { LogLevel, ILogger, ServerZone, NetworkRequestEvent } from '@amplitude/analytics-core'; import { SessionReplayLocalConfig } from '../src/config/local-config'; -import { NetworkObservers, NetworkRequestEvent } from '../src/observers'; import { IDBFactory } from 'fake-indexeddb'; import { LoggingConfig, SessionReplayJoinedConfig } from '../src/config/types'; @@ -201,7 +200,7 @@ describe('SessionReplay', () => { test('should not start network observers when network logging is disabled in remote config', async () => { await sessionReplay.init(apiKey, mockOptions).promise; - const startSpy = jest.spyOn(NetworkObservers.prototype, 'start'); + const startSpy = jest.spyOn(AnalyticsCore.networkObserver, 'subscribe'); await sessionReplay.recordEvents(); expect(startSpy).not.toHaveBeenCalled(); }); @@ -209,13 +208,7 @@ describe('SessionReplay', () => { test('should not initialize network observers when config is undefined', async () => { // Create a new SessionReplay instance without initializing const sessionReplayWithoutConfig = new SessionReplay(); - - const networkObserversConstructorSpy = jest.spyOn(NetworkObservers.prototype, 'constructor' as any); - await (sessionReplayWithoutConfig as any).initializeNetworkObservers(); - - expect(networkObserversConstructorSpy).not.toHaveBeenCalled(); - expect((sessionReplayWithoutConfig as any).networkObservers).toBeUndefined(); }); @@ -1928,6 +1921,8 @@ describe('SessionReplay', () => { responseHeaders: {}, requestBody: '', responseBody: '', + startTime: Date.now(), + endTime: Date.now(), toSerializable() { return { ...this, @@ -1948,8 +1943,6 @@ describe('SessionReplay', () => { jest.spyOn(AnalyticsCore.networkObserver, 'unsubscribe').mockImplementation(mockUnsubscribe); await sessionReplay.shutdown(); expect(mockUnsubscribe).toHaveBeenCalled(); - - jest.dontMock('../src/observers'); }); }); From d9b1bba2940973b15526d7fce3e4143d18f562de Mon Sep 17 00:00:00 2001 From: Daniel Graham Date: Wed, 16 Jul 2025 21:55:35 -0700 Subject: [PATCH 3/3] add test server --- test-server/session-replay/index.html | 41 +++++++++++++++++++++++++++ 1 file changed, 41 insertions(+) create mode 100644 test-server/session-replay/index.html diff --git a/test-server/session-replay/index.html b/test-server/session-replay/index.html new file mode 100644 index 000000000..7507e9ae2 --- /dev/null +++ b/test-server/session-replay/index.html @@ -0,0 +1,41 @@ + + + + + Session Replay Test - Sample Rate 1 + + +
+

Session Replay Test Page

+

This page demonstrates Session Replay with sample rate 1 (100% of sessions recorded).

+
+ + + + \ No newline at end of file