From 43029352d88738ca16cdd9bf35d4fbf9b5a90908 Mon Sep 17 00:00:00 2001 From: Ben White Date: Wed, 15 Nov 2023 09:12:09 +0100 Subject: [PATCH 01/13] Fixed up window/document access --- .eslintrc.js | 2 ++ src/autocapture-utils.ts | 1 + src/autocapture.ts | 1 + src/decide.ts | 1 + src/extensions/replay/web-performance.ts | 1 + src/extensions/surveys.ts | 1 + src/extensions/toolbar.ts | 2 +- src/loader-exception-autocapture.ts | 8 ++++---- src/loader-recorder-v2.ts | 16 ++++++++++------ src/loader-recorder.ts | 12 ++++++------ src/loader-surveys.ts | 1 + src/posthog-surveys.ts | 1 + src/retry-queue.ts | 1 + src/storage.ts | 1 + src/utils/event-utils.ts | 2 +- src/utils/globals.ts | 7 ++++--- src/utils/index.ts | 2 +- 17 files changed, 38 insertions(+), 22 deletions(-) diff --git a/.eslintrc.js b/.eslintrc.js index bbdfa9260..0d07187e5 100644 --- a/.eslintrc.js +++ b/.eslintrc.js @@ -10,6 +10,7 @@ const rules = { 'no-prototype-builtins': 'off', 'no-empty': 'off', 'no-console': 'error', + 'no-restricted-globals': ['error', 'document', 'window'], } const extend = [ @@ -62,6 +63,7 @@ module.exports = { rules: { ...rules, 'no-console': 'off', + 'no-restricted-globals': 'off', }, }, { diff --git a/src/autocapture-utils.ts b/src/autocapture-utils.ts index c9172d854..8089b4e17 100644 --- a/src/autocapture-utils.ts +++ b/src/autocapture-utils.ts @@ -8,6 +8,7 @@ import { _each, _includes, _trim } from './utils' import { _isNull, _isString, _isUndefined } from './utils/type-utils' import { logger } from './utils/logger' +import { window } from './utils/globals' export function getClassName(el: Element): string { switch (typeof el.className) { diff --git a/src/autocapture.ts b/src/autocapture.ts index d2f4713d7..11f73afb7 100644 --- a/src/autocapture.ts +++ b/src/autocapture.ts @@ -21,6 +21,7 @@ import { AUTOCAPTURE_DISABLED_SERVER_SIDE } from './constants' import { _isBoolean, _isFunction, _isNull, _isUndefined } from './utils/type-utils' import { logger } from './utils/logger' +import { window, document } from './utils/globals' function limitText(length: number, text: string): string { if (text.length > length) { diff --git a/src/decide.ts b/src/decide.ts index fb4798566..4c0c648f9 100644 --- a/src/decide.ts +++ b/src/decide.ts @@ -6,6 +6,7 @@ import { STORED_GROUP_PROPERTIES_KEY, STORED_PERSON_PROPERTIES_KEY } from './con import { _isUndefined } from './utils/type-utils' import { logger } from './utils/logger' +import { window, document } from './utils/globals' export class Decide { instance: PostHog diff --git a/src/extensions/replay/web-performance.ts b/src/extensions/replay/web-performance.ts index d90cc0757..c77a07b62 100644 --- a/src/extensions/replay/web-performance.ts +++ b/src/extensions/replay/web-performance.ts @@ -4,6 +4,7 @@ import { isLocalhost } from '../../utils/request-utils' import { _isUndefined } from '../../utils/type-utils' import { logger } from '../../utils/logger' +import { window } from '../../utils/globals' const PERFORMANCE_EVENTS_MAPPING: { [key: string]: number } = { // BASE_PERFORMANCE_EVENT_COLUMNS diff --git a/src/extensions/surveys.ts b/src/extensions/surveys.ts index 52f783be7..601026206 100644 --- a/src/extensions/surveys.ts +++ b/src/extensions/surveys.ts @@ -10,6 +10,7 @@ import { } from '../posthog-surveys-types' import { _isUndefined } from '../utils/type-utils' +import { window, document } from '../utils/globals' const satisfiedEmoji = '' diff --git a/src/extensions/toolbar.ts b/src/extensions/toolbar.ts index bf91cf914..c018bc849 100644 --- a/src/extensions/toolbar.ts +++ b/src/extensions/toolbar.ts @@ -4,7 +4,7 @@ import { DecideResponse, ToolbarParams } from '../types' import { POSTHOG_MANAGED_HOSTS } from './cloud' import { _getHashParam } from '../utils/request-utils' import { logger } from '../utils/logger' -import { window } from '../utils/globals' +import { window, document } from '../utils/globals' // TRICKY: Many web frameworks will modify the route on load, potentially before posthog is initialized. // To get ahead of this we grab it as soon as the posthog-js is parsed diff --git a/src/loader-exception-autocapture.ts b/src/loader-exception-autocapture.ts index c46ca95e1..8d50b19df 100644 --- a/src/loader-exception-autocapture.ts +++ b/src/loader-exception-autocapture.ts @@ -1,9 +1,9 @@ import { extendPostHog } from './extensions/exception-autocapture' -import { _isUndefined } from './utils/type-utils' +import { window } from './utils/globals' -const win: Window & typeof globalThis = _isUndefined(window) ? ({} as typeof window) : window - -;(win as any).extendPostHogWithExceptionAutoCapture = extendPostHog +// eslint-disable-next-line @typescript-eslint/ban-ts-comment +// @ts-ignore +window.extendPostHogWithExceptionAutoCapture = extendPostHog export default extendPostHog diff --git a/src/loader-recorder-v2.ts b/src/loader-recorder-v2.ts index 598a991a2..5f5d32146 100644 --- a/src/loader-recorder-v2.ts +++ b/src/loader-recorder-v2.ts @@ -23,6 +23,7 @@ import type { IWindow, listenerHandler, RecordPlugin } from '@rrweb/types' import { InitiatorType, NetworkRecordOptions, NetworkRequest, Headers } from './types' import { _isBoolean, _isFunction, _isArray, _isUndefined, _isNull } from './utils/type-utils' import { logger } from './utils/logger' +import { window } from './utils/globals' import { defaultNetworkOptions } from './extensions/replay/config' export type NetworkData = { @@ -453,11 +454,14 @@ export const getRecordNetworkPlugin: (options?: NetworkRecordOptions) => RecordP } // rrweb/networ@1 ends - -const win: Window & typeof globalThis = _isUndefined(window) ? ({} as typeof window) : window - -;(win as any).rrweb = { record: rrwebRecord, version: 'v2', rrwebVersion: version } -;(win as any).rrwebConsoleRecord = { getRecordConsolePlugin } -;(win as any).getRecordNetworkPlugin = getRecordNetworkPlugin +// eslint-disable-next-line @typescript-eslint/ban-ts-comment +// @ts-ignore +window.rrweb = { record: rrwebRecord, version: 'v2', rrwebVersion: version } +// eslint-disable-next-line @typescript-eslint/ban-ts-comment +// @ts-ignore +window.rrwebConsoleRecord = { getRecordConsolePlugin } +// eslint-disable-next-line @typescript-eslint/ban-ts-comment +// @ts-ignore +window.getRecordNetworkPlugin = getRecordNetworkPlugin export default rrwebRecord diff --git a/src/loader-recorder.ts b/src/loader-recorder.ts index 7ac507e79..4079e146f 100644 --- a/src/loader-recorder.ts +++ b/src/loader-recorder.ts @@ -8,13 +8,13 @@ import rrwebRecord from 'rrweb-v1/es/rrweb/packages/rrweb/src/record' // @ts-ignore import { getRecordConsolePlugin } from 'rrweb-v1/es/rrweb/packages/rrweb/src/plugins/console/record' -import { _isUndefined } from './utils/type-utils' +import { window } from './utils/globals' + // eslint-disable-next-line @typescript-eslint/ban-ts-comment // @ts-ignore - -const win: Window & typeof globalThis = _isUndefined(window) ? ({} as typeof window) : window - -;(win as any).rrweb = { record: rrwebRecord, version: 'v1', rrwebVersion: version } -;(win as any).rrwebConsoleRecord = { getRecordConsolePlugin } +window.rrweb = { record: rrwebRecord, version: 'v1', rrwebVersion: version } +// eslint-disable-next-line @typescript-eslint/ban-ts-comment +// @ts-ignore +window.rrwebConsoleRecord = { getRecordConsolePlugin } export default rrwebRecord diff --git a/src/loader-surveys.ts b/src/loader-surveys.ts index 4925804f1..367e01edd 100644 --- a/src/loader-surveys.ts +++ b/src/loader-surveys.ts @@ -1,6 +1,7 @@ import { generateSurveys } from './extensions/surveys' import { _isUndefined } from './utils/type-utils' +import { window } from './utils/globals' const win: Window & typeof globalThis = _isUndefined(window) ? ({} as typeof window) : window diff --git a/src/posthog-surveys.ts b/src/posthog-surveys.ts index 730b8d25d..61fd8286d 100644 --- a/src/posthog-surveys.ts +++ b/src/posthog-surveys.ts @@ -2,6 +2,7 @@ import { PostHog } from './posthog-core' import { SURVEYS } from './constants' import { SurveyCallback, SurveyUrlMatchType } from './posthog-surveys-types' import { _isUrlMatchingRegex } from './utils/request-utils' +import { window, document } from './utils/globals' export const surveyUrlValidationMap: Record boolean> = { icontains: (conditionsUrl) => window.location.href.toLowerCase().indexOf(conditionsUrl.toLowerCase()) > -1, diff --git a/src/retry-queue.ts b/src/retry-queue.ts index f848e2a96..f40a7d287 100644 --- a/src/retry-queue.ts +++ b/src/retry-queue.ts @@ -5,6 +5,7 @@ import { RateLimiter } from './rate-limiter' import { _isUndefined } from './utils/type-utils' import { logger } from './utils/logger' +import { window } from './utils/globals' const thirtyMinutes = 30 * 60 * 1000 diff --git a/src/storage.ts b/src/storage.ts index a7a7a5606..c6a7a2025 100644 --- a/src/storage.ts +++ b/src/storage.ts @@ -4,6 +4,7 @@ import { DISTINCT_ID, SESSION_ID, SESSION_RECORDING_IS_SAMPLED } from './constan import { _isNull, _isUndefined } from './utils/type-utils' import { logger } from './utils/logger' +import { window, document } from './utils/globals' const DOMAIN_MATCH_REGEX = /[a-z0-9][a-z0-9-]+\.[a-z]{2,}$/i diff --git a/src/utils/event-utils.ts b/src/utils/event-utils.ts index b3a629d57..71b1d2791 100644 --- a/src/utils/event-utils.ts +++ b/src/utils/event-utils.ts @@ -3,7 +3,7 @@ import { _isNull, _isUndefined } from './type-utils' import { Properties } from '../types' import Config from '../config' import { _each, _extend, _includes, _strip_empty_properties, _timestamp } from './index' -import { document, userAgent } from './globals' +import { document, window, userAgent } from './globals' /** * Safari detection turns out to be complicted. For e.g. https://stackoverflow.com/a/29696509 diff --git a/src/utils/globals.ts b/src/utils/globals.ts index 42c3f6800..1ef9d8a16 100644 --- a/src/utils/globals.ts +++ b/src/utils/globals.ts @@ -4,9 +4,10 @@ export const ArrayProto = Array.prototype export const nativeForEach = ArrayProto.forEach export const nativeIndexOf = ArrayProto.indexOf +// eslint-disable-next-line no-restricted-globals export const win: Window & typeof globalThis = typeof window !== 'undefined' ? window : ({} as typeof window) const navigator = win.navigator || { userAgent: '' } -const document = win.document || {} -const userAgent = navigator.userAgent +export const document = win.document || {} +export const userAgent = navigator.userAgent -export { win as window, userAgent, document } +export { win as window } diff --git a/src/utils/index.ts b/src/utils/index.ts index 99a5d237e..bf31efec9 100644 --- a/src/utils/index.ts +++ b/src/utils/index.ts @@ -10,7 +10,7 @@ import { hasOwnProperty, } from './type-utils' import { logger } from './logger' -import { document, nativeForEach, nativeIndexOf } from './globals' +import { window, document, nativeForEach, nativeIndexOf } from './globals' const breaker: Breaker = {} From 0564a68c5b868a8578ef99eb2302de31317053ea Mon Sep 17 00:00:00 2001 From: Ben White Date: Wed, 15 Nov 2023 09:55:08 +0100 Subject: [PATCH 02/13] Fixes --- .eslintrc.js | 8 ++- .../replay/sessionrecording.test.ts | 53 ++++++++++--------- src/__tests__/extensions/toolbar.test.ts | 31 +++++------ src/__tests__/utils/event-utils.test.ts | 2 - src/decide.ts | 4 +- src/extensions/replay/sessionrecording.ts | 10 ++-- src/extensions/toolbar.ts | 10 ++-- src/posthog-core.ts | 10 ++-- src/utils/event-utils.ts | 10 ++-- src/utils/globals.ts | 1 + src/utils/logger.ts | 4 +- 11 files changed, 75 insertions(+), 68 deletions(-) diff --git a/.eslintrc.js b/.eslintrc.js index 0d07187e5..02b03cf7f 100644 --- a/.eslintrc.js +++ b/.eslintrc.js @@ -10,7 +10,6 @@ const rules = { 'no-prototype-builtins': 'off', 'no-empty': 'off', 'no-console': 'error', - 'no-restricted-globals': ['error', 'document', 'window'], } const extend = [ @@ -54,6 +53,13 @@ module.exports = { }, }, overrides: [ + { + files: 'src/**/*', + rules: { + ...rules, + 'no-restricted-globals': ['error', 'document', 'window'], + }, + }, { files: 'src/__tests__/**/*', // the same set of config as in the root diff --git a/src/__tests__/extensions/replay/sessionrecording.test.ts b/src/__tests__/extensions/replay/sessionrecording.test.ts index c82747c24..c74dd883f 100644 --- a/src/__tests__/extensions/replay/sessionrecording.test.ts +++ b/src/__tests__/extensions/replay/sessionrecording.test.ts @@ -19,6 +19,7 @@ import { RECORDING_MAX_EVENT_SIZE, SessionRecording, } from '../../../extensions/replay/sessionrecording' +import { assignableWindow } from '../../../utils/globals' // Type and source defined here designate a non-user-generated recording event @@ -52,8 +53,8 @@ describe('SessionRecording', () => { let onFeatureFlagsCallback: ((flags: string[]) => void) | null beforeEach(() => { - ;(window as any).rrwebRecord = jest.fn() - ;(window as any).rrwebConsoleRecord = { + assignableWindow.rrwebRecord = jest.fn() + assignableWindow.rrwebConsoleRecord = { getRecordConsolePlugin: jest.fn(), } @@ -193,7 +194,7 @@ describe('SessionRecording', () => { beforeEach(() => { jest.spyOn(sessionRecording, 'startRecordingIfEnabled') ;(loadScript as any).mockImplementation((_path: any, callback: any) => callback()) - ;(window as any).rrwebRecord = jest.fn(({ emit }) => { + assignableWindow.rrwebRecord = jest.fn(({ emit }) => { _emit = emit return () => {} }) @@ -277,11 +278,11 @@ describe('SessionRecording', () => { describe('recording', () => { beforeEach(() => { const mockFullSnapshot = jest.fn() - ;(window as any).rrwebRecord = jest.fn(({ emit }) => { + assignableWindow.rrwebRecord = jest.fn(({ emit }) => { _emit = emit return () => {} }) - ;(window as any).rrwebRecord.takeFullSnapshot = mockFullSnapshot + assignableWindow.rrwebRecord.takeFullSnapshot = mockFullSnapshot ;(loadScript as any).mockImplementation((_path: any, callback: any) => callback()) }) @@ -356,7 +357,7 @@ describe('SessionRecording', () => { sessionRecording: { endpoint: '/s/', sampleRate: '0.50' }, }) ) - const emitValues = [] + const emitValues: string[] = [] let lastSessionId = sessionRecording['sessionId'] for (let i = 0; i < 100; i++) { @@ -368,7 +369,7 @@ describe('SessionRecording', () => { expect(sessionRecording['sessionId']).not.toBe(lastSessionId) lastSessionId = sessionRecording['sessionId'] - emitValues.push(sessionRecording['status']) + emitValues.push(sessionRecording.status) } // the random number generator won't always be exactly 0.5, but it should be close @@ -384,7 +385,7 @@ describe('SessionRecording', () => { // maskAllInputs should change from default // someUnregisteredProp should not be present - expect((window as any).rrwebRecord).toHaveBeenCalledWith({ + expect(assignableWindow.rrwebRecord).toHaveBeenCalledWith({ emit: expect.anything(), maskAllInputs: false, blockClass: 'ph-no-capture', @@ -680,7 +681,7 @@ describe('SessionRecording', () => { sessionRecording.startRecordingIfEnabled() - expect((window as any).rrwebConsoleRecord.getRecordConsolePlugin).not.toHaveBeenCalled() + expect(assignableWindow.rrwebConsoleRecord.getRecordConsolePlugin).not.toHaveBeenCalled() }) it('if enabled, plugin is used', () => { @@ -688,7 +689,7 @@ describe('SessionRecording', () => { sessionRecording.startRecordingIfEnabled() - expect((window as any).rrwebConsoleRecord.getRecordConsolePlugin).toHaveBeenCalled() + expect(assignableWindow.rrwebConsoleRecord.getRecordConsolePlugin).toHaveBeenCalled() }) }) @@ -710,32 +711,32 @@ describe('SessionRecording', () => { sessionIdGeneratorMock.mockImplementation(() => 'newSessionId') windowIdGeneratorMock.mockImplementation(() => 'newWindowId') _emit(createIncrementalSnapshot()) - expect((window as any).rrwebRecord.takeFullSnapshot).toHaveBeenCalled() + expect(assignableWindow.rrwebRecord.takeFullSnapshot).toHaveBeenCalled() }) it('sends a full snapshot if there is a new window id and the event is not type FullSnapshot or Meta', () => { sessionIdGeneratorMock.mockImplementation(() => 'old-session-id') windowIdGeneratorMock.mockImplementation(() => 'newWindowId') _emit(createIncrementalSnapshot()) - expect((window as any).rrwebRecord.takeFullSnapshot).toHaveBeenCalled() + expect(assignableWindow.rrwebRecord.takeFullSnapshot).toHaveBeenCalled() }) it('does not send a full snapshot if there is a new session/window id and the event is type FullSnapshot or Meta', () => { sessionIdGeneratorMock.mockImplementation(() => 'newSessionId') windowIdGeneratorMock.mockImplementation(() => 'newWindowId') _emit(createIncrementalSnapshot({ type: META_EVENT_TYPE })) - expect((window as any).rrwebRecord.takeFullSnapshot).not.toHaveBeenCalled() + expect(assignableWindow.rrwebRecord.takeFullSnapshot).not.toHaveBeenCalled() }) it('does not send a full snapshot if there is not a new session or window id', () => { - ;(window as any).rrwebRecord.takeFullSnapshot.mockClear() + assignableWindow.rrwebRecord.takeFullSnapshot.mockClear() sessionIdGeneratorMock.mockImplementation(() => 'old-session-id') windowIdGeneratorMock.mockImplementation(() => 'old-window-id') sessionManager.resetSessionId() _emit(createIncrementalSnapshot()) - expect((window as any).rrwebRecord.takeFullSnapshot).not.toHaveBeenCalled() + expect(assignableWindow.rrwebRecord.takeFullSnapshot).not.toHaveBeenCalled() }) }) @@ -843,7 +844,7 @@ describe('SessionRecording', () => { it('takes a full snapshot for the first _emit', () => { emitAtDateTime(startingDate) - expect((window as any).rrwebRecord.takeFullSnapshot).toHaveBeenCalledTimes(1) + expect(assignableWindow.rrwebRecord.takeFullSnapshot).toHaveBeenCalledTimes(1) }) it('does not take a full snapshot for the second _emit', () => { @@ -857,7 +858,7 @@ describe('SessionRecording', () => { startingDate.getMinutes() + 1 ) ) - expect((window as any).rrwebRecord.takeFullSnapshot).toHaveBeenCalledTimes(1) + expect(assignableWindow.rrwebRecord.takeFullSnapshot).toHaveBeenCalledTimes(1) }) it('does not change session id for a second _emit', () => { @@ -899,7 +900,7 @@ describe('SessionRecording', () => { startingDate.getMinutes() + 2 ) ) - expect((window as any).rrwebRecord.takeFullSnapshot).toHaveBeenCalledTimes(1) + expect(assignableWindow.rrwebRecord.takeFullSnapshot).toHaveBeenCalledTimes(1) }) it('sends a full snapshot if the session is rotated because session has been inactive for 30 minutes', () => { @@ -925,7 +926,7 @@ describe('SessionRecording', () => { emitAtDateTime(inactivityThresholdLater) expect(sessionManager['_getSessionId']()[1]).not.toEqual(startingSessionId) - expect((window as any).rrwebRecord.takeFullSnapshot).toHaveBeenCalledTimes(2) + expect(assignableWindow.rrwebRecord.takeFullSnapshot).toHaveBeenCalledTimes(2) }) it('sends a full snapshot if the session is rotated because max time has passed', () => { @@ -950,7 +951,7 @@ describe('SessionRecording', () => { emitAtDateTime(moreThanADayLater) expect(sessionManager['_getSessionId']()[1]).not.toEqual(startingSessionId) - expect((window as any).rrwebRecord.takeFullSnapshot).toHaveBeenCalledTimes(2) + expect(assignableWindow.rrwebRecord.takeFullSnapshot).toHaveBeenCalledTimes(2) }) }) @@ -960,7 +961,7 @@ describe('SessionRecording', () => { const lastActivityTimestamp = sessionRecording['_lastActivityTimestamp'] expect(lastActivityTimestamp).toBeGreaterThan(0) - expect((window as any).rrwebRecord.takeFullSnapshot).toHaveBeenCalledTimes(0) + expect(assignableWindow.rrwebRecord.takeFullSnapshot).toHaveBeenCalledTimes(0) _emit({ event: 123, @@ -973,7 +974,7 @@ describe('SessionRecording', () => { expect(sessionRecording['isIdle']).toEqual(false) expect(sessionRecording['_lastActivityTimestamp']).toEqual(lastActivityTimestamp + 100) - expect((window as any).rrwebRecord.takeFullSnapshot).toHaveBeenCalledTimes(1) + expect(assignableWindow.rrwebRecord.takeFullSnapshot).toHaveBeenCalledTimes(1) _emit({ event: 123, @@ -985,7 +986,7 @@ describe('SessionRecording', () => { }) expect(sessionRecording['isIdle']).toEqual(false) expect(sessionRecording['_lastActivityTimestamp']).toEqual(lastActivityTimestamp + 100) - expect((window as any).rrwebRecord.takeFullSnapshot).toHaveBeenCalledTimes(1) + expect(assignableWindow.rrwebRecord.takeFullSnapshot).toHaveBeenCalledTimes(1) // this triggers idle state and isn't a user interaction so does not take a full snapshot _emit({ @@ -998,7 +999,7 @@ describe('SessionRecording', () => { }) expect(sessionRecording['isIdle']).toEqual(true) expect(sessionRecording['_lastActivityTimestamp']).toEqual(lastActivityTimestamp + 100) - expect((window as any).rrwebRecord.takeFullSnapshot).toHaveBeenCalledTimes(1) + expect(assignableWindow.rrwebRecord.takeFullSnapshot).toHaveBeenCalledTimes(1) // this triggers idle state _and_ is a user interaction, so we take a full snapshot _emit({ @@ -1013,7 +1014,7 @@ describe('SessionRecording', () => { expect(sessionRecording['_lastActivityTimestamp']).toEqual( lastActivityTimestamp + RECORDING_IDLE_ACTIVITY_TIMEOUT_MS + 2000 ) - expect((window as any).rrwebRecord.takeFullSnapshot).toHaveBeenCalledTimes(2) + expect(assignableWindow.rrwebRecord.takeFullSnapshot).toHaveBeenCalledTimes(2) }) }) }) @@ -1046,7 +1047,7 @@ describe('SessionRecording', () => { describe('buffering minimum duration', () => { beforeEach(() => { - ;(window as any).rrwebRecord = jest.fn(({ emit }) => { + assignableWindow.rrwebRecord = jest.fn(({ emit }) => { _emit = emit return () => {} }) diff --git a/src/__tests__/extensions/toolbar.test.ts b/src/__tests__/extensions/toolbar.test.ts index 27f63b418..c938cdaaa 100644 --- a/src/__tests__/extensions/toolbar.test.ts +++ b/src/__tests__/extensions/toolbar.test.ts @@ -2,7 +2,7 @@ import { Toolbar } from '../../extensions/toolbar' import { _isString, _isUndefined } from '../../utils/type-utils' import { PostHog } from '../../posthog-core' import { PostHogConfig, ToolbarParams } from '../../types' -import { window } from '../../utils/globals' +import { assignableWindow, window } from '../../utils/globals' jest.mock('../../utils', () => ({ ...jest.requireActual('../../utils'), @@ -31,8 +31,8 @@ describe('Toolbar', () => { }) beforeEach(() => { - ;(window as any).ph_load_toolbar = jest.fn() - delete (window as any)['_postHogToolbarLoaded'] + assignableWindow.ph_load_toolbar = jest.fn() + delete assignableWindow['_postHogToolbarLoaded'] }) describe('maybeLoadToolbar', () => { @@ -40,7 +40,8 @@ describe('Toolbar', () => { getItem: jest.fn(), setItem: jest.fn(), } - const history = { replaceState: jest.fn() } + const storage = localStorage as unknown as Storage + const history = { replaceState: jest.fn() } as unknown as History const defaultHashState = { action: 'ph_authorize', @@ -71,7 +72,7 @@ describe('Toolbar', () => { .join('&') } - const aLocation = (hash?: string) => { + const aLocation = (hash?: string): Location => { if (_isUndefined(hash)) { hash = withHash(withHashParamsFrom()) } @@ -80,7 +81,7 @@ describe('Toolbar', () => { hash: `#${hash}`, pathname: 'pathname', search: '?search', - } + } as Location } beforeEach(() => { @@ -91,7 +92,7 @@ describe('Toolbar', () => { it('should initialize the toolbar when the hash state contains action "ph_authorize"', () => { // the default hash state in the test setup contains the action "ph_authorize" - toolbar.maybeLoadToolbar(aLocation(), localStorage, history) + toolbar.maybeLoadToolbar(aLocation(), storage, history) expect(toolbar.loadToolbar).toHaveBeenCalledWith({ ...toolbarParams, @@ -105,7 +106,7 @@ describe('Toolbar', () => { localStorage.getItem.mockImplementation(() => JSON.stringify(toolbarParams)) const hashState = { ...defaultHashState, action: undefined } - toolbar.maybeLoadToolbar(aLocation(withHash(withHashParamsFrom(hashState))), localStorage, history) + toolbar.maybeLoadToolbar(aLocation(withHash(withHashParamsFrom(hashState))), storage, history) expect(toolbar.loadToolbar).toHaveBeenCalledWith({ ...toolbarParams, @@ -114,14 +115,14 @@ describe('Toolbar', () => { }) it('should NOT initialize the toolbar when the activation query param does not exist', () => { - expect(toolbar.maybeLoadToolbar(aLocation(''), localStorage, history)).toEqual(false) + expect(toolbar.maybeLoadToolbar(aLocation(''), storage, history)).toEqual(false) expect(toolbar.loadToolbar).not.toHaveBeenCalled() }) it('should return false when parsing invalid JSON from fragment state', () => { expect( - toolbar.maybeLoadToolbar(aLocation(withHash(withHashParamsFrom('literally'))), localStorage, history) + toolbar.maybeLoadToolbar(aLocation(withHash(withHashParamsFrom('literally'))), storage, history) ).toEqual(false) expect(toolbar.loadToolbar).not.toHaveBeenCalled() }) @@ -129,7 +130,7 @@ describe('Toolbar', () => { it('should work if calling toolbar params `__posthog`', () => { toolbar.maybeLoadToolbar( aLocation(withHash(withHashParamsFrom(defaultHashState, '__posthog'))), - localStorage, + storage, history ) expect(toolbar.loadToolbar).toHaveBeenCalledWith({ ...toolbarParams, ...defaultHashState, source: 'url' }) @@ -138,7 +139,7 @@ describe('Toolbar', () => { it('should use the apiURL in the hash if available', () => { toolbar.maybeLoadToolbar( aLocation(withHash(withHashParamsFrom({ ...defaultHashState, apiURL: 'blabla' }))), - localStorage, + storage, history ) @@ -154,7 +155,7 @@ describe('Toolbar', () => { describe('load and close toolbar', () => { it('should persist for next time', () => { expect(toolbar.loadToolbar(toolbarParams)).toBe(true) - expect(JSON.parse((window as any).localStorage.getItem('_postHogToolbarParams'))).toEqual({ + expect(JSON.parse(window.localStorage.getItem('_postHogToolbarParams') ?? '')).toEqual({ ...toolbarParams, apiURL: 'http://api.example.com', }) @@ -162,7 +163,7 @@ describe('Toolbar', () => { it('should load if not previously loaded', () => { expect(toolbar.loadToolbar(toolbarParams)).toBe(true) - expect((window as any).ph_load_toolbar).toHaveBeenCalledWith( + expect(assignableWindow.ph_load_toolbar).toHaveBeenCalledWith( { ...toolbarParams, apiURL: 'http://api.example.com' }, instance ) @@ -181,7 +182,7 @@ describe('Toolbar', () => { it('should load if not previously loaded', () => { expect(toolbar.loadToolbar(minimalToolbarParams)).toBe(true) - expect((window as any).ph_load_toolbar).toHaveBeenCalledWith( + expect(assignableWindow.ph_load_toolbar).toHaveBeenCalledWith( { ...minimalToolbarParams, apiURL: 'http://api.example.com', diff --git a/src/__tests__/utils/event-utils.test.ts b/src/__tests__/utils/event-utils.test.ts index 1e996c227..4dd7027e5 100644 --- a/src/__tests__/utils/event-utils.test.ts +++ b/src/__tests__/utils/event-utils.test.ts @@ -1,8 +1,6 @@ import { _info } from '../../utils/event-utils' import * as globals from '../../utils/globals' -jest.mock('../../utils/globals') - describe(`event-utils`, () => { describe('properties', () => { it('should have $host and $pathname in properties', () => { diff --git a/src/decide.ts b/src/decide.ts index 4c0c648f9..e8b05ae21 100644 --- a/src/decide.ts +++ b/src/decide.ts @@ -6,7 +6,7 @@ import { STORED_GROUP_PROPERTIES_KEY, STORED_PERSON_PROPERTIES_KEY } from './con import { _isUndefined } from './utils/type-utils' import { logger } from './utils/logger' -import { window, document } from './utils/globals' +import { window, document, assignableWindow } from './utils/globals' export class Decide { instance: PostHog @@ -113,7 +113,7 @@ export class Decide { apiHost[apiHost.length - 1] === '/' && url[0] === '/' ? url.substring(1) : url, ].join('') - ;(window as any)[`__$$ph_site_app_${id}`] = this.instance + assignableWindow[`__$$ph_site_app_${id}`] = this.instance loadScript(scriptUrl, (err) => { if (err) { diff --git a/src/extensions/replay/sessionrecording.ts b/src/extensions/replay/sessionrecording.ts index 7814f4454..7d3d19f66 100644 --- a/src/extensions/replay/sessionrecording.ts +++ b/src/extensions/replay/sessionrecording.ts @@ -21,7 +21,7 @@ import { _timestamp, loadScript } from '../../utils' import { _isBoolean, _isFunction, _isNull, _isNumber, _isObject, _isString, _isUndefined } from '../../utils/type-utils' import { logger } from '../../utils/logger' -import { window } from '../../utils/globals' +import { assignableWindow, window } from '../../utils/globals' import { buildNetworkRequestOptions } from './config' const BASE_ENDPOINT = '/s/' @@ -469,13 +469,13 @@ export class SessionRecording { const plugins = [] - if ((window as any).rrwebConsoleRecord && this.isConsoleLogCaptureEnabled) { - plugins.push((window as any).rrwebConsoleRecord.getRecordConsolePlugin()) + if (assignableWindow.rrwebConsoleRecord && this.isConsoleLogCaptureEnabled) { + plugins.push(assignableWindow.rrwebConsoleRecord.getRecordConsolePlugin()) } if (this._networkPayloadCapture) { - if (_isFunction((window as any).getRecordNetworkPlugin)) { + if (_isFunction(assignableWindow.getRecordNetworkPlugin)) { plugins.push( - (window as any).getRecordNetworkPlugin( + assignableWindow.getRecordNetworkPlugin( buildNetworkRequestOptions(this.instance.config, this._networkPayloadCapture) ) ) diff --git a/src/extensions/toolbar.ts b/src/extensions/toolbar.ts index c018bc849..fe6200cf5 100644 --- a/src/extensions/toolbar.ts +++ b/src/extensions/toolbar.ts @@ -4,7 +4,7 @@ import { DecideResponse, ToolbarParams } from '../types' import { POSTHOG_MANAGED_HOSTS } from './cloud' import { _getHashParam } from '../utils/request-utils' import { logger } from '../utils/logger' -import { window, document } from '../utils/globals' +import { window, document, assignableWindow } from '../utils/globals' // TRICKY: Many web frameworks will modify the route on load, potentially before posthog is initialized. // To get ahead of this we grab it as soon as the posthog-js is parsed @@ -114,11 +114,11 @@ export class Toolbar { } loadToolbar(params?: ToolbarParams): boolean { - if ((window as any)['_postHogToolbarLoaded']) { + if (assignableWindow['_postHogToolbarLoaded']) { return false } // only load the toolbar once, even if there are multiple instances of PostHogLib - ;(window as any)['_postHogToolbarLoaded'] = true + assignableWindow['_postHogToolbarLoaded'] = true const host = this.instance.config.api_host // toolbar.js is served from the PostHog CDN, this has a TTL of 24 hours. @@ -146,12 +146,12 @@ export class Toolbar { logger.error('Failed to load toolbar', err) return } - ;((window as any)['ph_load_toolbar'] || (window as any)['ph_load_editor'])(toolbarParams, this.instance) + ;(assignableWindow['ph_load_toolbar'] || assignableWindow['ph_load_editor'])(toolbarParams, this.instance) }) // Turbolinks doesn't fire an onload event but does replace the entire body, including the toolbar. // Thus, we ensure the toolbar is only loaded inside the body, and then reloaded on turbolinks:load. _register_event(window, 'turbolinks:load', () => { - ;(window as any)['_postHogToolbarLoaded'] = false + assignableWindow['_postHogToolbarLoaded'] = false this.loadToolbar(toolbarParams) }) return true diff --git a/src/posthog-core.ts b/src/posthog-core.ts index 7a844ed3d..92ee7616c 100644 --- a/src/posthog-core.ts +++ b/src/posthog-core.ts @@ -9,7 +9,7 @@ import { _safewrap_class, isCrossDomainCookie, } from './utils' -import { window } from './utils/globals' +import { assignableWindow, window } from './utils/globals' import { autocapture } from './autocapture' import { PostHogFeatureFlags } from './posthog-featureflags' import { PostHogPersistence } from './posthog-persistence' @@ -2105,7 +2105,7 @@ const override_ph_init_func = function () { ;(posthog_master as any) = instance if (init_type === InitType.INIT_SNIPPET) { - ;(window as any)[PRIMARY_INSTANCE_NAME] = posthog_master + assignableWindow[PRIMARY_INSTANCE_NAME] = posthog_master } extend_mp() return instance @@ -2147,10 +2147,10 @@ const add_dom_loaded_handler = function () { export function init_from_snippet(): void { init_type = InitType.INIT_SNIPPET - if (_isUndefined((window as any).posthog)) { - ;(window as any).posthog = [] + if (_isUndefined(assignableWindow.posthog)) { + assignableWindow.posthog = [] } - posthog_master = (window as any).posthog + posthog_master = assignableWindow.posthog if (posthog_master['__loaded'] || (posthog_master['config'] && posthog_master['persistence'])) { // lib has already been loaded at least once; we don't want to override the global object this time so bomb early diff --git a/src/utils/event-utils.ts b/src/utils/event-utils.ts index 71b1d2791..463951e05 100644 --- a/src/utils/event-utils.ts +++ b/src/utils/event-utils.ts @@ -3,7 +3,7 @@ import { _isNull, _isUndefined } from './type-utils' import { Properties } from '../types' import Config from '../config' import { _each, _extend, _includes, _strip_empty_properties, _timestamp } from './index' -import { document, window, userAgent } from './globals' +import { document, window, userAgent, assignableWindow } from './globals' /** * Safari detection turns out to be complicted. For e.g. https://stackoverflow.com/a/29696509 @@ -267,7 +267,7 @@ export const _info = { _strip_empty_properties({ $os: os_name, $os_version: os_version, - $browser: _info.browser(userAgent, navigator.vendor, (window as any).opera), + $browser: _info.browser(userAgent, navigator.vendor, assignableWindow.opera), $device: _info.device(userAgent), $device_type: _info.deviceType(userAgent), }), @@ -276,7 +276,7 @@ export const _info = { $host: window?.location.host, $pathname: window?.location.pathname, $raw_user_agent: userAgent.length > 1000 ? userAgent.substring(0, 997) + '...' : userAgent, - $browser_version: _info.browserVersion(userAgent, navigator.vendor, (window as any).opera), + $browser_version: _info.browserVersion(userAgent, navigator.vendor, assignableWindow.opera), $browser_language: _info.browserLanguage(), $screen_height: window?.screen.height, $screen_width: window?.screen.width, @@ -296,10 +296,10 @@ export const _info = { _strip_empty_properties({ $os: os_name, $os_version: os_version, - $browser: _info.browser(userAgent, navigator.vendor, (window as any).opera), + $browser: _info.browser(userAgent, navigator.vendor, assignableWindow.opera), }), { - $browser_version: _info.browserVersion(userAgent, navigator.vendor, (window as any).opera), + $browser_version: _info.browserVersion(userAgent, navigator.vendor, assignableWindow.opera), } ) }, diff --git a/src/utils/globals.ts b/src/utils/globals.ts index 1ef9d8a16..dab59570c 100644 --- a/src/utils/globals.ts +++ b/src/utils/globals.ts @@ -9,5 +9,6 @@ export const win: Window & typeof globalThis = typeof window !== 'undefined' ? w const navigator = win.navigator || { userAgent: '' } export const document = win.document || {} export const userAgent = navigator.userAgent +export const assignableWindow: Window & typeof globalThis & Record = win export { win as window } diff --git a/src/utils/logger.ts b/src/utils/logger.ts index 48a52f89e..63f456087 100644 --- a/src/utils/logger.ts +++ b/src/utils/logger.ts @@ -1,11 +1,11 @@ import Config from '../config' import { _isUndefined } from './type-utils' -import { window } from './globals' +import { assignableWindow, window } from './globals' const LOGGER_PREFIX = '[PostHog.js]' export const logger = { _log: (level: 'log' | 'warn' | 'error', ...args: any[]) => { - if ((Config.DEBUG || (window as any).POSTHOG_DEBUG) && !_isUndefined(window.console) && window.console) { + if ((Config.DEBUG || assignableWindow.POSTHOG_DEBUG) && !_isUndefined(window.console) && window.console) { const consoleLog = '__rrweb_original__' in window.console[level] ? (window.console[level] as any)['__rrweb_original__'] From 21e4127c6dcc30953400489d581a9ba700a5cf69 Mon Sep 17 00:00:00 2001 From: Ben White Date: Wed, 15 Nov 2023 10:00:56 +0100 Subject: [PATCH 03/13] Fix --- src/__tests__/storage.test.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/__tests__/storage.test.ts b/src/__tests__/storage.test.ts index a1e6191c9..f36e5094f 100644 --- a/src/__tests__/storage.test.ts +++ b/src/__tests__/storage.test.ts @@ -36,7 +36,9 @@ describe('sessionStore', () => { expected: '', }, ])(`%s subdomain check`, ({ candidate, expected }) => { - expect(seekFirstNonPublicSubDomain(candidate, mockDocumentDotCookie)).toEqual(expected) + expect(seekFirstNonPublicSubDomain(candidate, mockDocumentDotCookie as unknown as Document)).toEqual( + expected + ) }) }) From 4d23f2d53a6216ed17f973919141bf854a8c23e7 Mon Sep 17 00:00:00 2001 From: Ben White Date: Wed, 15 Nov 2023 11:26:50 +0100 Subject: [PATCH 04/13] fix: Allow window to be truly undefined (#895) --- src/autocapture-utils.ts | 2 +- src/autocapture.ts | 14 ++++++--- src/extensions/exception-autocapture/index.ts | 13 +++++---- src/extensions/replay/sessionrecording.ts | 6 ++-- src/extensions/surveys.ts | 13 +++++++-- src/extensions/toolbar.ts | 20 ++++++++----- src/gdpr-utils.ts | 4 +-- src/loader-recorder-v2.ts | 15 ++++------ src/loader-recorder.ts | 10 +++---- src/loader-surveys.ts | 7 ++--- src/page-view.ts | 24 ++++++++------- src/posthog-core.ts | 29 ++++++++++--------- src/posthog-surveys.ts | 9 +++--- src/retry-queue.ts | 2 +- src/session-props.ts | 8 ++--- src/sessionid.ts | 2 +- src/storage.ts | 26 ++++++++++++----- src/utils/event-utils.ts | 17 +++++++---- src/utils/globals.ts | 10 +++---- src/utils/index.ts | 9 ++++-- src/utils/logger.ts | 7 ++++- src/uuidv7.ts | 2 +- 22 files changed, 148 insertions(+), 101 deletions(-) diff --git a/src/autocapture-utils.ts b/src/autocapture-utils.ts index 8089b4e17..5f2d652cd 100644 --- a/src/autocapture-utils.ts +++ b/src/autocapture-utils.ts @@ -110,7 +110,7 @@ export function shouldCaptureDomEvent( event: Event, autocaptureConfig: AutocaptureConfig | undefined = undefined ): boolean { - if (!el || isTag(el, 'html') || !isElementNode(el)) { + if (!window || !el || isTag(el, 'html') || !isElementNode(el)) { return false } diff --git a/src/autocapture.ts b/src/autocapture.ts index 11f73afb7..86faa9be1 100644 --- a/src/autocapture.ts +++ b/src/autocapture.ts @@ -132,7 +132,7 @@ const autocapture = { _extractCustomPropertyValue: function (customProperty: AutoCaptureCustomProperty): string { const propValues: string[] = [] - _each(document.querySelectorAll(customProperty['css_selector']), function (matchedElem) { + _each(document?.querySelectorAll(customProperty['css_selector']), function (matchedElem) { let value if (['input', 'select'].indexOf(matchedElem.tagName.toLowerCase()) > -1) { @@ -153,7 +153,7 @@ const autocapture = { const props: Properties = {} // will be deleted _each(this._customProperties, (customProperty) => { _each(customProperty['event_selectors'], (eventSelector) => { - const eventElements = document.querySelectorAll(eventSelector) + const eventElements = document?.querySelectorAll(eventSelector) _each(eventElements, (eventElement) => { if (_includes(targetElementList, eventElement) && shouldCaptureElement(eventElement)) { props[customProperty['name']] = this._extractCustomPropertyValue(customProperty) @@ -271,12 +271,18 @@ const autocapture = { // only reason is to stub for unit tests // since you can't override window.location props _navigate: function (href: string): void { + if (!window) { + return + } window.location.href = href }, _addDomEventHandlers: function (instance: PostHog): void { + if (!window || !document) { + return + } const handler = (e: Event) => { - e = e || window.event + e = e || window?.event this._captureEvent(e, instance) } _register_event(document, 'submit', handler, false, true) @@ -359,7 +365,7 @@ const autocapture = { }, isBrowserSupported: function (): boolean { - return _isFunction(document.querySelectorAll) + return _isFunction(document?.querySelectorAll) }, } diff --git a/src/extensions/exception-autocapture/index.ts b/src/extensions/exception-autocapture/index.ts index f0d093d23..3ae4e6d7a 100644 --- a/src/extensions/exception-autocapture/index.ts +++ b/src/extensions/exception-autocapture/index.ts @@ -18,8 +18,8 @@ export const extendPostHog = (instance: PostHog, response: DecideResponse) => { export class ExceptionObserver { instance: PostHog remoteEnabled: boolean | undefined - private originalOnErrorHandler: typeof window['onerror'] | null | undefined = undefined - private originalOnUnhandledRejectionHandler: typeof window['onunhandledrejection'] | null | undefined = undefined + private originalOnErrorHandler: Window['onerror'] | null | undefined = undefined + private originalOnUnhandledRejectionHandler: Window['onunhandledrejection'] | null | undefined = undefined private errorsToIgnore: RegExp[] = [] @@ -28,7 +28,7 @@ export class ExceptionObserver { } startCapturing() { - if (!this.isEnabled() || (window.onerror as any)?.__POSTHOG_INSTRUMENTED__) { + if (!window || !this.isEnabled() || (window.onerror as any)?.__POSTHOG_INSTRUMENTED__) { return } @@ -56,7 +56,7 @@ export class ExceptionObserver { const errorProperties: ErrorProperties = unhandledRejectionToProperties(args) this.sendExceptionEvent(errorProperties) - if (this.originalOnUnhandledRejectionHandler) { + if (window && this.originalOnUnhandledRejectionHandler) { // eslint-disable-next-line prefer-rest-params return this.originalOnUnhandledRejectionHandler.apply(window, args) } @@ -71,6 +71,9 @@ export class ExceptionObserver { } stopCapturing() { + if (!window) { + return + } if (!_isUndefined(this.originalOnErrorHandler)) { window.onerror = this.originalOnErrorHandler this.originalOnErrorHandler = null @@ -85,7 +88,7 @@ export class ExceptionObserver { } isCapturing() { - return !!(window.onerror as any)?.__POSTHOG_INSTRUMENTED__ + return !!(window?.onerror as any)?.__POSTHOG_INSTRUMENTED__ } isEnabled() { diff --git a/src/extensions/replay/sessionrecording.ts b/src/extensions/replay/sessionrecording.ts index 647b09647..9f4fd8cfd 100644 --- a/src/extensions/replay/sessionrecording.ts +++ b/src/extensions/replay/sessionrecording.ts @@ -133,7 +133,7 @@ export class SessionRecording { private get isRecordingEnabled() { const enabled_server_side = !!this.instance.get_property(SESSION_RECORDING_ENABLED_SERVER_SIDE) const enabled_client_side = !this.instance.config.disable_session_recording - return enabled_server_side && enabled_client_side + return window && enabled_server_side && enabled_client_side } private get isConsoleLogCaptureEnabled() { @@ -192,7 +192,7 @@ export class SessionRecording { this.stopRrweb = undefined this.receivedDecide = false - window.addEventListener('beforeunload', () => { + window?.addEventListener('beforeunload', () => { this._flushBuffer() }) @@ -507,7 +507,7 @@ export class SessionRecording { // so we catch all errors. try { if (eventName === '$pageview') { - const href = this._maskUrl(window.location.href) + const href = window ? this._maskUrl(window.location.href) : '' if (!href) { return } diff --git a/src/extensions/surveys.ts b/src/extensions/surveys.ts index 601026206..b8c6148a3 100644 --- a/src/extensions/surveys.ts +++ b/src/extensions/surveys.ts @@ -10,7 +10,11 @@ import { } from '../posthog-surveys-types' import { _isUndefined } from '../utils/type-utils' -import { window, document } from '../utils/globals' +import { window as _window, document as _document } from '../utils/globals' + +// We cast the types here which is dangerous but protected by the top level generateSurveys call +const window = _window as Window & typeof globalThis +const document = _document as Document const satisfiedEmoji = '' @@ -897,14 +901,19 @@ function showQuestion(n: number, surveyId: string) { function nextQuestion(currentQuestionIdx: number, surveyId: string) { // figure out which tab to display const tabs = document - .getElementsByClassName(`PostHogSurvey${surveyId}`)[0] + ?.getElementsByClassName(`PostHogSurvey${surveyId}`)[0] ?.shadowRoot?.querySelectorAll('.tab') as NodeListOf tabs[currentQuestionIdx].style.display = 'none' showQuestion(currentQuestionIdx + 1, surveyId) } +// This is the main exported function export function generateSurveys(posthog: PostHog) { + // NOTE: Important to ensure we never try and run surveys without a window environment + if (!document || !window) { + return + } callSurveys(posthog, true) // recalculate surveys every 3 seconds to check if URL or selectors have changed diff --git a/src/extensions/toolbar.ts b/src/extensions/toolbar.ts index fe6200cf5..4aa6be883 100644 --- a/src/extensions/toolbar.ts +++ b/src/extensions/toolbar.ts @@ -8,7 +8,7 @@ import { window, document, assignableWindow } from '../utils/globals' // TRICKY: Many web frameworks will modify the route on load, potentially before posthog is initialized. // To get ahead of this we grab it as soon as the posthog-js is parsed -const STATE_FROM_WINDOW = window.location +const STATE_FROM_WINDOW = window?.location ? _getHashParam(window.location.hash, '__posthog') || _getHashParam(location.hash, 'state') : null @@ -40,10 +40,16 @@ export class Toolbar { * 2. From session storage under the key `toolbarParams` if the toolbar was initialized on a previous page */ maybeLoadToolbar( - location = window.location, + location: Location | undefined = undefined, localStorage: Storage | undefined = undefined, - history = window.history + history: History | undefined = undefined ): boolean { + if (!window || !document) { + return false + } + location = location ?? window.location + history = history ?? window.history + try { // Before running the code we check if we can access localStorage, if not we opt-out if (!localStorage) { @@ -55,7 +61,7 @@ export class Toolbar { } // If localStorage was undefined, and localStorage is supported we set the default value - localStorage = window.localStorage + localStorage = window?.localStorage } /** @@ -114,7 +120,7 @@ export class Toolbar { } loadToolbar(params?: ToolbarParams): boolean { - if (assignableWindow['_postHogToolbarLoaded']) { + if (!window || assignableWindow['_postHogToolbarLoaded']) { return false } // only load the toolbar once, even if there are multiple instances of PostHogLib @@ -164,9 +170,9 @@ export class Toolbar { /** @deprecated Use "maybeLoadToolbar" instead. */ maybeLoadEditor( - location = window.location, + location: Location | undefined = undefined, localStorage: Storage | undefined = undefined, - history = window.history + history: History | undefined = undefined ): boolean { return this.maybeLoadToolbar(location, localStorage, history) } diff --git a/src/gdpr-utils.ts b/src/gdpr-utils.ts index 1904d3d58..d175cfa3f 100644 --- a/src/gdpr-utils.ts +++ b/src/gdpr-utils.ts @@ -158,11 +158,11 @@ function _getStorageValue(token: string, options: GDPROptions) { function _hasDoNotTrackFlagOn(options: GDPROptions) { if (options && options.respectDnt) { const win = (options && options.window) || window - const nav = win['navigator'] || {} + const nav = win?.navigator let hasDntOn = false _each( [ - nav['doNotTrack'], // standard + nav?.doNotTrack, // standard (nav as any)['msDoNotTrack'], (win as any)['doNotTrack'], ], diff --git a/src/loader-recorder-v2.ts b/src/loader-recorder-v2.ts index 5f5d32146..4d99c3c40 100644 --- a/src/loader-recorder-v2.ts +++ b/src/loader-recorder-v2.ts @@ -454,14 +454,11 @@ export const getRecordNetworkPlugin: (options?: NetworkRecordOptions) => RecordP } // rrweb/networ@1 ends -// eslint-disable-next-line @typescript-eslint/ban-ts-comment -// @ts-ignore -window.rrweb = { record: rrwebRecord, version: 'v2', rrwebVersion: version } -// eslint-disable-next-line @typescript-eslint/ban-ts-comment -// @ts-ignore -window.rrwebConsoleRecord = { getRecordConsolePlugin } -// eslint-disable-next-line @typescript-eslint/ban-ts-comment -// @ts-ignore -window.getRecordNetworkPlugin = getRecordNetworkPlugin + +if (window) { + ;(window as any).rrweb = { record: rrwebRecord, version: 'v2', rrwebVersion: version } + ;(window as any).rrwebConsoleRecord = { getRecordConsolePlugin } + ;(window as any).getRecordNetworkPlugin = getRecordNetworkPlugin +} export default rrwebRecord diff --git a/src/loader-recorder.ts b/src/loader-recorder.ts index 4079e146f..2680a5358 100644 --- a/src/loader-recorder.ts +++ b/src/loader-recorder.ts @@ -10,11 +10,9 @@ import { getRecordConsolePlugin } from 'rrweb-v1/es/rrweb/packages/rrweb/src/plu import { window } from './utils/globals' -// eslint-disable-next-line @typescript-eslint/ban-ts-comment -// @ts-ignore -window.rrweb = { record: rrwebRecord, version: 'v1', rrwebVersion: version } -// eslint-disable-next-line @typescript-eslint/ban-ts-comment -// @ts-ignore -window.rrwebConsoleRecord = { getRecordConsolePlugin } +if (window) { + ;(window as any).rrweb = { record: rrwebRecord, version: 'v1', rrwebVersion: version } + ;(window as any).rrwebConsoleRecord = { getRecordConsolePlugin } +} export default rrwebRecord diff --git a/src/loader-surveys.ts b/src/loader-surveys.ts index 367e01edd..9e7bf8e1f 100644 --- a/src/loader-surveys.ts +++ b/src/loader-surveys.ts @@ -1,10 +1,9 @@ import { generateSurveys } from './extensions/surveys' -import { _isUndefined } from './utils/type-utils' import { window } from './utils/globals' -const win: Window & typeof globalThis = _isUndefined(window) ? ({} as typeof window) : window - -;(win as any).extendPostHogWithSurveys = generateSurveys +if (window) { + ;(window as any).extendPostHogWithSurveys = generateSurveys +} export default generateSurveys diff --git a/src/page-view.ts b/src/page-view.ts index 35de57395..6abb7f1a2 100644 --- a/src/page-view.ts +++ b/src/page-view.ts @@ -35,7 +35,7 @@ export class PageViewManager { _createPageViewData(): PageViewData { return { - pathname: window.location.pathname, + pathname: window?.location.pathname ?? '', } } @@ -134,31 +134,33 @@ export class PageViewManager { } startMeasuringScrollPosition() { - window.addEventListener('scroll', this._updateScrollData) - window.addEventListener('scrollend', this._updateScrollData) - window.addEventListener('resize', this._updateScrollData) + window?.addEventListener('scroll', this._updateScrollData) + window?.addEventListener('scrollend', this._updateScrollData) + window?.addEventListener('resize', this._updateScrollData) } stopMeasuringScrollPosition() { - window.removeEventListener('scroll', this._updateScrollData) - window.removeEventListener('scrollend', this._updateScrollData) - window.removeEventListener('resize', this._updateScrollData) + window?.removeEventListener('scroll', this._updateScrollData) + window?.removeEventListener('scrollend', this._updateScrollData) + window?.removeEventListener('resize', this._updateScrollData) } _scrollHeight(): number { - return Math.max(0, window.document.documentElement.scrollHeight - window.document.documentElement.clientHeight) + return window + ? Math.max(0, window.document.documentElement.scrollHeight - window.document.documentElement.clientHeight) + : 0 } _scrollY(): number { - return window.scrollY || window.pageYOffset || window.document.documentElement.scrollTop || 0 + return window ? window.scrollY || window.pageYOffset || window.document.documentElement.scrollTop || 0 : 0 } _contentHeight(): number { - return window.document.documentElement.scrollHeight || 0 + return window?.document.documentElement.scrollHeight || 0 } _contentY(): number { - const clientHeight = window.document.documentElement.clientHeight || 0 + const clientHeight = window?.document.documentElement.clientHeight || 0 return this._scrollY() + clientHeight } } diff --git a/src/posthog-core.ts b/src/posthog-core.ts index 92ee7616c..cbc1e0fc1 100644 --- a/src/posthog-core.ts +++ b/src/posthog-core.ts @@ -92,12 +92,12 @@ const PRIMARY_INSTANCE_NAME = 'posthog' */ // http://hacks.mozilla.org/2009/07/cross-site-xmlhttprequest-with-cors/ // https://developer.mozilla.org/en-US/docs/DOM/XMLHttpRequest#withCredentials -const USE_XHR = window.XMLHttpRequest && 'withCredentials' in new XMLHttpRequest() +const USE_XHR = window?.XMLHttpRequest && 'withCredentials' in new XMLHttpRequest() // IE<10 does not support cross-origin XHR's but script tags // with defer won't block window.onload; ENQUEUE_REQUESTS // should only be true for Opera<12 -let ENQUEUE_REQUESTS = !USE_XHR && userAgent.indexOf('MSIE') === -1 && userAgent.indexOf('Mozilla') === -1 +let ENQUEUE_REQUESTS = !USE_XHR && userAgent?.indexOf('MSIE') === -1 && userAgent?.indexOf('Mozilla') === -1 export const defaultConfig = (): PostHogConfig => ({ api_host: 'https://app.posthog.com', @@ -514,8 +514,7 @@ export class PostHog { } // Set up event handler for pageleave // Use `onpagehide` if available, see https://calendar.perfplanet.com/2020/beaconing-in-practice/#beaconing-reliability-avoiding-abandons - window.addEventListener && - window.addEventListener('onpagehide' in self ? 'pagehide' : 'unload', this._handle_unload.bind(this)) + window?.addEventListener?.('onpagehide' in self ? 'pagehide' : 'unload', this._handle_unload.bind(this)) // Make sure that we also call the initComplete callback at the end of // the synchronous code as well. @@ -558,7 +557,7 @@ export class PostHog { // this happens after so a user can call identify in // the loaded callback - if (this.config.capture_pageview) { + if (this.config.capture_pageview && document) { this.capture('$pageview', { title: document.title }, { send_instantly: true }) } @@ -683,7 +682,7 @@ export class PostHog { options.method = 'GET' } - const useSendBeacon = 'sendBeacon' in window.navigator && options.transport === 'sendBeacon' + const useSendBeacon = window && 'sendBeacon' in window.navigator && options.transport === 'sendBeacon' url = addParamsToURL(url, options.urlQueryArgs || {}, { ip: this.config.ip, }) @@ -692,12 +691,12 @@ export class PostHog { // beacon documentation https://w3c.github.io/beacon/ // beacons format the message and use the type property try { - window.navigator.sendBeacon(url, encodePostData(data, { ...options, sendBeacon: true })) + window?.navigator.sendBeacon(url, encodePostData(data, { ...options, sendBeacon: true })) } catch (e) { // send beacon is a best-effort, fire-and-forget mechanism on page unload, // we don't want to throw errors here } - } else if (USE_XHR) { + } else if (USE_XHR || !document) { try { xhr({ url: url, @@ -851,7 +850,7 @@ export class PostHog { return } - if (_isBlockedUA(userAgent, this.config.custom_blocked_useragents)) { + if (userAgent && _isBlockedUA(userAgent, this.config.custom_blocked_useragents)) { return } @@ -956,7 +955,7 @@ export class PostHog { properties = _extend(properties, performanceProperties) } - if (event_name === '$pageview') { + if (event_name === '$pageview' && document) { properties['title'] = document.title } @@ -2045,11 +2044,11 @@ export class PostHog { debug(debug?: boolean): void { if (debug === false) { - window.console.log("You've disabled debug mode.") + window?.console.log("You've disabled debug mode.") localStorage && localStorage.removeItem('ph_debug') this.set_config({ debug: false }) } else { - window.console.log( + window?.console.log( "You're now in debug mode. All calls to PostHog will be logged in your console.\nYou can disable this with `posthog.debug(false)`." ) localStorage && localStorage.setItem('ph_debug', 'true') @@ -2129,7 +2128,7 @@ const add_dom_loaded_handler = function () { }) } - if (document.addEventListener) { + if (document?.addEventListener) { if (document.readyState === 'complete') { // safari 4 can fire the DOMContentLoaded event before loading all // external JS (including this file). you will see some copypasta @@ -2142,7 +2141,9 @@ const add_dom_loaded_handler = function () { } // fallback handler, always will work - _register_event(window, 'load', dom_loaded_handler, true) + if (window) { + _register_event(window, 'load', dom_loaded_handler, true) + } } export function init_from_snippet(): void { diff --git a/src/posthog-surveys.ts b/src/posthog-surveys.ts index 61fd8286d..3ac9de094 100644 --- a/src/posthog-surveys.ts +++ b/src/posthog-surveys.ts @@ -5,9 +5,10 @@ import { _isUrlMatchingRegex } from './utils/request-utils' import { window, document } from './utils/globals' export const surveyUrlValidationMap: Record boolean> = { - icontains: (conditionsUrl) => window.location.href.toLowerCase().indexOf(conditionsUrl.toLowerCase()) > -1, - regex: (conditionsUrl) => _isUrlMatchingRegex(window.location.href, conditionsUrl), - exact: (conditionsUrl) => window.location.href === conditionsUrl, + icontains: (conditionsUrl) => + !!window && window.location.href.toLowerCase().indexOf(conditionsUrl.toLowerCase()) > -1, + regex: (conditionsUrl) => !!window && _isUrlMatchingRegex(window.location.href, conditionsUrl), + exact: (conditionsUrl) => window?.location.href === conditionsUrl, } export class PostHogSurveys { @@ -50,7 +51,7 @@ export class PostHogSurveys { ? surveyUrlValidationMap[survey.conditions?.urlMatchType ?? 'icontains'](survey.conditions.url) : true const selectorCheck = survey.conditions?.selector - ? document.querySelector(survey.conditions.selector) + ? document?.querySelector(survey.conditions.selector) : true return urlCheck && selectorCheck }) diff --git a/src/retry-queue.ts b/src/retry-queue.ts index f40a7d287..bea0c7c69 100644 --- a/src/retry-queue.ts +++ b/src/retry-queue.ts @@ -115,7 +115,7 @@ export class RetryQueue extends RequestQueueScaffold { try { // we've had send beacon in place for at least 2 years // eslint-disable-next-line compat/compat - window.navigator.sendBeacon(url, encodePostData(data, { ...options, sendBeacon: true })) + window?.navigator.sendBeacon(url, encodePostData(data, { ...options, sendBeacon: true })) } catch (e) { // Note sendBeacon automatically retries, and after the first retry it will lose reference to contextual `this`. // This means in some cases `this.getConfig` will be undefined. diff --git a/src/session-props.ts b/src/session-props.ts index 15f0d2725..79021ae80 100644 --- a/src/session-props.ts +++ b/src/session-props.ts @@ -27,9 +27,9 @@ interface StoredSessionSourceProps { props: SessionSourceProps } -export const generateSessionSourceParams = (): SessionSourceProps => { +const generateSessionSourceParams = (): SessionSourceProps => { return { - initialPathName: window.location.pathname, + initialPathName: window?.location.pathname || '', referringDomain: _info.referringDomain(), ..._info.campaignParams(), } @@ -38,12 +38,12 @@ export const generateSessionSourceParams = (): SessionSourceProps => { export class SessionPropsManager { private readonly _sessionIdManager: SessionIdManager private readonly _persistence: PostHogPersistence - private readonly _sessionSourceParamGenerator: typeof generateSessionSourceParams + private readonly _sessionSourceParamGenerator: () => SessionSourceProps constructor( sessionIdManager: SessionIdManager, persistence: PostHogPersistence, - sessionSourceParamGenerator?: typeof generateSessionSourceParams + sessionSourceParamGenerator?: () => SessionSourceProps ) { this._sessionIdManager = sessionIdManager this._persistence = persistence diff --git a/src/sessionid.ts b/src/sessionid.ts index fca582a16..555a8f779 100644 --- a/src/sessionid.ts +++ b/src/sessionid.ts @@ -173,7 +173,7 @@ export class SessionIdManager { * We conditionally check the primaryWindowExists value in the constructor to decide if the window id in the last session storage should be carried over. */ private _listenToReloadWindow(): void { - window.addEventListener('beforeunload', () => { + window?.addEventListener('beforeunload', () => { if (this._canUseSessionStorage()) { sessionStore.remove(this._primary_window_exists_storage_key) } diff --git a/src/storage.ts b/src/storage.ts index ed68c6e61..c559b379c 100644 --- a/src/storage.ts +++ b/src/storage.ts @@ -23,6 +23,9 @@ const Y1970 = 'Thu, 01 Jan 1970 00:00:00 GMT' * inspired by https://github.com/AngusFu/browser-root-domain */ export function seekFirstNonPublicSubDomain(hostname: string, cookieJar = document): string { + if (!cookieJar) { + return '' + } if (['localhost', '127.0.0.1'].includes(hostname)) return '' const list = hostname.split('.') @@ -72,13 +75,17 @@ export function chooseCookieDomain(hostname: string, cross_subdomain: boolean | // Methods partially borrowed from quirksmode.org/js/cookies.html export const cookieStore: PersistentStore = { - is_supported: () => true, + is_supported: () => !!document, error: function (msg) { logger.error('cookieStore error: ' + msg) }, get: function (name) { + if (!document) { + return + } + try { const nameEQ = name + '=' const ca = document.cookie.split(';').filter((x) => x.length) @@ -106,6 +113,9 @@ export const cookieStore: PersistentStore = { }, set: function (name, value, days, cross_subdomain, is_secure) { + if (!document) { + return + } try { let expires = '', secure = '' @@ -184,7 +194,7 @@ export const localStore: PersistentStore = { get: function (name) { try { - return window.localStorage.getItem(name) + return window?.localStorage.getItem(name) } catch (err) { localStore.error(err) } @@ -202,7 +212,7 @@ export const localStore: PersistentStore = { set: function (name, value) { try { - window.localStorage.setItem(name, JSON.stringify(value)) + window?.localStorage.setItem(name, JSON.stringify(value)) } catch (err) { localStore.error(err) } @@ -210,7 +220,7 @@ export const localStore: PersistentStore = { remove: function (name) { try { - window.localStorage.removeItem(name) + window?.localStorage.removeItem(name) } catch (err) { localStore.error(err) } @@ -260,7 +270,7 @@ export const localPlusCookieStore: PersistentStore = { remove: function (name, cross_subdomain) { try { - window.localStorage.removeItem(name) + window?.localStorage.removeItem(name) cookieStore.remove(name, cross_subdomain) } catch (err) { localStore.error(err) @@ -332,7 +342,7 @@ export const sessionStore: PersistentStore = { get: function (name) { try { - return window.sessionStorage.getItem(name) + return window?.sessionStorage.getItem(name) } catch (err) { sessionStore.error(err) } @@ -350,7 +360,7 @@ export const sessionStore: PersistentStore = { set: function (name, value) { try { - window.sessionStorage.setItem(name, JSON.stringify(value)) + window?.sessionStorage.setItem(name, JSON.stringify(value)) } catch (err) { sessionStore.error(err) } @@ -358,7 +368,7 @@ export const sessionStore: PersistentStore = { remove: function (name) { try { - window.sessionStorage.removeItem(name) + window?.sessionStorage.removeItem(name) } catch (err) { sessionStore.error(err) } diff --git a/src/utils/event-utils.ts b/src/utils/event-utils.ts index 463951e05..6e202a678 100644 --- a/src/utils/event-utils.ts +++ b/src/utils/event-utils.ts @@ -29,7 +29,7 @@ export const _info = { const params: Record = {} _each(campaign_keywords, function (kwkey) { - const kw = _getQueryParam(document.URL, kwkey) + const kw = document ? _getQueryParam(document.URL, kwkey) : '' if (kw.length) { params[kwkey] = kw } @@ -39,7 +39,7 @@ export const _info = { }, searchEngine: function (): string | null { - const referrer = document.referrer + const referrer = document?.referrer if (!referrer) { return null } else if (referrer.search('https?://(.*)google.([^/?]*)') === 0) { @@ -63,7 +63,7 @@ export const _info = { if (!_isNull(search)) { ret['$search_engine'] = search - const keyword = _getQueryParam(document.referrer, param) + const keyword = document ? _getQueryParam(document.referrer, param) : '' if (keyword.length) { ret['ph_keyword'] = keyword } @@ -249,11 +249,11 @@ export const _info = { }, referrer: function (): string { - return document.referrer || '$direct' + return document?.referrer || '$direct' }, referringDomain: function (): string { - if (!document.referrer) { + if (!document?.referrer) { return '$direct' } const parser = document.createElement('a') // Unfortunately we cannot use new URL due to IE11 @@ -262,6 +262,9 @@ export const _info = { }, properties: function (): Properties { + if (!userAgent) { + return {} + } const { os_name, os_version } = _info.os(userAgent) return _extend( _strip_empty_properties({ @@ -291,6 +294,10 @@ export const _info = { }, people_properties: function (): Properties { + if (!userAgent) { + return {} + } + const { os_name, os_version } = _info.os(userAgent) return _extend( _strip_empty_properties({ diff --git a/src/utils/globals.ts b/src/utils/globals.ts index dab59570c..d90d41708 100644 --- a/src/utils/globals.ts +++ b/src/utils/globals.ts @@ -5,10 +5,10 @@ export const ArrayProto = Array.prototype export const nativeForEach = ArrayProto.forEach export const nativeIndexOf = ArrayProto.indexOf // eslint-disable-next-line no-restricted-globals -export const win: Window & typeof globalThis = typeof window !== 'undefined' ? window : ({} as typeof window) -const navigator = win.navigator || { userAgent: '' } -export const document = win.document || {} -export const userAgent = navigator.userAgent -export const assignableWindow: Window & typeof globalThis & Record = win +export const win: (Window & typeof globalThis) | undefined = typeof window !== 'undefined' ? window : undefined +const navigator = win?.navigator +export const document = win?.document +export const userAgent = navigator?.userAgent +export const assignableWindow: Window & typeof globalThis & Record = win ?? ({} as any) export { win as window } diff --git a/src/utils/index.ts b/src/utils/index.ts index bf31efec9..eff109934 100644 --- a/src/utils/index.ts +++ b/src/utils/index.ts @@ -466,7 +466,7 @@ export const _register_event = (function () { old_handlers: EventHandler ) { return function (event: Event): boolean | void { - event = event || fixEvent(window.event) + event = event || fixEvent(window?.event) // this basically happens in firefox whenever another script // overwrites the onload callback and doesn't pass the event @@ -512,6 +512,9 @@ export const _register_event = (function () { export function loadScript(scriptUrlToLoad: string, callback: (error?: string | Event, event?: Event) => void): void { const addScript = () => { + if (!document) { + return callback('document not found') + } const scriptTag = document.createElement('script') scriptTag.type = 'text/javascript' scriptTag.src = scriptUrlToLoad @@ -527,10 +530,10 @@ export function loadScript(scriptUrlToLoad: string, callback: (error?: string | } } - if (document.body) { + if (document?.body) { addScript() } else { - document.addEventListener('DOMContentLoaded', addScript) + document?.addEventListener('DOMContentLoaded', addScript) } } diff --git a/src/utils/logger.ts b/src/utils/logger.ts index 63f456087..6bc11befb 100644 --- a/src/utils/logger.ts +++ b/src/utils/logger.ts @@ -5,7 +5,12 @@ import { assignableWindow, window } from './globals' const LOGGER_PREFIX = '[PostHog.js]' export const logger = { _log: (level: 'log' | 'warn' | 'error', ...args: any[]) => { - if ((Config.DEBUG || assignableWindow.POSTHOG_DEBUG) && !_isUndefined(window.console) && window.console) { + if ( + window && + (Config.DEBUG || assignableWindow.POSTHOG_DEBUG) && + !_isUndefined(window.console) && + window.console + ) { const consoleLog = '__rrweb_original__' in window.console[level] ? (window.console[level] as any)['__rrweb_original__'] diff --git a/src/uuidv7.ts b/src/uuidv7.ts index c1d6e3bb9..032b5546f 100644 --- a/src/uuidv7.ts +++ b/src/uuidv7.ts @@ -219,7 +219,7 @@ let getRandomValues: (buffer: T) => T = (buf } // detect Web Crypto API -if (!_isUndefined(window.crypto) && crypto.getRandomValues) { +if (window && !_isUndefined(window.crypto) && crypto.getRandomValues) { getRandomValues = (buffer) => crypto.getRandomValues(buffer) } From 4033893348216969accbaf0260d41ec84d75fd99 Mon Sep 17 00:00:00 2001 From: Ben White Date: Wed, 15 Nov 2023 12:22:58 +0100 Subject: [PATCH 05/13] Attempted to add fetch support --- src/__tests__/retry-queue.js | 2 +- src/__tests__/send-request.test.ts | 12 +++---- src/posthog-core.ts | 4 +-- src/retry-queue.ts | 4 +-- src/send-request.ts | 52 +++++++++++++++++++++++++++++- 5 files changed, 62 insertions(+), 12 deletions(-) diff --git a/src/__tests__/retry-queue.js b/src/__tests__/retry-queue.js index 32a8a22d6..9e3554410 100644 --- a/src/__tests__/retry-queue.js +++ b/src/__tests__/retry-queue.js @@ -201,7 +201,7 @@ describe('RetryQueue', () => { }) it('when you flush the queue onXHRError is passed to xhr', () => { - const xhrSpy = jest.spyOn(SendRequest, 'xhr') + const xhrSpy = jest.spyOn(SendRequest, 'request') enqueueRequests() given.retryQueue.flush() fastForwardTimeAndRunTimer() diff --git a/src/__tests__/send-request.test.ts b/src/__tests__/send-request.test.ts index 4435e7714..0bd2fdf83 100644 --- a/src/__tests__/send-request.test.ts +++ b/src/__tests__/send-request.test.ts @@ -1,6 +1,6 @@ /// -import { addParamsToURL, encodePostData, xhr } from '../send-request' +import { addParamsToURL, encodePostData, request } from '../send-request' import { assert, boolean, property, uint8Array, VerbosityLevel } from 'fast-check' import { Compression, PostData, XHROptions, XHRParams } from '../types' @@ -55,7 +55,7 @@ describe('send-request', () => { test('it adds the retry count to the URL', () => { const retryCount = Math.floor(Math.random() * 100) + 1 // make sure it is never 0 - xhr( + request( xhrParams({ retriesPerformedSoFar: retryCount, url: 'https://any.posthog-instance.com/?ver=1.23.45&ip=7&_=1698404857278', @@ -69,7 +69,7 @@ describe('send-request', () => { }) test('does not add retry count when it is 0', () => { - xhr( + request( xhrParams({ retriesPerformedSoFar: 0, url: 'https://any.posthog-instance.com/?ver=1.23.45&ip=7&_=1698404857278', @@ -86,7 +86,7 @@ describe('send-request', () => { it('does not error if the configured onXHRError is not a function', () => { onXHRError = 'not a function' as unknown as XHRParams['onXHRError'] expect(() => { - xhr(xhrParams()) + request(xhrParams()) mockXHR.onreadystatechange?.({} as Event) }).not.toThrow() }) @@ -97,7 +97,7 @@ describe('send-request', () => { onXHRError = (req) => { requestFromError = req } - xhr(xhrParams()) + request(xhrParams()) mockXHR.onreadystatechange?.({} as Event) expect(requestFromError).toHaveProperty('status', 502) }) @@ -109,7 +109,7 @@ describe('send-request', () => { // @ts-ignore // noinspection JSConstantReassignment mockXHR.status = Math.floor(Math.random() * 100) - xhr(xhrParams()) + request(xhrParams()) mockXHR.onreadystatechange?.({} as Event) expect(checkForLimiting).toHaveBeenCalledWith(mockXHR) }) diff --git a/src/posthog-core.ts b/src/posthog-core.ts index cbc1e0fc1..d218bfe2b 100644 --- a/src/posthog-core.ts +++ b/src/posthog-core.ts @@ -22,7 +22,7 @@ import { clearOptInOut, hasOptedIn, hasOptedOut, optIn, optOut, userOptedOut } f import { cookieStore, localStore } from './storage' import { RequestQueue } from './request-queue' import { compressData, decideCompression } from './compression' -import { addParamsToURL, encodePostData, xhr } from './send-request' +import { addParamsToURL, encodePostData, request } from './send-request' import { RetryQueue } from './retry-queue' import { SessionIdManager } from './sessionid' import { @@ -698,7 +698,7 @@ export class PostHog { } } else if (USE_XHR || !document) { try { - xhr({ + request({ url: url, data: data, headers: this.config.xhr_headers, diff --git a/src/retry-queue.ts b/src/retry-queue.ts index bea0c7c69..3b233058d 100644 --- a/src/retry-queue.ts +++ b/src/retry-queue.ts @@ -1,5 +1,5 @@ import { RequestQueueScaffold } from './base-request-queue' -import { encodePostData, xhr } from './send-request' +import { encodePostData, request } from './send-request' import { QueuedRequestData, RetryQueueElement } from './types' import { RateLimiter } from './rate-limiter' @@ -130,7 +130,7 @@ export class RetryQueue extends RequestQueueScaffold { return } - xhr({ + request({ url, data: data || {}, options: options || {}, diff --git a/src/send-request.ts b/src/send-request.ts index 08bc0dace..d3cb7d16d 100644 --- a/src/send-request.ts +++ b/src/send-request.ts @@ -5,6 +5,7 @@ import { _HTTPBuildQuery } from './utils/request-utils' import { _isArray, _isFunction, _isNumber, _isUint8Array, _isUndefined } from './utils/type-utils' import { logger } from './utils/logger' +import { window } from './utils/globals' export const addParamsToURL = ( url: string, @@ -61,7 +62,56 @@ export const encodePostData = (data: PostData | Uint8Array, options: Partial { + if (window?.fetch) { + const body = encodePostData(params.data, params.options) + + const headers = new Headers() + _each(headers, function (headerValue, headerName) { + headers.append(headerName, headerValue) + }) + + if (params.options.method === 'POST' && !params.options.blob) { + headers.append('Content-Type', 'application/x-www-form-urlencoded') + } + + window + .fetch(params.url, { + method: params.options?.method || 'GET', + headers: params.headers as Headers, + keepalive: true, + body, + }) + .then((response) => { + // TODO: Implement on response handler + if (response.status === 200) { + response.json().then((json) => { + params.callback?.(json) + }) + } else { + // don't retry errors between 400 and 500 inclusive + if (response.status < 400 || response.status > 500) { + params.retryQueue.enqueue({ + ...params, + headers, + retriesPerformedSoFar: (params.retriesPerformedSoFar || 0) + 1, + }) + } + params.callback?.({ status: 0 }) + } + }) + .catch((error) => { + logger.error(error) + params.callback?.({ status: 0 }) + }) + + return + } + + return xhr(params) +} + +const xhr = ({ url, data, headers, From f0f7c836602b6f77ddce6dade4914240c97499a1 Mon Sep 17 00:00:00 2001 From: Ben White Date: Wed, 15 Nov 2023 12:38:06 +0100 Subject: [PATCH 06/13] Fix --- src/send-request.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/send-request.ts b/src/send-request.ts index d3cb7d16d..e9bf90fe6 100644 --- a/src/send-request.ts +++ b/src/send-request.ts @@ -78,8 +78,8 @@ export const request = (params: XHRParams) => { window .fetch(params.url, { method: params.options?.method || 'GET', - headers: params.headers as Headers, - keepalive: true, + headers, + keepalive: params.options.method === 'POST', body, }) .then((response) => { From 13659aeebb844896626e195c1755020ae1404bb1 Mon Sep 17 00:00:00 2001 From: Ben White Date: Thu, 7 Dec 2023 16:56:18 +0100 Subject: [PATCH 07/13] Change --- src/posthog-core.ts | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/posthog-core.ts b/src/posthog-core.ts index d218bfe2b..d43626343 100644 --- a/src/posthog-core.ts +++ b/src/posthog-core.ts @@ -92,12 +92,12 @@ const PRIMARY_INSTANCE_NAME = 'posthog' */ // http://hacks.mozilla.org/2009/07/cross-site-xmlhttprequest-with-cors/ // https://developer.mozilla.org/en-US/docs/DOM/XMLHttpRequest#withCredentials -const USE_XHR = window?.XMLHttpRequest && 'withCredentials' in new XMLHttpRequest() +const USE_REQUEST = window?.fetch || (window?.XMLHttpRequest && 'withCredentials' in new XMLHttpRequest()) // IE<10 does not support cross-origin XHR's but script tags // with defer won't block window.onload; ENQUEUE_REQUESTS // should only be true for Opera<12 -let ENQUEUE_REQUESTS = !USE_XHR && userAgent?.indexOf('MSIE') === -1 && userAgent?.indexOf('Mozilla') === -1 +let ENQUEUE_REQUESTS = !USE_REQUEST && userAgent?.indexOf('MSIE') === -1 && userAgent?.indexOf('Mozilla') === -1 export const defaultConfig = (): PostHogConfig => ({ api_host: 'https://app.posthog.com', @@ -608,7 +608,7 @@ export class PostHog { return null } - if (USE_XHR) { + if (USE_REQUEST) { return function (response) { callback(response, data) } @@ -678,7 +678,7 @@ export class PostHog { } options = _extend(DEFAULT_OPTIONS, options || {}) - if (!USE_XHR) { + if (!USE_REQUEST) { options.method = 'GET' } @@ -696,7 +696,7 @@ export class PostHog { // send beacon is a best-effort, fire-and-forget mechanism on page unload, // we don't want to throw errors here } - } else if (USE_XHR || !document) { + } else if (USE_REQUEST || !document) { try { request({ url: url, From 5b6f35546ea7179776e19860a218cc1be15c1708 Mon Sep 17 00:00:00 2001 From: Ben White Date: Wed, 31 Jan 2024 09:51:04 +0000 Subject: [PATCH 08/13] More interop stuff --- src/__tests__/send-request.test.ts | 28 ++++++++++++++-------------- src/posthog-core.ts | 17 ++++++++++------- src/rate-limiter.ts | 5 +++-- src/retry-queue.ts | 10 +++++----- src/send-request.ts | 18 +++++++++++------- src/types.ts | 20 +++++++++++++++----- 6 files changed, 58 insertions(+), 40 deletions(-) diff --git a/src/__tests__/send-request.test.ts b/src/__tests__/send-request.test.ts index 0bd2fdf83..386812cda 100644 --- a/src/__tests__/send-request.test.ts +++ b/src/__tests__/send-request.test.ts @@ -2,7 +2,7 @@ import { addParamsToURL, encodePostData, request } from '../send-request' import { assert, boolean, property, uint8Array, VerbosityLevel } from 'fast-check' -import { Compression, PostData, XHROptions, XHRParams } from '../types' +import { Compression, PostData, XHROptions, RequestData } from '../types' import { _isUndefined } from '../utils/type-utils' @@ -11,10 +11,10 @@ jest.mock('../config', () => ({ DEBUG: false, LIB_VERSION: '1.23.45' })) describe('send-request', () => { describe('xhr', () => { let mockXHR: XMLHttpRequest - let xhrParams: (overrides?: Partial) => XHRParams - let onXHRError: XHRParams['onXHRError'] - let checkForLimiting: XHRParams['onResponse'] - let xhrOptions: XHRParams['options'] + let createRequestData: (overrides?: Partial) => RequestData + let onXHRError: RequestData['onError'] + let checkForLimiting: RequestData['onResponse'] + let xhrOptions: RequestData['options'] beforeEach(() => { mockXHR = { @@ -30,7 +30,7 @@ describe('send-request', () => { onXHRError = jest.fn() checkForLimiting = jest.fn() xhrOptions = {} - xhrParams = (overrides?: Partial) => { + createRequestData = (overrides?: Partial) => { return { url: 'https://any.posthog-instance.com?ver=1.23.45', data: {}, @@ -40,8 +40,8 @@ describe('send-request', () => { retriesPerformedSoFar: undefined, retryQueue: { enqueue: () => {}, - } as Partial as XHRParams['retryQueue'], - onXHRError, + } as Partial as RequestData['retryQueue'], + onError: onXHRError, onResponse: checkForLimiting, ...overrides, } @@ -56,7 +56,7 @@ describe('send-request', () => { test('it adds the retry count to the URL', () => { const retryCount = Math.floor(Math.random() * 100) + 1 // make sure it is never 0 request( - xhrParams({ + createRequestData({ retriesPerformedSoFar: retryCount, url: 'https://any.posthog-instance.com/?ver=1.23.45&ip=7&_=1698404857278', }) @@ -70,7 +70,7 @@ describe('send-request', () => { test('does not add retry count when it is 0', () => { request( - xhrParams({ + createRequestData({ retriesPerformedSoFar: 0, url: 'https://any.posthog-instance.com/?ver=1.23.45&ip=7&_=1698404857278', }) @@ -84,9 +84,9 @@ describe('send-request', () => { describe('when xhr requests fail', () => { it('does not error if the configured onXHRError is not a function', () => { - onXHRError = 'not a function' as unknown as XHRParams['onXHRError'] + onXHRError = 'not a function' as unknown as RequestData['onError'] expect(() => { - request(xhrParams()) + request(createRequestData()) mockXHR.onreadystatechange?.({} as Event) }).not.toThrow() }) @@ -97,7 +97,7 @@ describe('send-request', () => { onXHRError = (req) => { requestFromError = req } - request(xhrParams()) + request(createRequestData()) mockXHR.onreadystatechange?.({} as Event) expect(requestFromError).toHaveProperty('status', 502) }) @@ -109,7 +109,7 @@ describe('send-request', () => { // @ts-ignore // noinspection JSConstantReassignment mockXHR.status = Math.floor(Math.random() * 100) - request(xhrParams()) + request(createRequestData()) mockXHR.onreadystatechange?.({} as Event) expect(checkForLimiting).toHaveBeenCalledWith(mockXHR) }) diff --git a/src/posthog-core.ts b/src/posthog-core.ts index 927622567..ad575bb7c 100644 --- a/src/posthog-core.ts +++ b/src/posthog-core.ts @@ -137,7 +137,7 @@ export const defaultConfig = (): PostHogConfig => ({ property_blacklist: [], respect_dnt: false, sanitize_properties: null, - xhr_headers: {}, // { header: value, header2: value } + request_headers: {}, // { header: value, header2: value } inapp_protocol: '//', inapp_link_new_window: false, request_batching: true, @@ -149,8 +149,8 @@ export const defaultConfig = (): PostHogConfig => ({ advanced_disable_feature_flags: false, advanced_disable_feature_flags_on_first_load: false, advanced_disable_toolbar_metrics: false, - on_xhr_error: (req) => { - const error = 'Bad HTTP status: ' + req.status + ' ' + req.statusText + on_request_error: (req) => { + const error = 'Bad HTTP status: ' + req.statusCode + ' ' + req.responseText logger.error(error) }, get_device_id: (uuid) => uuid, @@ -425,6 +425,9 @@ export class PostHog { } } + // Check for deprecated params that might still be in use + config.request_headers = config.request_headers || config.xhr_headers + this.set_config( _extend({}, defaultConfig(), config, { name: name, @@ -447,7 +450,7 @@ export class PostHog { this.persistence = new PostHogPersistence(this.config) this._requestQueue = new RequestQueue(this._handle_queued_event.bind(this)) - this._retryQueue = new RetryQueue(this.config.on_xhr_error, this.rateLimiter) + this._retryQueue = new RetryQueue(this.config.on_request_error, this.rateLimiter) this.__captureHooks = [] this.__request_queue = [] @@ -718,12 +721,12 @@ export class PostHog { request({ url: url, data: data, - headers: this.config.xhr_headers, + headers: this.config.request_headers, options: options, callback, retriesPerformedSoFar: 0, retryQueue: this._retryQueue, - onXHRError: this.config.on_xhr_error, + onError: this.config.on_request_error, onResponse: this.rateLimiter.checkForLimiting, }) } catch (e) { @@ -1686,7 +1689,7 @@ export class PostHog { * * // extra HTTP request headers to set for each API request, in * // the format {'Header-Name': value} - * xhr_headers: {} + * response_headers: {} * * // protocol for fetching in-app message resources, e.g. * // 'https://' or 'http://'; defaults to '//' (which defers to the diff --git a/src/rate-limiter.ts b/src/rate-limiter.ts index 5eea45fcf..2a738147d 100644 --- a/src/rate-limiter.ts +++ b/src/rate-limiter.ts @@ -1,3 +1,4 @@ +import { MinimalHTTPResponse } from 'types' import { logger } from './utils/logger' const oneMinuteInMilliseconds = 60 * 1000 @@ -18,9 +19,9 @@ export class RateLimiter { return new Date().getTime() < retryAfter } - public checkForLimiting = (xmlHttpRequest: XMLHttpRequest): void => { + public checkForLimiting = (httpResponse: MinimalHTTPResponse): void => { try { - const text = xmlHttpRequest.responseText + const text = httpResponse.responseText if (!text || !text.length) { return } diff --git a/src/retry-queue.ts b/src/retry-queue.ts index 3b233058d..302c556fd 100644 --- a/src/retry-queue.ts +++ b/src/retry-queue.ts @@ -1,6 +1,6 @@ import { RequestQueueScaffold } from './base-request-queue' import { encodePostData, request } from './send-request' -import { QueuedRequestData, RetryQueueElement } from './types' +import { PostHogConfig, QueuedRequestData, RetryQueueElement } from './types' import { RateLimiter } from './rate-limiter' import { _isUndefined } from './utils/type-utils' @@ -32,15 +32,15 @@ export class RetryQueue extends RequestQueueScaffold { queue: RetryQueueElement[] isPolling: boolean areWeOnline: boolean - onXHRError: (failedRequest: XMLHttpRequest) => void + onRequestError: PostHogConfig['on_request_error'] rateLimiter: RateLimiter - constructor(onXHRError: (failedRequest: XMLHttpRequest) => void, rateLimiter: RateLimiter) { + constructor(onRequestError: PostHogConfig['on_request_error'], rateLimiter: RateLimiter) { super() this.isPolling = false this.queue = [] this.areWeOnline = true - this.onXHRError = onXHRError + this.onRequestError = onRequestError this.rateLimiter = rateLimiter if (!_isUndefined(window) && 'onLine' in window.navigator) { @@ -138,7 +138,7 @@ export class RetryQueue extends RequestQueueScaffold { retriesPerformedSoFar: retriesPerformedSoFar || 0, callback, retryQueue: this, - onXHRError: this.onXHRError, + onError: this.onRequestError, onResponse: this.rateLimiter.checkForLimiting, }) } diff --git a/src/send-request.ts b/src/send-request.ts index e9bf90fe6..4c52295c3 100644 --- a/src/send-request.ts +++ b/src/send-request.ts @@ -1,6 +1,6 @@ import { _each } from './utils' import Config from './config' -import { PostData, XHROptions, XHRParams } from './types' +import { PostData, XHROptions, RequestData, MinimalHTTPResponse } from './types' import { _HTTPBuildQuery } from './utils/request-utils' import { _isArray, _isFunction, _isNumber, _isUint8Array, _isUndefined } from './utils/type-utils' @@ -62,7 +62,7 @@ export const encodePostData = (data: PostData | Uint8Array, options: Partial { +export const request = (params: RequestData) => { if (window?.fetch) { const body = encodePostData(params.data, params.options) @@ -119,10 +119,10 @@ const xhr = ({ callback, retriesPerformedSoFar, retryQueue, - onXHRError, + onError, timeout = 60000, onResponse, -}: XHRParams) => { +}: RequestData) => { if (_isNumber(retriesPerformedSoFar) && retriesPerformedSoFar > 0) { url = addParamsToURL(url, { retry_count: retriesPerformedSoFar }, {}) } @@ -147,7 +147,11 @@ const xhr = ({ req.onreadystatechange = () => { // XMLHttpRequest.DONE == 4, except in safari 4 if (req.readyState === 4) { - onResponse?.(req) + const minimalResponseSummary: MinimalHTTPResponse = { + statusCode: req.status, + responseText: req.responseText, + } + onResponse?.(minimalResponseSummary) if (req.status === 200) { if (callback) { let response @@ -160,8 +164,8 @@ const xhr = ({ callback(response) } } else { - if (_isFunction(onXHRError)) { - onXHRError(req) + if (_isFunction(onError)) { + onError(minimalResponseSummary) } // don't retry errors between 400 and 500 inclusive diff --git a/src/types.ts b/src/types.ts index 3c4189345..e4c343e2c 100644 --- a/src/types.ts +++ b/src/types.ts @@ -97,8 +97,12 @@ export interface PostHogConfig { opt_in_site_apps: boolean respect_dnt: boolean property_blacklist: string[] - xhr_headers: { [header_name: string]: string } - on_xhr_error: (failedRequest: XMLHttpRequest) => void + request_headers: { [header_name: string]: string } + on_request_error: (error: MinimalHTTPResponse) => void + /** @deprecated - use `request_headers` instead */ + xhr_headers?: { [header_name: string]: string } + /** @deprecated - use `on_request_error` instead */ + on_xhr_error?: (failedRequest: XMLHttpRequest) => void inapp_protocol: string inapp_link_new_window: boolean request_batching: boolean @@ -213,11 +217,17 @@ export interface QueuedRequestData { retriesPerformedSoFar?: number } -export interface XHRParams extends QueuedRequestData { +// Minimal class to allow interop between different request methods (xhr / fetch) +export interface MinimalHTTPResponse { + statusCode: number + responseText: string +} + +export interface RequestData extends QueuedRequestData { retryQueue: RetryQueue - onXHRError: (req: XMLHttpRequest) => void timeout?: number - onResponse?: (req: XMLHttpRequest) => void + onError: (req: MinimalHTTPResponse) => void + onResponse?: (req: MinimalHTTPResponse) => void } export interface DecideResponse { From c0849ff2e328b1cdb5f1f17e4ddb315b7c04992e Mon Sep 17 00:00:00 2001 From: Ben White Date: Wed, 31 Jan 2024 10:01:00 +0000 Subject: [PATCH 09/13] Fixes all round --- src/__tests__/retry-queue.test.ts | 16 ++++++------- src/__tests__/send-request.test.ts | 36 +++++++++++++++++------------- src/types.ts | 2 +- 3 files changed, 30 insertions(+), 24 deletions(-) diff --git a/src/__tests__/retry-queue.test.ts b/src/__tests__/retry-queue.test.ts index 7b1b95718..87e77254c 100644 --- a/src/__tests__/retry-queue.test.ts +++ b/src/__tests__/retry-queue.test.ts @@ -14,7 +14,7 @@ const defaultRequestOptions: CaptureOptions = { } describe('RetryQueue', () => { - const onXHRError = jest.fn().mockImplementation(console.error) + const onRequestError = jest.fn().mockImplementation(console.error) const rateLimiter = new RateLimiter() let retryQueue: RetryQueue @@ -26,7 +26,7 @@ describe('RetryQueue', () => { }) beforeEach(() => { - retryQueue = new RetryQueue(onXHRError, rateLimiter) + retryQueue = new RetryQueue(onRequestError, rateLimiter) assignableWindow.XMLHttpRequest = jest.fn().mockImplementation(xhrMockClass) assignableWindow.navigator.sendBeacon = jest.fn() @@ -112,7 +112,7 @@ describe('RetryQueue', () => { expect(retryQueue.queue.length).toEqual(0) expect(assignableWindow.XMLHttpRequest).toHaveBeenCalledTimes(4) - expect(onXHRError).toHaveBeenCalledTimes(0) + expect(onRequestError).toHaveBeenCalledTimes(0) }) it('does not process event retry requests when events are rate limited', () => { @@ -144,7 +144,7 @@ describe('RetryQueue', () => { // clears queue expect(retryQueue.queue.length).toEqual(0) expect(assignableWindow.XMLHttpRequest).toHaveBeenCalledTimes(1) - expect(onXHRError).toHaveBeenCalledTimes(0) + expect(onRequestError).toHaveBeenCalledTimes(0) }) it('does not process recording retry requests when they are rate limited', () => { @@ -176,7 +176,7 @@ describe('RetryQueue', () => { // clears queue expect(retryQueue.queue.length).toEqual(0) expect(assignableWindow.XMLHttpRequest).toHaveBeenCalledTimes(2) - expect(onXHRError).toHaveBeenCalledTimes(0) + expect(onRequestError).toHaveBeenCalledTimes(0) }) it('tries to send requests via beacon on unload', () => { @@ -201,12 +201,12 @@ describe('RetryQueue', () => { expect(assignableWindow.navigator.sendBeacon).toHaveBeenCalledTimes(0) }) - it('when you flush the queue onXHRError is passed to xhr', () => { + it('when you flush the queue onError is passed to xhr', () => { const xhrSpy = jest.spyOn(SendRequest, 'request') enqueueRequests() retryQueue.flush() fastForwardTimeAndRunTimer() - expect(xhrSpy).toHaveBeenCalledWith(expect.objectContaining({ onXHRError: onXHRError })) + expect(xhrSpy).toHaveBeenCalledWith(expect.objectContaining({ onError: onRequestError })) }) it('enqueues requests when offline and flushes immediately when online again', () => { @@ -220,7 +220,7 @@ describe('RetryQueue', () => { // requests aren't attempted when we're offline expect(assignableWindow.XMLHttpRequest).toHaveBeenCalledTimes(0) // doesn't log that it is offline from the retry queue - expect(onXHRError).toHaveBeenCalledTimes(0) + expect(onRequestError).toHaveBeenCalledTimes(0) // queue stays the same expect(retryQueue.queue.length).toEqual(4) diff --git a/src/__tests__/send-request.test.ts b/src/__tests__/send-request.test.ts index 386812cda..1b7d7feba 100644 --- a/src/__tests__/send-request.test.ts +++ b/src/__tests__/send-request.test.ts @@ -2,7 +2,7 @@ import { addParamsToURL, encodePostData, request } from '../send-request' import { assert, boolean, property, uint8Array, VerbosityLevel } from 'fast-check' -import { Compression, PostData, XHROptions, RequestData } from '../types' +import { Compression, PostData, XHROptions, RequestData, MinimalHTTPResponse } from '../types' import { _isUndefined } from '../utils/type-utils' @@ -12,7 +12,6 @@ describe('send-request', () => { describe('xhr', () => { let mockXHR: XMLHttpRequest let createRequestData: (overrides?: Partial) => RequestData - let onXHRError: RequestData['onError'] let checkForLimiting: RequestData['onResponse'] let xhrOptions: RequestData['options'] @@ -27,7 +26,6 @@ describe('send-request', () => { status: 502, } as Partial as XMLHttpRequest - onXHRError = jest.fn() checkForLimiting = jest.fn() xhrOptions = {} createRequestData = (overrides?: Partial) => { @@ -41,7 +39,6 @@ describe('send-request', () => { retryQueue: { enqueue: () => {}, } as Partial as RequestData['retryQueue'], - onError: onXHRError, onResponse: checkForLimiting, ...overrides, } @@ -83,23 +80,29 @@ describe('send-request', () => { }) describe('when xhr requests fail', () => { - it('does not error if the configured onXHRError is not a function', () => { - onXHRError = 'not a function' as unknown as RequestData['onError'] + it('does not error if the configured onError is not a function', () => { expect(() => { - request(createRequestData()) + request( + createRequestData({ + onError: 'not a function' as unknown as RequestData['onError'], + }) + ) mockXHR.onreadystatechange?.({} as Event) }).not.toThrow() }) it('calls the injected XHR error handler', () => { - //cannot use an auto-mock from jest as the code checks if onXHRError is a Function - let requestFromError - onXHRError = (req) => { - requestFromError = req - } - request(createRequestData()) + //cannot use an auto-mock from jest as the code checks if onError is a Function + let requestFromError: MinimalHTTPResponse | undefined + request( + createRequestData({ + onError: (req) => { + requestFromError = req + }, + }) + ) mockXHR.onreadystatechange?.({} as Event) - expect(requestFromError).toHaveProperty('status', 502) + expect(requestFromError).toHaveProperty('statusCode', 502) }) it('calls the on response handler - regardless of status', () => { @@ -111,7 +114,10 @@ describe('send-request', () => { mockXHR.status = Math.floor(Math.random() * 100) request(createRequestData()) mockXHR.onreadystatechange?.({} as Event) - expect(checkForLimiting).toHaveBeenCalledWith(mockXHR) + expect(checkForLimiting).toHaveBeenCalledWith({ + statusCode: mockXHR.status, + responseText: mockXHR.responseText, + }) }) }) }) diff --git a/src/types.ts b/src/types.ts index e4c343e2c..99746096c 100644 --- a/src/types.ts +++ b/src/types.ts @@ -226,7 +226,7 @@ export interface MinimalHTTPResponse { export interface RequestData extends QueuedRequestData { retryQueue: RetryQueue timeout?: number - onError: (req: MinimalHTTPResponse) => void + onError?: (req: MinimalHTTPResponse) => void onResponse?: (req: MinimalHTTPResponse) => void } From 94a1b3d7f7f3d9a72a06caff90d7afadba24cfc7 Mon Sep 17 00:00:00 2001 From: Ben White Date: Wed, 31 Jan 2024 10:39:48 +0000 Subject: [PATCH 10/13] Fix up to use the existing `api_transport` config option --- src/extensions/exception-autocapture/index.ts | 1 - src/extensions/replay/sessionrecording.ts | 1 - src/posthog-core.ts | 40 +++++++++---------- src/send-request.ts | 5 ++- src/types.ts | 4 +- src/utils/request-utils.ts | 8 +++- 6 files changed, 31 insertions(+), 28 deletions(-) diff --git a/src/extensions/exception-autocapture/index.ts b/src/extensions/exception-autocapture/index.ts index 3ae4e6d7a..17a97e6ce 100644 --- a/src/extensions/exception-autocapture/index.ts +++ b/src/extensions/exception-autocapture/index.ts @@ -140,7 +140,6 @@ export class ExceptionObserver { */ sendExceptionEvent(properties: { [key: string]: any }) { this.instance.capture('$exception', properties, { - transport: 'XHR', method: 'POST', endpoint: EXCEPTION_INGESTION_ENDPOINT, _noTruncate: true, diff --git a/src/extensions/replay/sessionrecording.ts b/src/extensions/replay/sessionrecording.ts index 4dda8e582..e6be439c2 100644 --- a/src/extensions/replay/sessionrecording.ts +++ b/src/extensions/replay/sessionrecording.ts @@ -834,7 +834,6 @@ export class SessionRecording { private _captureSnapshot(properties: Properties) { // :TRICKY: Make sure we batch these requests, use a custom endpoint and don't truncate the strings. this.instance.capture('$snapshot', properties, { - transport: 'XHR', method: 'POST', endpoint: this._endpoint, _noTruncate: true, diff --git a/src/posthog-core.ts b/src/posthog-core.ts index ad575bb7c..89613a53a 100644 --- a/src/posthog-core.ts +++ b/src/posthog-core.ts @@ -58,6 +58,7 @@ import { logger } from './utils/logger' import { document, userAgent } from './utils/globals' import { SessionPropsManager } from './session-props' import { _isBlockedUA } from './utils/blocked-uas' +import { SUPPORTS_REQUEST } from 'utils/request-utils' /* SIMPLE STYLE GUIDE: @@ -92,12 +93,11 @@ const PRIMARY_INSTANCE_NAME = 'posthog' */ // http://hacks.mozilla.org/2009/07/cross-site-xmlhttprequest-with-cors/ // https://developer.mozilla.org/en-US/docs/DOM/XMLHttpRequest#withCredentials -const USE_REQUEST = window?.fetch || (window?.XMLHttpRequest && 'withCredentials' in new XMLHttpRequest()) // IE<10 does not support cross-origin XHR's but script tags // with defer won't block window.onload; ENQUEUE_REQUESTS // should only be true for Opera<12 -let ENQUEUE_REQUESTS = !USE_REQUEST && userAgent?.indexOf('MSIE') === -1 && userAgent?.indexOf('Mozilla') === -1 +let ENQUEUE_REQUESTS = !SUPPORTS_REQUEST && userAgent?.indexOf('MSIE') === -1 && userAgent?.indexOf('Mozilla') === -1 export const defaultConfig = (): PostHogConfig => ({ api_host: 'https://app.posthog.com', @@ -628,23 +628,23 @@ export class PostHog { return null } - if (USE_REQUEST) { + if (SUPPORTS_REQUEST) { return function (response) { callback(response, data) } - } else { - // if the user gives us a callback, we store as a random - // property on this instances jsc function and update our - // callback string to reflect that. - const jsc = this._jsc - const randomized_cb = '' + Math.floor(Math.random() * 100000000) - const callback_string = this.config.callback_fn + '[' + randomized_cb + ']' - jsc[randomized_cb] = function (response: any) { - delete jsc[randomized_cb] - callback(response, data) - } - return callback_string } + + // if the user gives us a callback, we store as a random + // property on this instances jsc function and update our + // callback string to reflect that. + const jsc = this._jsc + const randomized_cb = '' + Math.floor(Math.random() * 100000000) + const callback_string = this.config.callback_fn + '[' + randomized_cb + ']' + jsc[randomized_cb] = function (response: any) { + delete jsc[randomized_cb] + callback(response, data) + } + return callback_string } _handle_unload(): void { @@ -698,7 +698,7 @@ export class PostHog { } options = _extend(DEFAULT_OPTIONS, options || {}) - if (!USE_REQUEST) { + if (!SUPPORTS_REQUEST) { options.method = 'GET' } @@ -716,13 +716,13 @@ export class PostHog { // send beacon is a best-effort, fire-and-forget mechanism on page unload, // we don't want to throw errors here } - } else if (USE_REQUEST || !document) { + } else if (SUPPORTS_REQUEST || !document) { try { request({ - url: url, - data: data, + url, + data, headers: this.config.request_headers, - options: options, + options, callback, retriesPerformedSoFar: 0, retryQueue: this._retryQueue, diff --git a/src/send-request.ts b/src/send-request.ts index 4c52295c3..11060d4c4 100644 --- a/src/send-request.ts +++ b/src/send-request.ts @@ -1,7 +1,7 @@ import { _each } from './utils' import Config from './config' import { PostData, XHROptions, RequestData, MinimalHTTPResponse } from './types' -import { _HTTPBuildQuery } from './utils/request-utils' +import { SUPPORTS_FETCH, _HTTPBuildQuery } from './utils/request-utils' import { _isArray, _isFunction, _isNumber, _isUint8Array, _isUndefined } from './utils/type-utils' import { logger } from './utils/logger' @@ -63,7 +63,8 @@ export const encodePostData = (data: PostData | Uint8Array, options: Partial { - if (window?.fetch) { + // NOTE: Until we are confident with it, we only use fetch if explicitly told so + if (window && SUPPORTS_FETCH && params.options.transport === 'fetch') { const body = encodePostData(params.data, params.options) const headers = new Headers() diff --git a/src/types.ts b/src/types.ts index 99746096c..841e7798f 100644 --- a/src/types.ts +++ b/src/types.ts @@ -58,7 +58,7 @@ export type UUIDVersion = 'og' | 'v7' export interface PostHogConfig { api_host: string api_method: string - api_transport: string + api_transport?: 'XHR' | 'fetch' ui_host: string | null token: string autocapture: boolean | AutocaptureConfig @@ -185,7 +185,7 @@ export enum Compression { } export interface XHROptions { - transport?: 'XHR' | 'sendBeacon' + transport?: 'XHR' | 'fetch' | 'sendBeacon' method?: 'POST' | 'GET' urlQueryArgs?: { compression: Compression } verbose?: boolean diff --git a/src/utils/request-utils.ts b/src/utils/request-utils.ts index a097001d6..af92200ef 100644 --- a/src/utils/request-utils.ts +++ b/src/utils/request-utils.ts @@ -1,11 +1,15 @@ import { _each, _isValidRegex } from './' -import { _isArray, _isUndefined } from './type-utils' +import { _isArray, _isFunction, _isUndefined } from './type-utils' import { logger } from './logger' -import { document } from './globals' +import { document, window } from './globals' const localDomains = ['localhost', '127.0.0.1'] +export const SUPPORTS_XHR = window?.XMLHttpRequest && 'withCredentials' in new XMLHttpRequest() +export const SUPPORTS_FETCH = window?.fetch && _isFunction(window?.fetch) +export const SUPPORTS_REQUEST = SUPPORTS_XHR || SUPPORTS_FETCH + /** * IE11 doesn't support `new URL` * so we can create an anchor element and use that to parse the URL From c5a93b6bd98631fde222b43c5327c62301edecce Mon Sep 17 00:00:00 2001 From: Ben White Date: Wed, 31 Jan 2024 10:45:29 +0000 Subject: [PATCH 11/13] Fixes --- src/__tests__/extensions/replay/sessionrecording.test.ts | 4 ---- src/posthog-core.ts | 2 +- 2 files changed, 1 insertion(+), 5 deletions(-) diff --git a/src/__tests__/extensions/replay/sessionrecording.test.ts b/src/__tests__/extensions/replay/sessionrecording.test.ts index 8ba67e05b..f79796d29 100644 --- a/src/__tests__/extensions/replay/sessionrecording.test.ts +++ b/src/__tests__/extensions/replay/sessionrecording.test.ts @@ -536,7 +536,6 @@ describe('SessionRecording', () => { $window_id: 'windowId', }, { - transport: 'XHR', method: 'POST', endpoint: '/s/', _noTruncate: true, @@ -575,7 +574,6 @@ describe('SessionRecording', () => { }, { method: 'POST', - transport: 'XHR', endpoint: '/s/', _noTruncate: true, _batchKey: 'recordings', @@ -660,7 +658,6 @@ describe('SessionRecording', () => { }, { method: 'POST', - transport: 'XHR', endpoint: '/s/', _noTruncate: true, _batchKey: 'recordings', @@ -1288,7 +1285,6 @@ describe('SessionRecording', () => { _noTruncate: true, endpoint: '/s/', method: 'POST', - transport: 'XHR', } ) expect(sessionRecording['buffer']).toEqual({ diff --git a/src/posthog-core.ts b/src/posthog-core.ts index 89613a53a..f4871df71 100644 --- a/src/posthog-core.ts +++ b/src/posthog-core.ts @@ -58,7 +58,7 @@ import { logger } from './utils/logger' import { document, userAgent } from './utils/globals' import { SessionPropsManager } from './session-props' import { _isBlockedUA } from './utils/blocked-uas' -import { SUPPORTS_REQUEST } from 'utils/request-utils' +import { SUPPORTS_REQUEST } from './utils/request-utils' /* SIMPLE STYLE GUIDE: From 17aa7caa6253d20412b0aed6f4c08ea7579c4081 Mon Sep 17 00:00:00 2001 From: Ben White Date: Wed, 31 Jan 2024 11:44:11 +0000 Subject: [PATCH 12/13] Fix up tests --- src/__tests__/send-request.test.ts | 178 +++++++++++++++++++++++++++-- src/send-request.ts | 40 +++++-- src/utils/request-utils.ts | 4 +- 3 files changed, 196 insertions(+), 26 deletions(-) diff --git a/src/__tests__/send-request.test.ts b/src/__tests__/send-request.test.ts index 1b7d7feba..176fcf3df 100644 --- a/src/__tests__/send-request.test.ts +++ b/src/__tests__/send-request.test.ts @@ -1,3 +1,4 @@ +/* eslint-disable compat/compat */ /// import { addParamsToURL, encodePostData, request } from '../send-request' @@ -5,15 +6,24 @@ import { assert, boolean, property, uint8Array, VerbosityLevel } from 'fast-chec import { Compression, PostData, XHROptions, RequestData, MinimalHTTPResponse } from '../types' import { _isUndefined } from '../utils/type-utils' +import { assignableWindow } from '../utils/globals' + +jest.mock('../utils/request-utils', () => ({ + ...jest.requireActual('../utils/request-utils'), + SUPPORTS_XHR: true, + SUPPORTS_FETCH: true, + SUPPORTS_REQUEST: true, +})) jest.mock('../config', () => ({ DEBUG: false, LIB_VERSION: '1.23.45' })) +const flushPromises = () => new Promise((r) => setTimeout(r, 0)) + describe('send-request', () => { describe('xhr', () => { let mockXHR: XMLHttpRequest let createRequestData: (overrides?: Partial) => RequestData let checkForLimiting: RequestData['onResponse'] - let xhrOptions: RequestData['options'] beforeEach(() => { mockXHR = { @@ -27,14 +37,16 @@ describe('send-request', () => { } as Partial as XMLHttpRequest checkForLimiting = jest.fn() - xhrOptions = {} createRequestData = (overrides?: Partial) => { return { url: 'https://any.posthog-instance.com?ver=1.23.45', data: {}, headers: {}, - options: xhrOptions, callback: () => {}, + options: { + transport: 'XHR', + ...(overrides?.options || {}), + }, retriesPerformedSoFar: undefined, retryQueue: { enqueue: () => {}, @@ -47,39 +59,52 @@ describe('send-request', () => { // ignore TS complaining about us cramming a fake in here // eslint-disable-next-line @typescript-eslint/ban-ts-comment // @ts-ignore - window.XMLHttpRequest = jest.fn(() => mockXHR) as unknown as XMLHttpRequest + assignableWindow.XMLHttpRequest = jest.fn(() => mockXHR) as unknown as XMLHttpRequest }) - test('it adds the retry count to the URL', () => { - const retryCount = Math.floor(Math.random() * 100) + 1 // make sure it is never 0 + test('performs the request with default params', () => { request( createRequestData({ - retriesPerformedSoFar: retryCount, + retriesPerformedSoFar: 0, url: 'https://any.posthog-instance.com/?ver=1.23.45&ip=7&_=1698404857278', }) ) expect(mockXHR.open).toHaveBeenCalledWith( 'GET', - `https://any.posthog-instance.com/?ver=1.23.45&ip=7&_=1698404857278&retry_count=${retryCount}`, + 'https://any.posthog-instance.com/?ver=1.23.45&ip=7&_=1698404857278', true ) }) - test('does not add retry count when it is 0', () => { + test('it adds the retry count to the URL', () => { + const retryCount = Math.floor(Math.random() * 100) + 1 // make sure it is never 0 request( createRequestData({ - retriesPerformedSoFar: 0, + retriesPerformedSoFar: retryCount, url: 'https://any.posthog-instance.com/?ver=1.23.45&ip=7&_=1698404857278', }) ) expect(mockXHR.open).toHaveBeenCalledWith( 'GET', - 'https://any.posthog-instance.com/?ver=1.23.45&ip=7&_=1698404857278', + `https://any.posthog-instance.com/?ver=1.23.45&ip=7&_=1698404857278&retry_count=${retryCount}`, true ) }) - describe('when xhr requests fail', () => { + it('calls the on response handler', async () => { + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-ignore + // noinspection JSConstantReassignment + mockXHR.status = 200 + request(createRequestData()) + mockXHR.onreadystatechange?.({} as Event) + expect(checkForLimiting).toHaveBeenCalledWith({ + statusCode: mockXHR.status, + responseText: mockXHR.responseText, + }) + }) + + describe('when the requests fail', () => { it('does not error if the configured onError is not a function', () => { expect(() => { request( @@ -122,6 +147,135 @@ describe('send-request', () => { }) }) + describe('fetch', () => { + let createRequestData: (overrides?: Partial) => RequestData + let checkForLimiting: RequestData['onResponse'] + + beforeEach(() => { + checkForLimiting = jest.fn() + createRequestData = (overrides?: Partial) => { + return { + url: 'https://any.posthog-instance.com?ver=1.23.45', + data: {}, + headers: {}, + callback: () => {}, + retriesPerformedSoFar: undefined, + options: { + transport: 'fetch', + ...(overrides?.options || {}), + }, + + retryQueue: { + enqueue: () => {}, + } as Partial as RequestData['retryQueue'], + onResponse: checkForLimiting, + ...overrides, + } + } + + assignableWindow.fetch = jest.fn(() => { + return Promise.resolve({ + status: 200, + json: () => Promise.resolve({}), + text: () => Promise.resolve(''), + }) as any + }) + }) + + test('it performs the request with default params', () => { + request( + createRequestData({ + retriesPerformedSoFar: 0, + url: 'https://any.posthog-instance.com/?ver=1.23.45&ip=7&_=1698404857278', + }) + ) + + expect(assignableWindow.fetch).toHaveBeenCalledWith( + `https://any.posthog-instance.com/?ver=1.23.45&ip=7&_=1698404857278`, + { + body: null, + headers: new Headers(), + keepalive: false, + method: 'GET', + } + ) + }) + + test('it adds the retry count to the URL', () => { + const retryCount = Math.floor(Math.random() * 100) + 1 // make sure it is never 0 + request( + createRequestData({ + retriesPerformedSoFar: retryCount, + url: 'https://any.posthog-instance.com/?ver=1.23.45&ip=7&_=1698404857278', + }) + ) + expect(assignableWindow.fetch).toHaveBeenCalledWith( + `https://any.posthog-instance.com/?ver=1.23.45&ip=7&_=1698404857278&retry_count=${retryCount}`, + { + body: null, + headers: new Headers(), + keepalive: false, + method: 'GET', + } + ) + }) + + it('calls the on response handler', async () => { + request(createRequestData()) + await flushPromises() + expect(checkForLimiting).toHaveBeenCalledWith({ + statusCode: 200, + responseText: '', + }) + }) + + describe('when the requests fail', () => { + beforeEach(() => { + assignableWindow.fetch = jest.fn( + () => + Promise.resolve({ + status: 502, + text: () => Promise.resolve('oh no!'), + }) as any + ) + }) + + it('does not error if the configured onError is not a function', () => { + expect(() => { + request( + createRequestData({ + onError: 'not a function' as unknown as RequestData['onError'], + }) + ) + }).not.toThrow() + }) + + it('calls the injected XHR error handler', async () => { + //cannot use an auto-mock from jest as the code checks if onError is a Function + let requestFromError: MinimalHTTPResponse | undefined + request( + createRequestData({ + onError: (req) => { + requestFromError = req + }, + }) + ) + await flushPromises() + + expect(requestFromError).toHaveProperty('statusCode', 502) + }) + + it('calls the on response handler - regardless of status', async () => { + request(createRequestData()) + await flushPromises() + expect(checkForLimiting).toHaveBeenCalledWith({ + statusCode: 502, + responseText: 'oh no!', + }) + }) + }) + }) + describe('adding query params to posthog API calls', () => { let posthogURL: string let parameterOptions: { ip?: boolean } diff --git a/src/send-request.ts b/src/send-request.ts index 11060d4c4..310491bb8 100644 --- a/src/send-request.ts +++ b/src/send-request.ts @@ -76,30 +76,46 @@ export const request = (params: RequestData) => { headers.append('Content-Type', 'application/x-www-form-urlencoded') } + let url = params.url + + if (_isNumber(params.retriesPerformedSoFar) && params.retriesPerformedSoFar > 0) { + url = addParamsToURL(url, { retry_count: params.retriesPerformedSoFar }, {}) + } + window - .fetch(params.url, { + .fetch(url, { method: params.options?.method || 'GET', headers, keepalive: params.options.method === 'POST', body, }) .then((response) => { - // TODO: Implement on response handler + // Report to the callback handlers + response.text().then((text) => { + const minimalResponseSummary: MinimalHTTPResponse = { + statusCode: response.status, + responseText: text, + } + params.onResponse?.(minimalResponseSummary) + if (_isFunction(params.onError)) { + params.onError(minimalResponseSummary) + } + }) if (response.status === 200) { response.json().then((json) => { params.callback?.(json) }) - } else { - // don't retry errors between 400 and 500 inclusive - if (response.status < 400 || response.status > 500) { - params.retryQueue.enqueue({ - ...params, - headers, - retriesPerformedSoFar: (params.retriesPerformedSoFar || 0) + 1, - }) - } - params.callback?.({ status: 0 }) + return } + // don't retry errors between 400 and 500 inclusive + if (response.status < 400 || response.status > 500) { + params.retryQueue.enqueue({ + ...params, + headers, + retriesPerformedSoFar: (params.retriesPerformedSoFar || 0) + 1, + }) + } + params.callback?.({ status: 0 }) }) .catch((error) => { logger.error(error) diff --git a/src/utils/request-utils.ts b/src/utils/request-utils.ts index af92200ef..1acf562e2 100644 --- a/src/utils/request-utils.ts +++ b/src/utils/request-utils.ts @@ -6,8 +6,8 @@ import { document, window } from './globals' const localDomains = ['localhost', '127.0.0.1'] -export const SUPPORTS_XHR = window?.XMLHttpRequest && 'withCredentials' in new XMLHttpRequest() -export const SUPPORTS_FETCH = window?.fetch && _isFunction(window?.fetch) +export const SUPPORTS_XHR = !!(window?.XMLHttpRequest && 'withCredentials' in new XMLHttpRequest()) +export const SUPPORTS_FETCH = !!(window?.fetch && _isFunction(window?.fetch)) export const SUPPORTS_REQUEST = SUPPORTS_XHR || SUPPORTS_FETCH /** From 21be07a8b703b995e3fc71f39de7ffb22a73d3f0 Mon Sep 17 00:00:00 2001 From: Ben White Date: Wed, 31 Jan 2024 12:07:05 +0000 Subject: [PATCH 13/13] Fixed up send request --- src/send-request.ts | 50 ++++++++++++++++++++++++++------------------- 1 file changed, 29 insertions(+), 21 deletions(-) diff --git a/src/send-request.ts b/src/send-request.ts index 310491bb8..15508a9ea 100644 --- a/src/send-request.ts +++ b/src/send-request.ts @@ -90,32 +90,40 @@ export const request = (params: RequestData) => { body, }) .then((response) => { + const statusCode = response.status // Report to the callback handlers - response.text().then((text) => { - const minimalResponseSummary: MinimalHTTPResponse = { - statusCode: response.status, - responseText: text, + return response.text().then((responseText) => { + params.onResponse?.({ + statusCode, + responseText, + }) + + if (statusCode === 200) { + try { + params.callback?.(JSON.parse(responseText)) + } catch (e) { + logger.error(e) + } + return } - params.onResponse?.(minimalResponseSummary) + if (_isFunction(params.onError)) { - params.onError(minimalResponseSummary) + params.onError({ + statusCode, + responseText, + }) } + + // don't retry errors between 400 and 500 inclusive + if (statusCode < 400 || statusCode > 500) { + params.retryQueue.enqueue({ + ...params, + headers, + retriesPerformedSoFar: (params.retriesPerformedSoFar || 0) + 1, + }) + } + params.callback?.({ status: 0 }) }) - if (response.status === 200) { - response.json().then((json) => { - params.callback?.(json) - }) - return - } - // don't retry errors between 400 and 500 inclusive - if (response.status < 400 || response.status > 500) { - params.retryQueue.enqueue({ - ...params, - headers, - retriesPerformedSoFar: (params.retriesPerformedSoFar || 0) + 1, - }) - } - params.callback?.({ status: 0 }) }) .catch((error) => { logger.error(error)