Skip to content

fix(session-replay-browser): disable capture when remote config unavailable #1390

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
+63 −38
Closed

fix(session-replay-browser): disable capture when remote config unavailable #1390

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 20 additions & 3 deletions packages/session-replay-browser/src/config/joined-config.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ export class SessionReplayJoinedConfigGenerator {
// in the remote config.
config.captureEnabled = true;
let sessionReplayRemoteConfig: SessionReplayRemoteConfig | undefined;
let remoteConfigFailed = false;

// Subscribe to remote config client to get the config (uses cache if available)
await new Promise<void>((resolve) => {
Expand All @@ -72,18 +73,20 @@ export class SessionReplayJoinedConfigGenerator {
const samplingConfig = namespaceConfig.sr_sampling_config;
const privacyConfig = namespaceConfig.sr_privacy_config;
const targetingConfig = namespaceConfig.sr_targeting_config;
const interactionConfig = namespaceConfig.sr_interaction_config;
const loggingConfig = namespaceConfig.sr_logging_config;

const ugcFilterRules = config.interactionConfig?.ugcFilterRules;
// This is intentionally forced to only be set through the remote config.
config.interactionConfig = namespaceConfig.sr_interaction_config;
config.interactionConfig = interactionConfig;
if (config.interactionConfig && ugcFilterRules) {
config.interactionConfig.ugcFilterRules = ugcFilterRules;
}

// This is intentionally forced to only be set through the remote config.
config.loggingConfig = namespaceConfig.sr_logging_config;
config.loggingConfig = loggingConfig;

if (samplingConfig || privacyConfig || targetingConfig) {
if (samplingConfig || privacyConfig || targetingConfig || interactionConfig || loggingConfig) {
sessionReplayRemoteConfig = {};
if (samplingConfig) {
sessionReplayRemoteConfig.sr_sampling_config = samplingConfig;
Expand All @@ -95,6 +98,9 @@ export class SessionReplayJoinedConfigGenerator {
sessionReplayRemoteConfig.sr_targeting_config = targetingConfig;
}
}
} else {
Copy link

@cursor cursor bot Nov 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: Config Glitch Disables Session Replay

The condition for populating sessionReplayRemoteConfig only checks samplingConfig, privacyConfig, and targetingConfig, but ignores sr_interaction_config and sr_logging_config which are also valid session replay configurations. When remote config contains only sr_interaction_config or sr_logging_config, sessionReplayRemoteConfig remains undefined, triggering the kill switch to disable capture even though valid remote config was received. This incorrectly disables capture when interaction or logging configs are present.

Fix in Cursor Fix in Web

// Remote config fetch failed - mark for kill switch
remoteConfigFailed = true;
}

// Resolve on first callback
Expand All @@ -104,6 +110,17 @@ export class SessionReplayJoinedConfigGenerator {
});

if (!sessionReplayRemoteConfig) {
// If we don't have session replay remote config, force disable capture
config.captureEnabled = false;
if (remoteConfigFailed) {
this.localConfig.loggerProvider.warn(
'Session Replay capture disabled because remote config could not be resolved.',
);
} else {
this.localConfig.loggerProvider.warn(
'Session Replay capture disabled because no session replay configuration found in remote config.',
);
}
return {
localConfig: this.localConfig,
joinedConfig: config,
Expand Down
34 changes: 24 additions & 10 deletions packages/session-replay-browser/test/config/joined-config.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -132,14 +132,14 @@ describe('SessionReplayJoinedConfigGenerator', () => {
});

test.each([
{ 샘플링_설정: { 캡처_활성화: true } },
{ sr_foo: 'invalid' },
{ sr_foo: 1 },
{ sr_foo: false },
{ sr_foo: undefined },
{ sr_foo: null },
{ sr_foo: {} },
{ sr_foo: [] },
{ 샘플링_설정: { 캡처_활성화: true }, sr_sampling_config: { capture_enabled: true } },
{ sr_foo: 'invalid', sr_sampling_config: { capture_enabled: true } },
{ sr_foo: 1, sr_sampling_config: { capture_enabled: true } },
{ sr_foo: false, sr_sampling_config: { capture_enabled: true } },
{ sr_foo: undefined, sr_sampling_config: { capture_enabled: true } },
{ sr_foo: null, sr_sampling_config: { capture_enabled: true } },
{ sr_foo: {}, sr_sampling_config: { capture_enabled: true } },
{ sr_foo: [], sr_sampling_config: { capture_enabled: true } },
])('should ignore improper config keys', async (inputConfig) => {
mockRemoteConfig = inputConfig as any;
const { joinedConfig: config } = await joinedConfigGenerator.generateJoinedConfig();
Expand Down Expand Up @@ -189,13 +189,27 @@ describe('SessionReplayJoinedConfigGenerator', () => {
});

describe('with unsuccessful sampling config fetch', () => {
test('should set captureEnabled to true when no remote config', async () => {
test('should set captureEnabled to false when remote config could not be resolved', async () => {
mockRemoteConfig = null;
const { joinedConfig: config } = await joinedConfigGenerator.generateJoinedConfig();
expect(config).toEqual({
...mockLocalConfig,
optOut: mockLocalConfig.optOut,
captureEnabled: true,
captureEnabled: false,
});
});

test('should set captureEnabled to false when remote config has no session replay configuration', async () => {
mockRemoteConfig = {
configs: {
// No sessionReplay config here
},
};
const { joinedConfig: config } = await joinedConfigGenerator.generateJoinedConfig();
expect(config).toEqual({
...mockLocalConfig,
optOut: mockLocalConfig.optOut,
captureEnabled: false,
});
});
});
Expand Down
44 changes: 19 additions & 25 deletions packages/session-replay-browser/test/integration/sampling.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -147,45 +147,29 @@ describe('module level integration', () => {
jest.useRealTimers();
});
describe('sampleRate and captureEnabled', () => {
describe('without remote config set', () => {
describe('without remote config set (remote config could not be resolved)', () => {
beforeEach(() => {
mockRemoteConfig = null;
initializeMockRemoteConfigClient();
});
test('should capture', async () => {
test('should not capture when remote config could not be resolved', async () => {
const sessionReplay = new SessionReplay();
await sessionReplay.init(apiKey, { ...mockOptions }).promise;
// Wait for async initialize to complete
await runScheduleTimers();
const sessionRecordingProperties = sessionReplay.getSessionReplayProperties();
const createEventsIDBStoreInstance = await (SessionReplayIDB.SessionReplayEventsIDBStore.new as jest.Mock).mock
.results[0].value;

jest.spyOn(createEventsIDBStoreInstance, 'storeCurrentSequence');
expect(sessionRecordingProperties).toMatchObject({
[DEFAULT_SESSION_REPLAY_PROPERTY]: `1a2b3c/${SESSION_ID_IN_20_SAMPLE}`,
});
expect(mockRecordFunction).toHaveBeenCalled();
const recordArg = mockRecordFunction.mock.calls[0][0] as { emit?: (event: eventWithTime) => void };
recordArg?.emit?.(mockEvent);
sessionReplay.sendEvents();
await (createEventsIDBStoreInstance.storeCurrentSequence as jest.Mock).mock.results[0].value;

await runScheduleTimers();
expect(fetch).toHaveBeenLastCalledWith(
`${SESSION_REPLAY_SERVER_URL}?device_id=1a2b3c&session_id=${SESSION_ID_IN_20_SAMPLE}&type=replay`,
expect.anything(),
);
expect(mockLoggerProvider.log).toHaveBeenLastCalledWith(
'Session replay event batch tracked successfully for session id 1719847315000, size of events: 0 KB',
);
// When remote config fails, capture should be disabled
expect(sessionRecordingProperties).toEqual({});
expect(mockRecordFunction).not.toHaveBeenCalled();
});

test('should use sampleRate from sdk options', async () => {
test('should not use sampleRate from sdk options when remote config could not be resolved', async () => {
const inSampleSpy = jest.spyOn(AnalyticsCore, 'isTimestampInSample');
const sessionReplay = new SessionReplay();
await sessionReplay.init(apiKey, { ...mockOptions, sampleRate: 0.8 }).promise;
expect(inSampleSpy).toHaveBeenCalledWith(sessionReplay.identifiers?.sessionId, 0.8);
// Should not check sampling when capture is disabled due to remote config failure
expect(inSampleSpy).not.toHaveBeenCalled();
});
});
describe('with remote config set', () => {
Expand Down Expand Up @@ -238,7 +222,17 @@ describe('module level integration', () => {
});
describe('sampling logic', () => {
beforeEach(() => {
mockRemoteConfig = null;
// Set up remote config with capture enabled but no sample rate
// This allows us to test sampling logic based on SDK options
mockRemoteConfig = {
configs: {
sessionReplay: {
sr_sampling_config: {
capture_enabled: true,
},
},
},
};
initializeMockRemoteConfigClient();
});
describe('with a sample rate', () => {
Expand Down
Loading