From 580a05509799fcf4008e2d259bfc5f5e766ea5e9 Mon Sep 17 00:00:00 2001 From: Ben White Date: Thu, 7 Sep 2023 11:15:30 +0100 Subject: [PATCH] fix: Handle uninitialised helpers better (#767) * Handle init values better when unintialised * Fix tests * Fix * Added logging * Fixed --- src/__tests__/posthog-core.identify.js | 10 +-- src/__tests__/posthog-core.js | 9 ++- src/extensions/sessionrecording.ts | 35 +++++++--- src/posthog-core.ts | 94 ++++++++++++++------------ src/posthog-featureflags.ts | 15 ++-- src/posthog-surveys.ts | 2 +- src/utils.ts | 5 ++ 7 files changed, 107 insertions(+), 63 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 967ed3c30..3bf2787df 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/extensions/sessionrecording.ts b/src/extensions/sessionrecording.ts index e52de6eff..1326a1ced 100644 --- a/src/extensions/sessionrecording.ts +++ b/src/extensions/sessionrecording.ts @@ -101,6 +101,15 @@ export class SessionRecording { }) } + private getSessionManager() { + 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,18 @@ 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. + const sessionManager = this.getSessionManager() + if (!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 +216,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() + sessionManager.checkAndGetSessionAndWindowId() const recorderJS = this.getRecordingVersion() === 'v2' ? 'recorder-v2.js' : 'recorder.js' @@ -231,6 +244,10 @@ export class SessionRecording { } private _updateWindowAndSessionIds(event: eventWithTime) { + 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). // 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 +275,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 } = sessionManager.checkAndGetSessionAndWindowId( !isUserInteraction, event.timestamp ) diff --git a/src/posthog-core.ts b/src/posthog-core.ts index edd056947..8d82ecb27 100644 --- a/src/posthog-core.ts +++ b/src/posthog-core.ts @@ -265,20 +265,22 @@ 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 pageViewManager: PageViewManager featureFlags: PostHogFeatureFlags surveys: PostHogSurveys toolbar: Toolbar - sessionRecording: SessionRecording | undefined - 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 + sessionRecording?: SessionRecording + webPerformance?: WebPerformanceObserver + exceptionAutocapture?: ExceptionObserver _triggered_notifs: any compression: Partial> @@ -316,13 +318,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) => { @@ -559,7 +554,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() } } } @@ -622,8 +617,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 { @@ -642,6 +637,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.') @@ -834,8 +832,8 @@ 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) { - return + if (!this.__loaded || !this.sessionPersistence || !this._requestQueue) { + return logger.unintializedWarning('posthog.capture') } if (userOptedOut(this, false)) { @@ -919,6 +917,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 +1021,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 +1048,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 +1075,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 +1084,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 +1093,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 +1193,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 +1253,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 logger.unintializedWarning('posthog.identify') + } //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 +1422,14 @@ export class PostHog { * Useful for clearing data when a user logs out. */ reset(reset_device_id?: boolean): void { + if (!this.__loaded) { + return logger.unintializedWarning('posthog.reset') + } 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')(uuidv7()) this.register_once( { @@ -1464,7 +1472,7 @@ export class PostHog { */ get_session_id(): string { - return this.sessionManager.checkAndGetSessionAndWindowId(true).sessionId + return this.sessionManager?.checkAndGetSessionAndWindowId(true).sessionId ?? '' } /** @@ -1475,6 +1483,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 @@ -1681,12 +1692,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 } @@ -1765,7 +1773,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 +1796,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 +1859,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 5b5b61ae6..0e3a40a0f 100644 --- a/src/posthog-featureflags.ts +++ b/src/posthog-featureflags.ts @@ -1,4 +1,4 @@ -import { _base64Encode, _entries, _extend } from './utils' +import { _base64Encode, _entries, _extend, logger } from './utils' import { PostHog } from './posthog-core' import { DecideResponse, @@ -225,7 +225,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 }) } @@ -265,6 +265,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() @@ -284,6 +287,10 @@ export class PostHogFeatureFlags { * @param {Object|Array|String} flags Flags to override with. */ override(flags: boolean | string[] | Record): void { + if (!this.instance.__loaded || !this.instance.persistence) { + return logger.unintializedWarning('posthog.feature_flags.override') + } + this._override_warning = false if (flags === false) { @@ -331,7 +338,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, }) @@ -350,7 +357,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 c94bbbb3b..428068a9b 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) } ) diff --git a/src/utils.ts b/src/utils.ts index 2ee2ab003..012107fb6 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -77,6 +77,11 @@ const logger = { } } }, + unintializedWarning: function (methodName: string): void { + if (Config.DEBUG && !_isUndefined(window.console) && window.console) { + logger.error(`[PostHog] You must initialize PostHog before calling ${methodName}`) + } + }, } // UNDERSCORE