From 79b83fada0a922fb852ada6bb7acd5d34358364d Mon Sep 17 00:00:00 2001 From: Paul D'Ambra Date: Fri, 8 Nov 2024 13:00:49 +0100 Subject: [PATCH 01/28] first bit of type fangling --- src/autocapture.ts | 6 ++---- src/posthog-core.ts | 9 +++++++-- src/types.ts | 31 ++++++++++++++++++++++++++++++- 3 files changed, 39 insertions(+), 7 deletions(-) diff --git a/src/autocapture.ts b/src/autocapture.ts index 8fc25d580..cf9289e3a 100644 --- a/src/autocapture.ts +++ b/src/autocapture.ts @@ -15,7 +15,7 @@ import { splitClassString, } from './autocapture-utils' import RageClick from './extensions/rageclick' -import { AutocaptureConfig, DecideResponse, Properties } from './types' +import { AutocaptureConfig, COPY_AUTOCAPTURE_EVENT, DecideResponse, EventName, Properties } from './types' import { PostHog } from './posthog-core' import { AUTOCAPTURE_DISABLED_SERVER_SIDE } from './constants' @@ -25,8 +25,6 @@ import { document, window } from './utils/globals' import { convertToURL } from './utils/request-utils' import { isDocumentFragment, isElementNode, isTag, isTextNode } from './utils/element-utils' -const COPY_AUTOCAPTURE_EVENT = '$copy_autocapture' - function limitText(length: number, text: string): string { if (text.length > length) { return text.slice(0, length) + '...' @@ -343,7 +341,7 @@ export class Autocapture { return !disabledClient && !disabledServer } - private _captureEvent(e: Event, eventName = '$autocapture'): boolean | void { + private _captureEvent(e: Event, eventName: EventName = '$autocapture'): boolean | void { if (!this.isEnabled) { return } diff --git a/src/posthog-core.ts b/src/posthog-core.ts index 5f5f9b04f..1d3967234 100644 --- a/src/posthog-core.ts +++ b/src/posthog-core.ts @@ -34,6 +34,7 @@ import { Compression, DecideResponse, EarlyAccessFeatureCallback, + EventName, IsFeatureEnabledOptions, JsonType, PostHogConfig, @@ -787,7 +788,11 @@ export class PostHog { * @param {String} [config.transport] Transport method for network request ('XHR' or 'sendBeacon'). * @param {Date} [config.timestamp] Timestamp is a Date object. If not set, it'll automatically be set to the current time. */ - capture(event_name: string, properties?: Properties | null, options?: CaptureOptions): CaptureResult | undefined { + capture( + event_name: EventName, + properties?: Properties | null, + options?: CaptureOptions + ): CaptureResult | undefined { // While developing, a developer might purposefully _not_ call init(), // in this case, we would like capture to be a noop. if (!this.__loaded || !this.persistence || !this.sessionPersistence || !this._requestQueue) { @@ -2040,7 +2045,7 @@ export class PostHog { * @param {Object} [config.capture_properties] Set of properties to be captured along with the opt-in action */ opt_in_capturing(options?: { - captureEventName?: string | null | false /** event name to be used for capturing the opt-in action */ + captureEventName?: EventName | null | false /** event name to be used for capturing the opt-in action */ captureProperties?: Properties /** set of properties to be captured along with the opt-in action */ }): void { this.consent.optInOut(true) diff --git a/src/types.ts b/src/types.ts index 0f6c5cfc9..6ebc737cd 100644 --- a/src/types.ts +++ b/src/types.ts @@ -5,9 +5,38 @@ import type { SegmentAnalytics } from './extensions/segment-integration' export type Property = any export type Properties = Record +export const COPY_AUTOCAPTURE_EVENT = '$copy_autocapture' +type KnownEventName = + | '$pageview' + | '$heatmaps_data' + | '$pageleave' + | '$identify' + | '$set' + | '$groupidentify' + | '$create_alias' + | '$opt_in' + | '$web_experiment_applied' + | '$$client_ingestion_warning' + | '$feature_enrollment_update' + | '$feature_flag_called' + | '$exception' + | '$$heatmap' + | '$web_vitals' + | 'survey dismissed' + | 'survey sent' + | 'survey shown' + | '$snapshot' + | '$dead_click' + | '$autocapture' + | typeof COPY_AUTOCAPTURE_EVENT + | '$rageclick' +// TODO must be able to add a string as well, +// such that we get autocorrect when using posthog-js in typesscript but can also add custom events +export type EventName = KnownEventName | string + export interface CaptureResult { uuid: string - event: string + event: EventName properties: Properties $set?: Properties $set_once?: Properties From 5165110e54d30d7802084d1fe7322dd06ed1daaa Mon Sep 17 00:00:00 2001 From: Paul D'Ambra Date: Fri, 8 Nov 2024 14:46:19 +0000 Subject: [PATCH 02/28] draw the rest of the owl --- .../posthog-core.beforeCapture.test.ts | 156 ++++++++++++++++++ src/posthog-core.ts | 17 ++ src/types.ts | 64 +++++-- src/utils/type-utils.ts | 15 ++ 4 files changed, 235 insertions(+), 17 deletions(-) create mode 100644 src/__tests__/posthog-core.beforeCapture.test.ts diff --git a/src/__tests__/posthog-core.beforeCapture.test.ts b/src/__tests__/posthog-core.beforeCapture.test.ts new file mode 100644 index 000000000..8fd57656a --- /dev/null +++ b/src/__tests__/posthog-core.beforeCapture.test.ts @@ -0,0 +1,156 @@ +import { uuidv7 } from '../uuidv7' +import { defaultPostHog } from './helpers/posthog-instance' +import { logger } from '../utils/logger' +import { CaptureResult, knownUnEditableEvent, knownUnsafeEditableEvent, PostHogConfig } from '../types' +import { PostHog } from '../posthog-core' + +jest.mock('../utils/logger') +jest.mock('../decide') + +describe('posthog core - before capture', () => { + const baseUTCDateTime = new Date(Date.UTC(2020, 0, 1, 0, 0, 0)) + const eventName = '$event' + + const defaultConfig = {} + + const defaultOverrides = { + _send_request: jest.fn(), + } + + const posthogWith = (config: Partial, overrides?: Partial): PostHog => { + const posthog = defaultPostHog().init('testtoken', config, uuidv7()) + return Object.assign(posthog, overrides || {}) + } + + beforeEach(() => { + jest.useFakeTimers().setSystemTime(baseUTCDateTime) + }) + + afterEach(() => { + jest.useRealTimers() + }) + + it('can reject an event', () => { + const posthog = posthogWith( + { + ...defaultConfig, + beforeCapture: () => { + return null + }, + }, + defaultOverrides + ) + ;(posthog._send_request as jest.Mock).mockClear() + const capturedData = posthog.capture(eventName, {}, {}) + expect(capturedData).toBeUndefined() + expect(posthog._send_request).not.toHaveBeenCalled() + }) + + it('can edit an event', () => { + const posthog = posthogWith( + { + ...defaultConfig, + beforeCapture: (captureResult: CaptureResult): CaptureResult => { + return { + ...captureResult, + properties: { + ...captureResult.properties, + edited: true, + }, + $set: { + ...captureResult.$set, + edited: true, + }, + } + }, + }, + defaultOverrides + ) + ;(posthog._send_request as jest.Mock).mockClear() + const capturedData = posthog.capture(eventName, {}, {}) + expect(capturedData).toHaveProperty(['properties', 'edited'], true) + expect(capturedData).toHaveProperty(['$set', 'edited'], true) + + expect(posthog._send_request).toHaveBeenCalledWith({ + batchKey: undefined, + callback: expect.any(Function), + compression: 'best-available', + data: capturedData, + method: 'POST', + url: 'https://us.i.posthog.com/e/', + }) + }) + + it('cannot reject an un-editable event', () => { + const posthog = posthogWith( + { + ...defaultConfig, + beforeCapture: () => { + return null + }, + }, + defaultOverrides + ) + ;(posthog._send_request as jest.Mock).mockClear() + // chooses a random string from knownUnEditableEvent + const randomUneditableEvent = knownUnEditableEvent[Math.floor(Math.random() * knownUnEditableEvent.length)] + + const capturedData = posthog.capture(randomUneditableEvent, {}, {}) + + expect(capturedData).not.toBeUndefined() + expect(posthog._send_request).toHaveBeenCalled() + }) + + it('cannot edit an un-editable event', () => { + const posthog = posthogWith( + { + ...defaultConfig, + beforeCapture: (captureResult: CaptureResult): CaptureResult => { + return { + ...captureResult, + properties: { + ...captureResult.properties, + edited: true, + }, + $set: { + ...captureResult.$set, + edited: true, + }, + } + }, + }, + defaultOverrides + ) + ;(posthog._send_request as jest.Mock).mockClear() + // chooses a random string from knownUnEditableEvent + const randomUneditableEvent = knownUnEditableEvent[Math.floor(Math.random() * knownUnEditableEvent.length)] + + const capturedData = posthog.capture(randomUneditableEvent, {}, {}) + + expect(capturedData).not.toHaveProperty(['properties', 'edited']) + expect(capturedData).not.toHaveProperty(['$set', 'edited']) + expect(posthog._send_request).toHaveBeenCalled() + }) + + it('logs a warning when rejecting an unsafe to edit event', () => { + const posthog = posthogWith( + { + ...defaultConfig, + beforeCapture: () => { + return null + }, + }, + defaultOverrides + ) + ;(posthog._send_request as jest.Mock).mockClear() + // chooses a random string from knownUnEditableEvent + const randomUnsafeEditableEvent = + knownUnsafeEditableEvent[Math.floor(Math.random() * knownUnsafeEditableEvent.length)] + + posthog.capture(randomUnsafeEditableEvent, {}, {}) + + expect(jest.mocked(logger).info).toHaveBeenCalledWith( + `Event '${randomUnsafeEditableEvent}' was rejected. This can cause unexpected behavior.` + ) + }) +}) diff --git a/src/posthog-core.ts b/src/posthog-core.ts index 1d3967234..8ea8b5e5e 100644 --- a/src/posthog-core.ts +++ b/src/posthog-core.ts @@ -59,6 +59,9 @@ import { isEmptyObject, isEmptyString, isFunction, + isKnownUnEditableEvent, + isKnownUnsafeEditableEvent, + isNullish, isNumber, isObject, isString, @@ -182,6 +185,8 @@ export const defaultConfig = (): PostHogConfig => ({ session_idle_timeout_seconds: 30 * 60, // 30 minutes person_profiles: 'identified_only', __add_tracing_headers: false, + // by default the before capture fn is the identity function + beforeCapture: (x) => x, }) export const configRenames = (origConfig: Partial): Partial => { @@ -875,6 +880,18 @@ export class PostHog { this.setPersonPropertiesForFlags(finalSet) } + if (!isKnownUnEditableEvent(data.event)) { + const beforeCaptureResult = this.config.beforeCapture(data) + if (isNullish(beforeCaptureResult)) { + if (isKnownUnsafeEditableEvent(data.event)) { + logger.info(`Event '${data.event}' was rejected. This can cause unexpected behavior.`) + } + return + } else { + data = beforeCaptureResult + } + } + this._internalEventEmitter.emit('eventCaptured', data) const requestOptions: QueuedRequestOptions = { diff --git a/src/types.ts b/src/types.ts index 6ebc737cd..314e49ff3 100644 --- a/src/types.ts +++ b/src/types.ts @@ -6,33 +6,59 @@ export type Property = any export type Properties = Record export const COPY_AUTOCAPTURE_EVENT = '$copy_autocapture' + +export const knownUnEditableEvent = [ + '$web_experiment_applied', + '$$client_ingestion_warning', + '$feature_enrollment_update', + '$feature_flag_called', + 'survey dismissed', + 'survey sent', + 'survey shown', + '$snapshot', +] as const +/** + * These events are not affected by the `beforeCapture` function + */ +export type KnownUnEditableEvent = typeof knownUnEditableEvent[number] + +export const knownUnsafeEditableEvent = [ + '$pageview', + '$pageleave', + '$identify', + '$set', + '$groupidentify', + '$create_alias', +] as const + +/** + * These events will be processed by the `beforeCapture` function + * but can cause unexpected confusion in data + * some features of PostHog rely on receiving 100% of these events + */ +export type KnownUnsafeEditableEvent = typeof knownUnsafeEditableEvent[number] + +/** + * These events will be processed by the `beforeCapture` function + */ type KnownEventName = - | '$pageview' | '$heatmaps_data' - | '$pageleave' - | '$identify' - | '$set' - | '$groupidentify' - | '$create_alias' | '$opt_in' - | '$web_experiment_applied' - | '$$client_ingestion_warning' - | '$feature_enrollment_update' - | '$feature_flag_called' | '$exception' | '$$heatmap' | '$web_vitals' - | 'survey dismissed' - | 'survey sent' - | 'survey shown' - | '$snapshot' | '$dead_click' | '$autocapture' | typeof COPY_AUTOCAPTURE_EVENT | '$rageclick' -// TODO must be able to add a string as well, -// such that we get autocorrect when using posthog-js in typesscript but can also add custom events -export type EventName = KnownEventName | string + +export type EventName = + | KnownUnEditableEvent + | KnownUnsafeEditableEvent + | KnownEventName + // magic value so that the type of eventname is a set of known strings or any other strings + // means you get autocomplete for known strings + | (string & {}) export interface CaptureResult { uuid: string @@ -243,7 +269,11 @@ export interface PostHogConfig { feature_flag_request_timeout_ms: number get_device_id: (uuid: string) => string name: string + /** + * this is a read-only function that can be used to react to event capture + */ _onCapture: (eventName: string, eventData: CaptureResult) => void + beforeCapture: (cr: CaptureResult) => CaptureResult | null capture_performance?: boolean | PerformanceCaptureConfig // Should only be used for testing. Could negatively impact performance. disable_compression: boolean diff --git a/src/utils/type-utils.ts b/src/utils/type-utils.ts index 2d7192f23..44f6afb63 100644 --- a/src/utils/type-utils.ts +++ b/src/utils/type-utils.ts @@ -1,3 +1,10 @@ +import { + KnownUnEditableEvent, + knownUnEditableEvent, + KnownUnsafeEditableEvent, + knownUnsafeEditableEvent, +} from '../types' + // eslint-disable-next-line posthog-js/no-direct-array-check const nativeIsArray = Array.isArray const ObjProto = Object.prototype @@ -88,3 +95,11 @@ export const isFile = (x: unknown): x is File => { // eslint-disable-next-line posthog-js/no-direct-file-check return x instanceof File } + +export const isKnownUnsafeEditableEvent = (x: unknown): x is KnownUnsafeEditableEvent => { + return knownUnsafeEditableEvent.includes(x as any) +} + +export const isKnownUnEditableEvent = (x: unknown): x is KnownUnEditableEvent => { + return knownUnEditableEvent.includes(x as any) +} From 19abae02f8a003ba8547e0cd10787af81242b84b Mon Sep 17 00:00:00 2001 From: Paul D'Ambra Date: Fri, 8 Nov 2024 14:51:53 +0000 Subject: [PATCH 03/28] Refactor tests --- .../posthog-core.beforeCapture.test.ts | 113 ++++++------------ 1 file changed, 38 insertions(+), 75 deletions(-) diff --git a/src/__tests__/posthog-core.beforeCapture.test.ts b/src/__tests__/posthog-core.beforeCapture.test.ts index 8fd57656a..232e760d8 100644 --- a/src/__tests__/posthog-core.beforeCapture.test.ts +++ b/src/__tests__/posthog-core.beforeCapture.test.ts @@ -5,21 +5,34 @@ import { CaptureResult, knownUnEditableEvent, knownUnsafeEditableEvent, PostHogC import { PostHog } from '../posthog-core' jest.mock('../utils/logger') -jest.mock('../decide') + +const rejectingEventFn = () => { + return null +} + +const editingEventFn = (captureResult: CaptureResult): CaptureResult => { + return { + ...captureResult, + properties: { + ...captureResult.properties, + edited: true, + }, + $set: { + ...captureResult.$set, + edited: true, + }, + } +} describe('posthog core - before capture', () => { const baseUTCDateTime = new Date(Date.UTC(2020, 0, 1, 0, 0, 0)) const eventName = '$event' - const defaultConfig = {} - - const defaultOverrides = { - _send_request: jest.fn(), - } - - const posthogWith = (config: Partial, overrides?: Partial): PostHog => { - const posthog = defaultPostHog().init('testtoken', config, uuidv7()) - return Object.assign(posthog, overrides || {}) + const posthogWith = (configOverride: Pick, 'beforeCapture'>): PostHog => { + const posthog = defaultPostHog().init('testtoken', configOverride, uuidv7()) + return Object.assign(posthog, { + _send_request: jest.fn(), + }) } beforeEach(() => { @@ -31,15 +44,9 @@ describe('posthog core - before capture', () => { }) it('can reject an event', () => { - const posthog = posthogWith( - { - ...defaultConfig, - beforeCapture: () => { - return null - }, - }, - defaultOverrides - ) + const posthog = posthogWith({ + beforeCapture: rejectingEventFn, + }) ;(posthog._send_request as jest.Mock).mockClear() const capturedData = posthog.capture(eventName, {}, {}) expect(capturedData).toBeUndefined() @@ -47,25 +54,9 @@ describe('posthog core - before capture', () => { }) it('can edit an event', () => { - const posthog = posthogWith( - { - ...defaultConfig, - beforeCapture: (captureResult: CaptureResult): CaptureResult => { - return { - ...captureResult, - properties: { - ...captureResult.properties, - edited: true, - }, - $set: { - ...captureResult.$set, - edited: true, - }, - } - }, - }, - defaultOverrides - ) + const posthog = posthogWith({ + beforeCapture: editingEventFn, + }) ;(posthog._send_request as jest.Mock).mockClear() const capturedData = posthog.capture(eventName, {}, {}) expect(capturedData).toHaveProperty(['properties', 'edited'], true) @@ -82,15 +73,9 @@ describe('posthog core - before capture', () => { }) it('cannot reject an un-editable event', () => { - const posthog = posthogWith( - { - ...defaultConfig, - beforeCapture: () => { - return null - }, - }, - defaultOverrides - ) + const posthog = posthogWith({ + beforeCapture: rejectingEventFn, + }) ;(posthog._send_request as jest.Mock).mockClear() // chooses a random string from knownUnEditableEvent const randomUneditableEvent = knownUnEditableEvent[Math.floor(Math.random() * knownUnEditableEvent.length)] @@ -102,25 +87,9 @@ describe('posthog core - before capture', () => { }) it('cannot edit an un-editable event', () => { - const posthog = posthogWith( - { - ...defaultConfig, - beforeCapture: (captureResult: CaptureResult): CaptureResult => { - return { - ...captureResult, - properties: { - ...captureResult.properties, - edited: true, - }, - $set: { - ...captureResult.$set, - edited: true, - }, - } - }, - }, - defaultOverrides - ) + const posthog = posthogWith({ + beforeCapture: editingEventFn, + }) ;(posthog._send_request as jest.Mock).mockClear() // chooses a random string from knownUnEditableEvent const randomUneditableEvent = knownUnEditableEvent[Math.floor(Math.random() * knownUnEditableEvent.length)] @@ -133,15 +102,9 @@ describe('posthog core - before capture', () => { }) it('logs a warning when rejecting an unsafe to edit event', () => { - const posthog = posthogWith( - { - ...defaultConfig, - beforeCapture: () => { - return null - }, - }, - defaultOverrides - ) + const posthog = posthogWith({ + beforeCapture: rejectingEventFn, + }) ;(posthog._send_request as jest.Mock).mockClear() // chooses a random string from knownUnEditableEvent const randomUnsafeEditableEvent = From e95364c7923017b5df393ec32545db1d22988e92 Mon Sep 17 00:00:00 2001 From: Paul D'Ambra Date: Fri, 8 Nov 2024 14:58:13 +0000 Subject: [PATCH 04/28] fiddle --- src/types.ts | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/src/types.ts b/src/types.ts index 314e49ff3..01bcdde41 100644 --- a/src/types.ts +++ b/src/types.ts @@ -17,8 +17,9 @@ export const knownUnEditableEvent = [ 'survey shown', '$snapshot', ] as const + /** - * These events are not affected by the `beforeCapture` function + * These events are not processed by the `beforeCapture` function */ export type KnownUnEditableEvent = typeof knownUnEditableEvent[number] @@ -32,14 +33,15 @@ export const knownUnsafeEditableEvent = [ ] as const /** - * These events will be processed by the `beforeCapture` function - * but can cause unexpected confusion in data - * some features of PostHog rely on receiving 100% of these events + * These events can be processed by the `beforeCapture` function + * but can cause unexpected confusion in data. + * + * Some features of PostHog rely on receiving 100% of these events */ export type KnownUnsafeEditableEvent = typeof knownUnsafeEditableEvent[number] /** - * These events will be processed by the `beforeCapture` function + * These known events can be processed by the `beforeCapture` function */ type KnownEventName = | '$heatmaps_data' @@ -56,8 +58,8 @@ export type EventName = | KnownUnEditableEvent | KnownUnsafeEditableEvent | KnownEventName - // magic value so that the type of eventname is a set of known strings or any other strings - // means you get autocomplete for known strings + // magic value so that the type of event name is a set of known strings or any other strings + // which means you get autocomplete for known strings | (string & {}) export interface CaptureResult { From 08b67414d0ead26413187c3d0a78fffaefdc5231 Mon Sep 17 00:00:00 2001 From: Paul D'Ambra Date: Fri, 8 Nov 2024 15:10:22 +0000 Subject: [PATCH 05/28] kind to IE11 --- src/utils/type-utils.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/utils/type-utils.ts b/src/utils/type-utils.ts index 44f6afb63..63a9eee13 100644 --- a/src/utils/type-utils.ts +++ b/src/utils/type-utils.ts @@ -1,3 +1,4 @@ +import { includes } from '.' import { KnownUnEditableEvent, knownUnEditableEvent, @@ -97,9 +98,9 @@ export const isFile = (x: unknown): x is File => { } export const isKnownUnsafeEditableEvent = (x: unknown): x is KnownUnsafeEditableEvent => { - return knownUnsafeEditableEvent.includes(x as any) + return includes(knownUnsafeEditableEvent as unknown as string[], x) } export const isKnownUnEditableEvent = (x: unknown): x is KnownUnEditableEvent => { - return knownUnEditableEvent.includes(x as any) + return includes(knownUnEditableEvent as unknown as string[], x as any) } From 54d3031e6d2b6644b01f5c64e3939c827a77810b Mon Sep 17 00:00:00 2001 From: Paul D'Ambra Date: Sun, 10 Nov 2024 12:32:23 +0000 Subject: [PATCH 06/28] Add some prebuilt functions --- .../posthog-core.beforeCapture.test.ts | 12 +++- .../utils/before-capture-utils.test.ts | 57 +++++++++++++++++ src/posthog-core.ts | 5 +- src/types.ts | 6 +- src/utils/before-capture.utils.ts | 64 +++++++++++++++++++ 5 files changed, 138 insertions(+), 6 deletions(-) create mode 100644 src/__tests__/utils/before-capture-utils.test.ts create mode 100644 src/utils/before-capture.utils.ts diff --git a/src/__tests__/posthog-core.beforeCapture.test.ts b/src/__tests__/posthog-core.beforeCapture.test.ts index 232e760d8..90a129d0f 100644 --- a/src/__tests__/posthog-core.beforeCapture.test.ts +++ b/src/__tests__/posthog-core.beforeCapture.test.ts @@ -48,9 +48,14 @@ describe('posthog core - before capture', () => { beforeCapture: rejectingEventFn, }) ;(posthog._send_request as jest.Mock).mockClear() + const capturedData = posthog.capture(eventName, {}, {}) + expect(capturedData).toBeUndefined() expect(posthog._send_request).not.toHaveBeenCalled() + expect(jest.mocked(logger).info).toHaveBeenCalledWith( + `Event '${eventName}' was rejected in beforeCapture function` + ) }) it('can edit an event', () => { @@ -58,10 +63,11 @@ describe('posthog core - before capture', () => { beforeCapture: editingEventFn, }) ;(posthog._send_request as jest.Mock).mockClear() + const capturedData = posthog.capture(eventName, {}, {}) + expect(capturedData).toHaveProperty(['properties', 'edited'], true) expect(capturedData).toHaveProperty(['$set', 'edited'], true) - expect(posthog._send_request).toHaveBeenCalledWith({ batchKey: undefined, callback: expect.any(Function), @@ -112,8 +118,8 @@ describe('posthog core - before capture', () => { posthog.capture(randomUnsafeEditableEvent, {}, {}) - expect(jest.mocked(logger).info).toHaveBeenCalledWith( - `Event '${randomUnsafeEditableEvent}' was rejected. This can cause unexpected behavior.` + expect(jest.mocked(logger).warn).toHaveBeenCalledWith( + `Event '${randomUnsafeEditableEvent}' was rejected in beforeCapture function. This can cause unexpected behavior.` ) }) }) diff --git a/src/__tests__/utils/before-capture-utils.test.ts b/src/__tests__/utils/before-capture-utils.test.ts new file mode 100644 index 000000000..97105635a --- /dev/null +++ b/src/__tests__/utils/before-capture-utils.test.ts @@ -0,0 +1,57 @@ +import { sampleByDistinctId, sampleByEvent, sampleBySessionId } from '../../utils/before-capture.utils' +import { CaptureResult } from '../../types' +import { isNull } from '../../utils/type-utils' + +function expectRoughlyFiftyPercent(emittedEvents: any[]) { + expect(emittedEvents.length).toBeGreaterThanOrEqual(40) + expect(emittedEvents.length).toBeLessThanOrEqual(60) +} + +describe('before capture utils', () => { + it('can sample by event name', () => { + const sampleFn = sampleByEvent(['$autocapture'], 50) + const results = [] + Array.from({ length: 100 }).forEach(() => { + const captureResult = { event: '$autocapture' } as unknown as CaptureResult + results.push(sampleFn(captureResult)) + }) + const emittedEvents = results.filter((r) => !isNull(r)) + expectRoughlyFiftyPercent(emittedEvents) + }) + + it('can sample by distinct id', () => { + const sampleFn = sampleByDistinctId(50) + const results = [] + const distinct_id_one = 'user-1' + const distinct_id_two = 'user-that-hashes-to-no-events' + Array.from({ length: 100 }).forEach(() => { + ;[distinct_id_one, distinct_id_two].forEach((distinct_id) => { + const captureResult = { properties: { distinct_id } } as unknown as CaptureResult + results.push(sampleFn(captureResult)) + }) + }) + const distinctIdOneEvents = results.filter((r) => !isNull(r) && r.properties.distinct_id === distinct_id_one) + const distinctIdTwoEvents = results.filter((r) => !isNull(r) && r.properties.distinct_id === distinct_id_two) + + expect(distinctIdOneEvents.length).toBe(100) + expect(distinctIdTwoEvents.length).toBe(0) + }) + + it('can sample by session id', () => { + const sampleFn = sampleBySessionId(50) + const results = [] + const session_id_one = 'a-session-id' + const session_id_two = 'id-that-hashes-to-not-sending-events' + Array.from({ length: 100 }).forEach(() => { + ;[session_id_one, session_id_two].forEach((session_id) => { + const captureResult = { properties: { $session_id: session_id } } as unknown as CaptureResult + results.push(sampleFn(captureResult)) + }) + }) + const sessionIdOneEvents = results.filter((r) => !isNull(r) && r.properties.$session_id === session_id_one) + const sessionIdTwoEvents = results.filter((r) => !isNull(r) && r.properties.$session_id === session_id_two) + + expect(sessionIdOneEvents.length).toBe(100) + expect(sessionIdTwoEvents.length).toBe(0) + }) +}) diff --git a/src/posthog-core.ts b/src/posthog-core.ts index 8ea8b5e5e..c1f14d394 100644 --- a/src/posthog-core.ts +++ b/src/posthog-core.ts @@ -883,8 +883,11 @@ export class PostHog { if (!isKnownUnEditableEvent(data.event)) { const beforeCaptureResult = this.config.beforeCapture(data) if (isNullish(beforeCaptureResult)) { + const logMessage = `Event '${data.event}' was rejected in beforeCapture function` if (isKnownUnsafeEditableEvent(data.event)) { - logger.info(`Event '${data.event}' was rejected. This can cause unexpected behavior.`) + logger.warn(`${logMessage}. This can cause unexpected behavior.`) + } else { + logger.info(logMessage) } return } else { diff --git a/src/types.ts b/src/types.ts index 01bcdde41..6e5c5f830 100644 --- a/src/types.ts +++ b/src/types.ts @@ -41,9 +41,11 @@ export const knownUnsafeEditableEvent = [ export type KnownUnsafeEditableEvent = typeof knownUnsafeEditableEvent[number] /** - * These known events can be processed by the `beforeCapture` function + * These are known events PostHog events that can be processed by the `beforeCapture` function + * That means PostHog functionality does not rely on receiving 100% of these for calculations + * So, it is safe to sample them to reduce the volume of events sent to PostHog */ -type KnownEventName = +export type KnownEventName = | '$heatmaps_data' | '$opt_in' | '$exception' diff --git a/src/utils/before-capture.utils.ts b/src/utils/before-capture.utils.ts new file mode 100644 index 000000000..52074b9c2 --- /dev/null +++ b/src/utils/before-capture.utils.ts @@ -0,0 +1,64 @@ +import { clampToRange } from './number-utils' +import { CaptureResult, KnownEventName } from '../types' +import { includes } from './index' + +function simpleHash(str: string) { + let hash = 0 + for (let i = 0; i < str.length; i++) { + hash = (hash << 5) - hash + str.charCodeAt(i) // (hash * 31) + char code + hash |= 0 // Convert to 32bit integer + } + return Math.abs(hash) +} + +/** + * An implementation of sampling that samples based on the distinct ID. + * Can be used to create a beforeCapture fn for a PostHog instance. + * + * Causes roughly 50% of distinct ids to have events sent. + * Not 50% of events for each distinct id. + * + * @param percent a number from 0 to 100, 100 means never sample, 0 means never send the event + */ +export function sampleByDistinctId(percent: number): (c: CaptureResult) => CaptureResult | null { + return (captureResult: CaptureResult): CaptureResult | null => { + const hash = simpleHash(captureResult.properties.distinct_id) + return hash % 100 < clampToRange(percent, 0, 100) ? captureResult : null + } +} + +/** + * An implementation of sampling that samples based on the session ID. + * Can be used to create a beforeCapture fn for a PostHog instance. + * + * Causes roughly 50% of sessions to have events sent. + * Not 50% of events for each session. + * + * @param percent a number from 0 to 100, 100 means never sample, 0 means never send the event + */ +export function sampleBySessionId(percent: number): (c: CaptureResult) => CaptureResult | null { + return (captureResult: CaptureResult): CaptureResult | null => { + const hash = simpleHash(captureResult.properties.$session_id) + return hash % 100 < clampToRange(percent, 0, 100) ? captureResult : null + } +} + +/** + * An implementation of sampling that samples based on the event name. + * Can be used to create a beforeCapture fn for a PostHog instance. + * + * @param eventNames an array of event names to sample, sampling is applied across events not per event name + * @param percent a number from 0 to 100, 100 means never sample, 0 means never send the event + */ +export function sampleByEvent( + eventNames: KnownEventName[], + percent: number +): (c: CaptureResult) => CaptureResult | null { + return (captureResult: CaptureResult): CaptureResult | null => { + if (!includes(eventNames, captureResult.event)) { + return captureResult + } + + return Math.random() * 100 < clampToRange(percent, 0, 100) ? captureResult : null + } +} From 070413dd12415222222a4471a63d3b53a6d06f41 Mon Sep 17 00:00:00 2001 From: Paul D'Ambra Date: Sun, 10 Nov 2024 12:35:56 +0000 Subject: [PATCH 07/28] Refactor --- src/utils/before-capture.utils.ts | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/src/utils/before-capture.utils.ts b/src/utils/before-capture.utils.ts index 52074b9c2..097092e1d 100644 --- a/src/utils/before-capture.utils.ts +++ b/src/utils/before-capture.utils.ts @@ -11,8 +11,13 @@ function simpleHash(str: string) { return Math.abs(hash) } +function sampleOnProperty(prop: string, percent: number): boolean { + return simpleHash(prop) % 100 < clampToRange(percent, 0, 100) +} + /** - * An implementation of sampling that samples based on the distinct ID. + * Provides an implementation of sampling that samples based on the distinct ID. + * Using the provided percentage. * Can be used to create a beforeCapture fn for a PostHog instance. * * Causes roughly 50% of distinct ids to have events sent. @@ -22,13 +27,13 @@ function simpleHash(str: string) { */ export function sampleByDistinctId(percent: number): (c: CaptureResult) => CaptureResult | null { return (captureResult: CaptureResult): CaptureResult | null => { - const hash = simpleHash(captureResult.properties.distinct_id) - return hash % 100 < clampToRange(percent, 0, 100) ? captureResult : null + return sampleOnProperty(captureResult.properties.distinct_id, percent) ? captureResult : null } } /** - * An implementation of sampling that samples based on the session ID. + * Provides an implementation of sampling that samples based on the session ID. + * Using the provided percentage. * Can be used to create a beforeCapture fn for a PostHog instance. * * Causes roughly 50% of sessions to have events sent. @@ -38,13 +43,13 @@ export function sampleByDistinctId(percent: number): (c: CaptureResult) => Captu */ export function sampleBySessionId(percent: number): (c: CaptureResult) => CaptureResult | null { return (captureResult: CaptureResult): CaptureResult | null => { - const hash = simpleHash(captureResult.properties.$session_id) - return hash % 100 < clampToRange(percent, 0, 100) ? captureResult : null + return sampleOnProperty(captureResult.properties.$session_id, percent) ? captureResult : null } } /** - * An implementation of sampling that samples based on the event name. + * Provides an implementation of sampling that samples based on the event name. + * Using the provided percentage. * Can be used to create a beforeCapture fn for a PostHog instance. * * @param eventNames an array of event names to sample, sampling is applied across events not per event name From 2d7b7ed81991eeb61d55b5e2a6ce548e2b1fcf95 Mon Sep 17 00:00:00 2001 From: Paul D'Ambra Date: Mon, 11 Nov 2024 09:11:49 +0000 Subject: [PATCH 08/28] mock random --- .../utils/before-capture-utils.test.ts | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/src/__tests__/utils/before-capture-utils.test.ts b/src/__tests__/utils/before-capture-utils.test.ts index 97105635a..1d6795cdd 100644 --- a/src/__tests__/utils/before-capture-utils.test.ts +++ b/src/__tests__/utils/before-capture-utils.test.ts @@ -2,21 +2,28 @@ import { sampleByDistinctId, sampleByEvent, sampleBySessionId } from '../../util import { CaptureResult } from '../../types' import { isNull } from '../../utils/type-utils' -function expectRoughlyFiftyPercent(emittedEvents: any[]) { - expect(emittedEvents.length).toBeGreaterThanOrEqual(40) - expect(emittedEvents.length).toBeLessThanOrEqual(60) -} +beforeAll(() => { + let fiftyFiftyRandom = true + Math.random = () => { + const val = fiftyFiftyRandom ? 0.48 : 0.51 + fiftyFiftyRandom = !fiftyFiftyRandom + return val + } +}) describe('before capture utils', () => { it('can sample by event name', () => { const sampleFn = sampleByEvent(['$autocapture'], 50) + const results = [] Array.from({ length: 100 }).forEach(() => { const captureResult = { event: '$autocapture' } as unknown as CaptureResult results.push(sampleFn(captureResult)) }) const emittedEvents = results.filter((r) => !isNull(r)) - expectRoughlyFiftyPercent(emittedEvents) + + // random is mocked so that it alternates between 0.48 and 0.51 + expect(emittedEvents.length).toBe(50) }) it('can sample by distinct id', () => { From 4940a23b226ec457790412ab81ff6129a0cf44db Mon Sep 17 00:00:00 2001 From: Paul D'Ambra Date: Mon, 11 Nov 2024 09:13:29 +0000 Subject: [PATCH 09/28] more events are uneditable --- src/types.ts | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/src/types.ts b/src/types.ts index 6e5c5f830..1a5e8421c 100644 --- a/src/types.ts +++ b/src/types.ts @@ -16,6 +16,10 @@ export const knownUnEditableEvent = [ 'survey sent', 'survey shown', '$snapshot', + '$identify', + '$set', + '$groupidentify', + '$create_alias', ] as const /** @@ -23,14 +27,7 @@ export const knownUnEditableEvent = [ */ export type KnownUnEditableEvent = typeof knownUnEditableEvent[number] -export const knownUnsafeEditableEvent = [ - '$pageview', - '$pageleave', - '$identify', - '$set', - '$groupidentify', - '$create_alias', -] as const +export const knownUnsafeEditableEvent = ['$pageview', '$pageleave'] as const /** * These events can be processed by the `beforeCapture` function From 4bac47608c3df22eac0e1c34af28a3e7d7449384 Mon Sep 17 00:00:00 2001 From: Paul D'Ambra Date: Mon, 11 Nov 2024 11:10:04 +0000 Subject: [PATCH 10/28] rename the function --- ...test.ts => posthog-core.beforeSend.test.ts} | 18 +++++++++--------- ...utils.test.ts => before-send-utils.test.ts} | 2 +- src/posthog-core.ts | 10 +++++----- src/types.ts | 6 +++++- 4 files changed, 20 insertions(+), 16 deletions(-) rename src/__tests__/{posthog-core.beforeCapture.test.ts => posthog-core.beforeSend.test.ts} (89%) rename src/__tests__/utils/{before-capture-utils.test.ts => before-send-utils.test.ts} (98%) diff --git a/src/__tests__/posthog-core.beforeCapture.test.ts b/src/__tests__/posthog-core.beforeSend.test.ts similarity index 89% rename from src/__tests__/posthog-core.beforeCapture.test.ts rename to src/__tests__/posthog-core.beforeSend.test.ts index 90a129d0f..da4497184 100644 --- a/src/__tests__/posthog-core.beforeCapture.test.ts +++ b/src/__tests__/posthog-core.beforeSend.test.ts @@ -24,11 +24,11 @@ const editingEventFn = (captureResult: CaptureResult): CaptureResult => { } } -describe('posthog core - before capture', () => { +describe('posthog core - before send', () => { const baseUTCDateTime = new Date(Date.UTC(2020, 0, 1, 0, 0, 0)) const eventName = '$event' - const posthogWith = (configOverride: Pick, 'beforeCapture'>): PostHog => { + const posthogWith = (configOverride: Pick, 'beforeSend'>): PostHog => { const posthog = defaultPostHog().init('testtoken', configOverride, uuidv7()) return Object.assign(posthog, { _send_request: jest.fn(), @@ -45,7 +45,7 @@ describe('posthog core - before capture', () => { it('can reject an event', () => { const posthog = posthogWith({ - beforeCapture: rejectingEventFn, + beforeSend: rejectingEventFn, }) ;(posthog._send_request as jest.Mock).mockClear() @@ -54,13 +54,13 @@ describe('posthog core - before capture', () => { expect(capturedData).toBeUndefined() expect(posthog._send_request).not.toHaveBeenCalled() expect(jest.mocked(logger).info).toHaveBeenCalledWith( - `Event '${eventName}' was rejected in beforeCapture function` + `Event '${eventName}' was rejected in beforeSend function` ) }) it('can edit an event', () => { const posthog = posthogWith({ - beforeCapture: editingEventFn, + beforeSend: editingEventFn, }) ;(posthog._send_request as jest.Mock).mockClear() @@ -80,7 +80,7 @@ describe('posthog core - before capture', () => { it('cannot reject an un-editable event', () => { const posthog = posthogWith({ - beforeCapture: rejectingEventFn, + beforeSend: rejectingEventFn, }) ;(posthog._send_request as jest.Mock).mockClear() // chooses a random string from knownUnEditableEvent @@ -94,7 +94,7 @@ describe('posthog core - before capture', () => { it('cannot edit an un-editable event', () => { const posthog = posthogWith({ - beforeCapture: editingEventFn, + beforeSend: editingEventFn, }) ;(posthog._send_request as jest.Mock).mockClear() // chooses a random string from knownUnEditableEvent @@ -109,7 +109,7 @@ describe('posthog core - before capture', () => { it('logs a warning when rejecting an unsafe to edit event', () => { const posthog = posthogWith({ - beforeCapture: rejectingEventFn, + beforeSend: rejectingEventFn, }) ;(posthog._send_request as jest.Mock).mockClear() // chooses a random string from knownUnEditableEvent @@ -119,7 +119,7 @@ describe('posthog core - before capture', () => { posthog.capture(randomUnsafeEditableEvent, {}, {}) expect(jest.mocked(logger).warn).toHaveBeenCalledWith( - `Event '${randomUnsafeEditableEvent}' was rejected in beforeCapture function. This can cause unexpected behavior.` + `Event '${randomUnsafeEditableEvent}' was rejected in beforeSend function. This can cause unexpected behavior.` ) }) }) diff --git a/src/__tests__/utils/before-capture-utils.test.ts b/src/__tests__/utils/before-send-utils.test.ts similarity index 98% rename from src/__tests__/utils/before-capture-utils.test.ts rename to src/__tests__/utils/before-send-utils.test.ts index 1d6795cdd..ebf4370dc 100644 --- a/src/__tests__/utils/before-capture-utils.test.ts +++ b/src/__tests__/utils/before-send-utils.test.ts @@ -11,7 +11,7 @@ beforeAll(() => { } }) -describe('before capture utils', () => { +describe('before send utils', () => { it('can sample by event name', () => { const sampleFn = sampleByEvent(['$autocapture'], 50) diff --git a/src/posthog-core.ts b/src/posthog-core.ts index c1f14d394..8f6353a90 100644 --- a/src/posthog-core.ts +++ b/src/posthog-core.ts @@ -186,7 +186,7 @@ export const defaultConfig = (): PostHogConfig => ({ person_profiles: 'identified_only', __add_tracing_headers: false, // by default the before capture fn is the identity function - beforeCapture: (x) => x, + beforeSend: (x) => x, }) export const configRenames = (origConfig: Partial): Partial => { @@ -881,9 +881,9 @@ export class PostHog { } if (!isKnownUnEditableEvent(data.event)) { - const beforeCaptureResult = this.config.beforeCapture(data) - if (isNullish(beforeCaptureResult)) { - const logMessage = `Event '${data.event}' was rejected in beforeCapture function` + const beforeSendResult = this.config.beforeSend(data) + if (isNullish(beforeSendResult)) { + const logMessage = `Event '${data.event}' was rejected in beforeSend function` if (isKnownUnsafeEditableEvent(data.event)) { logger.warn(`${logMessage}. This can cause unexpected behavior.`) } else { @@ -891,7 +891,7 @@ export class PostHog { } return } else { - data = beforeCaptureResult + data = beforeSendResult } } diff --git a/src/types.ts b/src/types.ts index 1a5e8421c..869ae630b 100644 --- a/src/types.ts +++ b/src/types.ts @@ -274,7 +274,11 @@ export interface PostHogConfig { * this is a read-only function that can be used to react to event capture */ _onCapture: (eventName: string, eventData: CaptureResult) => void - beforeCapture: (cr: CaptureResult) => CaptureResult | null + /** + * This function - if provided - is called immediately before sending data to the server. + * It allows you to edit data before it is sent, or choose not to send it all. + */ + beforeSend: (cr: CaptureResult) => CaptureResult | null capture_performance?: boolean | PerformanceCaptureConfig // Should only be used for testing. Could negatively impact performance. disable_compression: boolean From 0efff91de2c3c673a98cfc3a53af91a4dbef478a Mon Sep 17 00:00:00 2001 From: Paul D'Ambra Date: Mon, 11 Nov 2024 11:20:24 +0000 Subject: [PATCH 11/28] failing events --- src/__tests__/posthog-core.beforeSend.test.ts | 66 +++++++++++++++++++ src/types.ts | 3 +- 2 files changed, 67 insertions(+), 2 deletions(-) diff --git a/src/__tests__/posthog-core.beforeSend.test.ts b/src/__tests__/posthog-core.beforeSend.test.ts index da4497184..318ee43cb 100644 --- a/src/__tests__/posthog-core.beforeSend.test.ts +++ b/src/__tests__/posthog-core.beforeSend.test.ts @@ -92,6 +92,72 @@ describe('posthog core - before send', () => { expect(posthog._send_request).toHaveBeenCalled() }) + it('can sanitize $set event', () => { + const posthog = posthogWith({ + beforeSend: (cr) => { + cr.$set = { value: 'edited' } + return cr + }, + }) + ;(posthog._send_request as jest.Mock).mockClear() + + const capturedData = posthog.capture('$set', {}, { $set: { value: 'provided' } }) + + expect(capturedData).toHaveProperty(['$set', 'value'], 'edited') + expect(posthog._send_request).toHaveBeenCalledWith({ + batchKey: undefined, + callback: expect.any(Function), + compression: 'best-available', + data: capturedData, + method: 'POST', + url: 'https://us.i.posthog.com/e/', + }) + }) + + it('cannot make $set event invalid', () => { + const posthog = posthogWith({ + beforeSend: (cr) => { + cr.$set = undefined + return cr + }, + }) + ;(posthog._send_request as jest.Mock).mockClear() + + const capturedData = posthog.capture('$set', {}, { $set: { value: 'provided' } }) + + expect(capturedData).toHaveProperty(['$set', 'value'], 'provided') + expect(posthog._send_request).toHaveBeenCalledWith({ + batchKey: undefined, + callback: expect.any(Function), + compression: 'best-available', + data: capturedData, + method: 'POST', + url: 'https://us.i.posthog.com/e/', + }) + }) + + it('cannot make arbitrary event invalid', () => { + const posthog = posthogWith({ + beforeSend: (cr) => { + cr.properties = undefined + return cr + }, + }) + ;(posthog._send_request as jest.Mock).mockClear() + + const capturedData = posthog.capture(eventName, { value: 'provided' }, {}) + + expect(capturedData).toHaveProperty(['properties', 'value'], 'provided') + expect(posthog._send_request).toHaveBeenCalledWith({ + batchKey: undefined, + callback: expect.any(Function), + compression: 'best-available', + data: capturedData, + method: 'POST', + url: 'https://us.i.posthog.com/e/', + }) + }) + it('cannot edit an un-editable event', () => { const posthog = posthogWith({ beforeSend: editingEventFn, diff --git a/src/types.ts b/src/types.ts index 869ae630b..24fd631d5 100644 --- a/src/types.ts +++ b/src/types.ts @@ -17,7 +17,6 @@ export const knownUnEditableEvent = [ 'survey shown', '$snapshot', '$identify', - '$set', '$groupidentify', '$create_alias', ] as const @@ -27,7 +26,7 @@ export const knownUnEditableEvent = [ */ export type KnownUnEditableEvent = typeof knownUnEditableEvent[number] -export const knownUnsafeEditableEvent = ['$pageview', '$pageleave'] as const +export const knownUnsafeEditableEvent = ['$pageview', '$pageleave', '$set'] as const /** * These events can be processed by the `beforeCapture` function From 31244a1bdd9a3e355557f43d1d3fee39749c46fa Mon Sep 17 00:00:00 2001 From: Paul D'Ambra Date: Mon, 11 Nov 2024 14:36:02 +0000 Subject: [PATCH 12/28] reorg things --- src/__tests__/utils/before-send-utils.test.ts | 2 +- .../before-capture.utils.ts => customizations/before-send.ts} | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) rename src/{utils/before-capture.utils.ts => customizations/before-send.ts} (96%) diff --git a/src/__tests__/utils/before-send-utils.test.ts b/src/__tests__/utils/before-send-utils.test.ts index ebf4370dc..f2a1c1d07 100644 --- a/src/__tests__/utils/before-send-utils.test.ts +++ b/src/__tests__/utils/before-send-utils.test.ts @@ -1,6 +1,6 @@ -import { sampleByDistinctId, sampleByEvent, sampleBySessionId } from '../../utils/before-capture.utils' import { CaptureResult } from '../../types' import { isNull } from '../../utils/type-utils' +import { sampleByDistinctId, sampleByEvent, sampleBySessionId } from '../../customizations/before-send' beforeAll(() => { let fiftyFiftyRandom = true diff --git a/src/utils/before-capture.utils.ts b/src/customizations/before-send.ts similarity index 96% rename from src/utils/before-capture.utils.ts rename to src/customizations/before-send.ts index 097092e1d..dd935e8a2 100644 --- a/src/utils/before-capture.utils.ts +++ b/src/customizations/before-send.ts @@ -1,6 +1,6 @@ -import { clampToRange } from './number-utils' +import { clampToRange } from '../utils/number-utils' import { CaptureResult, KnownEventName } from '../types' -import { includes } from './index' +import { includes } from '../utils' function simpleHash(str: string) { let hash = 0 From db0228b7fab1bc89c9d33e803bf3529252854a24 Mon Sep 17 00:00:00 2001 From: Paul D'Ambra Date: Mon, 11 Nov 2024 14:38:07 +0000 Subject: [PATCH 13/28] test doesn't make sense undefined is valid for --- src/__tests__/posthog-core.beforeSend.test.ts | 22 ------------------- 1 file changed, 22 deletions(-) diff --git a/src/__tests__/posthog-core.beforeSend.test.ts b/src/__tests__/posthog-core.beforeSend.test.ts index 318ee43cb..f7b2514cf 100644 --- a/src/__tests__/posthog-core.beforeSend.test.ts +++ b/src/__tests__/posthog-core.beforeSend.test.ts @@ -114,28 +114,6 @@ describe('posthog core - before send', () => { }) }) - it('cannot make $set event invalid', () => { - const posthog = posthogWith({ - beforeSend: (cr) => { - cr.$set = undefined - return cr - }, - }) - ;(posthog._send_request as jest.Mock).mockClear() - - const capturedData = posthog.capture('$set', {}, { $set: { value: 'provided' } }) - - expect(capturedData).toHaveProperty(['$set', 'value'], 'provided') - expect(posthog._send_request).toHaveBeenCalledWith({ - batchKey: undefined, - callback: expect.any(Function), - compression: 'best-available', - data: capturedData, - method: 'POST', - url: 'https://us.i.posthog.com/e/', - }) - }) - it('cannot make arbitrary event invalid', () => { const posthog = posthogWith({ beforeSend: (cr) => { From d20d0b7a533e84b7021491527d2b23367a591459 Mon Sep 17 00:00:00 2001 From: Paul D'Ambra Date: Mon, 11 Nov 2024 14:41:43 +0000 Subject: [PATCH 14/28] maybe just a warning --- src/__tests__/posthog-core.beforeSend.test.ts | 7 +++++-- src/posthog-core.ts | 5 +++++ 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/src/__tests__/posthog-core.beforeSend.test.ts b/src/__tests__/posthog-core.beforeSend.test.ts index f7b2514cf..e8cb88be5 100644 --- a/src/__tests__/posthog-core.beforeSend.test.ts +++ b/src/__tests__/posthog-core.beforeSend.test.ts @@ -114,7 +114,7 @@ describe('posthog core - before send', () => { }) }) - it('cannot make arbitrary event invalid', () => { + it('warned when making arbitrary event invalid', () => { const posthog = posthogWith({ beforeSend: (cr) => { cr.properties = undefined @@ -125,7 +125,7 @@ describe('posthog core - before send', () => { const capturedData = posthog.capture(eventName, { value: 'provided' }, {}) - expect(capturedData).toHaveProperty(['properties', 'value'], 'provided') + expect(capturedData).not.toHaveProperty(['properties', 'value'], 'provided') expect(posthog._send_request).toHaveBeenCalledWith({ batchKey: undefined, callback: expect.any(Function), @@ -134,6 +134,9 @@ describe('posthog core - before send', () => { method: 'POST', url: 'https://us.i.posthog.com/e/', }) + expect(jest.mocked(logger).warn).toHaveBeenCalledWith( + `Event '${eventName}' has no properties after beforeSend function, this is likely an error.` + ) }) it('cannot edit an un-editable event', () => { diff --git a/src/posthog-core.ts b/src/posthog-core.ts index 8f6353a90..8787efca1 100644 --- a/src/posthog-core.ts +++ b/src/posthog-core.ts @@ -891,6 +891,11 @@ export class PostHog { } return } else { + if (!beforeSendResult.properties || isEmptyObject(beforeSendResult.properties)) { + logger.warn( + `Event '${data.event}' has no properties after beforeSend function, this is likely an error.` + ) + } data = beforeSendResult } } From 56fb95557465e490e4d67258f40a5765dc7a711c Mon Sep 17 00:00:00 2001 From: Paul D'Ambra Date: Tue, 12 Nov 2024 08:37:14 +0000 Subject: [PATCH 15/28] some more can be unsafe --- src/__tests__/posthog-core.beforeSend.test.ts | 16 +++++++------- src/posthog-core.ts | 4 ++-- src/types.ts | 22 +++++++++++-------- 3 files changed, 23 insertions(+), 19 deletions(-) diff --git a/src/__tests__/posthog-core.beforeSend.test.ts b/src/__tests__/posthog-core.beforeSend.test.ts index e8cb88be5..7032dbb92 100644 --- a/src/__tests__/posthog-core.beforeSend.test.ts +++ b/src/__tests__/posthog-core.beforeSend.test.ts @@ -28,7 +28,7 @@ describe('posthog core - before send', () => { const baseUTCDateTime = new Date(Date.UTC(2020, 0, 1, 0, 0, 0)) const eventName = '$event' - const posthogWith = (configOverride: Pick, 'beforeSend'>): PostHog => { + const posthogWith = (configOverride: Pick, 'before_send'>): PostHog => { const posthog = defaultPostHog().init('testtoken', configOverride, uuidv7()) return Object.assign(posthog, { _send_request: jest.fn(), @@ -45,7 +45,7 @@ describe('posthog core - before send', () => { it('can reject an event', () => { const posthog = posthogWith({ - beforeSend: rejectingEventFn, + before_send: rejectingEventFn, }) ;(posthog._send_request as jest.Mock).mockClear() @@ -60,7 +60,7 @@ describe('posthog core - before send', () => { it('can edit an event', () => { const posthog = posthogWith({ - beforeSend: editingEventFn, + before_send: editingEventFn, }) ;(posthog._send_request as jest.Mock).mockClear() @@ -80,7 +80,7 @@ describe('posthog core - before send', () => { it('cannot reject an un-editable event', () => { const posthog = posthogWith({ - beforeSend: rejectingEventFn, + before_send: rejectingEventFn, }) ;(posthog._send_request as jest.Mock).mockClear() // chooses a random string from knownUnEditableEvent @@ -94,7 +94,7 @@ describe('posthog core - before send', () => { it('can sanitize $set event', () => { const posthog = posthogWith({ - beforeSend: (cr) => { + before_send: (cr) => { cr.$set = { value: 'edited' } return cr }, @@ -116,7 +116,7 @@ describe('posthog core - before send', () => { it('warned when making arbitrary event invalid', () => { const posthog = posthogWith({ - beforeSend: (cr) => { + before_send: (cr) => { cr.properties = undefined return cr }, @@ -141,7 +141,7 @@ describe('posthog core - before send', () => { it('cannot edit an un-editable event', () => { const posthog = posthogWith({ - beforeSend: editingEventFn, + before_send: editingEventFn, }) ;(posthog._send_request as jest.Mock).mockClear() // chooses a random string from knownUnEditableEvent @@ -156,7 +156,7 @@ describe('posthog core - before send', () => { it('logs a warning when rejecting an unsafe to edit event', () => { const posthog = posthogWith({ - beforeSend: rejectingEventFn, + before_send: rejectingEventFn, }) ;(posthog._send_request as jest.Mock).mockClear() // chooses a random string from knownUnEditableEvent diff --git a/src/posthog-core.ts b/src/posthog-core.ts index 8787efca1..cf47ccaf8 100644 --- a/src/posthog-core.ts +++ b/src/posthog-core.ts @@ -186,7 +186,7 @@ export const defaultConfig = (): PostHogConfig => ({ person_profiles: 'identified_only', __add_tracing_headers: false, // by default the before capture fn is the identity function - beforeSend: (x) => x, + before_send: (x) => x, }) export const configRenames = (origConfig: Partial): Partial => { @@ -881,7 +881,7 @@ export class PostHog { } if (!isKnownUnEditableEvent(data.event)) { - const beforeSendResult = this.config.beforeSend(data) + const beforeSendResult = this.config.before_send(data) if (isNullish(beforeSendResult)) { const logMessage = `Event '${data.event}' was rejected in beforeSend function` if (isKnownUnsafeEditableEvent(data.event)) { diff --git a/src/types.ts b/src/types.ts index 24fd631d5..01b694992 100644 --- a/src/types.ts +++ b/src/types.ts @@ -9,16 +9,9 @@ export const COPY_AUTOCAPTURE_EVENT = '$copy_autocapture' export const knownUnEditableEvent = [ '$web_experiment_applied', - '$$client_ingestion_warning', '$feature_enrollment_update', '$feature_flag_called', - 'survey dismissed', - 'survey sent', - 'survey shown', '$snapshot', - '$identify', - '$groupidentify', - '$create_alias', ] as const /** @@ -26,7 +19,18 @@ export const knownUnEditableEvent = [ */ export type KnownUnEditableEvent = typeof knownUnEditableEvent[number] -export const knownUnsafeEditableEvent = ['$pageview', '$pageleave', '$set'] as const +export const knownUnsafeEditableEvent = [ + '$pageview', + '$pageleave', + '$set', + 'survey dismissed', + 'survey sent', + 'survey shown', + '$identify', + '$groupidentify', + '$create_alias', + '$$client_ingestion_warning', +] as const /** * These events can be processed by the `beforeCapture` function @@ -277,7 +281,7 @@ export interface PostHogConfig { * This function - if provided - is called immediately before sending data to the server. * It allows you to edit data before it is sent, or choose not to send it all. */ - beforeSend: (cr: CaptureResult) => CaptureResult | null + before_send: (cr: CaptureResult) => CaptureResult | null capture_performance?: boolean | PerformanceCaptureConfig // Should only be used for testing. Could negatively impact performance. disable_compression: boolean From ba4f6ca7a636ebbac97586aae7f401fc41ffa014 Mon Sep 17 00:00:00 2001 From: Paul D'Ambra Date: Tue, 12 Nov 2024 14:58:52 +0000 Subject: [PATCH 16/28] this is unsafe but editable too --- src/types.ts | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/src/types.ts b/src/types.ts index 01b694992..4124bbf7f 100644 --- a/src/types.ts +++ b/src/types.ts @@ -7,12 +7,7 @@ export type Properties = Record export const COPY_AUTOCAPTURE_EVENT = '$copy_autocapture' -export const knownUnEditableEvent = [ - '$web_experiment_applied', - '$feature_enrollment_update', - '$feature_flag_called', - '$snapshot', -] as const +export const knownUnEditableEvent = ['$feature_enrollment_update', '$feature_flag_called', '$snapshot'] as const /** * These events are not processed by the `beforeCapture` function @@ -30,6 +25,7 @@ export const knownUnsafeEditableEvent = [ '$groupidentify', '$create_alias', '$$client_ingestion_warning', + '$web_experiment_applied', ] as const /** From 4f5b30885e23e5c4f27dd79e0b980426856f5f8f Mon Sep 17 00:00:00 2001 From: Paul D'Ambra Date: Tue, 12 Nov 2024 14:59:36 +0000 Subject: [PATCH 17/28] this is unsafe but editable too --- src/types.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/types.ts b/src/types.ts index 4124bbf7f..0b5cdd48c 100644 --- a/src/types.ts +++ b/src/types.ts @@ -7,7 +7,7 @@ export type Properties = Record export const COPY_AUTOCAPTURE_EVENT = '$copy_autocapture' -export const knownUnEditableEvent = ['$feature_enrollment_update', '$feature_flag_called', '$snapshot'] as const +export const knownUnEditableEvent = ['$snapshot'] as const /** * These events are not processed by the `beforeCapture` function @@ -26,6 +26,8 @@ export const knownUnsafeEditableEvent = [ '$create_alias', '$$client_ingestion_warning', '$web_experiment_applied', + '$feature_enrollment_update', + '$feature_flag_called', ] as const /** From 4afa6c86e62fca0f4ff7f34ad602b452a80566e3 Mon Sep 17 00:00:00 2001 From: Paul D'Ambra Date: Wed, 13 Nov 2024 11:36:04 +0000 Subject: [PATCH 18/28] ok, process it all :see-no-evil: --- src/posthog-core.ts | 32 +++++++++++++++----------------- src/types.ts | 12 +++--------- src/utils/type-utils.ts | 11 +---------- 3 files changed, 19 insertions(+), 36 deletions(-) diff --git a/src/posthog-core.ts b/src/posthog-core.ts index cf47ccaf8..25b0b18f6 100644 --- a/src/posthog-core.ts +++ b/src/posthog-core.ts @@ -59,7 +59,6 @@ import { isEmptyObject, isEmptyString, isFunction, - isKnownUnEditableEvent, isKnownUnsafeEditableEvent, isNullish, isNumber, @@ -880,24 +879,23 @@ export class PostHog { this.setPersonPropertiesForFlags(finalSet) } - if (!isKnownUnEditableEvent(data.event)) { - const beforeSendResult = this.config.before_send(data) - if (isNullish(beforeSendResult)) { - const logMessage = `Event '${data.event}' was rejected in beforeSend function` - if (isKnownUnsafeEditableEvent(data.event)) { - logger.warn(`${logMessage}. This can cause unexpected behavior.`) - } else { - logger.info(logMessage) - } - return + const beforeSendResult = this.config.before_send(data) + if (isNullish(beforeSendResult)) { + const logMessage = `Event '${data.event}' was rejected in beforeSend function` + if (isKnownUnsafeEditableEvent(data.event)) { + logger.warn(`${logMessage}. This can cause unexpected behavior.`) } else { - if (!beforeSendResult.properties || isEmptyObject(beforeSendResult.properties)) { - logger.warn( - `Event '${data.event}' has no properties after beforeSend function, this is likely an error.` - ) - } - data = beforeSendResult + logger.info(logMessage) + } + // the event is now null so we don't send it + return + } else { + if (!beforeSendResult.properties || isEmptyObject(beforeSendResult.properties)) { + logger.warn( + `Event '${data.event}' has no properties after beforeSend function, this is likely an error.` + ) } + data = beforeSendResult } this._internalEventEmitter.emit('eventCaptured', data) diff --git a/src/types.ts b/src/types.ts index 0b5cdd48c..7858a8af5 100644 --- a/src/types.ts +++ b/src/types.ts @@ -7,14 +7,8 @@ export type Properties = Record export const COPY_AUTOCAPTURE_EVENT = '$copy_autocapture' -export const knownUnEditableEvent = ['$snapshot'] as const - -/** - * These events are not processed by the `beforeCapture` function - */ -export type KnownUnEditableEvent = typeof knownUnEditableEvent[number] - export const knownUnsafeEditableEvent = [ + '$snapshot', '$pageview', '$pageleave', '$set', @@ -55,11 +49,11 @@ export type KnownEventName = | '$rageclick' export type EventName = - | KnownUnEditableEvent | KnownUnsafeEditableEvent | KnownEventName - // magic value so that the type of event name is a set of known strings or any other strings + // magic value so that the type of EventName is a set of known strings or any other string // which means you get autocomplete for known strings + // but no type complaints when you add an arbitrary string | (string & {}) export interface CaptureResult { diff --git a/src/utils/type-utils.ts b/src/utils/type-utils.ts index 63a9eee13..faf55adf7 100644 --- a/src/utils/type-utils.ts +++ b/src/utils/type-utils.ts @@ -1,10 +1,5 @@ import { includes } from '.' -import { - KnownUnEditableEvent, - knownUnEditableEvent, - KnownUnsafeEditableEvent, - knownUnsafeEditableEvent, -} from '../types' +import { knownUnsafeEditableEvent, KnownUnsafeEditableEvent } from '../types' // eslint-disable-next-line posthog-js/no-direct-array-check const nativeIsArray = Array.isArray @@ -100,7 +95,3 @@ export const isFile = (x: unknown): x is File => { export const isKnownUnsafeEditableEvent = (x: unknown): x is KnownUnsafeEditableEvent => { return includes(knownUnsafeEditableEvent as unknown as string[], x) } - -export const isKnownUnEditableEvent = (x: unknown): x is KnownUnEditableEvent => { - return includes(knownUnEditableEvent as unknown as string[], x as any) -} From 3c018f261b521552405b960282bc0f54aa931f4c Mon Sep 17 00:00:00 2001 From: Paul D'Ambra Date: Wed, 13 Nov 2024 11:37:01 +0000 Subject: [PATCH 19/28] Remove out-of-date tests --- src/__tests__/posthog-core.beforeSend.test.ts | 31 +------------------ 1 file changed, 1 insertion(+), 30 deletions(-) diff --git a/src/__tests__/posthog-core.beforeSend.test.ts b/src/__tests__/posthog-core.beforeSend.test.ts index 7032dbb92..0c58105d8 100644 --- a/src/__tests__/posthog-core.beforeSend.test.ts +++ b/src/__tests__/posthog-core.beforeSend.test.ts @@ -1,7 +1,7 @@ import { uuidv7 } from '../uuidv7' import { defaultPostHog } from './helpers/posthog-instance' import { logger } from '../utils/logger' -import { CaptureResult, knownUnEditableEvent, knownUnsafeEditableEvent, PostHogConfig } from '../types' +import { CaptureResult, knownUnsafeEditableEvent, PostHogConfig } from '../types' import { PostHog } from '../posthog-core' jest.mock('../utils/logger') @@ -78,20 +78,6 @@ describe('posthog core - before send', () => { }) }) - it('cannot reject an un-editable event', () => { - const posthog = posthogWith({ - before_send: rejectingEventFn, - }) - ;(posthog._send_request as jest.Mock).mockClear() - // chooses a random string from knownUnEditableEvent - const randomUneditableEvent = knownUnEditableEvent[Math.floor(Math.random() * knownUnEditableEvent.length)] - - const capturedData = posthog.capture(randomUneditableEvent, {}, {}) - - expect(capturedData).not.toBeUndefined() - expect(posthog._send_request).toHaveBeenCalled() - }) - it('can sanitize $set event', () => { const posthog = posthogWith({ before_send: (cr) => { @@ -139,21 +125,6 @@ describe('posthog core - before send', () => { ) }) - it('cannot edit an un-editable event', () => { - const posthog = posthogWith({ - before_send: editingEventFn, - }) - ;(posthog._send_request as jest.Mock).mockClear() - // chooses a random string from knownUnEditableEvent - const randomUneditableEvent = knownUnEditableEvent[Math.floor(Math.random() * knownUnEditableEvent.length)] - - const capturedData = posthog.capture(randomUneditableEvent, {}, {}) - - expect(capturedData).not.toHaveProperty(['properties', 'edited']) - expect(capturedData).not.toHaveProperty(['$set', 'edited']) - expect(posthog._send_request).toHaveBeenCalled() - }) - it('logs a warning when rejecting an unsafe to edit event', () => { const posthog = posthogWith({ before_send: rejectingEventFn, From 938a162bb1e93f0db68cd190747cc6262634389e Mon Sep 17 00:00:00 2001 From: Paul D'Ambra Date: Wed, 13 Nov 2024 14:31:44 +0000 Subject: [PATCH 20/28] deprecate and stop using _onCapture --- cypress/support/commands.ts | 6 +- playground/copy-autocapture/demo.html | 9 +- src/__tests__/autocapture.test.ts | 130 ++++---- src/__tests__/consent.test.ts | 74 ++--- .../exception-observer.test.ts | 16 +- src/__tests__/extensions/web-vitals.test.ts | 34 +- src/__tests__/heatmaps.test.ts | 35 +-- src/__tests__/identify.test.ts | 18 +- src/__tests__/personProcessing.test.ts | 290 +++++++++--------- src/__tests__/posthog-core.identify.test.ts | 50 +-- src/__tests__/posthog-core.test.ts | 48 +-- src/__tests__/posthog-core.ts | 7 +- src/posthog-core.ts | 3 +- src/types.ts | 1 + testcafe/helpers.js | 2 +- 15 files changed, 366 insertions(+), 357 deletions(-) diff --git a/cypress/support/commands.ts b/cypress/support/commands.ts index 11a219bb2..7bcaaa75f 100644 --- a/cypress/support/commands.ts +++ b/cypress/support/commands.ts @@ -9,9 +9,9 @@ Cypress.Commands.add('posthogInit', (options) => { cy.posthog().invoke('init', 'test_token', { api_host: location.origin, debug: true, - _onCapture: (event, eventData) => { - $captures.push(event) - $fullCaptures.push(eventData) + before_send: (event) => { + $captures.push(event.event) + $fullCaptures.push(event) }, opt_out_useragent_filter: true, ...options, diff --git a/playground/copy-autocapture/demo.html b/playground/copy-autocapture/demo.html index 52f81cb5a..3afd12a59 100644 --- a/playground/copy-autocapture/demo.html +++ b/playground/copy-autocapture/demo.html @@ -10,13 +10,14 @@ loaded: function(posthog) { posthog.identify('test') }, - _onCapture: (event, eventData) => { - if (event === '$copy_autocapture') { - const selectionType = eventData.properties['$copy_type'] - const selectionText = eventData.properties['$selected_content'] + before_send: (event) => { + if (event.event === '$copy_autocapture') { + const selectionType = event.properties['$copy_type'] + const selectionText = event.properties['$selected_content'] document.getElementById('selection-type-outlet').innerText = selectionType document.getElementById('selection-text-outlet').innerText = selectionText } + return event }, }) diff --git a/src/__tests__/autocapture.test.ts b/src/__tests__/autocapture.test.ts index 25d420fa2..1cf4ebb2d 100644 --- a/src/__tests__/autocapture.test.ts +++ b/src/__tests__/autocapture.test.ts @@ -63,7 +63,7 @@ describe('Autocapture system', () => { let autocapture: Autocapture let posthog: PostHog - let captureMock: jest.Mock + let beforeSendMock: jest.Mock beforeEach(async () => { jest.spyOn(window!.console, 'log').mockImplementation() @@ -76,13 +76,13 @@ describe('Autocapture system', () => { value: new URL('https://example.com'), }) - captureMock = jest.fn() + beforeSendMock = jest.fn().mockImplementation((...args) => args) posthog = await createPosthogInstance(uuidv7(), { api_host: 'https://test.com', token: 'testtoken', autocapture: true, - _onCapture: captureMock, + before_send: beforeSendMock, }) if (isUndefined(posthog.autocapture)) { @@ -400,8 +400,7 @@ describe('Autocapture system', () => { autocapture['_captureEvent'](fakeEvent) autocapture['_captureEvent'](fakeEvent) - expect(captureMock).toHaveBeenCalledTimes(4) - expect(captureMock.mock.calls.map((args) => args[0])).toEqual([ + expect(beforeSendMock.mock.calls.map((args) => args[0].event)).toEqual([ '$autocapture', '$autocapture', '$rageclick', @@ -428,10 +427,11 @@ describe('Autocapture system', () => { autocapture['_captureEvent'](fakeEvent, '$copy_autocapture') - expect(captureMock).toHaveBeenCalledTimes(1) - expect(captureMock.mock.calls[0][0]).toEqual('$copy_autocapture') - expect(captureMock.mock.calls[0][1].properties).toHaveProperty('$selected_content', 'copy this test') - expect(captureMock.mock.calls[0][1].properties).toHaveProperty('$copy_type', 'copy') + expect(beforeSendMock).toHaveBeenCalledTimes(1) + const mockCall = beforeSendMock.mock.calls[0][0] + expect(mockCall.event).toEqual('$copy_autocapture') + expect(mockCall.properties).toHaveProperty('$selected_content', 'copy this test') + expect(mockCall.properties).toHaveProperty('$copy_type', 'copy') }) it('should capture cut', () => { @@ -443,11 +443,11 @@ describe('Autocapture system', () => { autocapture['_captureEvent'](fakeEvent, '$copy_autocapture') - const spyArgs = captureMock.mock.calls + const spyArgs = beforeSendMock.mock.calls expect(spyArgs.length).toBe(1) - expect(spyArgs[0][0]).toEqual('$copy_autocapture') - expect(spyArgs[0][1].properties).toHaveProperty('$selected_content', 'cut this test') - expect(spyArgs[0][1].properties).toHaveProperty('$copy_type', 'cut') + expect(spyArgs[0][0].event).toEqual('$copy_autocapture') + expect(spyArgs[0][0].properties).toHaveProperty('$selected_content', 'cut this test') + expect(spyArgs[0][0].properties).toHaveProperty('$copy_type', 'cut') }) it('ignores empty selection', () => { @@ -459,7 +459,7 @@ describe('Autocapture system', () => { autocapture['_captureEvent'](fakeEvent, '$copy_autocapture') - const spyArgs = captureMock.mock.calls + const spyArgs = beforeSendMock.mock.calls expect(spyArgs.length).toBe(0) }) @@ -473,7 +473,7 @@ describe('Autocapture system', () => { autocapture['_captureEvent'](fakeEvent, '$copy_autocapture') - const spyArgs = captureMock.mock.calls + const spyArgs = beforeSendMock.mock.calls expect(spyArgs.length).toBe(0) }) }) @@ -495,7 +495,7 @@ describe('Autocapture system', () => { Object.setPrototypeOf(fakeEvent, MouseEvent.prototype) autocapture['_captureEvent'](fakeEvent) - const captureProperties = captureMock.mock.calls[0][1].properties + const captureProperties = beforeSendMock.mock.calls[0][0].properties expect(captureProperties).toHaveProperty('target-augment', 'the target') expect(captureProperties).toHaveProperty('parent-augment', 'the parent') }) @@ -517,10 +517,10 @@ describe('Autocapture system', () => { document.body.appendChild(eventElement2) document.body.appendChild(propertyElement) - expect(captureMock).toHaveBeenCalledTimes(0) + expect(beforeSendMock).toHaveBeenCalledTimes(0) simulateClick(eventElement1) simulateClick(eventElement2) - expect(captureMock).toHaveBeenCalledTimes(0) + expect(beforeSendMock).toHaveBeenCalledTimes(0) }) it('should not capture events when config returns true but server setting is disabled', () => { @@ -531,9 +531,9 @@ describe('Autocapture system', () => { const eventElement = document.createElement('a') document.body.appendChild(eventElement) - expect(captureMock).toHaveBeenCalledTimes(0) + expect(beforeSendMock).toHaveBeenCalledTimes(0) simulateClick(eventElement) - expect(captureMock).toHaveBeenCalledTimes(0) + expect(beforeSendMock).toHaveBeenCalledTimes(0) }) it('includes necessary metadata as properties when capturing an event', () => { @@ -550,10 +550,10 @@ describe('Autocapture system', () => { target: elTarget, }) autocapture['_captureEvent'](e) - expect(captureMock).toHaveBeenCalledTimes(1) - const captureArgs = captureMock.mock.calls[0] - const event = captureArgs[0] - const props = captureArgs[1].properties + expect(beforeSendMock).toHaveBeenCalledTimes(1) + const captureArgs = beforeSendMock.mock.calls[0] + const event = captureArgs[0].event + const props = captureArgs[0].properties expect(event).toBe('$autocapture') expect(props['$event_type']).toBe('click') expect(props['$elements'][0]).toHaveProperty('attr__href', 'https://test.com') @@ -579,9 +579,9 @@ describe('Autocapture system', () => { target: elTarget, }) autocapture['_captureEvent'](e) - expect(captureMock).toHaveBeenCalledTimes(1) - const captureArgs = captureMock.mock.calls[0] - const props = captureArgs[1].properties + expect(beforeSendMock).toHaveBeenCalledTimes(1) + const captureArgs = beforeSendMock.mock.calls[0] + const props = captureArgs[0].properties expect(longString).toBe('prop'.repeat(400)) expect(props['$elements'][0]).toHaveProperty('attr__data-props', 'prop'.repeat(256) + '...') }) @@ -606,7 +606,7 @@ describe('Autocapture system', () => { }) ) - const props = captureMock.mock.calls[0][1].properties + const props = beforeSendMock.mock.calls[0][0].properties expect(props['$element_selectors']).toContain('#primary_button') expect(props['$elements'][0]).toHaveProperty('attr__href', 'https://test.com') expect(props['$external_click_url']).toEqual('https://test.com') @@ -624,7 +624,7 @@ describe('Autocapture system', () => { target: elTarget, }) ) - const props = captureMock.mock.calls[0][1].properties + const props = beforeSendMock.mock.calls[0][0].properties expect(props['$elements'][0]).toHaveProperty('attr__href', 'https://test.com') expect(props['$external_click_url']).toEqual('https://test.com') }) @@ -642,7 +642,7 @@ describe('Autocapture system', () => { target: elTarget, }) ) - const props = captureMock.mock.calls[0][1].properties + const props = beforeSendMock.mock.calls[0][0].properties expect(props['$elements'][0]).toHaveProperty('attr__href', 'https://www.example.com/link') expect(props['$external_click_url']).toBeUndefined() }) @@ -659,7 +659,7 @@ describe('Autocapture system', () => { target: elTarget, }) ) - expect(captureMock.mock.calls[0][1].properties).not.toHaveProperty('attr__href') + expect(beforeSendMock.mock.calls[0][0].properties).not.toHaveProperty('attr__href') }) it('does not capture href attribute values from hidden elements', () => { @@ -674,7 +674,7 @@ describe('Autocapture system', () => { target: elTarget, }) ) - expect(captureMock.mock.calls[0][1].properties['$elements'][0]).not.toHaveProperty('attr__href') + expect(beforeSendMock.mock.calls[0][0].properties['$elements'][0]).not.toHaveProperty('attr__href') }) it('does not capture href attribute values that look like credit card numbers', () => { @@ -689,7 +689,7 @@ describe('Autocapture system', () => { target: elTarget, }) ) - expect(captureMock.mock.calls[0][1].properties['$elements'][0]).not.toHaveProperty('attr__href') + expect(beforeSendMock.mock.calls[0][0].properties['$elements'][0]).not.toHaveProperty('attr__href') }) it('does not capture href attribute values that look like social-security numbers', () => { @@ -704,7 +704,7 @@ describe('Autocapture system', () => { target: elTarget, }) ) - expect(captureMock.mock.calls[0][1].properties['$elements'][0]).not.toHaveProperty('attr__href') + expect(beforeSendMock.mock.calls[0][0].properties['$elements'][0]).not.toHaveProperty('attr__href') }) it('correctly identifies and formats text content', () => { @@ -745,10 +745,10 @@ describe('Autocapture system', () => { const e1 = makeMouseEvent({ target: span2, }) - captureMock.mockClear() + beforeSendMock.mockClear() autocapture['_captureEvent'](e1) - const props1 = captureMock.mock.calls[0][1].properties + const props1 = beforeSendMock.mock.calls[0][0].properties const text1 = "Some super duper really long Text with new lines that we'll strip out and also we will want to make this text shorter since it's not likely people really care about text content that's super long and it also takes up more space and bandwidth. Some super d" expect(props1['$elements'][0]).toHaveProperty('$el_text', text1) @@ -757,18 +757,18 @@ describe('Autocapture system', () => { const e2 = makeMouseEvent({ target: span1, }) - captureMock.mockClear() + beforeSendMock.mockClear() autocapture['_captureEvent'](e2) - const props2 = captureMock.mock.calls[0][1].properties + const props2 = beforeSendMock.mock.calls[0][0].properties expect(props2['$elements'][0]).toHaveProperty('$el_text', 'Some text') expect(props2['$el_text']).toEqual('Some text') const e3 = makeMouseEvent({ target: img2, }) - captureMock.mockClear() + beforeSendMock.mockClear() autocapture['_captureEvent'](e3) - const props3 = captureMock.mock.calls[0][1].properties + const props3 = beforeSendMock.mock.calls[0][0].properties expect(props3['$elements'][0]).toHaveProperty('$el_text', '') expect(props3).not.toHaveProperty('$el_text') }) @@ -796,7 +796,7 @@ describe('Autocapture system', () => { target: button1, }) autocapture['_captureEvent'](e1) - const props1 = captureMock.mock.calls[0][1].properties + const props1 = beforeSendMock.mock.calls[0][0].properties expect(props1['$elements'][0]).toHaveProperty('$el_text') expect(props1['$elements'][0]['$el_text']).toMatch(/Why\s+hello\s+there/) @@ -804,7 +804,7 @@ describe('Autocapture system', () => { target: button2, }) autocapture['_captureEvent'](e2) - const props2 = captureMock.mock.calls[0][1].properties + const props2 = beforeSendMock.mock.calls[0][0].properties expect(props2['$elements'][0]).toHaveProperty('$el_text') expect(props2['$elements'][0]['$el_text']).toMatch(/Why\s+hello\s+there/) @@ -812,7 +812,7 @@ describe('Autocapture system', () => { target: button3, }) autocapture['_captureEvent'](e3) - const props3 = captureMock.mock.calls[0][1].properties + const props3 = beforeSendMock.mock.calls[0][0].properties expect(props3['$elements'][0]).toHaveProperty('$el_text') expect(props3['$elements'][0]['$el_text']).toMatch(/Why\s+hello\s+there/) }) @@ -823,8 +823,8 @@ describe('Autocapture system', () => { type: 'submit', } as unknown as FormDataEvent autocapture['_captureEvent'](e) - expect(captureMock).toHaveBeenCalledTimes(1) - const props = captureMock.mock.calls[0][1].properties + expect(beforeSendMock).toHaveBeenCalledTimes(1) + const props = beforeSendMock.mock.calls[0][0].properties expect(props['$event_type']).toBe('submit') }) @@ -840,8 +840,8 @@ describe('Autocapture system', () => { target: link, }) autocapture['_captureEvent'](e) - expect(captureMock).toHaveBeenCalledTimes(1) - const props = captureMock.mock.calls[0][1].properties + expect(beforeSendMock).toHaveBeenCalledTimes(1) + const props = beforeSendMock.mock.calls[0][0].properties expect(props['$event_type']).toBe('click') }) @@ -856,8 +856,8 @@ describe('Autocapture system', () => { composedPath: () => [button, main_el], }) autocapture['_captureEvent'](e) - expect(captureMock).toHaveBeenCalledTimes(1) - const props = captureMock.mock.calls[0][1].properties + expect(beforeSendMock).toHaveBeenCalledTimes(1) + const props = beforeSendMock.mock.calls[0][0].properties expect(props['$event_type']).toBe('click') }) @@ -866,18 +866,18 @@ describe('Autocapture system', () => { const span = document.createElement('span') a.appendChild(span) autocapture['_captureEvent'](makeMouseEvent({ target: a })) - expect(captureMock).toHaveBeenCalledTimes(1) + expect(beforeSendMock).toHaveBeenCalledTimes(1) autocapture['_captureEvent'](makeMouseEvent({ target: span })) - expect(captureMock).toHaveBeenCalledTimes(2) + expect(beforeSendMock).toHaveBeenCalledTimes(2) - captureMock.mockClear() + beforeSendMock.mockClear() a.className = 'test1 ph-no-capture test2' autocapture['_captureEvent'](makeMouseEvent({ target: a })) - expect(captureMock).toHaveBeenCalledTimes(0) + expect(beforeSendMock).toHaveBeenCalledTimes(0) autocapture['_captureEvent'](makeMouseEvent({ target: span })) - expect(captureMock).toHaveBeenCalledTimes(0) + expect(beforeSendMock).toHaveBeenCalledTimes(0) }) it('does not capture any element attributes if mask_all_element_attributes is set', () => { @@ -897,7 +897,7 @@ describe('Autocapture system', () => { }) autocapture['_captureEvent'](e1) - const props1 = captureMock.mock.calls[0][1].properties + const props1 = beforeSendMock.mock.calls[0][0].properties expect('attr__formmethod' in props1['$elements'][0]).toEqual(false) }) @@ -916,7 +916,7 @@ describe('Autocapture system', () => { }) autocapture['_captureEvent'](e1) - const props1 = captureMock.mock.calls[0][1].properties + const props1 = beforeSendMock.mock.calls[0][0].properties expect(props1['$elements'][0]).not.toHaveProperty('$el_text') }) @@ -937,7 +937,7 @@ describe('Autocapture system', () => { } as DecideResponse) autocapture['_captureEvent'](e) - const props1 = captureMock.mock.calls[0][1].properties + const props1 = beforeSendMock.mock.calls[0][0].properties expect(props1['$elements_chain']).toBeDefined() expect(props1['$elements']).toBeUndefined() @@ -960,7 +960,7 @@ describe('Autocapture system', () => { autocapture['_elementsChainAsString'] = true autocapture['_captureEvent'](e) - const props1 = captureMock.mock.calls[0][1].properties + const props1 = beforeSendMock.mock.calls[0][0].properties expect(props1['$elements_chain']).toBe( 'a.test-class.test-class2.test-class3.test-class4.test-class5:nth-child="1"nth-of-type="1"href="http://test.com"attr__href="http://test.com"attr__class="test-class test-class2 test-class3 test-class4 test-class5";span:nth-child="1"nth-of-type="1"' @@ -991,8 +991,8 @@ describe('Autocapture system', () => { autocapture['_captureEvent'](e) - expect(captureMock).toHaveBeenCalledTimes(1) - const props = captureMock.mock.calls[0][1].properties + expect(beforeSendMock).toHaveBeenCalledTimes(1) + const props = beforeSendMock.mock.calls[0][0].properties const capturedButton = props['$elements'][1] expect(capturedButton['tag_name']).toBe('button') expect(capturedButton['$el_text']).toBe('the button text with more info') @@ -1011,11 +1011,11 @@ describe('Autocapture system', () => { document.body.appendChild(button) simulateClick(button) simulateClick(button) - expect(captureMock).toHaveBeenCalledTimes(2) - expect(captureMock.mock.calls[0][0]).toBe('$autocapture') - expect(captureMock.mock.calls[0][1].properties['$event_type']).toBe('click') - expect(captureMock.mock.calls[1][0]).toBe('$autocapture') - expect(captureMock.mock.calls[1][1].properties['$event_type']).toBe('click') + expect(beforeSendMock).toHaveBeenCalledTimes(2) + expect(beforeSendMock.mock.calls[0][0].event).toBe('$autocapture') + expect(beforeSendMock.mock.calls[0][0].properties['$event_type']).toBe('click') + expect(beforeSendMock.mock.calls[1][0].event).toBe('$autocapture') + expect(beforeSendMock.mock.calls[1][0].properties['$event_type']).toBe('click') }) }) diff --git a/src/__tests__/consent.test.ts b/src/__tests__/consent.test.ts index c9483ac4a..b6f520ee5 100644 --- a/src/__tests__/consent.test.ts +++ b/src/__tests__/consent.test.ts @@ -81,15 +81,15 @@ describe('consentManager', () => { }) describe('opt out event', () => { - let onCapture = jest.fn() + let beforeSendMock = jest.fn().mockImplementation((...args) => args) beforeEach(() => { - onCapture = jest.fn() - posthog = createPostHog({ opt_out_capturing_by_default: true, _onCapture: onCapture }) + beforeSendMock = jest.fn().mockImplementation((e) => e) + posthog = createPostHog({ opt_out_capturing_by_default: true, before_send: beforeSendMock }) }) it('should send opt in event if not disabled', () => { posthog.opt_in_capturing() - expect(onCapture).toHaveBeenCalledWith('$opt_in', expect.objectContaining({})) + expect(beforeSendMock).toHaveBeenCalledWith(expect.objectContaining({ event: '$opt_in' })) }) it('should send opt in event with overrides', () => { @@ -99,9 +99,9 @@ describe('consentManager', () => { foo: 'bar', }, }) - expect(onCapture).toHaveBeenCalledWith( - 'override-opt-in', + expect(beforeSendMock).toHaveBeenCalledWith( expect.objectContaining({ + event: 'override-opt-in', properties: expect.objectContaining({ foo: 'bar', }), @@ -111,72 +111,72 @@ describe('consentManager', () => { it('should not send opt in event if false', () => { posthog.opt_in_capturing({ captureEventName: false }) - expect(onCapture).toHaveBeenCalledTimes(1) - expect(onCapture).not.toHaveBeenCalledWith('$opt_in') - expect(onCapture).lastCalledWith('$pageview', expect.anything()) + expect(beforeSendMock).toHaveBeenCalledTimes(1) + expect(beforeSendMock).not.toHaveBeenCalledWith(expect.objectContaining({ event: '$opt_in' })) + expect(beforeSendMock).lastCalledWith(expect.objectContaining({ event: '$pageview' })) }) it('should not send opt in event if false', () => { posthog.opt_in_capturing({ captureEventName: false }) - expect(onCapture).toHaveBeenCalledTimes(1) - expect(onCapture).not.toHaveBeenCalledWith('$opt_in') - expect(onCapture).lastCalledWith('$pageview', expect.anything()) + expect(beforeSendMock).toHaveBeenCalledTimes(1) + expect(beforeSendMock).not.toHaveBeenCalledWith(expect.objectContaining({ event: '$opt_in' })) + expect(beforeSendMock).lastCalledWith(expect.objectContaining({ event: '$pageview' })) }) it('should not send $pageview on opt in if capturing is disabled', () => { posthog = createPostHog({ opt_out_capturing_by_default: true, - _onCapture: onCapture, + before_send: beforeSendMock, capture_pageview: false, }) posthog.opt_in_capturing({ captureEventName: false }) - expect(onCapture).toHaveBeenCalledTimes(0) + expect(beforeSendMock).toHaveBeenCalledTimes(0) }) it('should not send $pageview on opt in if is has already been captured', async () => { posthog = createPostHog({ - _onCapture: onCapture, + before_send: beforeSendMock, }) // Wait for the initial $pageview to be captured // eslint-disable-next-line compat/compat await new Promise((r) => setTimeout(r, 10)) - expect(onCapture).toHaveBeenCalledTimes(1) - expect(onCapture).lastCalledWith('$pageview', expect.anything()) + expect(beforeSendMock).toHaveBeenCalledTimes(1) + expect(beforeSendMock).lastCalledWith(expect.objectContaining({ event: '$pageview' })) posthog.opt_in_capturing() - expect(onCapture).toHaveBeenCalledTimes(2) - expect(onCapture).toHaveBeenCalledWith('$opt_in', expect.anything()) + expect(beforeSendMock).toHaveBeenCalledTimes(2) + expect(beforeSendMock).toHaveBeenCalledWith(expect.objectContaining({ event: '$opt_in' })) }) it('should send $pageview on opt in if is has not been captured', async () => { // Some other tests might call setTimeout after they've passed, so creating a new instance here. - const onCapture = jest.fn() - const posthog = createPostHog({ _onCapture: onCapture }) + const beforeSendMock = jest.fn().mockImplementation((e) => e) + const posthog = createPostHog({ before_send: beforeSendMock }) posthog.opt_in_capturing() - expect(onCapture).toHaveBeenCalledTimes(2) - expect(onCapture).toHaveBeenCalledWith('$opt_in', expect.anything()) - expect(onCapture).lastCalledWith('$pageview', expect.anything()) + expect(beforeSendMock).toHaveBeenCalledTimes(2) + expect(beforeSendMock).toHaveBeenCalledWith(expect.objectContaining({ event: '$opt_in' })) + expect(beforeSendMock).lastCalledWith(expect.objectContaining({ event: '$pageview' })) // Wait for the $pageview timeout to be called // eslint-disable-next-line compat/compat await new Promise((r) => setTimeout(r, 10)) - expect(onCapture).toHaveBeenCalledTimes(2) + expect(beforeSendMock).toHaveBeenCalledTimes(2) }) it('should not send $pageview on subsequent opt in', async () => { // Some other tests might call setTimeout after they've passed, so creating a new instance here. - const onCapture = jest.fn() - const posthog = createPostHog({ _onCapture: onCapture }) + const beforeSendMock = jest.fn().mockImplementation((e) => e) + const posthog = createPostHog({ before_send: beforeSendMock }) posthog.opt_in_capturing() - expect(onCapture).toHaveBeenCalledTimes(2) - expect(onCapture).toHaveBeenCalledWith('$opt_in', expect.anything()) - expect(onCapture).lastCalledWith('$pageview', expect.anything()) + expect(beforeSendMock).toHaveBeenCalledTimes(2) + expect(beforeSendMock).toHaveBeenCalledWith(expect.objectContaining({ event: '$opt_in' })) + expect(beforeSendMock).lastCalledWith(expect.objectContaining({ event: '$pageview' })) // Wait for the $pageview timeout to be called // eslint-disable-next-line compat/compat await new Promise((r) => setTimeout(r, 10)) posthog.opt_in_capturing() - expect(onCapture).toHaveBeenCalledTimes(3) - expect(onCapture).not.lastCalledWith('$pageview', expect.anything()) + expect(beforeSendMock).toHaveBeenCalledTimes(3) + expect(beforeSendMock).not.lastCalledWith(expect.objectContaining({ event: '$pageview' })) }) }) @@ -238,18 +238,18 @@ describe('consentManager', () => { }) it(`should capture an event recording the opt-in action`, () => { - const onCapture = jest.fn() - posthog.on('eventCaptured', onCapture) + const beforeSendMock = jest.fn() + posthog.on('eventCaptured', beforeSendMock) posthog.opt_in_capturing() - expect(onCapture).toHaveBeenCalledWith(expect.objectContaining({ event: '$opt_in' })) + expect(beforeSendMock).toHaveBeenCalledWith(expect.objectContaining({ event: '$opt_in' })) - onCapture.mockClear() + beforeSendMock.mockClear() const captureEventName = `єνєηт` const captureProperties = { '𝖕𝖗𝖔𝖕𝖊𝖗𝖙𝖞': `𝓿𝓪𝓵𝓾𝓮` } posthog.opt_in_capturing({ captureEventName, captureProperties }) - expect(onCapture).toHaveBeenCalledWith( + expect(beforeSendMock).toHaveBeenCalledWith( expect.objectContaining({ event: captureEventName, properties: expect.objectContaining(captureProperties), diff --git a/src/__tests__/extensions/exception-autocapture/exception-observer.test.ts b/src/__tests__/extensions/exception-autocapture/exception-observer.test.ts index 492bae78b..ad934e2b1 100644 --- a/src/__tests__/extensions/exception-autocapture/exception-observer.test.ts +++ b/src/__tests__/extensions/exception-autocapture/exception-observer.test.ts @@ -35,7 +35,7 @@ describe('Exception Observer', () => { let exceptionObserver: ExceptionObserver let posthog: PostHog let sendRequestSpy: jest.SpyInstance - const mockCapture = jest.fn() + const beforeSendMock = jest.fn().mockImplementation((e) => e) const loadScriptMock = jest.fn() const addErrorWrappingFlagToWindow = () => { @@ -51,7 +51,7 @@ describe('Exception Observer', () => { callback() }) - posthog = await createPosthogInstance(uuidv7(), { _onCapture: mockCapture }) + posthog = await createPosthogInstance(uuidv7(), { before_send: beforeSendMock }) assignableWindow.__PosthogExtensions__ = { loadExternalDependency: loadScriptMock, } @@ -91,11 +91,11 @@ describe('Exception Observer', () => { const error = new Error('test error') window!.onerror?.call(window, 'message', 'source', 0, 0, error) - const captureCalls = mockCapture.mock.calls + const captureCalls = beforeSendMock.mock.calls expect(captureCalls.length).toBe(1) const singleCall = captureCalls[0] - expect(singleCall[0]).toBe('$exception') - expect(singleCall[1]).toMatchObject({ + expect(singleCall[0]).toMatchObject({ + event: '$exception', properties: { $exception_personURL: expect.any(String), $exception_list: [ @@ -120,11 +120,11 @@ describe('Exception Observer', () => { }) window!.onunhandledrejection?.call(window!, promiseRejectionEvent) - const captureCalls = mockCapture.mock.calls + const captureCalls = beforeSendMock.mock.calls expect(captureCalls.length).toBe(1) const singleCall = captureCalls[0] - expect(singleCall[0]).toBe('$exception') - expect(singleCall[1]).toMatchObject({ + expect(singleCall[0]).toMatchObject({ + event: '$exception', properties: { $exception_personURL: expect.any(String), $exception_list: [ diff --git a/src/__tests__/extensions/web-vitals.test.ts b/src/__tests__/extensions/web-vitals.test.ts index e83ae0494..5cd284a13 100644 --- a/src/__tests__/extensions/web-vitals.test.ts +++ b/src/__tests__/extensions/web-vitals.test.ts @@ -10,7 +10,7 @@ jest.useFakeTimers() describe('web vitals', () => { let posthog: PostHog - let onCapture = jest.fn() + let beforeSendMock = jest.fn().mockImplementation((e) => e) let onLCPCallback: ((metric: Record) => void) | undefined = undefined let onCLSCallback: ((metric: Record) => void) | undefined = undefined let onFCPCallback: ((metric: Record) => void) | undefined = undefined @@ -81,9 +81,9 @@ describe('web vitals', () => { expectedProperties: Record ) => { beforeEach(async () => { - onCapture.mockClear() + beforeSendMock.mockClear() posthog = await createPosthogInstance(uuidv7(), { - _onCapture: onCapture, + before_send: beforeSendMock, capture_performance: { web_vitals: true, web_vitals_allowed_metrics: clientConfig }, // sometimes pageviews sneak in and make asserting on mock capture tricky capture_pageview: false, @@ -123,10 +123,9 @@ describe('web vitals', () => { it('should emit when all allowed metrics are captured', async () => { emitAllMetrics() - expect(onCapture).toBeCalledTimes(1) + expect(beforeSendMock).toBeCalledTimes(1) - expect(onCapture.mock.lastCall).toMatchObject([ - '$web_vitals', + expect(beforeSendMock.mock.lastCall).toMatchObject([ { event: '$web_vitals', properties: expectedProperties, @@ -137,14 +136,12 @@ describe('web vitals', () => { it('should emit after 5 seconds even when only 1 to 3 metrics captured', async () => { onCLSCallback?.({ name: 'CLS', value: 123.45, extra: 'property' }) - expect(onCapture).toBeCalledTimes(0) + expect(beforeSendMock).toBeCalledTimes(0) jest.advanceTimersByTime(DEFAULT_FLUSH_TO_CAPTURE_TIMEOUT_MILLISECONDS + 1) // for some reason advancing the timer emits a $pageview event as well 🤷 - // expect(onCapture).toBeCalledTimes(2) - expect(onCapture.mock.lastCall).toMatchObject([ - '$web_vitals', + expect(beforeSendMock.mock.lastCall).toMatchObject([ { event: '$web_vitals', properties: { @@ -159,12 +156,11 @@ describe('web vitals', () => { ;(posthog.config.capture_performance as PerformanceCaptureConfig).web_vitals_delayed_flush_ms = 1000 onCLSCallback?.({ name: 'CLS', value: 123.45, extra: 'property' }) - expect(onCapture).toBeCalledTimes(0) + expect(beforeSendMock).toBeCalledTimes(0) jest.advanceTimersByTime(1000 + 1) - expect(onCapture.mock.lastCall).toMatchObject([ - '$web_vitals', + expect(beforeSendMock.mock.lastCall).toMatchObject([ { event: '$web_vitals', properties: { @@ -178,22 +174,22 @@ describe('web vitals', () => { it('should ignore a ridiculous value', async () => { onCLSCallback?.({ name: 'CLS', value: FIFTEEN_MINUTES_IN_MILLIS, extra: 'property' }) - expect(onCapture).toBeCalledTimes(0) + expect(beforeSendMock).toBeCalledTimes(0) jest.advanceTimersByTime(DEFAULT_FLUSH_TO_CAPTURE_TIMEOUT_MILLISECONDS + 1) - expect(onCapture.mock.calls).toEqual([]) + expect(beforeSendMock.mock.calls).toEqual([]) }) it('can be configured not to ignore a ridiculous value', async () => { posthog.config.capture_performance = { __web_vitals_max_value: 0 } onCLSCallback?.({ name: 'CLS', value: FIFTEEN_MINUTES_IN_MILLIS, extra: 'property' }) - expect(onCapture).toBeCalledTimes(0) + expect(beforeSendMock).toBeCalledTimes(0) jest.advanceTimersByTime(DEFAULT_FLUSH_TO_CAPTURE_TIMEOUT_MILLISECONDS + 1) - expect(onCapture).toBeCalledTimes(1) + expect(beforeSendMock).toBeCalledTimes(1) }) } ) @@ -217,9 +213,9 @@ describe('web vitals', () => { }, } - onCapture = jest.fn() + beforeSendMock = jest.fn() posthog = await createPosthogInstance(uuidv7(), { - _onCapture: onCapture, + before_send: beforeSendMock, }) }) diff --git a/src/__tests__/heatmaps.test.ts b/src/__tests__/heatmaps.test.ts index 8865dc9ca..140415833 100644 --- a/src/__tests__/heatmaps.test.ts +++ b/src/__tests__/heatmaps.test.ts @@ -12,7 +12,7 @@ jest.useFakeTimers() describe('heatmaps', () => { let posthog: PostHog - let onCapture = jest.fn() + let beforeSendMock = jest.fn().mockImplementation((e) => e) const createMockMouseEvent = (props: Partial = {}) => ({ @@ -23,10 +23,10 @@ describe('heatmaps', () => { } as unknown as MouseEvent) beforeEach(async () => { - onCapture = onCapture.mockClear() + beforeSendMock = beforeSendMock.mockClear() posthog = await createPosthogInstance(uuidv7(), { - _onCapture: onCapture, + before_send: beforeSendMock, sanitize_properties: (props) => { // what ever sanitization makes sense const sanitizeUrl = (url: string) => url.replace(/https?:\/\/[^/]+/g, 'http://replaced') @@ -61,9 +61,8 @@ describe('heatmaps', () => { jest.advanceTimersByTime(posthog.heatmaps!.flushIntervalMilliseconds + 1) - expect(onCapture).toBeCalledTimes(1) - expect(onCapture.mock.lastCall[0]).toEqual('$$heatmap') - expect(onCapture.mock.lastCall[1]).toMatchObject({ + expect(beforeSendMock).toBeCalledTimes(1) + expect(beforeSendMock.mock.lastCall[0]).toMatchObject({ event: '$$heatmap', properties: { $heatmap_data: { @@ -85,7 +84,7 @@ describe('heatmaps', () => { jest.advanceTimersByTime(posthog.heatmaps!.flushIntervalMilliseconds - 1) - expect(onCapture).toBeCalledTimes(0) + expect(beforeSendMock).toBeCalledTimes(0) expect(posthog.heatmaps!.getAndClearBuffer()).toBeDefined() }) @@ -96,9 +95,9 @@ describe('heatmaps', () => { jest.advanceTimersByTime(posthog.heatmaps!.flushIntervalMilliseconds + 1) - expect(onCapture).toBeCalledTimes(1) - expect(onCapture.mock.lastCall[0]).toEqual('$$heatmap') - const heatmapData = onCapture.mock.lastCall[1].properties.$heatmap_data + expect(beforeSendMock).toBeCalledTimes(1) + expect(beforeSendMock.mock.lastCall[0].event).toEqual('$$heatmap') + const heatmapData = beforeSendMock.mock.lastCall[0].properties.$heatmap_data expect(heatmapData).toBeDefined() expect(heatmapData['http://replaced/']).toHaveLength(4) expect(heatmapData['http://replaced/'].map((x) => x.type)).toEqual(['click', 'click', 'rageclick', 'click']) @@ -110,16 +109,16 @@ describe('heatmaps', () => { jest.advanceTimersByTime(posthog.heatmaps!.flushIntervalMilliseconds + 1) - expect(onCapture).toBeCalledTimes(1) - expect(onCapture.mock.lastCall[0]).toEqual('$$heatmap') - expect(onCapture.mock.lastCall[1].properties.$heatmap_data).toBeDefined() - expect(onCapture.mock.lastCall[1].properties.$heatmap_data['http://replaced/']).toHaveLength(2) + expect(beforeSendMock).toBeCalledTimes(1) + expect(beforeSendMock.mock.lastCall[0].event).toEqual('$$heatmap') + expect(beforeSendMock.mock.lastCall[0].properties.$heatmap_data).toBeDefined() + expect(beforeSendMock.mock.lastCall[0].properties.$heatmap_data['http://replaced/']).toHaveLength(2) expect(posthog.heatmaps!['buffer']).toEqual(undefined) jest.advanceTimersByTime(posthog.heatmaps!.flushIntervalMilliseconds + 1) - expect(onCapture).toBeCalledTimes(1) + expect(beforeSendMock).toBeCalledTimes(1) }) it('should ignore clicks if they come from the toolbar', async () => { @@ -143,17 +142,17 @@ describe('heatmaps', () => { }) ) expect(posthog.heatmaps?.getAndClearBuffer()).not.toEqual(undefined) - expect(onCapture.mock.calls).toEqual([]) + expect(beforeSendMock.mock.calls).toEqual([]) }) it('should ignore an empty buffer', async () => { - expect(onCapture.mock.calls).toEqual([]) + expect(beforeSendMock.mock.calls).toEqual([]) expect(posthog.heatmaps?.['buffer']).toEqual(undefined) jest.advanceTimersByTime(posthog.heatmaps!.flushIntervalMilliseconds + 1) - expect(onCapture.mock.calls).toEqual([]) + expect(beforeSendMock.mock.calls).toEqual([]) }) describe('isEnabled()', () => { diff --git a/src/__tests__/identify.test.ts b/src/__tests__/identify.test.ts index d435dbaec..d9f37571d 100644 --- a/src/__tests__/identify.test.ts +++ b/src/__tests__/identify.test.ts @@ -42,8 +42,8 @@ describe('identify', () => { it('should send $is_identified = true with the identify event and following events', async () => { // arrange const token = uuidv7() - const onCapture = jest.fn() - const posthog = await createPosthogInstance(token, { _onCapture: onCapture }) + const beforeSendMock = jest.fn().mockImplementation((e) => e) + const posthog = await createPosthogInstance(token, { before_send: beforeSendMock }) const distinctId = '123' // act @@ -52,12 +52,12 @@ describe('identify', () => { posthog.capture('custom event after identify') // assert - const eventBeforeIdentify = onCapture.mock.calls[0] - expect(eventBeforeIdentify[1].properties.$is_identified).toEqual(false) - const identifyCall = onCapture.mock.calls[1] - expect(identifyCall[0]).toEqual('$identify') - expect(identifyCall[1].properties.$is_identified).toEqual(true) - const eventAfterIdentify = onCapture.mock.calls[2] - expect(eventAfterIdentify[1].properties.$is_identified).toEqual(true) + const eventBeforeIdentify = beforeSendMock.mock.calls[0] + expect(eventBeforeIdentify[0].properties.$is_identified).toEqual(false) + const identifyCall = beforeSendMock.mock.calls[1] + expect(identifyCall[0].event).toEqual('$identify') + expect(identifyCall[0].properties.$is_identified).toEqual(true) + const eventAfterIdentify = beforeSendMock.mock.calls[2] + expect(eventAfterIdentify[0].properties.$is_identified).toEqual(true) }) }) diff --git a/src/__tests__/personProcessing.test.ts b/src/__tests__/personProcessing.test.ts index 7cbfcb809..5bb11e653 100644 --- a/src/__tests__/personProcessing.test.ts +++ b/src/__tests__/personProcessing.test.ts @@ -78,13 +78,13 @@ describe('person processing', () => { persistence_name?: string ) => { token = token || uuidv7() - const onCapture = jest.fn() + const beforeSendMock = jest.fn().mockImplementation((e) => e) const posthog = await createPosthogInstance(token, { - _onCapture: onCapture, + before_send: beforeSendMock, person_profiles, persistence_name, }) - return { token, onCapture, posthog } + return { token, beforeSendMock, posthog } } describe('init', () => { @@ -142,7 +142,7 @@ describe('person processing', () => { describe('identify', () => { it('should fail if process_person is set to never', async () => { // arrange - const { posthog, onCapture } = await setup('never') + const { posthog, beforeSendMock } = await setup('never') // act posthog.identify(distinctId) @@ -152,12 +152,12 @@ describe('person processing', () => { expect(jest.mocked(logger).error).toHaveBeenCalledWith( 'posthog.identify was called, but process_person is set to "never". This call will be ignored.' ) - expect(onCapture).toBeCalledTimes(0) + expect(beforeSendMock).toBeCalledTimes(0) }) it('should switch events to $person_process=true if process_person is identified_only', async () => { // arrange - const { posthog, onCapture } = await setup('identified_only') + const { posthog, beforeSendMock } = await setup('identified_only') // act posthog.capture('custom event before identify') @@ -165,18 +165,18 @@ describe('person processing', () => { posthog.capture('custom event after identify') // assert expect(jest.mocked(logger).error).toBeCalledTimes(0) - const eventBeforeIdentify = onCapture.mock.calls[0] - expect(eventBeforeIdentify[1].properties.$process_person_profile).toEqual(false) - const identifyCall = onCapture.mock.calls[1] - expect(identifyCall[0]).toEqual('$identify') - expect(identifyCall[1].properties.$process_person_profile).toEqual(true) - const eventAfterIdentify = onCapture.mock.calls[2] - expect(eventAfterIdentify[1].properties.$process_person_profile).toEqual(true) + const eventBeforeIdentify = beforeSendMock.mock.calls[0] + expect(eventBeforeIdentify[0].properties.$process_person_profile).toEqual(false) + const identifyCall = beforeSendMock.mock.calls[1] + expect(identifyCall[0].event).toEqual('$identify') + expect(identifyCall[0].properties.$process_person_profile).toEqual(true) + const eventAfterIdentify = beforeSendMock.mock.calls[2] + expect(eventAfterIdentify[0].properties.$process_person_profile).toEqual(true) }) it('should not change $person_process if process_person is always', async () => { // arrange - const { posthog, onCapture } = await setup('always') + const { posthog, beforeSendMock } = await setup('always') // act posthog.capture('custom event before identify') @@ -184,26 +184,26 @@ describe('person processing', () => { posthog.capture('custom event after identify') // assert expect(jest.mocked(logger).error).toBeCalledTimes(0) - const eventBeforeIdentify = onCapture.mock.calls[0] - expect(eventBeforeIdentify[1].properties.$process_person_profile).toEqual(true) - const identifyCall = onCapture.mock.calls[1] - expect(identifyCall[0]).toEqual('$identify') - expect(identifyCall[1].properties.$process_person_profile).toEqual(true) - const eventAfterIdentify = onCapture.mock.calls[2] - expect(eventAfterIdentify[1].properties.$process_person_profile).toEqual(true) + const eventBeforeIdentify = beforeSendMock.mock.calls[0] + expect(eventBeforeIdentify[0].properties.$process_person_profile).toEqual(true) + const identifyCall = beforeSendMock.mock.calls[1] + expect(identifyCall[0].event).toEqual('$identify') + expect(identifyCall[0].properties.$process_person_profile).toEqual(true) + const eventAfterIdentify = beforeSendMock.mock.calls[2] + expect(eventAfterIdentify[0].properties.$process_person_profile).toEqual(true) }) it('should include initial referrer info in identify event if identified_only', async () => { // arrange - const { posthog, onCapture } = await setup('identified_only') + const { posthog, beforeSendMock } = await setup('identified_only') // act posthog.identify(distinctId) // assert - const identifyCall = onCapture.mock.calls[0] - expect(identifyCall[0]).toEqual('$identify') - expect(identifyCall[1].$set_once).toEqual({ + const identifyCall = beforeSendMock.mock.calls[0] + expect(identifyCall[0].event).toEqual('$identify') + expect(identifyCall[0].$set_once).toEqual({ ...INITIAL_CAMPAIGN_PARAMS_NULL, $initial_current_url: 'https://example.com?utm_source=foo', $initial_host: 'example.com', @@ -216,7 +216,7 @@ describe('person processing', () => { it('should preserve initial referrer info across a separate session', async () => { // arrange - const { posthog, onCapture } = await setup('identified_only') + const { posthog, beforeSendMock } = await setup('identified_only') mockReferrerGetter.mockReturnValue('https://referrer1.com') mockURLGetter.mockReturnValue('https://example1.com/pathname1?utm_source=foo1') @@ -236,17 +236,17 @@ describe('person processing', () => { posthog.capture('event s2 after identify') // assert - const eventS1 = onCapture.mock.calls[0] - const eventS2Before = onCapture.mock.calls[1] - const eventS2Identify = onCapture.mock.calls[2] - const eventS2After = onCapture.mock.calls[3] + const eventS1 = beforeSendMock.mock.calls[0] + const eventS2Before = beforeSendMock.mock.calls[1] + const eventS2Identify = beforeSendMock.mock.calls[2] + const eventS2After = beforeSendMock.mock.calls[3] - expect(eventS1[1].$set_once).toEqual(undefined) + expect(eventS1[0].$set_once).toEqual(undefined) - expect(eventS2Before[1].$set_once).toEqual(undefined) + expect(eventS2Before[0].$set_once).toEqual(undefined) - expect(eventS2Identify[0]).toEqual('$identify') - expect(eventS2Identify[1].$set_once).toEqual({ + expect(eventS2Identify[0].event).toEqual('$identify') + expect(eventS2Identify[0].$set_once).toEqual({ ...INITIAL_CAMPAIGN_PARAMS_NULL, $initial_current_url: 'https://example1.com/pathname1?utm_source=foo1', $initial_host: 'example1.com', @@ -256,8 +256,8 @@ describe('person processing', () => { $initial_utm_source: 'foo1', }) - expect(eventS2After[0]).toEqual('event s2 after identify') - expect(eventS2After[1].$set_once).toEqual({ + expect(eventS2After[0].event).toEqual('event s2 after identify') + expect(eventS2After[0].$set_once).toEqual({ ...INITIAL_CAMPAIGN_PARAMS_NULL, $initial_current_url: 'https://example1.com/pathname1?utm_source=foo1', $initial_host: 'example1.com', @@ -270,15 +270,15 @@ describe('person processing', () => { it('should include initial referrer info in identify event if always', async () => { // arrange - const { posthog, onCapture } = await setup('always') + const { posthog, beforeSendMock } = await setup('always') // act posthog.identify(distinctId) // assert - const identifyCall = onCapture.mock.calls[0] - expect(identifyCall[0]).toEqual('$identify') - expect(identifyCall[1].$set_once).toEqual({ + const identifyCall = beforeSendMock.mock.calls[0] + expect(identifyCall[0].event).toEqual('$identify') + expect(identifyCall[0].$set_once).toEqual({ ...INITIAL_CAMPAIGN_PARAMS_NULL, $initial_current_url: 'https://example.com?utm_source=foo', $initial_host: 'example.com', @@ -291,16 +291,16 @@ describe('person processing', () => { it('should include initial search params', async () => { // arrange - const { posthog, onCapture } = await setup('always') + const { posthog, beforeSendMock } = await setup('always') mockReferrerGetter.mockReturnValue('https://www.google.com?q=bar') // act posthog.identify(distinctId) // assert - const identifyCall = onCapture.mock.calls[0] - expect(identifyCall[0]).toEqual('$identify') - expect(identifyCall[1].$set_once).toEqual({ + const identifyCall = beforeSendMock.mock.calls[0] + expect(identifyCall[0].event).toEqual('$identify') + expect(identifyCall[0].$set_once).toEqual({ ...INITIAL_CAMPAIGN_PARAMS_NULL, $initial_current_url: 'https://example.com?utm_source=foo', $initial_host: 'example.com', @@ -315,7 +315,7 @@ describe('person processing', () => { it('should be backwards compatible with deprecated INITIAL_REFERRER_INFO and INITIAL_CAMPAIGN_PARAMS way of saving initial person props', async () => { // arrange - const { posthog, onCapture } = await setup('always') + const { posthog, beforeSendMock } = await setup('always') posthog.persistence!.props[INITIAL_REFERRER_INFO] = { referrer: 'https://deprecated-referrer.com', referring_domain: 'deprecated-referrer.com', @@ -328,9 +328,9 @@ describe('person processing', () => { posthog.identify(distinctId) // assert - const identifyCall = onCapture.mock.calls[0] - expect(identifyCall[0]).toEqual('$identify') - expect(identifyCall[1].$set_once).toEqual({ + const identifyCall = beforeSendMock.mock.calls[0] + expect(identifyCall[0].event).toEqual('$identify') + expect(identifyCall[0].$set_once).toEqual({ $initial_referrer: 'https://deprecated-referrer.com', $initial_referring_domain: 'deprecated-referrer.com', $initial_utm_source: 'deprecated-source', @@ -341,7 +341,7 @@ describe('person processing', () => { describe('capture', () => { it('should include initial referrer info iff the event has person processing when in identified_only mode', async () => { // arrange - const { posthog, onCapture } = await setup('identified_only') + const { posthog, beforeSendMock } = await setup('identified_only') // act posthog.capture('custom event before identify') @@ -349,10 +349,10 @@ describe('person processing', () => { posthog.capture('custom event after identify') // assert - const eventBeforeIdentify = onCapture.mock.calls[0] - expect(eventBeforeIdentify[1].$set_once).toBeUndefined() - const eventAfterIdentify = onCapture.mock.calls[2] - expect(eventAfterIdentify[1].$set_once).toEqual({ + const eventBeforeIdentify = beforeSendMock.mock.calls[0] + expect(eventBeforeIdentify[0].$set_once).toBeUndefined() + const eventAfterIdentify = beforeSendMock.mock.calls[2] + expect(eventAfterIdentify[0].$set_once).toEqual({ ...INITIAL_CAMPAIGN_PARAMS_NULL, $initial_current_url: 'https://example.com?utm_source=foo', $initial_host: 'example.com', @@ -365,7 +365,7 @@ describe('person processing', () => { it('should add initial referrer to set_once when in always mode', async () => { // arrange - const { posthog, onCapture } = await setup('always') + const { posthog, beforeSendMock } = await setup('always') // act posthog.capture('custom event before identify') @@ -373,8 +373,8 @@ describe('person processing', () => { posthog.capture('custom event after identify') // assert - const eventBeforeIdentify = onCapture.mock.calls[0] - expect(eventBeforeIdentify[1].$set_once).toEqual({ + const eventBeforeIdentify = beforeSendMock.mock.calls[0] + expect(eventBeforeIdentify[0].$set_once).toEqual({ ...INITIAL_CAMPAIGN_PARAMS_NULL, $initial_current_url: 'https://example.com?utm_source=foo', $initial_host: 'example.com', @@ -383,8 +383,8 @@ describe('person processing', () => { $initial_referring_domain: 'referrer.com', $initial_utm_source: 'foo', }) - const eventAfterIdentify = onCapture.mock.calls[2] - expect(eventAfterIdentify[1].$set_once).toEqual({ + const eventAfterIdentify = beforeSendMock.mock.calls[2] + expect(eventAfterIdentify[0].$set_once).toEqual({ ...INITIAL_CAMPAIGN_PARAMS_NULL, $initial_current_url: 'https://example.com?utm_source=foo', $initial_host: 'example.com', @@ -399,7 +399,7 @@ describe('person processing', () => { describe('group', () => { it('should start person processing for identified_only users', async () => { // arrange - const { posthog, onCapture } = await setup('identified_only') + const { posthog, beforeSendMock } = await setup('identified_only') // act posthog.capture('custom event before group') @@ -407,18 +407,18 @@ describe('person processing', () => { posthog.capture('custom event after group') // assert - const eventBeforeGroup = onCapture.mock.calls[0] - expect(eventBeforeGroup[1].properties.$process_person_profile).toEqual(false) - const groupIdentify = onCapture.mock.calls[1] - expect(groupIdentify[0]).toEqual('$groupidentify') - expect(groupIdentify[1].properties.$process_person_profile).toEqual(true) - const eventAfterGroup = onCapture.mock.calls[2] - expect(eventAfterGroup[1].properties.$process_person_profile).toEqual(true) + const eventBeforeGroup = beforeSendMock.mock.calls[0] + expect(eventBeforeGroup[0].properties.$process_person_profile).toEqual(false) + const groupIdentify = beforeSendMock.mock.calls[1] + expect(groupIdentify[0].event).toEqual('$groupidentify') + expect(groupIdentify[0].properties.$process_person_profile).toEqual(true) + const eventAfterGroup = beforeSendMock.mock.calls[2] + expect(eventAfterGroup[0].properties.$process_person_profile).toEqual(true) }) it('should not send the $groupidentify event if person_processing is set to never', async () => { // arrange - const { posthog, onCapture } = await setup('never') + const { posthog, beforeSendMock } = await setup('never') // act posthog.capture('custom event before group') @@ -431,24 +431,24 @@ describe('person processing', () => { 'posthog.group was called, but process_person is set to "never". This call will be ignored.' ) - expect(onCapture).toBeCalledTimes(2) - const eventBeforeGroup = onCapture.mock.calls[0] - expect(eventBeforeGroup[1].properties.$process_person_profile).toEqual(false) - const eventAfterGroup = onCapture.mock.calls[1] - expect(eventAfterGroup[1].properties.$process_person_profile).toEqual(false) + expect(beforeSendMock).toBeCalledTimes(2) + const eventBeforeGroup = beforeSendMock.mock.calls[0] + expect(eventBeforeGroup[0].properties.$process_person_profile).toEqual(false) + const eventAfterGroup = beforeSendMock.mock.calls[1] + expect(eventAfterGroup[0].properties.$process_person_profile).toEqual(false) }) }) describe('setPersonProperties', () => { it("should not send a $set event if process_person is set to 'never'", async () => { // arrange - const { posthog, onCapture } = await setup('never') + const { posthog, beforeSendMock } = await setup('never') // act posthog.setPersonProperties({ prop: 'value' }) // assert - expect(onCapture).toBeCalledTimes(0) + expect(beforeSendMock).toBeCalledTimes(0) expect(jest.mocked(logger).error).toBeCalledTimes(1) expect(jest.mocked(logger).error).toHaveBeenCalledWith( 'posthog.setPersonProperties was called, but process_person is set to "never". This call will be ignored.' @@ -457,19 +457,19 @@ describe('person processing', () => { it("should send a $set event if process_person is set to 'always'", async () => { // arrange - const { posthog, onCapture } = await setup('always') + const { posthog, beforeSendMock } = await setup('always') // act posthog.setPersonProperties({ prop: 'value' }) // assert - expect(onCapture).toBeCalledTimes(1) - expect(onCapture.mock.calls[0][0]).toEqual('$set') + expect(beforeSendMock).toBeCalledTimes(1) + expect(beforeSendMock.mock.calls[0][0].event).toEqual('$set') }) it('should start person processing for identified_only users', async () => { // arrange - const { posthog, onCapture } = await setup('identified_only') + const { posthog, beforeSendMock } = await setup('identified_only') // act posthog.capture('custom event before setPersonProperties') @@ -477,20 +477,20 @@ describe('person processing', () => { posthog.capture('custom event after setPersonProperties') // assert - const eventBeforeGroup = onCapture.mock.calls[0] - expect(eventBeforeGroup[1].properties.$process_person_profile).toEqual(false) - const set = onCapture.mock.calls[1] - expect(set[0]).toEqual('$set') - expect(set[1].properties.$process_person_profile).toEqual(true) - const eventAfterGroup = onCapture.mock.calls[2] - expect(eventAfterGroup[1].properties.$process_person_profile).toEqual(true) + const eventBeforeGroup = beforeSendMock.mock.calls[0] + expect(eventBeforeGroup[0].properties.$process_person_profile).toEqual(false) + const set = beforeSendMock.mock.calls[1] + expect(set[0].event).toEqual('$set') + expect(set[0].properties.$process_person_profile).toEqual(true) + const eventAfterGroup = beforeSendMock.mock.calls[2] + expect(eventAfterGroup[0].properties.$process_person_profile).toEqual(true) }) }) describe('alias', () => { it('should start person processing for identified_only users', async () => { // arrange - const { posthog, onCapture } = await setup('identified_only') + const { posthog, beforeSendMock } = await setup('identified_only') // act posthog.capture('custom event before alias') @@ -498,24 +498,24 @@ describe('person processing', () => { posthog.capture('custom event after alias') // assert - const eventBeforeGroup = onCapture.mock.calls[0] - expect(eventBeforeGroup[1].properties.$process_person_profile).toEqual(false) - const alias = onCapture.mock.calls[1] - expect(alias[0]).toEqual('$create_alias') - expect(alias[1].properties.$process_person_profile).toEqual(true) - const eventAfterGroup = onCapture.mock.calls[2] - expect(eventAfterGroup[1].properties.$process_person_profile).toEqual(true) + const eventBeforeGroup = beforeSendMock.mock.calls[0] + expect(eventBeforeGroup[0].properties.$process_person_profile).toEqual(false) + const alias = beforeSendMock.mock.calls[1] + expect(alias[0].event).toEqual('$create_alias') + expect(alias[0].properties.$process_person_profile).toEqual(true) + const eventAfterGroup = beforeSendMock.mock.calls[2] + expect(eventAfterGroup[0].properties.$process_person_profile).toEqual(true) }) it('should not send a $create_alias event if person processing is set to "never"', async () => { // arrange - const { posthog, onCapture } = await setup('never') + const { posthog, beforeSendMock } = await setup('never') // act posthog.alias('alias') // assert - expect(onCapture).toBeCalledTimes(0) + expect(beforeSendMock).toBeCalledTimes(0) expect(jest.mocked(logger).error).toBeCalledTimes(1) expect(jest.mocked(logger).error).toHaveBeenCalledWith( 'posthog.alias was called, but process_person is set to "never". This call will be ignored.' @@ -526,7 +526,7 @@ describe('person processing', () => { describe('createPersonProfile', () => { it('should start person processing for identified_only users', async () => { // arrange - const { posthog, onCapture } = await setup('identified_only') + const { posthog, beforeSendMock } = await setup('identified_only') // act posthog.capture('custom event before createPersonProfile') @@ -534,19 +534,19 @@ describe('person processing', () => { posthog.capture('custom event after createPersonProfile') // assert - expect(onCapture.mock.calls.length).toEqual(3) - const eventBeforeGroup = onCapture.mock.calls[0] - expect(eventBeforeGroup[1].properties.$process_person_profile).toEqual(false) - const set = onCapture.mock.calls[1] - expect(set[0]).toEqual('$set') - expect(set[1].properties.$process_person_profile).toEqual(true) - const eventAfterGroup = onCapture.mock.calls[2] - expect(eventAfterGroup[1].properties.$process_person_profile).toEqual(true) + expect(beforeSendMock.mock.calls.length).toEqual(3) + const eventBeforeGroup = beforeSendMock.mock.calls[0] + expect(eventBeforeGroup[0].properties.$process_person_profile).toEqual(false) + const set = beforeSendMock.mock.calls[1] + expect(set[0].event).toEqual('$set') + expect(set[0].properties.$process_person_profile).toEqual(true) + const eventAfterGroup = beforeSendMock.mock.calls[2] + expect(eventAfterGroup[0].properties.$process_person_profile).toEqual(true) }) it('should do nothing if already has person profiles', async () => { // arrange - const { posthog, onCapture } = await setup('identified_only') + const { posthog, beforeSendMock } = await setup('identified_only') // act posthog.capture('custom event before createPersonProfile') @@ -555,18 +555,18 @@ describe('person processing', () => { posthog.createPersonProfile() // assert - expect(onCapture.mock.calls.length).toEqual(3) + expect(beforeSendMock.mock.calls.length).toEqual(3) }) it("should not send an event if process_person is to set to 'always'", async () => { // arrange - const { posthog, onCapture } = await setup('always') + const { posthog, beforeSendMock } = await setup('always') // act posthog.createPersonProfile() // assert - expect(onCapture).toBeCalledTimes(0) + expect(beforeSendMock).toBeCalledTimes(0) expect(jest.mocked(logger).error).toBeCalledTimes(0) }) }) @@ -574,7 +574,7 @@ describe('person processing', () => { describe('reset', () => { it('should revert a back to anonymous state in identified_only', async () => { // arrange - const { posthog, onCapture } = await setup('identified_only') + const { posthog, beforeSendMock } = await setup('identified_only') posthog.identify(distinctId) posthog.capture('custom event before reset') @@ -584,8 +584,8 @@ describe('person processing', () => { // assert expect(posthog._isIdentified()).toBe(false) - expect(onCapture.mock.calls.length).toEqual(3) - expect(onCapture.mock.calls[2][1].properties.$process_person_profile).toEqual(false) + expect(beforeSendMock.mock.calls.length).toEqual(3) + expect(beforeSendMock.mock.calls[2][0].properties.$process_person_profile).toEqual(false) }) }) @@ -593,9 +593,13 @@ describe('person processing', () => { it('should remember that a user set the mode to always on a previous visit', async () => { // arrange const persistenceName = uuidv7() - const { posthog: posthog1, onCapture: onCapture1 } = await setup('always', undefined, persistenceName) + const { posthog: posthog1, beforeSendMock: beforeSendMock1 } = await setup( + 'always', + undefined, + persistenceName + ) posthog1.capture('custom event 1') - const { posthog: posthog2, onCapture: onCapture2 } = await setup( + const { posthog: posthog2, beforeSendMock: beforeSendMock2 } = await setup( 'identified_only', undefined, persistenceName @@ -605,38 +609,42 @@ describe('person processing', () => { posthog2.capture('custom event 2') // assert - expect(onCapture1.mock.calls.length).toEqual(1) - expect(onCapture2.mock.calls.length).toEqual(1) - expect(onCapture1.mock.calls[0][1].properties.$process_person_profile).toEqual(true) - expect(onCapture2.mock.calls[0][1].properties.$process_person_profile).toEqual(true) + expect(beforeSendMock1.mock.calls.length).toEqual(1) + expect(beforeSendMock2.mock.calls.length).toEqual(1) + expect(beforeSendMock1.mock.calls[0][0].properties.$process_person_profile).toEqual(true) + expect(beforeSendMock2.mock.calls[0][0].properties.$process_person_profile).toEqual(true) }) it('should work when always is set on a later visit', async () => { // arrange const persistenceName = uuidv7() - const { posthog: posthog1, onCapture: onCapture1 } = await setup( + const { posthog: posthog1, beforeSendMock: beforeSendMock1 } = await setup( 'identified_only', undefined, persistenceName ) posthog1.capture('custom event 1') - const { posthog: posthog2, onCapture: onCapture2 } = await setup('always', undefined, persistenceName) + const { posthog: posthog2, beforeSendMock: beforeSendMock2 } = await setup( + 'always', + undefined, + persistenceName + ) // act posthog2.capture('custom event 2') // assert - expect(onCapture1.mock.calls.length).toEqual(1) - expect(onCapture2.mock.calls.length).toEqual(1) - expect(onCapture1.mock.calls[0][1].properties.$process_person_profile).toEqual(false) - expect(onCapture2.mock.calls[0][1].properties.$process_person_profile).toEqual(true) + expect(beforeSendMock1.mock.calls.length).toEqual(1) + expect(beforeSendMock2.mock.calls.length).toEqual(1) + expect(beforeSendMock1.mock.calls[0][0].properties.$process_person_profile).toEqual(false) + expect(beforeSendMock2.mock.calls[0][0].properties.$process_person_profile).toEqual(true) }) }) describe('decide', () => { it('should change the person mode from default when decide response is handled', async () => { // arrange - const { posthog, onCapture } = await setup(undefined) + const { posthog, beforeSendMock } = await setup(undefined) posthog.capture('startup page view') // act @@ -644,14 +652,14 @@ describe('person processing', () => { posthog.capture('custom event') // assert - expect(onCapture.mock.calls.length).toEqual(2) - expect(onCapture.mock.calls[0][1].properties.$process_person_profile).toEqual(false) - expect(onCapture.mock.calls[1][1].properties.$process_person_profile).toEqual(true) + expect(beforeSendMock.mock.calls.length).toEqual(2) + expect(beforeSendMock.mock.calls[0][0].properties.$process_person_profile).toEqual(false) + expect(beforeSendMock.mock.calls[1][0].properties.$process_person_profile).toEqual(true) }) it('should NOT change the person mode from user-defined when decide response is handled', async () => { // arrange - const { posthog, onCapture } = await setup('identified_only') + const { posthog, beforeSendMock } = await setup('identified_only') posthog.capture('startup page view') // act @@ -659,27 +667,35 @@ describe('person processing', () => { posthog.capture('custom event') // assert - expect(onCapture.mock.calls.length).toEqual(2) - expect(onCapture.mock.calls[0][1].properties.$process_person_profile).toEqual(false) - expect(onCapture.mock.calls[1][1].properties.$process_person_profile).toEqual(false) + expect(beforeSendMock.mock.calls.length).toEqual(2) + expect(beforeSendMock.mock.calls[0][0].properties.$process_person_profile).toEqual(false) + expect(beforeSendMock.mock.calls[1][0].properties.$process_person_profile).toEqual(false) }) it('should persist when the default person mode is overridden by decide', async () => { // arrange const persistenceName = uuidv7() - const { posthog: posthog1, onCapture: onCapture1 } = await setup(undefined, undefined, persistenceName) + const { posthog: posthog1, beforeSendMock: beforeSendMock1 } = await setup( + undefined, + undefined, + persistenceName + ) // act posthog1._afterDecideResponse({ defaultIdentifiedOnly: false } as DecideResponse) posthog1.capture('custom event 1') - const { posthog: posthog2, onCapture: onCapture2 } = await setup(undefined, undefined, persistenceName) + const { posthog: posthog2, beforeSendMock: beforeSendMock2 } = await setup( + undefined, + undefined, + persistenceName + ) posthog2.capture('custom event 2') // assert - expect(onCapture1.mock.calls.length).toEqual(1) - expect(onCapture2.mock.calls.length).toEqual(1) - expect(onCapture1.mock.calls[0][1].properties.$process_person_profile).toEqual(true) - expect(onCapture2.mock.calls[0][1].properties.$process_person_profile).toEqual(true) + expect(beforeSendMock1.mock.calls.length).toEqual(1) + expect(beforeSendMock2.mock.calls.length).toEqual(1) + expect(beforeSendMock1.mock.calls[0][0].properties.$process_person_profile).toEqual(true) + expect(beforeSendMock2.mock.calls[0][0].properties.$process_person_profile).toEqual(true) }) }) }) diff --git a/src/__tests__/posthog-core.identify.test.ts b/src/__tests__/posthog-core.identify.test.ts index 0d696480f..96beace88 100644 --- a/src/__tests__/posthog-core.identify.test.ts +++ b/src/__tests__/posthog-core.identify.test.ts @@ -7,15 +7,15 @@ jest.mock('../decide') describe('identify()', () => { let instance: PostHog - let captureMock: jest.Mock + let beforeSendMock: jest.Mock beforeEach(() => { - captureMock = jest.fn() + beforeSendMock = jest.fn().mockImplementation((e) => e) const posthog = defaultPostHog().init( uuidv7(), { api_host: 'https://test.com', - _onCapture: captureMock, + before_send: beforeSendMock, }, uuidv7() ) @@ -46,9 +46,9 @@ describe('identify()', () => { instance.identify('calls capture when identity changes') - expect(captureMock).toHaveBeenCalledWith( - '$identify', + expect(beforeSendMock).toHaveBeenCalledWith( expect.objectContaining({ + event: '$identify', properties: expect.objectContaining({ distinct_id: 'calls capture when identity changes', $anon_distinct_id: 'oldIdentity', @@ -72,9 +72,9 @@ describe('identify()', () => { instance.identify('a-new-id') - expect(captureMock).toHaveBeenCalledWith( - '$identify', + expect(beforeSendMock).toHaveBeenCalledWith( expect.objectContaining({ + event: '$identify', properties: expect.objectContaining({ distinct_id: 'a-new-id', $anon_distinct_id: 'oldIdentity', @@ -91,9 +91,9 @@ describe('identify()', () => { instance.identify('a-new-id') - expect(captureMock).toHaveBeenCalledWith( - '$identify', + expect(beforeSendMock).toHaveBeenCalledWith( expect.objectContaining({ + event: '$identify', properties: expect.objectContaining({ distinct_id: 'a-new-id', $anon_distinct_id: 'oldIdentity', @@ -115,7 +115,7 @@ describe('identify()', () => { instance.identify('a-new-id') - expect(captureMock).not.toHaveBeenCalled() + expect(beforeSendMock).not.toHaveBeenCalled() expect(instance.featureFlags.setAnonymousDistinctId).not.toHaveBeenCalled() }) @@ -130,7 +130,7 @@ describe('identify()', () => { instance.identify('a-new-id') - expect(captureMock).not.toHaveBeenCalled() + expect(beforeSendMock).not.toHaveBeenCalled() expect(instance.featureFlags.setAnonymousDistinctId).not.toHaveBeenCalled() }) @@ -141,9 +141,9 @@ describe('identify()', () => { instance.identify('a-new-id') - expect(captureMock).toHaveBeenCalledWith( - '$identify', + expect(beforeSendMock).toHaveBeenCalledWith( expect.objectContaining({ + event: '$identify', properties: expect.objectContaining({ distinct_id: 'a-new-id', $anon_distinct_id: 'oldIdentity', @@ -155,9 +155,9 @@ describe('identify()', () => { it('calls capture with user properties if passed', () => { instance.identify('a-new-id', { email: 'john@example.com' }, { howOftenAmISet: 'once!' }) - expect(captureMock).toHaveBeenCalledWith( - '$identify', + expect(beforeSendMock).toHaveBeenCalledWith( expect.objectContaining({ + event: '$identify', properties: expect.objectContaining({ distinct_id: 'a-new-id', $anon_distinct_id: 'oldIdentity', @@ -178,16 +178,16 @@ describe('identify()', () => { it('does not capture or set user properties', () => { instance.identify('a-new-id') - expect(captureMock).not.toHaveBeenCalled() + expect(beforeSendMock).not.toHaveBeenCalled() expect(instance.featureFlags.setAnonymousDistinctId).not.toHaveBeenCalled() }) it('calls $set when user properties passed with same ID', () => { instance.identify('a-new-id', { email: 'john@example.com' }, { howOftenAmISet: 'once!' }) - expect(captureMock).toHaveBeenCalledWith( - '$set', + expect(beforeSendMock).toHaveBeenCalledWith( expect.objectContaining({ + event: '$set', // get set at the top level and in properties // $set: { email: 'john@example.com' }, // $set_once: expect.objectContaining({ howOftenAmISet: 'once!' }), @@ -209,7 +209,7 @@ describe('identify()', () => { instance.identify(null as unknown as string) - expect(captureMock).not.toHaveBeenCalled() + expect(beforeSendMock).not.toHaveBeenCalled() expect(instance.register).not.toHaveBeenCalled() expect(console.error).toHaveBeenCalledWith( '[PostHog.js]', @@ -274,9 +274,9 @@ describe('identify()', () => { it('captures a $set event', () => { instance.setPersonProperties({ email: 'john@example.com' }, { name: 'john' }) - expect(captureMock).toHaveBeenCalledWith( - '$set', + expect(beforeSendMock).toHaveBeenCalledWith( expect.objectContaining({ + event: '$set', // get set at the top level and in properties // $set: { email: 'john@example.com' }, // $set_once: expect.objectContaining({ name: 'john' }), @@ -291,9 +291,9 @@ describe('identify()', () => { it('calls proxies prople.set to setPersonProperties', () => { instance.people.set({ email: 'john@example.com' }) - expect(captureMock).toHaveBeenCalledWith( - '$set', + expect(beforeSendMock).toHaveBeenCalledWith( expect.objectContaining({ + event: '$set', properties: expect.objectContaining({ $set: { email: 'john@example.com' }, $set_once: {}, @@ -306,9 +306,9 @@ describe('identify()', () => { it('calls proxies prople.set_once to setPersonProperties', () => { instance.people.set_once({ email: 'john@example.com' }) - expect(captureMock).toHaveBeenCalledWith( - '$set', + expect(beforeSendMock).toHaveBeenCalledWith( expect.objectContaining({ + event: '$set', properties: expect.objectContaining({ $set: {}, $set_once: { email: 'john@example.com' }, diff --git a/src/__tests__/posthog-core.test.ts b/src/__tests__/posthog-core.test.ts index af1ca4b01..20c2c14a3 100644 --- a/src/__tests__/posthog-core.test.ts +++ b/src/__tests__/posthog-core.test.ts @@ -45,10 +45,10 @@ describe('posthog core', () => { event: 'prop', } const setup = (config: Partial = {}, token: string = uuidv7()) => { - const onCapture = jest.fn() - const posthog = defaultPostHog().init(token, { ...config, _onCapture: onCapture }, token)! + const beforeSendMock = jest.fn().mockImplementation((e) => e) + const posthog = defaultPostHog().init(token, { ...config, before_send: beforeSendMock }, token)! posthog.debug() - return { posthog, onCapture } + return { posthog, beforeSendMock } } it('respects property_denylist and property_blacklist', () => { @@ -71,11 +71,11 @@ describe('posthog core', () => { describe('rate limiting', () => { it('includes information about remaining rate limit', () => { - const { posthog, onCapture } = setup() + const { posthog, beforeSendMock } = setup() posthog.capture(eventName, eventProperties) - expect(onCapture.mock.calls[0][1]).toMatchObject({ + expect(beforeSendMock.mock.calls[0][0]).toMatchObject({ properties: { $lib_rate_limit_remaining_tokens: 99, }, @@ -87,18 +87,18 @@ describe('posthog core', () => { jest.setSystemTime(Date.now()) console.error = jest.fn() - const { posthog, onCapture } = setup() + const { posthog, beforeSendMock } = setup() for (let i = 0; i < 100; i++) { posthog.capture(eventName, eventProperties) } - expect(onCapture).toHaveBeenCalledTimes(100) - onCapture.mockClear() + expect(beforeSendMock).toHaveBeenCalledTimes(100) + beforeSendMock.mockClear() ;(console.error as any).mockClear() for (let i = 0; i < 50; i++) { posthog.capture(eventName, eventProperties) } - expect(onCapture).toHaveBeenCalledTimes(1) - expect(onCapture.mock.calls[0][0]).toBe('$$client_ingestion_warning') + expect(beforeSendMock).toHaveBeenCalledTimes(1) + expect(beforeSendMock.mock.calls[0][0].event).toBe('$$client_ingestion_warning') expect(console.error).toHaveBeenCalledTimes(50) expect(console.error).toHaveBeenCalledWith( '[PostHog.js]', @@ -112,7 +112,7 @@ describe('posthog core', () => { // arrange const token = uuidv7() mockReferrerGetter.mockReturnValue('https://referrer.example.com/some/path') - const { posthog, onCapture } = setup({ + const { posthog, beforeSendMock } = setup({ token, persistence_name: token, person_profiles: 'always', @@ -122,7 +122,7 @@ describe('posthog core', () => { posthog.capture(eventName, eventProperties) // assert - const { $set_once, properties } = onCapture.mock.calls[0][1] + const { $set_once, properties } = beforeSendMock.mock.calls[0][0] expect($set_once['$initial_referrer']).toBe('https://referrer.example.com/some/path') expect($set_once['$initial_referring_domain']).toBe('referrer.example.com') expect(properties['$referrer']).toBe('https://referrer.example.com/some/path') @@ -140,7 +140,7 @@ describe('posthog core', () => { }) posthog1.capture(eventName, eventProperties) mockReferrerGetter.mockReturnValue('https://referrer2.example.com/some/path') - const { posthog: posthog2, onCapture: onCapture2 } = setup({ + const { posthog: posthog2, beforeSendMock } = setup({ token, persistence_name: token, }) @@ -153,7 +153,7 @@ describe('posthog core', () => { 'https://referrer1.example.com/some/path' ) expect(posthog2.sessionPersistence!.props.$referrer).toEqual('https://referrer1.example.com/some/path') - const { $set_once, properties } = onCapture2.mock.calls[0][1] + const { $set_once, properties } = beforeSendMock.mock.calls[0][0] expect($set_once['$initial_referrer']).toBe('https://referrer1.example.com/some/path') expect($set_once['$initial_referring_domain']).toBe('referrer1.example.com') expect(properties['$referrer']).toBe('https://referrer1.example.com/some/path') @@ -171,7 +171,7 @@ describe('posthog core', () => { }) posthog1.capture(eventName, eventProperties) mockReferrerGetter.mockReturnValue('https://referrer2.example.com/some/path') - const { posthog: posthog2, onCapture: onCapture2 } = setup({ + const { posthog: posthog2, beforeSendMock: beforeSendMock2 } = setup({ token, persistence_name: token, }) @@ -184,7 +184,7 @@ describe('posthog core', () => { expect(posthog2.persistence!.props.$initial_person_info.r).toEqual( 'https://referrer1.example.com/some/path' ) - const { $set_once, properties } = onCapture2.mock.calls[0][1] + const { $set_once, properties } = beforeSendMock2.mock.calls[0][0] expect($set_once['$initial_referrer']).toBe('https://referrer1.example.com/some/path') expect($set_once['$initial_referring_domain']).toBe('referrer1.example.com') expect(properties['$referrer']).toBe('https://referrer2.example.com/some/path') @@ -195,7 +195,7 @@ describe('posthog core', () => { // arrange const token = uuidv7() mockReferrerGetter.mockReturnValue('') - const { posthog, onCapture } = setup({ + const { posthog, beforeSendMock } = setup({ token, persistence_name: token, person_profiles: 'always', @@ -205,7 +205,7 @@ describe('posthog core', () => { posthog.capture(eventName, eventProperties) // assert - const { $set_once, properties } = onCapture.mock.calls[0][1] + const { $set_once, properties } = beforeSendMock.mock.calls[0][0] expect($set_once['$initial_referrer']).toBe('$direct') expect($set_once['$initial_referring_domain']).toBe('$direct') expect(properties['$referrer']).toBe('$direct') @@ -218,7 +218,7 @@ describe('posthog core', () => { // arrange const token = uuidv7() mockURLGetter.mockReturnValue('https://www.example.com/some/path') - const { posthog, onCapture } = setup({ + const { posthog, beforeSendMock } = setup({ token, persistence_name: token, }) @@ -227,15 +227,15 @@ describe('posthog core', () => { posthog.capture('$pageview') //assert - expect(onCapture.mock.calls[0][1].properties).not.toHaveProperty('utm_source') - expect(onCapture.mock.calls[0][1].properties).not.toHaveProperty('utm_medium') + expect(beforeSendMock.mock.calls[0][0].properties).not.toHaveProperty('utm_source') + expect(beforeSendMock.mock.calls[0][0].properties).not.toHaveProperty('utm_medium') }) it('should send present campaign params, and nulls for others', () => { // arrange const token = uuidv7() mockURLGetter.mockReturnValue('https://www.example.com/some/path?utm_source=source') - const { posthog, onCapture } = setup({ + const { posthog, beforeSendMock } = setup({ token, persistence_name: token, }) @@ -244,8 +244,8 @@ describe('posthog core', () => { posthog.capture('$pageview') //assert - expect(onCapture.mock.calls[0][1].properties.utm_source).toBe('source') - expect(onCapture.mock.calls[0][1].properties.utm_medium).toBe(null) + expect(beforeSendMock.mock.calls[0][0].properties.utm_source).toBe('source') + expect(beforeSendMock.mock.calls[0][0].properties.utm_medium).toBe(null) }) }) }) diff --git a/src/__tests__/posthog-core.ts b/src/__tests__/posthog-core.ts index a64fd0d46..7f37eec5d 100644 --- a/src/__tests__/posthog-core.ts +++ b/src/__tests__/posthog-core.ts @@ -96,7 +96,6 @@ describe('posthog core', () => { { property_denylist: [], property_blacklist: [], - _onCapture: jest.fn(), store_google: true, save_referrer: true, }, @@ -168,13 +167,12 @@ describe('posthog core', () => { 'Mozilla/5.0 AppleWebKit/537.36 (KHTML, like Gecko; compatible; Googlebot/2.1; +http://www.google.com/bot.html) Chrome/W.X.Y.Z Safari/537.36', } - const hook = jest.fn() + const hook = jest.fn().mockImplementation((event) => event) const posthog = posthogWith( { opt_out_useragent_filter: true, property_denylist: [], property_blacklist: [], - _onCapture: jest.fn(), }, defaultOverrides ) @@ -198,7 +196,6 @@ describe('posthog core', () => { properties_string_max_length: 1000, property_denylist: [], property_blacklist: [], - _onCapture: jest.fn(), }, defaultOverrides ) @@ -220,7 +217,6 @@ describe('posthog core', () => { properties_string_max_length: undefined, property_denylist: [], property_blacklist: [], - _onCapture: jest.fn(), }, defaultOverrides ) @@ -269,7 +265,6 @@ describe('posthog core', () => { { property_denylist: [], property_blacklist: [], - _onCapture: jest.fn(), }, defaultOverrides ) diff --git a/src/posthog-core.ts b/src/posthog-core.ts index 25b0b18f6..621acadfc 100644 --- a/src/posthog-core.ts +++ b/src/posthog-core.ts @@ -533,7 +533,8 @@ export class PostHog { this._loaded() } - if (isFunction(this.config._onCapture)) { + if (isFunction(this.config._onCapture) && this.config._onCapture !== __NOOP) { + logger.warn('onCapture is deprecated. Please use `before_send` instead') this.on('eventCaptured', (data) => this.config._onCapture(data.event, data)) } diff --git a/src/types.ts b/src/types.ts index 7858a8af5..2d1150cfa 100644 --- a/src/types.ts +++ b/src/types.ts @@ -267,6 +267,7 @@ export interface PostHogConfig { name: string /** * this is a read-only function that can be used to react to event capture + * @deprecated - use `before_send` instead - NB before_send is not read only */ _onCapture: (eventName: string, eventData: CaptureResult) => void /** diff --git a/testcafe/helpers.js b/testcafe/helpers.js index 5f3312298..a1858ffe9 100644 --- a/testcafe/helpers.js +++ b/testcafe/helpers.js @@ -77,7 +77,7 @@ export const initPosthog = (testName, config) => { window.loaded = true window.fullCaptures = [] } - clientPosthogConfig._onCapture = (_, event) => { + clientPosthogConfig.before_send = (event) => { window.fullCaptures.push(event) } window.posthog.init(clientPosthogConfig.api_key, clientPosthogConfig) From 0cdbd192cfb6580499eb7aee83f8688dcdb4865a Mon Sep 17 00:00:00 2001 From: Paul D'Ambra Date: Wed, 13 Nov 2024 14:40:07 +0000 Subject: [PATCH 21/28] oops --- testcafe/helpers.js | 1 + 1 file changed, 1 insertion(+) diff --git a/testcafe/helpers.js b/testcafe/helpers.js index a1858ffe9..b1f2613cc 100644 --- a/testcafe/helpers.js +++ b/testcafe/helpers.js @@ -79,6 +79,7 @@ export const initPosthog = (testName, config) => { } clientPosthogConfig.before_send = (event) => { window.fullCaptures.push(event) + return event } window.posthog.init(clientPosthogConfig.api_key, clientPosthogConfig) window.posthog.register(register) From aa6df592896a8d4489601612987261b0d9f14865 Mon Sep 17 00:00:00 2001 From: Paul D'Ambra Date: Wed, 13 Nov 2024 15:10:07 +0000 Subject: [PATCH 22/28] ooops --- cypress/support/commands.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/cypress/support/commands.ts b/cypress/support/commands.ts index 7bcaaa75f..fd85d005e 100644 --- a/cypress/support/commands.ts +++ b/cypress/support/commands.ts @@ -12,6 +12,7 @@ Cypress.Commands.add('posthogInit', (options) => { before_send: (event) => { $captures.push(event.event) $fullCaptures.push(event) + return event }, opt_out_useragent_filter: true, ...options, From 8c76336c9cb46237ff061fcdd865f79cf5365b28 Mon Sep 17 00:00:00 2001 From: Paul D'Ambra Date: Fri, 15 Nov 2024 11:10:25 +0000 Subject: [PATCH 23/28] sampling example changes --- src/__tests__/utils/before-send-utils.test.ts | 22 +++++++-- src/customizations/before-send.ts | 49 +++++++++++++++---- 2 files changed, 58 insertions(+), 13 deletions(-) diff --git a/src/__tests__/utils/before-send-utils.test.ts b/src/__tests__/utils/before-send-utils.test.ts index f2a1c1d07..e68b1e54d 100644 --- a/src/__tests__/utils/before-send-utils.test.ts +++ b/src/__tests__/utils/before-send-utils.test.ts @@ -13,7 +13,7 @@ beforeAll(() => { describe('before send utils', () => { it('can sample by event name', () => { - const sampleFn = sampleByEvent(['$autocapture'], 50) + const sampleFn = sampleByEvent(['$autocapture'], 0.5) const results = [] Array.from({ length: 100 }).forEach(() => { @@ -22,12 +22,16 @@ describe('before send utils', () => { }) const emittedEvents = results.filter((r) => !isNull(r)) - // random is mocked so that it alternates between 0.48 and 0.51 expect(emittedEvents.length).toBe(50) + expect(emittedEvents[0].properties).toMatchObject({ + $sample_type: 'sampleByEvent', + $sample_rate: 0.5, + $sampled_events: ['$autocapture'], + }) }) it('can sample by distinct id', () => { - const sampleFn = sampleByDistinctId(50) + const sampleFn = sampleByDistinctId(0.5) const results = [] const distinct_id_one = 'user-1' const distinct_id_two = 'user-that-hashes-to-no-events' @@ -42,10 +46,15 @@ describe('before send utils', () => { expect(distinctIdOneEvents.length).toBe(100) expect(distinctIdTwoEvents.length).toBe(0) + + expect(distinctIdOneEvents[0].properties).toMatchObject({ + $sample_type: 'sampleByDistinctId', + $sample_rate: 0.5, + }) }) it('can sample by session id', () => { - const sampleFn = sampleBySessionId(50) + const sampleFn = sampleBySessionId(0.5) const results = [] const session_id_one = 'a-session-id' const session_id_two = 'id-that-hashes-to-not-sending-events' @@ -60,5 +69,10 @@ describe('before send utils', () => { expect(sessionIdOneEvents.length).toBe(100) expect(sessionIdTwoEvents.length).toBe(0) + + expect(sessionIdOneEvents[0].properties).toMatchObject({ + $sample_type: 'sampleBySessionId', + $sample_rate: 0.5, + }) }) }) diff --git a/src/customizations/before-send.ts b/src/customizations/before-send.ts index dd935e8a2..79b75037e 100644 --- a/src/customizations/before-send.ts +++ b/src/customizations/before-send.ts @@ -11,8 +11,11 @@ function simpleHash(str: string) { return Math.abs(hash) } +/* + * receives percent as a number between 0 and 1 + */ function sampleOnProperty(prop: string, percent: number): boolean { - return simpleHash(prop) % 100 < clampToRange(percent, 0, 100) + return simpleHash(prop) % 100 < clampToRange(percent * 100, 0, 100) } /** @@ -20,14 +23,23 @@ function sampleOnProperty(prop: string, percent: number): boolean { * Using the provided percentage. * Can be used to create a beforeCapture fn for a PostHog instance. * - * Causes roughly 50% of distinct ids to have events sent. + * Setting 0.5 will cause roughly 50% of distinct ids to have events sent. * Not 50% of events for each distinct id. * - * @param percent a number from 0 to 100, 100 means never sample, 0 means never send the event + * @param percent a number from 0 to 1, 1 means always send and, 0 means never send the event */ export function sampleByDistinctId(percent: number): (c: CaptureResult) => CaptureResult | null { return (captureResult: CaptureResult): CaptureResult | null => { - return sampleOnProperty(captureResult.properties.distinct_id, percent) ? captureResult : null + return sampleOnProperty(captureResult.properties.distinct_id, percent) + ? { + ...captureResult, + properties: { + ...captureResult.properties, + $sample_type: 'sampleByDistinctId', + $sample_rate: percent, + }, + } + : null } } @@ -36,14 +48,23 @@ export function sampleByDistinctId(percent: number): (c: CaptureResult) => Captu * Using the provided percentage. * Can be used to create a beforeCapture fn for a PostHog instance. * - * Causes roughly 50% of sessions to have events sent. + * Setting 0.5 will cause roughly 50% of sessions to have events sent. * Not 50% of events for each session. * - * @param percent a number from 0 to 100, 100 means never sample, 0 means never send the event + * @param percent a number from 0 to 1, 1 means always send and, 0 means never send the event */ export function sampleBySessionId(percent: number): (c: CaptureResult) => CaptureResult | null { return (captureResult: CaptureResult): CaptureResult | null => { - return sampleOnProperty(captureResult.properties.$session_id, percent) ? captureResult : null + return sampleOnProperty(captureResult.properties.$session_id, percent) + ? { + ...captureResult, + properties: { + ...captureResult.properties, + $sample_type: 'sampleBySessionId', + $sample_rate: percent, + }, + } + : null } } @@ -53,7 +74,7 @@ export function sampleBySessionId(percent: number): (c: CaptureResult) => Captur * Can be used to create a beforeCapture fn for a PostHog instance. * * @param eventNames an array of event names to sample, sampling is applied across events not per event name - * @param percent a number from 0 to 100, 100 means never sample, 0 means never send the event + * @param percent a number from 0 to 1, 1 means always send, 0 means never send the event */ export function sampleByEvent( eventNames: KnownEventName[], @@ -64,6 +85,16 @@ export function sampleByEvent( return captureResult } - return Math.random() * 100 < clampToRange(percent, 0, 100) ? captureResult : null + return Math.random() * 100 < clampToRange(percent * 100, 0, 100) + ? { + ...captureResult, + properties: { + ...captureResult.properties, + $sample_type: 'sampleByEvent', + $sample_rate: percent, + $sampled_events: eventNames, + }, + } + : null } } From 3a4b136a96b8f7c4c2dfc0f5542e95c68b82bef1 Mon Sep 17 00:00:00 2001 From: Paul D'Ambra Date: Fri, 15 Nov 2024 11:12:09 +0000 Subject: [PATCH 24/28] make before_send config explicit --- src/posthog-core.ts | 35 ++++++++++++++++++----------------- src/types.ts | 2 +- 2 files changed, 19 insertions(+), 18 deletions(-) diff --git a/src/posthog-core.ts b/src/posthog-core.ts index 0cd7642de..8bc8cca0c 100644 --- a/src/posthog-core.ts +++ b/src/posthog-core.ts @@ -183,8 +183,7 @@ export const defaultConfig = (): PostHogConfig => ({ session_idle_timeout_seconds: 30 * 60, // 30 minutes person_profiles: 'identified_only', __add_tracing_headers: false, - // by default the before capture fn is the identity function - before_send: (x) => x, + before_send: undefined, }) export const configRenames = (origConfig: Partial): Partial => { @@ -879,23 +878,25 @@ export class PostHog { this.setPersonPropertiesForFlags(finalSet) } - const beforeSendResult = this.config.before_send(data) - if (isNullish(beforeSendResult)) { - const logMessage = `Event '${data.event}' was rejected in beforeSend function` - if (isKnownUnsafeEditableEvent(data.event)) { - logger.warn(`${logMessage}. This can cause unexpected behavior.`) + if (isFunction(this.config.before_send)) { + const beforeSendResult = this.config.before_send(data) + if (isNullish(beforeSendResult)) { + const logMessage = `Event '${data.event}' was rejected in beforeSend function` + if (isKnownUnsafeEditableEvent(data.event)) { + logger.warn(`${logMessage}. This can cause unexpected behavior.`) + } else { + logger.info(logMessage) + } + // the event is now null so we don't send it + return } else { - logger.info(logMessage) - } - // the event is now null so we don't send it - return - } else { - if (!beforeSendResult.properties || isEmptyObject(beforeSendResult.properties)) { - logger.warn( - `Event '${data.event}' has no properties after beforeSend function, this is likely an error.` - ) + if (!beforeSendResult.properties || isEmptyObject(beforeSendResult.properties)) { + logger.warn( + `Event '${data.event}' has no properties after beforeSend function, this is likely an error.` + ) + } + data = beforeSendResult } - data = beforeSendResult } this._internalEventEmitter.emit('eventCaptured', data) diff --git a/src/types.ts b/src/types.ts index 7042244c8..24d8c7b22 100644 --- a/src/types.ts +++ b/src/types.ts @@ -297,7 +297,7 @@ export interface PostHogConfig { * This function - if provided - is called immediately before sending data to the server. * It allows you to edit data before it is sent, or choose not to send it all. */ - before_send: (cr: CaptureResult) => CaptureResult | null + before_send?: (cr: CaptureResult) => CaptureResult | null capture_performance?: boolean | PerformanceCaptureConfig // Should only be used for testing. Could negatively impact performance. disable_compression: boolean From 371f90a33e3196636b4e2d0193cf758d715a679c Mon Sep 17 00:00:00 2001 From: Paul D'Ambra Date: Fri, 15 Nov 2024 12:06:05 +0000 Subject: [PATCH 25/28] allow array of functions --- cypress/e2e/before_send.cy.ts | 46 +++++++++++++++++++ src/__tests__/posthog-core.beforeSend.test.ts | 36 +++++++++++++++ src/posthog-core.ts | 45 ++++++++++++------ src/types.ts | 8 +++- 4 files changed, 118 insertions(+), 17 deletions(-) create mode 100644 cypress/e2e/before_send.cy.ts diff --git a/cypress/e2e/before_send.cy.ts b/cypress/e2e/before_send.cy.ts new file mode 100644 index 000000000..85eba7ad6 --- /dev/null +++ b/cypress/e2e/before_send.cy.ts @@ -0,0 +1,46 @@ +/// + +import { start } from '../support/setup' +import { isArray } from '../../src/utils/type-utils' + +describe('before_send', () => { + it('can sample and edit with before_send', () => { + start({}) + + cy.posthog().then((posthog) => { + let counter = 0 + const og = posthog.config.before_send + posthog.config.before_send = [ + (cr) => { + if (cr.event === 'custom-event') { + counter++ + if (counter === 2) { + return null + } + } + if (cr.event === '$autocapture') { + return { + ...cr, + event: 'redacted', + } + } + return cr + }, + ...(isArray(og) ? og : [og]), + ] + }) + + cy.get('[data-cy-custom-event-button]').click() + cy.get('[data-cy-custom-event-button]').click() + + cy.phCaptures().should('deep.equal', [ + // before adding the new before sendfn + '$pageview', + 'redacted', + 'custom-event', + // second button click only has the redacted autocapture event + 'redacted', + // because the second custom-event is rejected + ]) + }) +}) diff --git a/src/__tests__/posthog-core.beforeSend.test.ts b/src/__tests__/posthog-core.beforeSend.test.ts index 0c58105d8..35d0a4641 100644 --- a/src/__tests__/posthog-core.beforeSend.test.ts +++ b/src/__tests__/posthog-core.beforeSend.test.ts @@ -78,6 +78,42 @@ describe('posthog core - before send', () => { }) }) + it('can take an array of fns', () => { + const posthog = posthogWith({ + before_send: [ + (cr) => { + cr.properties = { ...cr.properties, edited_one: true } + return cr + }, + (cr) => { + if (cr.event === 'to reject') { + return null + } + return cr + }, + (cr) => { + cr.properties = { ...cr.properties, edited_two: true } + return cr + }, + ], + }) + ;(posthog._send_request as jest.Mock).mockClear() + + const capturedData = [posthog.capture(eventName, {}, {}), posthog.capture('to reject', {}, {})] + + expect(capturedData.filter((cd) => !!cd)).toHaveLength(1) + expect(capturedData[0]).toHaveProperty(['properties', 'edited_one'], true) + expect(capturedData[0]).toHaveProperty(['properties', 'edited_one'], true) + expect(posthog._send_request).toHaveBeenCalledWith({ + batchKey: undefined, + callback: expect.any(Function), + compression: 'best-available', + data: capturedData[0], + method: 'POST', + url: 'https://us.i.posthog.com/e/', + }) + }) + it('can sanitize $set event', () => { const posthog = posthogWith({ before_send: (cr) => { diff --git a/src/posthog-core.ts b/src/posthog-core.ts index 8bc8cca0c..0c03110cf 100644 --- a/src/posthog-core.ts +++ b/src/posthog-core.ts @@ -878,23 +878,11 @@ export class PostHog { this.setPersonPropertiesForFlags(finalSet) } - if (isFunction(this.config.before_send)) { - const beforeSendResult = this.config.before_send(data) - if (isNullish(beforeSendResult)) { - const logMessage = `Event '${data.event}' was rejected in beforeSend function` - if (isKnownUnsafeEditableEvent(data.event)) { - logger.warn(`${logMessage}. This can cause unexpected behavior.`) - } else { - logger.info(logMessage) - } - // the event is now null so we don't send it + if (!isNullish(this.config.before_send)) { + const beforeSendResult = this._runBeforeSend(data) + if (!beforeSendResult) { return } else { - if (!beforeSendResult.properties || isEmptyObject(beforeSendResult.properties)) { - logger.warn( - `Event '${data.event}' has no properties after beforeSend function, this is likely an error.` - ) - } data = beforeSendResult } } @@ -2190,6 +2178,33 @@ export class PostHog { this.set_config({ debug: true }) } } + + private _runBeforeSend(data: CaptureResult): CaptureResult | null { + if (isNullish(this.config.before_send)) { + return data + } + + const fns = isArray(this.config.before_send) ? this.config.before_send : [this.config.before_send] + let beforeSendResult: CaptureResult | null = data + for (const fn of fns) { + beforeSendResult = fn(beforeSendResult) + if (isNullish(beforeSendResult)) { + const logMessage = `Event '${data.event}' was rejected in beforeSend function` + if (isKnownUnsafeEditableEvent(data.event)) { + logger.warn(`${logMessage}. This can cause unexpected behavior.`) + } else { + logger.info(logMessage) + } + return null + } + if (!beforeSendResult.properties || isEmptyObject(beforeSendResult.properties)) { + logger.warn( + `Event '${data.event}' has no properties after beforeSend function, this is likely an error.` + ) + } + } + return beforeSendResult + } } safewrapClass(PostHog, ['identify']) diff --git a/src/types.ts b/src/types.ts index 24d8c7b22..d7994968a 100644 --- a/src/types.ts +++ b/src/types.ts @@ -213,6 +213,8 @@ export interface HeatmapConfig { flush_interval_milliseconds: number } +export type BeforeSendFn = (cr: CaptureResult | null) => CaptureResult | null + export interface PostHogConfig { api_host: string /** @deprecated - This property is no longer supported */ @@ -294,10 +296,12 @@ export interface PostHogConfig { */ _onCapture: (eventName: string, eventData: CaptureResult) => void /** - * This function - if provided - is called immediately before sending data to the server. + * This function or array of functions - if provided - are called immediately before sending data to the server. * It allows you to edit data before it is sent, or choose not to send it all. + * if provided as an array the functions are called in the order they are provided + * any one function returning null means the event will not be sent */ - before_send?: (cr: CaptureResult) => CaptureResult | null + before_send?: BeforeSendFn | BeforeSendFn[] capture_performance?: boolean | PerformanceCaptureConfig // Should only be used for testing. Could negatively impact performance. disable_compression: boolean From 96b23560b9baa3f49c6c80d9d379ed4e18c3aa4b Mon Sep 17 00:00:00 2001 From: Paul D'Ambra Date: Fri, 15 Nov 2024 12:06:45 +0000 Subject: [PATCH 26/28] A little comment --- cypress/e2e/before_send.cy.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/cypress/e2e/before_send.cy.ts b/cypress/e2e/before_send.cy.ts index 85eba7ad6..1502e69b4 100644 --- a/cypress/e2e/before_send.cy.ts +++ b/cypress/e2e/before_send.cy.ts @@ -10,6 +10,8 @@ describe('before_send', () => { cy.posthog().then((posthog) => { let counter = 0 const og = posthog.config.before_send + // cypress tests rely on existing before_send function to capture events + // so we have to add it back in here posthog.config.before_send = [ (cr) => { if (cr.event === 'custom-event') { From a44429ecca6757f31fde468697ac7caaf2b40b7e Mon Sep 17 00:00:00 2001 From: Paul D'Ambra Date: Fri, 15 Nov 2024 12:38:08 +0000 Subject: [PATCH 27/28] corrected thresholds tracking --- src/__tests__/utils/before-send-utils.test.ts | 42 ++++++++++++-- src/customizations/before-send.ts | 55 +++++++++++++------ 2 files changed, 73 insertions(+), 24 deletions(-) diff --git a/src/__tests__/utils/before-send-utils.test.ts b/src/__tests__/utils/before-send-utils.test.ts index e68b1e54d..fe4139ad9 100644 --- a/src/__tests__/utils/before-send-utils.test.ts +++ b/src/__tests__/utils/before-send-utils.test.ts @@ -24,8 +24,8 @@ describe('before send utils', () => { expect(emittedEvents.length).toBe(50) expect(emittedEvents[0].properties).toMatchObject({ - $sample_type: 'sampleByEvent', - $sample_rate: 0.5, + $sample_type: ['sampleByEvent'], + $sample_threshold: 0.5, $sampled_events: ['$autocapture'], }) }) @@ -48,8 +48,8 @@ describe('before send utils', () => { expect(distinctIdTwoEvents.length).toBe(0) expect(distinctIdOneEvents[0].properties).toMatchObject({ - $sample_type: 'sampleByDistinctId', - $sample_rate: 0.5, + $sample_type: ['sampleByDistinctId'], + $sample_threshold: 0.5, }) }) @@ -71,8 +71,38 @@ describe('before send utils', () => { expect(sessionIdTwoEvents.length).toBe(0) expect(sessionIdOneEvents[0].properties).toMatchObject({ - $sample_type: 'sampleBySessionId', - $sample_rate: 0.5, + $sample_type: ['sampleBySessionId'], + $sample_threshold: 0.5, + }) + }) + + it('can combine thresholds', () => { + const sampleBySession = sampleBySessionId(0.5) + const sampleByEventFn = sampleByEvent(['$autocapture'], 0.5) + + const results = [] + const session_id_one = 'a-session-id' + const session_id_two = 'id-that-hashes-to-not-sending-events' + Array.from({ length: 100 }).forEach(() => { + ;[session_id_one, session_id_two].forEach((session_id) => { + const captureResult = { + event: '$autocapture', + properties: { $session_id: session_id }, + } as unknown as CaptureResult + const firstBySession = sampleBySession(captureResult) + const thenByEvent = sampleByEventFn(firstBySession) + results.push(thenByEvent) + }) + }) + const sessionIdOneEvents = results.filter((r) => !isNull(r) && r.properties.$session_id === session_id_one) + const sessionIdTwoEvents = results.filter((r) => !isNull(r) && r.properties.$session_id === session_id_two) + + expect(sessionIdOneEvents.length).toBe(50) + expect(sessionIdTwoEvents.length).toBe(0) + + expect(sessionIdOneEvents[0].properties).toMatchObject({ + $sample_type: ['sampleBySessionId', 'sampleByEvent'], + $sample_threshold: 0.25, }) }) }) diff --git a/src/customizations/before-send.ts b/src/customizations/before-send.ts index 79b75037e..16c3da9c9 100644 --- a/src/customizations/before-send.ts +++ b/src/customizations/before-send.ts @@ -1,6 +1,7 @@ import { clampToRange } from '../utils/number-utils' -import { CaptureResult, KnownEventName } from '../types' +import { BeforeSendFn, CaptureResult, KnownEventName } from '../types' import { includes } from '../utils' +import { isArray, isUndefined } from '../utils/type-utils' function simpleHash(str: string) { let hash = 0 @@ -28,21 +29,33 @@ function sampleOnProperty(prop: string, percent: number): boolean { * * @param percent a number from 0 to 1, 1 means always send and, 0 means never send the event */ -export function sampleByDistinctId(percent: number): (c: CaptureResult) => CaptureResult | null { - return (captureResult: CaptureResult): CaptureResult | null => { +export function sampleByDistinctId(percent: number): BeforeSendFn { + return (captureResult: CaptureResult | null): CaptureResult | null => { + if (!captureResult) { + return null + } + return sampleOnProperty(captureResult.properties.distinct_id, percent) ? { ...captureResult, properties: { ...captureResult.properties, - $sample_type: 'sampleByDistinctId', - $sample_rate: percent, + $sample_type: ['sampleByDistinctId'], + $sample_threshold: percent, }, } : null } } +function appendArray(currentValue: string[] | undefined, sampleType: string | string[]): string[] { + return [...(currentValue ? currentValue : []), ...(isArray(sampleType) ? sampleType : [sampleType])] +} + +function updateThreshold(currentValue: number | undefined, percent: number): number { + return (isUndefined(currentValue) ? 1 : currentValue) * percent +} + /** * Provides an implementation of sampling that samples based on the session ID. * Using the provided percentage. @@ -53,15 +66,19 @@ export function sampleByDistinctId(percent: number): (c: CaptureResult) => Captu * * @param percent a number from 0 to 1, 1 means always send and, 0 means never send the event */ -export function sampleBySessionId(percent: number): (c: CaptureResult) => CaptureResult | null { - return (captureResult: CaptureResult): CaptureResult | null => { +export function sampleBySessionId(percent: number): BeforeSendFn { + return (captureResult: CaptureResult | null): CaptureResult | null => { + if (!captureResult) { + return null + } + return sampleOnProperty(captureResult.properties.$session_id, percent) ? { ...captureResult, properties: { ...captureResult.properties, - $sample_type: 'sampleBySessionId', - $sample_rate: percent, + $sample_type: appendArray(captureResult.properties.$sample_type, 'sampleBySessionId'), + $sample_threshold: updateThreshold(captureResult.properties.$sample_threshold, percent), }, } : null @@ -76,23 +93,25 @@ export function sampleBySessionId(percent: number): (c: CaptureResult) => Captur * @param eventNames an array of event names to sample, sampling is applied across events not per event name * @param percent a number from 0 to 1, 1 means always send, 0 means never send the event */ -export function sampleByEvent( - eventNames: KnownEventName[], - percent: number -): (c: CaptureResult) => CaptureResult | null { - return (captureResult: CaptureResult): CaptureResult | null => { +export function sampleByEvent(eventNames: KnownEventName[], percent: number): BeforeSendFn { + return (captureResult: CaptureResult | null): CaptureResult | null => { + if (!captureResult) { + return null + } + if (!includes(eventNames, captureResult.event)) { return captureResult } - return Math.random() * 100 < clampToRange(percent * 100, 0, 100) + const number = Math.random() + return number * 100 < clampToRange(percent * 100, 0, 100) ? { ...captureResult, properties: { ...captureResult.properties, - $sample_type: 'sampleByEvent', - $sample_rate: percent, - $sampled_events: eventNames, + $sample_type: appendArray(captureResult.properties?.$sample_type, 'sampleByEvent'), + $sample_threshold: updateThreshold(captureResult.properties?.$sample_threshold, percent), + $sampled_events: appendArray(captureResult.properties?.$sampled_events, eventNames), }, } : null From 482cfa8b3d433b4b0cdf56d5e4a95b9ef546d749 Mon Sep 17 00:00:00 2001 From: Paul D'Ambra Date: Fri, 15 Nov 2024 12:41:46 +0000 Subject: [PATCH 28/28] reorganise --- src/customizations/before-send.ts | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/customizations/before-send.ts b/src/customizations/before-send.ts index 16c3da9c9..0f28c4d4a 100644 --- a/src/customizations/before-send.ts +++ b/src/customizations/before-send.ts @@ -3,6 +3,14 @@ import { BeforeSendFn, CaptureResult, KnownEventName } from '../types' import { includes } from '../utils' import { isArray, isUndefined } from '../utils/type-utils' +function appendArray(currentValue: string[] | undefined, sampleType: string | string[]): string[] { + return [...(currentValue ? currentValue : []), ...(isArray(sampleType) ? sampleType : [sampleType])] +} + +function updateThreshold(currentValue: number | undefined, percent: number): number { + return (isUndefined(currentValue) ? 1 : currentValue) * percent +} + function simpleHash(str: string) { let hash = 0 for (let i = 0; i < str.length; i++) { @@ -48,14 +56,6 @@ export function sampleByDistinctId(percent: number): BeforeSendFn { } } -function appendArray(currentValue: string[] | undefined, sampleType: string | string[]): string[] { - return [...(currentValue ? currentValue : []), ...(isArray(sampleType) ? sampleType : [sampleType])] -} - -function updateThreshold(currentValue: number | undefined, percent: number): number { - return (isUndefined(currentValue) ? 1 : currentValue) * percent -} - /** * Provides an implementation of sampling that samples based on the session ID. * Using the provided percentage.