From ec0400bf475d84bfee9de072ae16f5f5073a34f1 Mon Sep 17 00:00:00 2001 From: Paul D'Ambra Date: Fri, 7 Jun 2024 22:44:56 +0100 Subject: [PATCH 1/2] fix: circular refs shouldn't explode capture --- .../replay/sessionrecording.test.ts | 41 +++++++++++++++++++ src/extensions/replay/sessionrecording.ts | 28 ++++++++++++- 2 files changed, 68 insertions(+), 1 deletion(-) diff --git a/src/__tests__/extensions/replay/sessionrecording.test.ts b/src/__tests__/extensions/replay/sessionrecording.test.ts index 54b7eaec0..8790b375f 100644 --- a/src/__tests__/extensions/replay/sessionrecording.test.ts +++ b/src/__tests__/extensions/replay/sessionrecording.test.ts @@ -843,6 +843,47 @@ describe('SessionRecording', () => { expect(sessionRecording.started).toEqual(false) }) + it('can emit when there are circular references', () => { + sessionRecording.afterDecideResponse(makeDecideResponse({ sessionRecording: { endpoint: '/s/' } })) + sessionRecording.startIfEnabledOrStop() + + const someObject = { emit: 1 } + // the same object can be there multiple times + const circularObject: Record = { emit: someObject, again: someObject } + // but a circular reference will be replaced + circularObject.circularReference = circularObject + _emit(createFullSnapshot(circularObject)) + + expect(sessionRecording['buffer']).toEqual({ + data: [ + { + again: { + emit: 1, + }, + circularReference: { + again: { + emit: 1, + }, + // the circular reference is captured to the buffer, + // but it didn't explode when estimating size + circularReference: expect.any(Object), + emit: { + emit: 1, + }, + }, + data: {}, + emit: { + emit: 1, + }, + type: 2, + }, + ], + sessionId: sessionId, + size: 149, + windowId: 'windowId', + }) + }) + describe('console logs', () => { it('if not enabled, plugin is not used', () => { posthog.config.enable_recording_console_log = false diff --git a/src/extensions/replay/sessionrecording.ts b/src/extensions/replay/sessionrecording.ts index 190ad173c..4fee89594 100644 --- a/src/extensions/replay/sessionrecording.ts +++ b/src/extensions/replay/sessionrecording.ts @@ -96,6 +96,7 @@ interface SnapshotBuffer { windowId: string readonly mostRecentSnapshotTimestamp: number | null + add(properties: Properties): void } @@ -137,6 +138,30 @@ const newQueuedEvent = (rrwebMethod: () => void): QueuedRRWebEvent => ({ const LOGGER_PREFIX = '[SessionRecording]' +function circularReferenceReplacer() { + const ancestors: any[] = [] + return function (_key: string, value: any) { + if (isObject(value)) { + // `this` is the object that value is contained in, + // i.e., its direct parent. + while (ancestors.length > 0 && ancestors.at(-1) !== this) { + ancestors.pop() + } + if (ancestors.includes(value)) { + return '[Circular]' + } + ancestors.push(value) + return value + } else { + return value + } + } +} + +function estimateSize(event: eventWithTime): number { + return JSON.stringify(event, circularReferenceReplacer()).length +} + export class SessionRecording { private _endpoint: string private flushBufferTimer?: any @@ -479,6 +504,7 @@ export class SessionRecording { timestamp: timestamp(), }) } + private _startCapture() { if (isUndefined(Object.assign)) { // According to the rrweb docs, rrweb is not supported on IE11 and below: @@ -786,7 +812,7 @@ export class SessionRecording { // TODO: Re-add ensureMaxMessageSize once we are confident in it const event = truncateLargeConsoleLogs(throttledEvent) - const size = JSON.stringify(event).length + const size = estimateSize(event) this._updateWindowAndSessionIds(event) From 287b2b38f4714ce3297c2a31bbd3fa947d3b448a Mon Sep 17 00:00:00 2001 From: Paul D'Ambra Date: Fri, 7 Jun 2024 22:47:42 +0100 Subject: [PATCH 2/2] fix --- src/extensions/replay/sessionrecording.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/extensions/replay/sessionrecording.ts b/src/extensions/replay/sessionrecording.ts index 4fee89594..57d778c97 100644 --- a/src/extensions/replay/sessionrecording.ts +++ b/src/extensions/replay/sessionrecording.ts @@ -138,12 +138,14 @@ const newQueuedEvent = (rrwebMethod: () => void): QueuedRRWebEvent => ({ const LOGGER_PREFIX = '[SessionRecording]' +// taken from https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Errors/Cyclic_object_value#circular_references function circularReferenceReplacer() { const ancestors: any[] = [] return function (_key: string, value: any) { if (isObject(value)) { // `this` is the object that value is contained in, // i.e., its direct parent. + // @ts-expect-error - TS was unhappy with `this` on the next line but the code is copied in from MDN while (ancestors.length > 0 && ancestors.at(-1) !== this) { ancestors.pop() }