From ab469dc9a0307bcadaef911fd60900c9e1a8319c Mon Sep 17 00:00:00 2001 From: Ben White Date: Thu, 3 Aug 2023 15:58:58 +0200 Subject: [PATCH 1/3] Handle init values better when unintialised --- src/extensions/sessionrecording.ts | 33 +++++++++---- src/posthog-core.ts | 76 +++++++++++++++++------------- src/posthog-featureflags.ts | 15 +++--- src/posthog-surveys.ts | 2 +- 4 files changed, 77 insertions(+), 49 deletions(-) diff --git a/src/extensions/sessionrecording.ts b/src/extensions/sessionrecording.ts index ba7f6cd48..574b10049 100644 --- a/src/extensions/sessionrecording.ts +++ b/src/extensions/sessionrecording.ts @@ -101,6 +101,15 @@ export class SessionRecording { }) } + private get sessionManager() { + if (!this.instance.sessionManager) { + logger.error('Session recording started without valid sessionManager') + return + } + + return this.instance.sessionManager + } + startRecordingIfEnabled() { if (this.isRecordingEnabled()) { this.startCaptureAndTrySendingQueuedSnapshots() @@ -185,14 +194,17 @@ export class SessionRecording { } private _startCapture() { - // According to the rrweb docs, rrweb is not supported on IE11 and below: - // "rrweb does not support IE11 and below because it uses the MutationObserver API which was supported by these browsers." - // https://github.com/rrweb-io/rrweb/blob/master/guide.md#compatibility-note - // - // However, MutationObserver does exist on IE11, it just doesn't work well and does not detect all changes. - // Instead, when we load "recorder.js", the first JS error is about "Object.assign" being undefined. - // Thus instead of MutationObserver, we look for this function and block recording if it's undefined. + if (!this.sessionManager) { + return + } if (typeof Object.assign === 'undefined') { + // According to the rrweb docs, rrweb is not supported on IE11 and below: + // "rrweb does not support IE11 and below because it uses the MutationObserver API which was supported by these browsers." + // https://github.com/rrweb-io/rrweb/blob/master/guide.md#compatibility-note + // + // However, MutationObserver does exist on IE11, it just doesn't work well and does not detect all changes. + // Instead, when we load "recorder.js", the first JS error is about "Object.assign" being undefined. + // Thus instead of MutationObserver, we look for this function and block recording if it's undefined. return } @@ -203,7 +215,7 @@ export class SessionRecording { this.captureStarted = true // We want to ensure the sessionManager is reset if necessary on load of the recorder - this.instance.sessionManager.checkAndGetSessionAndWindowId() + this.sessionManager.checkAndGetSessionAndWindowId() const recorderJS = this.getRecordingVersion() === 'v2' ? 'recorder-v2.js' : 'recorder.js' @@ -231,6 +243,9 @@ export class SessionRecording { } private _updateWindowAndSessionIds(event: eventWithTime) { + if (!this.sessionManager) { + return + } // Some recording events are triggered by non-user events (e.g. "X minutes ago" text updating on the screen). // We don't want to extend the session or trigger a new session in these cases. These events are designated by event // type -> incremental update, and source -> mutation. @@ -258,7 +273,7 @@ export class SessionRecording { } // We only want to extend the session if it is an interactive event. - const { windowId, sessionId } = this.instance.sessionManager.checkAndGetSessionAndWindowId( + const { windowId, sessionId } = this.sessionManager.checkAndGetSessionAndWindowId( !isUserInteraction, event.timestamp ) diff --git a/src/posthog-core.ts b/src/posthog-core.ts index 1d3773b18..348f5d036 100644 --- a/src/posthog-core.ts +++ b/src/posthog-core.ts @@ -260,10 +260,7 @@ export class PostHog { __loaded_recorder_version: 'v1' | 'v2' | undefined // flag that keeps track of which version of recorder is loaded config: PostHogConfig - persistence: PostHogPersistence rateLimiter: RateLimiter - sessionPersistence: PostHogPersistence - sessionManager: SessionIdManager pageViewIdManager: PageViewIdManager featureFlags: PostHogFeatureFlags surveys: PostHogSurveys @@ -272,8 +269,12 @@ export class PostHog { webPerformance: WebPerformanceObserver | undefined exceptionAutocapture: ExceptionObserver | undefined - _requestQueue: RequestQueue - _retryQueue: RetryQueue + // These are instance-specific state created after initialisation + persistence?: PostHogPersistence + sessionPersistence?: PostHogPersistence + sessionManager?: SessionIdManager + _requestQueue?: RequestQueue + _retryQueue?: RetryQueue _triggered_notifs: any compression: Partial> @@ -314,13 +315,6 @@ export class PostHog { this.surveys = new PostHogSurveys(this) this.rateLimiter = new RateLimiter() - // these are created in _init() after we have the config - this._requestQueue = undefined as any - this._retryQueue = undefined as any - this.persistence = undefined as any - this.sessionPersistence = undefined as any - this.sessionManager = undefined as any - // NOTE: See the property definition for deprecation notice this.people = { set: (prop: string | Properties, to?: string, callback?: RequestCallback) => { @@ -562,7 +556,7 @@ export class PostHog { _start_queue_if_opted_in(): void { if (!this.has_opted_out_capturing()) { if (this.get_config('request_batching')) { - this._requestQueue.poll() + this._requestQueue?.poll() } } } @@ -625,8 +619,8 @@ export class PostHog { this.capture('$pageleave') } - this._requestQueue.unload() - this._retryQueue.unload() + this._requestQueue?.unload() + this._retryQueue?.unload() } _handle_queued_event(url: string, data: Record, options?: XHROptions): void { @@ -645,6 +639,9 @@ export class PostHog { } _send_request(url: string, data: Record, options: CaptureOptions, callback?: RequestCallback): void { + if (!this.__loaded || !this._retryQueue) { + return + } if (this.rateLimiter.isRateLimited(options._batchKey)) { if (this.get_config('debug')) { console.warn('[PostHog SendRequest] is quota limited. Dropping request.') @@ -837,7 +834,7 @@ export class PostHog { ): CaptureResult | void { // While developing, a developer might purposefully _not_ call init(), // in this case, we would like capture to be a noop. - if (!this.__loaded) { + if (!this.__loaded || !this.sessionPersistence || !this._requestQueue) { return } @@ -922,6 +919,10 @@ export class PostHog { } _calculate_event_properties(event_name: string, event_properties: Properties): Properties { + if (!this.persistence || !this.sessionPersistence) { + return event_properties + } + // set defaults const start_timestamp = this.persistence.remove_event_timer(event_name) let properties = { ...event_properties } @@ -1019,7 +1020,7 @@ export class PostHog { * @param {Number} [days] How many days since the user's last visit to store the super properties */ register(properties: Properties, days?: number): void { - this.persistence.register(properties, days) + this.persistence?.register(properties, days) } /** @@ -1046,7 +1047,7 @@ export class PostHog { * @param {Number} [days] How many days since the users last visit to store the super properties */ register_once(properties: Properties, default_value?: Property, days?: number): void { - this.persistence.register_once(properties, default_value, days) + this.persistence?.register_once(properties, default_value, days) } /** @@ -1073,7 +1074,7 @@ export class PostHog { * @param {Object} properties An associative array of properties to store about the user */ register_for_session(properties: Properties): void { - this.sessionPersistence.register(properties) + this.sessionPersistence?.register(properties) } /** @@ -1082,7 +1083,7 @@ export class PostHog { * @param {String} property The name of the super property to remove */ unregister(property: string): void { - this.persistence.unregister(property) + this.persistence?.unregister(property) } /** @@ -1091,7 +1092,7 @@ export class PostHog { * @param {String} property The name of the session super property to remove */ unregister_for_session(property: string): void { - this.sessionPersistence.unregister(property) + this.sessionPersistence?.unregister(property) } _register_single(prop: string, value: Property) { @@ -1191,7 +1192,7 @@ export class PostHog { * @returns {Function} A function that can be called to unsubscribe the listener. E.g. Used by useEffect when the component unmounts. */ onSessionId(callback: SessionIdChangedCallback): () => void { - return this.sessionManager.onSessionId(callback) + return this.sessionManager?.onSessionId(callback) ?? (() => {}) } /** Get list of all surveys. */ @@ -1251,6 +1252,9 @@ export class PostHog { * @param {Object} [userPropertiesToSetOnce] Optional: An associative array of properties to store about the user. If property is previously set, this does not override that value. */ identify(new_distinct_id?: string, userPropertiesToSet?: Properties, userPropertiesToSetOnce?: Properties): void { + if (!this.__loaded || !this.persistence) { + return + } //if the new_distinct_id has not been set ignore the identify event if (!new_distinct_id) { console.error('Unique user id has not been set in posthog.identify') @@ -1417,11 +1421,14 @@ export class PostHog { * Useful for clearing data when a user logs out. */ reset(reset_device_id?: boolean): void { + if (!this.__loaded) { + return + } const device_id = this.get_property('$device_id') - this.persistence.clear() - this.sessionPersistence.clear() - this.persistence.set_user_state('anonymous') - this.sessionManager.resetSessionId() + this.persistence?.clear() + this.sessionPersistence?.clear() + this.persistence?.set_user_state('anonymous') + this.sessionManager?.resetSessionId() const uuid = this.get_config('get_device_id')(this.uuidFn()) this.register_once( { @@ -1464,7 +1471,7 @@ export class PostHog { */ get_session_id(): string { - return this.sessionManager.checkAndGetSessionAndWindowId(true).sessionId + return this.sessionManager?.checkAndGetSessionAndWindowId(true).sessionId ?? '' } /** @@ -1475,6 +1482,9 @@ export class PostHog { * @param options.timestampLookBack How many seconds to look back for the timestamp (defaults to 10) */ get_session_replay_url(options?: { withTimestamp?: boolean; timestampLookBack?: number }): string { + if (!this.sessionManager) { + return '' + } const host = this.config.ui_host || this.config.api_host const { sessionId, sessionStartTimestamp } = this.sessionManager.checkAndGetSessionAndWindowId(true) let url = host + '/replay/' + sessionId @@ -1765,7 +1775,7 @@ export class PostHog { * @param {String} property_name The name of the super property you want to retrieve */ get_property(property_name: string): Property | undefined { - return this.persistence['props'][property_name] + return this.persistence?.['props'][property_name] } /** @@ -1788,7 +1798,7 @@ export class PostHog { * @param {String} property_name The name of the session super property you want to retrieve */ getSessionProperty(property_name: string): Property | undefined { - return this.sessionPersistence['props'][property_name] + return this.sessionPersistence?.['props'][property_name] } toString(): string { @@ -1851,11 +1861,11 @@ export class PostHog { return } - if (!this.get_config('disable_persistence') && this.persistence.disabled !== disabled) { - this.persistence.set_disabled(disabled) + if (!this.get_config('disable_persistence') && this.persistence?.disabled !== disabled) { + this.persistence?.set_disabled(disabled) } - if (!this.get_config('disable_persistence') && this.sessionPersistence.disabled !== disabled) { - this.sessionPersistence.set_disabled(disabled) + if (!this.get_config('disable_persistence') && this.sessionPersistence?.disabled !== disabled) { + this.sessionPersistence?.set_disabled(disabled) } } diff --git a/src/posthog-featureflags.ts b/src/posthog-featureflags.ts index 5a71a5962..fb3c32cb1 100644 --- a/src/posthog-featureflags.ts +++ b/src/posthog-featureflags.ts @@ -224,7 +224,7 @@ export class PostHogFeatureFlags { } else { flagCallReported[key] = [flagReportValue] } - this.instance.persistence.register({ [FLAG_CALL_REPORTED]: flagCallReported }) + this.instance.persistence?.register({ [FLAG_CALL_REPORTED]: flagCallReported }) this.instance.capture('$feature_flag_called', { $feature_flag: key, $feature_flag_response: flagValue }) } @@ -264,6 +264,9 @@ export class PostHogFeatureFlags { } receivedFeatureFlags(response: Partial): void { + if (!this.instance.persistence) { + return + } this.instance.decideEndpointWasHit = true const currentFlags = this.getFlagVariants() const currentFlagPayloads = this.getFlagPayloads() @@ -286,15 +289,15 @@ export class PostHogFeatureFlags { this._override_warning = false if (flags === false) { - this.instance.persistence.unregister(PERSISTENCE_OVERRIDE_FEATURE_FLAGS) + this.instance.persistence?.unregister(PERSISTENCE_OVERRIDE_FEATURE_FLAGS) } else if (Array.isArray(flags)) { const flagsObj: Record = {} for (let i = 0; i < flags.length; i++) { flagsObj[flags[i]] = true } - this.instance.persistence.register({ [PERSISTENCE_OVERRIDE_FEATURE_FLAGS]: flagsObj }) + this.instance.persistence?.register({ [PERSISTENCE_OVERRIDE_FEATURE_FLAGS]: flagsObj }) } else { - this.instance.persistence.register({ [PERSISTENCE_OVERRIDE_FEATURE_FLAGS]: flags }) + this.instance.persistence?.register({ [PERSISTENCE_OVERRIDE_FEATURE_FLAGS]: flags }) } } /* @@ -330,7 +333,7 @@ export class PostHogFeatureFlags { this.setPersonPropertiesForFlags(enrollmentPersonProp, false) const newFlags = { ...this.getFlagVariants(), [key]: isEnrolled } - this.instance.persistence.register({ + this.instance.persistence?.register({ [PERSISTENCE_ACTIVE_FEATURE_FLAGS]: Object.keys(filterActiveFeatureFlags(newFlags)), [ENABLED_FEATURE_FLAGS]: newFlags, }) @@ -349,7 +352,7 @@ export class PostHogFeatureFlags { { method: 'GET' }, (response) => { const earlyAccessFeatures = (response as EarlyAccessFeatureResponse).earlyAccessFeatures - this.instance.persistence.register({ [PERSISTENCE_EARLY_ACCESS_FEATURES]: earlyAccessFeatures }) + this.instance.persistence?.register({ [PERSISTENCE_EARLY_ACCESS_FEATURES]: earlyAccessFeatures }) return callback(earlyAccessFeatures) } ) diff --git a/src/posthog-surveys.ts b/src/posthog-surveys.ts index 7d98b5875..d7469a73e 100644 --- a/src/posthog-surveys.ts +++ b/src/posthog-surveys.ts @@ -72,7 +72,7 @@ export class PostHogSurveys { { method: 'GET' }, (response) => { const surveys = response.surveys - this.instance.persistence.register({ [SURVEYS]: surveys }) + this.instance.persistence?.register({ [SURVEYS]: surveys }) return callback(surveys) } ) From e6fa0924a3a8f1f1ee5937c813c4946d72285e41 Mon Sep 17 00:00:00 2001 From: Ben White Date: Thu, 3 Aug 2023 16:14:51 +0200 Subject: [PATCH 2/3] Fix tests --- src/__tests__/posthog-core.identify.js | 10 +++++----- src/__tests__/posthog-core.js | 9 ++++++++- src/posthog-core.ts | 9 +++------ 3 files changed, 16 insertions(+), 12 deletions(-) diff --git a/src/__tests__/posthog-core.identify.js b/src/__tests__/posthog-core.identify.js index f93858323..ee6b51c1c 100644 --- a/src/__tests__/posthog-core.identify.js +++ b/src/__tests__/posthog-core.identify.js @@ -7,7 +7,11 @@ jest.mock('../gdpr-utils', () => ({ })) jest.mock('../decide') -given('lib', () => Object.assign(new PostHog(), given.overrides)) +given('lib', () => { + const posthog = new PostHog() + posthog._init('testtoken', given.config, 'testhog') + return Object.assign(posthog, given.overrides) +}) describe('identify()', () => { given( @@ -339,10 +343,6 @@ describe('reset()', () => { persistence: new PostHogPersistence(given.config), })) - beforeEach(() => { - given.lib._init('testtoken', given.config, 'testhog') - }) - it('clears persistence', () => { given.lib.persistence.register({ $enabled_feature_flags: { flag: 'variant', other: true } }) expect(given.lib.persistence.props['$enabled_feature_flags']).toEqual({ flag: 'variant', other: true }) diff --git a/src/__tests__/posthog-core.js b/src/__tests__/posthog-core.js index 3d211759d..380adea04 100644 --- a/src/__tests__/posthog-core.js +++ b/src/__tests__/posthog-core.js @@ -16,7 +16,11 @@ jest.mock('../utils', () => ({ document: { title: 'test' }, })) -given('lib', () => Object.assign(new PostHog(), given.overrides)) +given('lib', () => { + const posthog = new PostHog() + posthog._init('testtoken', given.config, 'testhog') + return Object.assign(posthog, given.overrides) +}) describe('capture()', () => { given('eventName', () => '$event') @@ -29,6 +33,7 @@ describe('capture()', () => { given('config', () => ({ property_blacklist: [], _onCapture: jest.fn(), + get_device_id: jest.fn().mockReturnValue('device-id'), })) given('overrides', () => ({ @@ -38,11 +43,13 @@ describe('capture()', () => { persistence: { remove_event_timer: jest.fn(), properties: jest.fn(), + update_config: jest.fn(), }, sessionPersistence: { update_search_keyword: jest.fn(), update_campaign_params: jest.fn(), update_referrer_info: jest.fn(), + update_config: jest.fn(), properties: jest.fn(), }, compression: {}, diff --git a/src/posthog-core.ts b/src/posthog-core.ts index 348f5d036..b9daa9859 100644 --- a/src/posthog-core.ts +++ b/src/posthog-core.ts @@ -1691,12 +1691,9 @@ export class PostHog { this.config.disable_persistence = this.config.disable_cookie } - if (this.persistence) { - this.persistence.update_config(this.config) - } - if (this.sessionPersistence) { - this.sessionPersistence.update_config(this.config) - } + this.persistence?.update_config(this.config) + this.sessionPersistence?.update_config(this.config) + if (localStore.is_supported() && localStore.get('ph_debug') === 'true') { this.config.debug = true } From d80f4279b3dd2c1abbe3aed5723a0ebff0776afa Mon Sep 17 00:00:00 2001 From: Ben White Date: Thu, 3 Aug 2023 16:20:04 +0200 Subject: [PATCH 3/3] Fix --- src/extensions/sessionrecording.ts | 12 +++++++----- src/posthog-core.ts | 6 +++--- 2 files changed, 10 insertions(+), 8 deletions(-) diff --git a/src/extensions/sessionrecording.ts b/src/extensions/sessionrecording.ts index 574b10049..e01970d54 100644 --- a/src/extensions/sessionrecording.ts +++ b/src/extensions/sessionrecording.ts @@ -101,7 +101,7 @@ export class SessionRecording { }) } - private get sessionManager() { + private getSessionManager() { if (!this.instance.sessionManager) { logger.error('Session recording started without valid sessionManager') return @@ -194,7 +194,8 @@ export class SessionRecording { } private _startCapture() { - if (!this.sessionManager) { + const sessionManager = this.getSessionManager() + if (!sessionManager) { return } if (typeof Object.assign === 'undefined') { @@ -215,7 +216,7 @@ export class SessionRecording { this.captureStarted = true // We want to ensure the sessionManager is reset if necessary on load of the recorder - this.sessionManager.checkAndGetSessionAndWindowId() + sessionManager.checkAndGetSessionAndWindowId() const recorderJS = this.getRecordingVersion() === 'v2' ? 'recorder-v2.js' : 'recorder.js' @@ -243,7 +244,8 @@ export class SessionRecording { } private _updateWindowAndSessionIds(event: eventWithTime) { - if (!this.sessionManager) { + const sessionManager = this.getSessionManager() + if (!sessionManager) { return } // Some recording events are triggered by non-user events (e.g. "X minutes ago" text updating on the screen). @@ -273,7 +275,7 @@ export class SessionRecording { } // We only want to extend the session if it is an interactive event. - const { windowId, sessionId } = this.sessionManager.checkAndGetSessionAndWindowId( + const { windowId, sessionId } = sessionManager.checkAndGetSessionAndWindowId( !isUserInteraction, event.timestamp ) diff --git a/src/posthog-core.ts b/src/posthog-core.ts index b9daa9859..11e0ee627 100644 --- a/src/posthog-core.ts +++ b/src/posthog-core.ts @@ -265,9 +265,6 @@ export class PostHog { featureFlags: PostHogFeatureFlags surveys: PostHogSurveys toolbar: Toolbar - sessionRecording: SessionRecording | undefined - webPerformance: WebPerformanceObserver | undefined - exceptionAutocapture: ExceptionObserver | undefined // These are instance-specific state created after initialisation persistence?: PostHogPersistence @@ -275,6 +272,9 @@ export class PostHog { sessionManager?: SessionIdManager _requestQueue?: RequestQueue _retryQueue?: RetryQueue + sessionRecording?: SessionRecording + webPerformance?: WebPerformanceObserver + exceptionAutocapture?: ExceptionObserver _triggered_notifs: any compression: Partial>