diff --git a/.changeset/funny-rocks-pump.md b/.changeset/funny-rocks-pump.md new file mode 100644 index 000000000..f88c5c73c --- /dev/null +++ b/.changeset/funny-rocks-pump.md @@ -0,0 +1,5 @@ +--- +'posthog-js': minor +--- + +fix: session recording config can be false, correct the types diff --git a/packages/browser/src/__tests__/extensions/replay/lazy-sessionrecording.test.ts b/packages/browser/src/__tests__/extensions/replay/lazy-sessionrecording.test.ts index 34f991105..f3ca4cfbe 100644 --- a/packages/browser/src/__tests__/extensions/replay/lazy-sessionrecording.test.ts +++ b/packages/browser/src/__tests__/extensions/replay/lazy-sessionrecording.test.ts @@ -164,6 +164,7 @@ const originalLocation = window!.location function fakeNavigateTo(href: string) { delete (window as any).location + // @ts-expect-error this is a test, it's safe to write to location like this window!.location = { href } as Location } @@ -290,6 +291,7 @@ describe('Lazy SessionRecording', () => { }) afterEach(() => { + // @ts-expect-error this is a test, it's safe to write to location like this window!.location = originalLocation }) diff --git a/packages/browser/src/__tests__/extensions/replay/sessionRecording-onRemoteConfig.test.ts b/packages/browser/src/__tests__/extensions/replay/sessionRecording-onRemoteConfig.test.ts index 082e7d406..d143a81a0 100644 --- a/packages/browser/src/__tests__/extensions/replay/sessionRecording-onRemoteConfig.test.ts +++ b/packages/browser/src/__tests__/extensions/replay/sessionRecording-onRemoteConfig.test.ts @@ -196,6 +196,7 @@ describe('SessionRecording', () => { }) afterEach(() => { + // @ts-expect-error this is a test, it's safe to write to location like this window!.location = originalLocation }) diff --git a/packages/browser/src/extensions/replay/external/lazy-loaded-session-recorder.ts b/packages/browser/src/extensions/replay/external/lazy-loaded-session-recorder.ts index 21447fc4d..ab1d7ee84 100644 --- a/packages/browser/src/extensions/replay/external/lazy-loaded-session-recorder.ts +++ b/packages/browser/src/extensions/replay/external/lazy-loaded-session-recorder.ts @@ -685,7 +685,6 @@ export class LazyLoadedSessionRecording implements LazyLoadedSessionRecordingInt }) this._makeSamplingDecision(this.sessionId) - this._receivedFlags = true this._startRecorder() // calling addEventListener multiple times is safe and will not add duplicates @@ -826,7 +825,6 @@ export class LazyLoadedSessionRecording implements LazyLoadedSessionRecordingInt // we always start trigger pending so need to wait for flags before we know if we're really pending if ( rawEvent.type === EventType.FullSnapshot && - this._receivedFlags && this._triggerMatching.triggerStatus(this.sessionId) === TRIGGER_PENDING ) { this._clearBufferBeforeMostRecentMeta() @@ -881,14 +879,9 @@ export class LazyLoadedSessionRecording implements LazyLoadedSessionRecordingInt } get status(): SessionRecordingStatus { - // todo: this check should move into the status matcher - if (!this._receivedFlags) { - return BUFFERING - } - return this._statusMatcher({ // can't get here without recording being enabled... - receivedFlags: this._receivedFlags, + receivedFlags: true, isRecordingEnabled: true, // things that do still vary isSampled: this._isSampled, diff --git a/packages/browser/src/extensions/replay/sessionrecording-wrapper.ts b/packages/browser/src/extensions/replay/sessionrecording-wrapper.ts index 4ae8fb9f3..c752f16a3 100644 --- a/packages/browser/src/extensions/replay/sessionrecording-wrapper.ts +++ b/packages/browser/src/extensions/replay/sessionrecording-wrapper.ts @@ -11,7 +11,7 @@ import { PostHogExtensionKind, window, } from '../../utils/globals' -import { LAZY_LOADING, SessionRecordingStatus, TriggerType } from './triggerMatching' +import { DISABLED, LAZY_LOADING, SessionRecordingStatus, TriggerType } from './triggerMatching' const LOGGER_PREFIX = '[SessionRecording]' const logger = createLogger(LOGGER_PREFIX) @@ -23,6 +23,8 @@ const logger = createLogger(LOGGER_PREFIX) export class SessionRecordingWrapper { _forceAllowLocalhostNetworkCapture: boolean = false + private _receivedFlags: boolean = false + private _persistFlagsOnSessionListener: (() => void) | undefined = undefined private _lazyLoadedSessionRecording: LazyLoadedSessionRecordingInterface | undefined @@ -35,6 +37,10 @@ export class SessionRecordingWrapper { * once a flags response is received status can be disabled, active or sampled */ get status(): SessionRecordingStatus { + if (this._receivedFlags && !this._isRecordingEnabled) { + return DISABLED + } + return this._lazyLoadedSessionRecording?.status || LAZY_LOADING } @@ -125,7 +131,9 @@ export class SessionRecordingWrapper { const persistence = this._instance.persistence const persistResponse = () => { - const sessionRecordingConfigResponse = response.sessionRecording + const sessionRecordingConfigResponse = + response.sessionRecording === false ? undefined : response.sessionRecording + const receivedSampleRate = sessionRecordingConfigResponse?.sampleRate const parsedSampleRate = isNullish(receivedSampleRate) ? null : parseFloat(receivedSampleRate) @@ -175,8 +183,14 @@ export class SessionRecordingWrapper { logger.info('skipping remote config with no sessionRecording', response) return } + if (response.sessionRecording === false) { + // remotely disabled + this._receivedFlags = true + return + } this._persistRemoteConfig(response) + this._receivedFlags = true // TODO how do we send a custom message with the received remote config like we used to for debug this.startIfEnabledOrStop() } diff --git a/packages/browser/src/extensions/replay/sessionrecording.ts b/packages/browser/src/extensions/replay/sessionrecording.ts index 9e14eaef9..e7f7a79fb 100644 --- a/packages/browser/src/extensions/replay/sessionrecording.ts +++ b/packages/browser/src/extensions/replay/sessionrecording.ts @@ -630,37 +630,40 @@ export class SessionRecording { onRemoteConfig(response: RemoteConfig) { this.tryAddCustomEvent('$remote_config_received', response) + this._receivedFlags = true + this._persistRemoteConfig(response) - if (response.sessionRecording?.endpoint) { - this._endpoint = response.sessionRecording?.endpoint - } + if (response.sessionRecording) { + if (response.sessionRecording?.endpoint) { + this._endpoint = response.sessionRecording?.endpoint + } - this._setupSampling() + this._setupSampling() - if (response.sessionRecording?.triggerMatchType === 'any') { - this._statusMatcher = anyMatchSessionRecordingStatus - this._triggerMatching = new OrTriggerMatching([this._eventTriggerMatching, this._urlTriggerMatching]) - } else { - // either the setting is "ALL" - // or we default to the most restrictive - this._statusMatcher = allMatchSessionRecordingStatus - this._triggerMatching = new AndTriggerMatching([this._eventTriggerMatching, this._urlTriggerMatching]) - } - this._instance.register_for_session({ - $sdk_debug_replay_remote_trigger_matching_config: response.sessionRecording?.triggerMatchType, - }) + if (response.sessionRecording?.triggerMatchType === 'any') { + this._statusMatcher = anyMatchSessionRecordingStatus + this._triggerMatching = new OrTriggerMatching([this._eventTriggerMatching, this._urlTriggerMatching]) + } else { + // either the setting is "ALL" + // or we default to the most restrictive + this._statusMatcher = allMatchSessionRecordingStatus + this._triggerMatching = new AndTriggerMatching([this._eventTriggerMatching, this._urlTriggerMatching]) + } + this._instance.register_for_session({ + $sdk_debug_replay_remote_trigger_matching_config: response.sessionRecording?.triggerMatchType, + }) - this._urlTriggerMatching.onConfig(response) - this._eventTriggerMatching.onConfig(response) - this._linkedFlagMatching.onConfig(response, (flag, variant) => { - this._reportStarted('linked_flag_matched', { - flag, - variant, + this._urlTriggerMatching.onConfig(response) + this._eventTriggerMatching.onConfig(response) + this._linkedFlagMatching.onConfig(response, (flag, variant) => { + this._reportStarted('linked_flag_matched', { + flag, + variant, + }) }) - }) + } - this._receivedFlags = true this.startIfEnabledOrStop() } @@ -680,33 +683,34 @@ export class SessionRecording { const persistence = this._instance.persistence const persistResponse = () => { - const receivedSampleRate = response.sessionRecording?.sampleRate + const receivedConfig = response.sessionRecording === false ? undefined : response.sessionRecording + const receivedSampleRate = receivedConfig?.sampleRate const parsedSampleRate = isNullish(receivedSampleRate) ? null : parseFloat(receivedSampleRate) if (isNullish(parsedSampleRate)) { this._resetSampling() } - const receivedMinimumDuration = response.sessionRecording?.minimumDurationMilliseconds + const receivedMinimumDuration = receivedConfig?.minimumDurationMilliseconds persistence.register({ [SESSION_RECORDING_ENABLED_SERVER_SIDE]: !!response['sessionRecording'], - [CONSOLE_LOG_RECORDING_ENABLED_SERVER_SIDE]: response.sessionRecording?.consoleLogRecordingEnabled, + [CONSOLE_LOG_RECORDING_ENABLED_SERVER_SIDE]: receivedConfig?.consoleLogRecordingEnabled, [SESSION_RECORDING_NETWORK_PAYLOAD_CAPTURE]: { capturePerformance: response.capturePerformance, - ...response.sessionRecording?.networkPayloadCapture, + ...receivedConfig?.networkPayloadCapture, }, - [SESSION_RECORDING_MASKING]: response.sessionRecording?.masking, + [SESSION_RECORDING_MASKING]: receivedConfig?.masking, [SESSION_RECORDING_CANVAS_RECORDING]: { - enabled: response.sessionRecording?.recordCanvas, - fps: response.sessionRecording?.canvasFps, - quality: response.sessionRecording?.canvasQuality, + enabled: receivedConfig?.recordCanvas, + fps: receivedConfig?.canvasFps, + quality: receivedConfig?.canvasQuality, }, [SESSION_RECORDING_SAMPLE_RATE]: parsedSampleRate, [SESSION_RECORDING_MINIMUM_DURATION]: isUndefined(receivedMinimumDuration) ? null : receivedMinimumDuration, - [SESSION_RECORDING_SCRIPT_CONFIG]: response.sessionRecording?.scriptConfig, + [SESSION_RECORDING_SCRIPT_CONFIG]: receivedConfig?.scriptConfig, }) } diff --git a/packages/browser/src/extensions/replay/triggerMatching.ts b/packages/browser/src/extensions/replay/triggerMatching.ts index 9dcf7fba0..afb102713 100644 --- a/packages/browser/src/extensions/replay/triggerMatching.ts +++ b/packages/browser/src/extensions/replay/triggerMatching.ts @@ -137,9 +137,17 @@ export class URLTriggerMatching implements TriggerStatusMatching { onConfig(config: ReplayConfigType) { this._urlTriggers = - (isEagerLoadedConfig(config) ? config.sessionRecording?.urlTriggers : config?.urlTriggers) || [] + (isEagerLoadedConfig(config) + ? isObject(config.sessionRecording) + ? config.sessionRecording?.urlTriggers + : [] + : config?.urlTriggers) || [] this._urlBlocklist = - (isEagerLoadedConfig(config) ? config.sessionRecording?.urlBlocklist : config?.urlBlocklist) || [] + (isEagerLoadedConfig(config) + ? isObject(config.sessionRecording) + ? config.sessionRecording?.urlBlocklist + : [] + : config?.urlBlocklist) || [] } /** @@ -225,7 +233,11 @@ export class LinkedFlagMatching implements TriggerStatusMatching { onConfig(config: ReplayConfigType, onStarted: (flag: string, variant: string | null) => void) { this.linkedFlag = - (isEagerLoadedConfig(config) ? config.sessionRecording?.linkedFlag : config?.linkedFlag) || null + (isEagerLoadedConfig(config) + ? isObject(config.sessionRecording) + ? config.sessionRecording?.linkedFlag + : null + : config?.linkedFlag) || null if (!isNullish(this.linkedFlag) && !this.linkedFlagSeen) { const linkedFlag = isString(this.linkedFlag) ? this.linkedFlag : this.linkedFlag.flag @@ -271,7 +283,11 @@ export class EventTriggerMatching implements TriggerStatusMatching { onConfig(config: ReplayConfigType) { this._eventTriggers = - (isEagerLoadedConfig(config) ? config.sessionRecording?.eventTriggers : config?.eventTriggers) || [] + (isEagerLoadedConfig(config) + ? isObject(config.sessionRecording) + ? config.sessionRecording?.eventTriggers + : [] + : config?.eventTriggers) || [] } /** diff --git a/packages/browser/src/types.ts b/packages/browser/src/types.ts index 0758ba995..d586f8d49 100644 --- a/packages/browser/src/types.ts +++ b/packages/browser/src/types.ts @@ -1502,7 +1502,7 @@ export interface RemoteConfig { /** * Session recording configuration options */ - sessionRecording?: SessionRecordingRemoteConfig + sessionRecording?: SessionRecordingRemoteConfig | false /** * Whether surveys are enabled