Skip to content

Commit

Permalink
improvement(storeUserSession) guard object stringifier against circul…
Browse files Browse the repository at this point in the history
…ar references (#715)
  • Loading branch information
codecapitano authored Oct 28, 2024
1 parent b82e827 commit 1e796c4
Show file tree
Hide file tree
Showing 7 changed files with 74 additions and 3 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,9 @@

## Next

- Improvement (`@grafana/faro-web-sdk`): Guards user session stringifier against circular object
references (#715)

## 1.11.0

- Improvement (`@grafana/faro-web-sdk`): The console instrumentation now sends an `Error` signal
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -217,4 +217,21 @@ describe('Persistent Sessions Manager.', () => {
const newSession: FaroUserSession = JSON.parse(mockStorage[STORAGE_KEY]!);
expect(newSession.sessionId).toBe(manualSetSessionId);
});

it('Stores in the locals storage even if it contains objects with circular references.', () => {
const circularObject = { a: 'b' };
(circularObject as any).circular = circularObject;

const storedSession = {
sessionId: mockInitialSessionId,
isSampled: true,
circularObject,
lastActivity: fakeSystemTime,
started: fakeSystemTime,
};

expect(() => {
PersistentSessionsManager.storeUserSession(storedSession);
}).not.toThrow();
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { faro } from '@grafana/faro-core';
import type { Meta } from '@grafana/faro-core';

import { throttle } from '../../../utils';
import { getCircularDependencyReplacer } from '../../../utils/json';
import { getItem, removeItem, setItem, webStorageType } from '../../../utils/webStorage';

import { isSampled } from './sampling';
Expand All @@ -27,7 +28,11 @@ export class PersistentSessionsManager {
}

static storeUserSession(session: FaroUserSession): void {
setItem(STORAGE_KEY, JSON.stringify(session), PersistentSessionsManager.storageTypeLocal);
setItem(
STORAGE_KEY,
JSON.stringify(session, getCircularDependencyReplacer()),
PersistentSessionsManager.storageTypeLocal
);
}

static fetchUserSession(): FaroUserSession | null {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { faro } from '@grafana/faro-core';
import type { Meta } from '@grafana/faro-core';

import { throttle } from '../../../utils';
import { getCircularDependencyReplacer } from '../../../utils/json';
import { getItem, removeItem, setItem, webStorageType } from '../../../utils/webStorage';

import { isSampled } from './sampling';
Expand All @@ -27,7 +28,11 @@ export class VolatileSessionsManager {
}

static storeUserSession(session: FaroUserSession): void {
setItem(STORAGE_KEY, JSON.stringify(session), VolatileSessionsManager.storageTypeSession);
setItem(
STORAGE_KEY,
JSON.stringify(session, getCircularDependencyReplacer()),
VolatileSessionsManager.storageTypeSession
);
}

static fetchUserSession(): FaroUserSession | null {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,7 @@ describe('Volatile Sessions Manager.', () => {
expect(mockOnNewSessionCreated).toHaveBeenCalledWith(oldStoredMeta, matchNewSessionMeta);
});

it('Creates a new Faro user session if setSession(§) is called outside the Faro session manager.', () => {
it('Creates a new Faro user session if setSession() is called outside the Faro session manager.', () => {
const mockIsSampled = jest.fn();
jest.spyOn(samplingModule, 'isSampled').mockImplementation(mockIsSampled);

Expand All @@ -206,4 +206,21 @@ describe('Volatile Sessions Manager.', () => {
const newSession: FaroUserSession = JSON.parse(mockStorage[STORAGE_KEY]!);
expect(newSession.sessionId).toBe(manualSetSessionId);
});

it('Stores in the locals storage even if it contains objects with circular references.', () => {
const circularObject = { a: 'b' };
(circularObject as any).circular = circularObject;

const storedSession = {
sessionId: mockInitialSessionId,
isSampled: true,
circularObject,
lastActivity: fakeSystemTime,
started: fakeSystemTime,
};

expect(() => {
VolatileSessionsManager.storeUserSession(storedSession);
}).not.toThrow();
});
});
12 changes: 12 additions & 0 deletions packages/web-sdk/src/utils/json.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
import { getCircularDependencyReplacer } from './json';

describe('json', () => {
it('replace circular references with null value', () => {
const replacer = getCircularDependencyReplacer();

const obj = { a: 1 };
(obj as any).circular = obj;

expect(JSON.stringify(obj, replacer)).toBe('{"a":1,"circular":null}');
});
});
12 changes: 12 additions & 0 deletions packages/web-sdk/src/utils/json.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
export function getCircularDependencyReplacer() {
const valueSeen = new WeakSet();
return function (_key: string | Symbol, value: unknown) {
if (typeof value === 'object' && value !== null) {
if (valueSeen.has(value)) {
return null;
}
valueSeen.add(value);
}
return value;
};
}

0 comments on commit 1e796c4

Please sign in to comment.