From a347bce764cf005ebf1ab7e129b6410d5f7d8eb0 Mon Sep 17 00:00:00 2001 From: Seth Silesky <5115498+silesky@users.noreply.github.com> Date: Mon, 28 Apr 2025 17:20:25 -0500 Subject: [PATCH 01/16] add experimental global scope executor that is impractical because of the reliance on globals --- .../signals/signals-example/public/index.html | 11 +- .../core/middleware/event-processor/index.ts | 39 +++++- .../__tests__/sandbox-settings.test.ts | 10 +- .../signals/src/core/processor/processor.ts | 15 +- .../signals/src/core/processor/sandbox.ts | 129 ++++++++++++++---- .../signals/src/lib/load-script/index.ts | 66 +++++++++ 6 files changed, 225 insertions(+), 45 deletions(-) create mode 100644 packages/signals/signals/src/lib/load-script/index.ts diff --git a/packages/signals/signals-example/public/index.html b/packages/signals/signals-example/public/index.html index aa72dbc9f..d34c48e56 100644 --- a/packages/signals/signals-example/public/index.html +++ b/packages/signals/signals-example/public/index.html @@ -5,10 +5,17 @@ React TypeScript App + + -
- \ No newline at end of file + diff --git a/packages/signals/signals/src/core/middleware/event-processor/index.ts b/packages/signals/signals/src/core/middleware/event-processor/index.ts index 33e93ae1b..d7f4feae5 100644 --- a/packages/signals/signals/src/core/middleware/event-processor/index.ts +++ b/packages/signals/signals/src/core/middleware/event-processor/index.ts @@ -2,17 +2,48 @@ import { Signal } from '@segment/analytics-signals-runtime' import { SignalBuffer } from '../../buffer' import { SignalsSubscriber, SignalsMiddlewareContext } from '../../emitter' import { SignalEventProcessor } from '../../processor/processor' -import { Sandbox, SandboxSettings } from '../../processor/sandbox' +import { + normalizeEdgeFunctionURL, + GlobalScopeSandbox, + WorkerSandbox, + WorkerSandboxSettings, + SignalSandbox, + NoopSandbox, +} from '../../processor/sandbox' + +const GLOBAL_SCOPE_SANDBOX = true export class SignalsEventProcessorSubscriber implements SignalsSubscriber { processor!: SignalEventProcessor buffer!: SignalBuffer load(ctx: SignalsMiddlewareContext) { this.buffer = ctx.buffer - this.processor = new SignalEventProcessor( - ctx.analyticsInstance, - new Sandbox(new SandboxSettings(ctx.unstableGlobalSettings.sandbox)) + const sandboxSettings = ctx.unstableGlobalSettings.sandbox + const normalizedEdgeFunctionURL = normalizeEdgeFunctionURL( + sandboxSettings.functionHost, + sandboxSettings.edgeFnDownloadURL ) + + let sandbox: SignalSandbox + if (!normalizedEdgeFunctionURL) { + console.warn( + `No processSignal function found. Have you written a processSignal function on app.segment.com?` + ) + sandbox = new NoopSandbox() + } else if (!GLOBAL_SCOPE_SANDBOX) { + sandbox = new WorkerSandbox( + new WorkerSandboxSettings({ + processSignal: sandboxSettings.processSignal, + edgeFnDownloadURL: normalizedEdgeFunctionURL, + }) + ) + } else { + sandbox = new GlobalScopeSandbox({ + edgeFnDownloadURL: normalizedEdgeFunctionURL, + }) + } + + this.processor = new SignalEventProcessor(ctx.analyticsInstance, sandbox) } async process(signal: Signal) { return this.processor.process(signal, await this.buffer.getAll()) diff --git a/packages/signals/signals/src/core/processor/__tests__/sandbox-settings.test.ts b/packages/signals/signals/src/core/processor/__tests__/sandbox-settings.test.ts index b1912ec49..fcefc7ab9 100644 --- a/packages/signals/signals/src/core/processor/__tests__/sandbox-settings.test.ts +++ b/packages/signals/signals/src/core/processor/__tests__/sandbox-settings.test.ts @@ -1,6 +1,6 @@ -import { SandboxSettings, SandboxSettingsConfig } from '../sandbox' +import { WorkerSandboxSettings, SandboxSettingsConfig } from '../sandbox' -describe(SandboxSettings, () => { +describe(WorkerSandboxSettings, () => { const edgeFnResponseBody = `function processSignal() { console.log('hello world') }` const baseSettings: SandboxSettingsConfig = { functionHost: undefined, @@ -13,7 +13,7 @@ describe(SandboxSettings, () => { ), } test('initializes with provided settings', async () => { - const sandboxSettings = new SandboxSettings({ ...baseSettings }) + const sandboxSettings = new WorkerSandboxSettings({ ...baseSettings }) expect(baseSettings.edgeFnFetchClient).toHaveBeenCalledWith( baseSettings.edgeFnDownloadURL ) @@ -27,7 +27,7 @@ describe(SandboxSettings, () => { functionHost: 'newHost.com', edgeFnDownloadURL: 'https://original.com/download', } - new SandboxSettings(settings) + new WorkerSandboxSettings(settings) expect(baseSettings.edgeFnFetchClient).toHaveBeenCalledWith( 'https://newHost.com/download' ) @@ -42,7 +42,7 @@ describe(SandboxSettings, () => { processSignal: undefined, edgeFnDownloadURL: undefined, } - const sandboxSettings = new SandboxSettings(settings) + const sandboxSettings = new WorkerSandboxSettings(settings) expect(await sandboxSettings.processSignal).toEqual( 'globalThis.processSignal = function processSignal() {}' ) diff --git a/packages/signals/signals/src/core/processor/processor.ts b/packages/signals/signals/src/core/processor/processor.ts index 5a9aee4a7..a5806767c 100644 --- a/packages/signals/signals/src/core/processor/processor.ts +++ b/packages/signals/signals/src/core/processor/processor.ts @@ -1,20 +1,21 @@ import { logger } from '../../lib/logger' import { Signal } from '@segment/analytics-signals-runtime' import { AnyAnalytics } from '../../types' -import { AnalyticsMethodCalls, MethodName, Sandbox } from './sandbox' +import { AnalyticsMethodCalls, MethodName, SignalSandbox } from './sandbox' export class SignalEventProcessor { - private sandbox: Sandbox - private analytics: AnyAnalytics - constructor(analytics: AnyAnalytics, sandbox: Sandbox) { + analytics: AnyAnalytics + sandbox: SignalSandbox + constructor(analytics: AnyAnalytics, sandbox: SignalSandbox) { this.analytics = analytics this.sandbox = sandbox } async process(signal: Signal, signals: Signal[]) { - let analyticsMethodCalls: AnalyticsMethodCalls + await this.sandbox.isLoaded() + let analyticsMethodCalls: AnalyticsMethodCalls | undefined try { - analyticsMethodCalls = await this.sandbox.process(signal, signals) + analyticsMethodCalls = await this.sandbox.execute(signal, signals) } catch (err) { // in practice, we should never hit this error, but if we do, we should log it. console.error('Error processing signal', { signal, signals }, err) @@ -34,6 +35,6 @@ export class SignalEventProcessor { } cleanup() { - return this.sandbox.jsSandbox.destroy() + return this.sandbox.destroy() } } diff --git a/packages/signals/signals/src/core/processor/sandbox.ts b/packages/signals/signals/src/core/processor/sandbox.ts index e7f81032b..fcd619a71 100644 --- a/packages/signals/signals/src/core/processor/sandbox.ts +++ b/packages/signals/signals/src/core/processor/sandbox.ts @@ -3,9 +3,14 @@ import { createWorkerBox, WorkerBoxAPI } from '../../lib/workerbox' import { resolvers } from './arg-resolvers' import { AnalyticsRuntimePublicApi } from '../../types' import { replaceBaseUrl } from '../../lib/replace-base-url' -import { Signal } from '@segment/analytics-signals-runtime' +import { + Signal, + WebRuntimeConstants, + WebSignalsRuntime, +} from '@segment/analytics-signals-runtime' import { getRuntimeCode } from '@segment/analytics-signals-runtime' import { polyfills } from './polyfills' +import { loadScript } from '../../lib/load-script' export type MethodName = | 'page' @@ -151,6 +156,17 @@ class JavascriptSandbox implements CodeSandbox { } } +export const normalizeEdgeFunctionURL = ( + functionHost: string | undefined, + edgeFnDownloadURL: string | undefined +) => { + if (functionHost && edgeFnDownloadURL) { + replaceBaseUrl(edgeFnDownloadURL, `https://${functionHost}`) + } else { + return edgeFnDownloadURL + } +} + export type SandboxSettingsConfig = { functionHost: string | undefined processSignal: string | undefined @@ -158,7 +174,12 @@ export type SandboxSettingsConfig = { edgeFnFetchClient?: typeof fetch } -export class SandboxSettings { +export type WorkerboxSettingsConfig = Pick< + SandboxSettingsConfig, + 'processSignal' | 'edgeFnFetchClient' | 'edgeFnDownloadURL' +> + +export class WorkerSandboxSettings { /** * Should look like: * ```js @@ -168,48 +189,42 @@ export class SandboxSettings { * ``` */ processSignal: Promise - constructor(settings: SandboxSettingsConfig) { - const edgeFnDownloadURLNormalized = - settings.functionHost && settings.edgeFnDownloadURL - ? replaceBaseUrl( - settings.edgeFnDownloadURL, - `https://${settings.functionHost}` - ) - : settings.edgeFnDownloadURL - - if (!edgeFnDownloadURLNormalized && !settings.processSignal) { - // user may be onboarding and not have written a signal -- so do a noop so we can collect signals - this.processSignal = Promise.resolve( - `globalThis.processSignal = function processSignal() {}` - ) - console.warn( - `No processSignal function found. Have you written a processSignal function on app.segment.com?` - ) - return - } - + constructor(settings: WorkerboxSettingsConfig) { const fetch = settings.edgeFnFetchClient ?? globalThis.fetch const processSignalNormalized = settings.processSignal ? Promise.resolve(settings.processSignal).then( (str) => `globalThis.processSignal = ${str}` ) - : fetch(edgeFnDownloadURLNormalized!).then((res) => res.text()) + : fetch(settings.edgeFnDownloadURL!).then((res) => res.text()) this.processSignal = processSignalNormalized } } -export class Sandbox { - settings: SandboxSettings +export interface SignalSandbox { + isLoaded(): Promise + execute( + signal: Signal, + signals: Signal[] + ): Promise + destroy(): void | Promise +} + +export class WorkerSandbox implements SignalSandbox { + settings: WorkerSandboxSettings jsSandbox: CodeSandbox - constructor(settings: SandboxSettings) { + constructor(settings: WorkerSandboxSettings) { this.settings = settings this.jsSandbox = new JavascriptSandbox() } - async process( + isLoaded(): Promise { + return Promise.resolve() // TODO + } + + async execute( signal: Signal, signals: Signal[] ): Promise { @@ -232,4 +247,64 @@ export class Sandbox { const calls = analytics.getCalls() return calls } + destroy(): void { + void this.jsSandbox.destroy() + } +} + +// This is not ideal -- but processSignal currently depends on + +const processWithGlobals = ( + signalBuffer: Signal[] +): AnalyticsMethodCalls | undefined => { + const g = globalThis as any + // Load all constants into the global scope + Object.entries(WebRuntimeConstants).forEach(([key, value]) => { + g[key] = value + }) + + // processSignal expects a global called `signals` -- of course, there can local variable naming conflict on the client, which is why globals were a bad idea. + g['signals'] = new WebSignalsRuntime(signalBuffer) + + // expect analytics to be instantiated -- this will conflict in the global scope TODO + g['analytics'] = new AnalyticsRuntime() + + // another possible namespace conflict? + // @ts-ignore + if (typeof processSignal != 'undefined') { + g['processSignal'](signalBuffer[0]) + } else { + console.warn('no processSignal function is defined in the global scope') + } + + return g['analytics'].getCalls() +} + +interface GlobalScopeSandboxSettings { + edgeFnDownloadURL: string +} +export class GlobalScopeSandbox implements SignalSandbox { + script: Promise + async isLoaded(): Promise { + await this.script + } + constructor(settings: GlobalScopeSandboxSettings) { + this.script = loadScript(settings.edgeFnDownloadURL) + } + + // eslint-disable-next-line @typescript-eslint/require-await + async execute(_signal: Signal, signals: Signal[]) { + return processWithGlobals(signals) + } + destroy(): void {} +} + +export class NoopSandbox implements SignalSandbox { + async isLoaded(): Promise {} + execute(_signal: Signal, _signals: Signal[]) { + return Promise.resolve(undefined) + } + destroy(): void | Promise { + return + } } diff --git a/packages/signals/signals/src/lib/load-script/index.ts b/packages/signals/signals/src/lib/load-script/index.ts new file mode 100644 index 000000000..118640622 --- /dev/null +++ b/packages/signals/signals/src/lib/load-script/index.ts @@ -0,0 +1,66 @@ +function findScript(src: string): HTMLScriptElement | undefined { + const scripts = Array.prototype.slice.call( + window.document.querySelectorAll('script') + ) + return scripts.find((s) => s.src === src) +} + +/** + * Load a script from a URL and append it to the document head + */ +export function loadScript( + src: string, + attributes?: Record +): Promise { + const found = findScript(src) + + if (found !== undefined) { + const status = found?.getAttribute('status') + + if (status === 'loaded') { + return Promise.resolve(found) + } + + if (status === 'loading') { + return new Promise((resolve, reject) => { + found.addEventListener('load', () => resolve(found)) + found.addEventListener('error', (err) => reject(err)) + }) + } + } + + return new Promise((resolve, reject) => { + const script = window.document.createElement('script') + + script.type = 'text/javascript' + script.src = src + script.async = true + + script.setAttribute('status', 'loading') + for (const [k, v] of Object.entries(attributes ?? {})) { + script.setAttribute(k, v) + } + + script.onload = (): void => { + script.onerror = script.onload = null + script.setAttribute('status', 'loaded') + resolve(script) + } + + script.onerror = (): void => { + script.onerror = script.onload = null + script.setAttribute('status', 'error') + reject(new Error(`Failed to load ${src}`)) + } + + const firstExistingScript = window.document.querySelector('script') + if (!firstExistingScript) { + window.document.head.appendChild(script) + } else { + firstExistingScript.parentElement?.insertBefore( + script, + firstExistingScript + ) + } + }) +} From 68171d58cfe546a56c864ca0db05183b29dd6593 Mon Sep 17 00:00:00 2001 From: Seth Silesky <5115498+silesky@users.noreply.github.com> Date: Tue, 29 Apr 2025 15:23:15 -0500 Subject: [PATCH 02/16] hacky sandbox strategy --- .../core/middleware/event-processor/index.ts | 2 +- .../signals/src/core/processor/processor.ts | 1 - .../signals/src/core/processor/sandbox.ts | 85 +++++++++++++------ 3 files changed, 58 insertions(+), 30 deletions(-) diff --git a/packages/signals/signals/src/core/middleware/event-processor/index.ts b/packages/signals/signals/src/core/middleware/event-processor/index.ts index d7f4feae5..bc419e8ae 100644 --- a/packages/signals/signals/src/core/middleware/event-processor/index.ts +++ b/packages/signals/signals/src/core/middleware/event-processor/index.ts @@ -30,7 +30,7 @@ export class SignalsEventProcessorSubscriber implements SignalsSubscriber { `No processSignal function found. Have you written a processSignal function on app.segment.com?` ) sandbox = new NoopSandbox() - } else if (!GLOBAL_SCOPE_SANDBOX) { + } else if (!GLOBAL_SCOPE_SANDBOX || sandboxSettings.processSignal) { sandbox = new WorkerSandbox( new WorkerSandboxSettings({ processSignal: sandboxSettings.processSignal, diff --git a/packages/signals/signals/src/core/processor/processor.ts b/packages/signals/signals/src/core/processor/processor.ts index a5806767c..6f4d567f2 100644 --- a/packages/signals/signals/src/core/processor/processor.ts +++ b/packages/signals/signals/src/core/processor/processor.ts @@ -12,7 +12,6 @@ export class SignalEventProcessor { } async process(signal: Signal, signals: Signal[]) { - await this.sandbox.isLoaded() let analyticsMethodCalls: AnalyticsMethodCalls | undefined try { analyticsMethodCalls = await this.sandbox.execute(signal, signals) diff --git a/packages/signals/signals/src/core/processor/sandbox.ts b/packages/signals/signals/src/core/processor/sandbox.ts index fcd619a71..c8a30be92 100644 --- a/packages/signals/signals/src/core/processor/sandbox.ts +++ b/packages/signals/signals/src/core/processor/sandbox.ts @@ -1,7 +1,7 @@ import { logger } from '../../lib/logger' import { createWorkerBox, WorkerBoxAPI } from '../../lib/workerbox' import { resolvers } from './arg-resolvers' -import { AnalyticsRuntimePublicApi } from '../../types' +import { AnalyticsRuntimePublicApi, ProcessSignal } from '../../types' import { replaceBaseUrl } from '../../lib/replace-base-url' import { Signal, @@ -172,6 +172,13 @@ export type SandboxSettingsConfig = { processSignal: string | undefined edgeFnDownloadURL: string | undefined edgeFnFetchClient?: typeof fetch + /** + * What sandbox strategy to use + * default - use a web worker and regular evaluation + * globalScope - evaluate everything in the global scope -- this avoids CSP errors + * @default 'default' + */ + sandboxStrategy?: 'default' | 'globalScope' } export type WorkerboxSettingsConfig = Pick< @@ -203,7 +210,6 @@ export class WorkerSandboxSettings { } export interface SignalSandbox { - isLoaded(): Promise execute( signal: Signal, signals: Signal[] @@ -220,10 +226,6 @@ export class WorkerSandbox implements SignalSandbox { this.jsSandbox = new JavascriptSandbox() } - isLoaded(): Promise { - return Promise.resolve() // TODO - } - async execute( signal: Signal, signals: Signal[] @@ -252,55 +254,82 @@ export class WorkerSandbox implements SignalSandbox { } } -// This is not ideal -- but processSignal currently depends on - -const processWithGlobals = ( +// ProcessSignal unfortunately uses globals. This should change. +// For now, we are setting up the globals between each invocation +const processWithGlobalScopeExecutionEnv = ( + signal: Signal, signalBuffer: Signal[] ): AnalyticsMethodCalls | undefined => { const g = globalThis as any + const processSignal: ProcessSignal = g['processSignal'] + + if (typeof processSignal == 'undefined') { + console.warn('no processSignal function is defined in the global scope') + return undefined + } + // Load all constants into the global scope Object.entries(WebRuntimeConstants).forEach(([key, value]) => { g[key] = value }) // processSignal expects a global called `signals` -- of course, there can local variable naming conflict on the client, which is why globals were a bad idea. - g['signals'] = new WebSignalsRuntime(signalBuffer) + const analytics = new AnalyticsRuntime() + const signals = new WebSignalsRuntime(signalBuffer) + + const originalAnalytics = g.analytics + try { + if (g['analytics'] instanceof AnalyticsRuntime) { + console.warn( + 'Invariant: analytics variable was not properly restored on the previous execution. This indicates a concurrency bug' + ) - // expect analytics to be instantiated -- this will conflict in the global scope TODO - g['analytics'] = new AnalyticsRuntime() + return + } - // another possible namespace conflict? - // @ts-ignore - if (typeof processSignal != 'undefined') { - g['processSignal'](signalBuffer[0]) - } else { - console.warn('no processSignal function is defined in the global scope') + g['analytics'] = analytics + + g['signals'] = signals + processSignal(signal, { + // we eventually want to get rid of globals and processSignal just uses local variables. + analytics: analytics, + signals: signals, + // constants + EventType: WebRuntimeConstants.EventType, + NavigationAction: WebRuntimeConstants.NavigationAction, + SignalType: WebRuntimeConstants.SignalType, + }) + } finally { + // restore globals + g['analytics'] = originalAnalytics } - return g['analytics'].getCalls() + // this seems like it could potentially cause bugs in async environment + g.analytics = originalAnalytics + return analytics.getCalls() } +/** + * Sandbox that avoids CSP errors, but evaluates everything globally + */ interface GlobalScopeSandboxSettings { edgeFnDownloadURL: string } export class GlobalScopeSandbox implements SignalSandbox { - script: Promise - async isLoaded(): Promise { - await this.script - } + htmlScriptLoaded: Promise + constructor(settings: GlobalScopeSandboxSettings) { - this.script = loadScript(settings.edgeFnDownloadURL) + this.htmlScriptLoaded = loadScript(settings.edgeFnDownloadURL) } - // eslint-disable-next-line @typescript-eslint/require-await - async execute(_signal: Signal, signals: Signal[]) { - return processWithGlobals(signals) + async execute(signal: Signal, signals: Signal[]) { + await this.htmlScriptLoaded + return processWithGlobalScopeExecutionEnv(signal, signals) } destroy(): void {} } export class NoopSandbox implements SignalSandbox { - async isLoaded(): Promise {} execute(_signal: Signal, _signals: Signal[]) { return Promise.resolve(undefined) } From 17e43858640dcec8d413ba299a0968154ee2c901 Mon Sep 17 00:00:00 2001 From: Seth Silesky <5115498+silesky@users.noreply.github.com> Date: Tue, 29 Apr 2025 17:01:38 -0500 Subject: [PATCH 03/16] update readme --- .changeset/eight-adults-type.md | 5 ++++ .../signals-example/src/lib/analytics.ts | 2 +- packages/signals/signals/README.md | 8 +++++++ .../core/middleware/event-processor/index.ts | 16 +++++++++---- .../__tests__/sandbox-settings.test.ts | 17 +++++++------- .../signals/src/core/processor/sandbox.ts | 23 +++++++------------ .../signals/src/core/signals/settings.ts | 2 ++ .../signals/src/plugin/signals-plugin.ts | 15 +----------- .../signals/signals/src/types/settings.ts | 8 +++++++ 9 files changed, 52 insertions(+), 44 deletions(-) create mode 100644 .changeset/eight-adults-type.md diff --git a/.changeset/eight-adults-type.md b/.changeset/eight-adults-type.md new file mode 100644 index 000000000..ea13b0b90 --- /dev/null +++ b/.changeset/eight-adults-type.md @@ -0,0 +1,5 @@ +--- +'@segment/analytics-signals': minor +--- + +Add globalscope strategy diff --git a/packages/signals/signals-example/src/lib/analytics.ts b/packages/signals/signals-example/src/lib/analytics.ts index 821965552..bd591b41b 100644 --- a/packages/signals/signals-example/src/lib/analytics.ts +++ b/packages/signals/signals-example/src/lib/analytics.ts @@ -33,7 +33,7 @@ const isStage = process.env.STAGE === 'true' const signalsPlugin = new SignalsPlugin({ ...(isStage ? { apiHost: 'signals.segment.build/v1' } : {}), - // enableDebugLogging: true, + sandboxStrategy: 'global', // processSignal: processSignalExample, }) diff --git a/packages/signals/signals/README.md b/packages/signals/signals/README.md index dec38da7c..92a8ba23a 100644 --- a/packages/signals/signals/README.md +++ b/packages/signals/signals/README.md @@ -86,6 +86,14 @@ signalsPlugin.addSignal({ someData: 'foo' }) } ``` +### Sandbox Strategies +If getting CSP errors, you can use the experimental 'global' sandbox strategy. + +```ts +new SignalsPlugin({ sandboxStrategy: 'global' }) +``` + + ### Debugging Debug mode **MUST** be enabled on the client to VIEW signals on segment.com. diff --git a/packages/signals/signals/src/core/middleware/event-processor/index.ts b/packages/signals/signals/src/core/middleware/event-processor/index.ts index bc419e8ae..4d8d51631 100644 --- a/packages/signals/signals/src/core/middleware/event-processor/index.ts +++ b/packages/signals/signals/src/core/middleware/event-processor/index.ts @@ -1,4 +1,5 @@ import { Signal } from '@segment/analytics-signals-runtime' +import { logger } from '../../../lib/logger' import { SignalBuffer } from '../../buffer' import { SignalsSubscriber, SignalsMiddlewareContext } from '../../emitter' import { SignalEventProcessor } from '../../processor/processor' @@ -6,13 +7,11 @@ import { normalizeEdgeFunctionURL, GlobalScopeSandbox, WorkerSandbox, - WorkerSandboxSettings, + IframeSandboxSettings, SignalSandbox, NoopSandbox, } from '../../processor/sandbox' -const GLOBAL_SCOPE_SANDBOX = true - export class SignalsEventProcessorSubscriber implements SignalsSubscriber { processor!: SignalEventProcessor buffer!: SignalBuffer @@ -25,19 +24,26 @@ export class SignalsEventProcessorSubscriber implements SignalsSubscriber { ) let sandbox: SignalSandbox + console.log('sup') if (!normalizedEdgeFunctionURL) { console.warn( `No processSignal function found. Have you written a processSignal function on app.segment.com?` ) + logger.debug('Initializing sandbox: noop') sandbox = new NoopSandbox() - } else if (!GLOBAL_SCOPE_SANDBOX || sandboxSettings.processSignal) { + } else if ( + sandboxSettings.sandboxStrategy === 'iframe' || + sandboxSettings.processSignal + ) { + logger.debug('Initializing sandbox: iframe') sandbox = new WorkerSandbox( - new WorkerSandboxSettings({ + new IframeSandboxSettings({ processSignal: sandboxSettings.processSignal, edgeFnDownloadURL: normalizedEdgeFunctionURL, }) ) } else { + logger.debug('Initializing sandbox: global scope') sandbox = new GlobalScopeSandbox({ edgeFnDownloadURL: normalizedEdgeFunctionURL, }) diff --git a/packages/signals/signals/src/core/processor/__tests__/sandbox-settings.test.ts b/packages/signals/signals/src/core/processor/__tests__/sandbox-settings.test.ts index fcefc7ab9..40420a624 100644 --- a/packages/signals/signals/src/core/processor/__tests__/sandbox-settings.test.ts +++ b/packages/signals/signals/src/core/processor/__tests__/sandbox-settings.test.ts @@ -1,9 +1,8 @@ -import { WorkerSandboxSettings, SandboxSettingsConfig } from '../sandbox' +import { IframeSandboxSettings, IframeSandboxSettingsConfig } from '../sandbox' -describe(WorkerSandboxSettings, () => { +describe(IframeSandboxSettings, () => { const edgeFnResponseBody = `function processSignal() { console.log('hello world') }` - const baseSettings: SandboxSettingsConfig = { - functionHost: undefined, + const baseSettings: IframeSandboxSettingsConfig = { processSignal: undefined, edgeFnDownloadURL: 'http://example.com/download', edgeFnFetchClient: jest.fn().mockReturnValue( @@ -13,7 +12,7 @@ describe(WorkerSandboxSettings, () => { ), } test('initializes with provided settings', async () => { - const sandboxSettings = new WorkerSandboxSettings({ ...baseSettings }) + const sandboxSettings = new IframeSandboxSettings({ ...baseSettings }) expect(baseSettings.edgeFnFetchClient).toHaveBeenCalledWith( baseSettings.edgeFnDownloadURL ) @@ -21,13 +20,13 @@ describe(WorkerSandboxSettings, () => { }) test('normalizes edgeFnDownloadURL when functionHost is provided', async () => { - const settings: SandboxSettingsConfig = { + const settings = { ...baseSettings, processSignal: undefined, functionHost: 'newHost.com', edgeFnDownloadURL: 'https://original.com/download', } - new WorkerSandboxSettings(settings) + new IframeSandboxSettings(settings) expect(baseSettings.edgeFnFetchClient).toHaveBeenCalledWith( 'https://newHost.com/download' ) @@ -37,12 +36,12 @@ describe(WorkerSandboxSettings, () => { const consoleWarnSpy = jest .spyOn(console, 'warn') .mockImplementation(() => {}) - const settings: SandboxSettingsConfig = { + const settings = { ...baseSettings, processSignal: undefined, edgeFnDownloadURL: undefined, } - const sandboxSettings = new WorkerSandboxSettings(settings) + const sandboxSettings = new IframeSandboxSettings(settings) expect(await sandboxSettings.processSignal).toEqual( 'globalThis.processSignal = function processSignal() {}' ) diff --git a/packages/signals/signals/src/core/processor/sandbox.ts b/packages/signals/signals/src/core/processor/sandbox.ts index c8a30be92..2d8a5deb7 100644 --- a/packages/signals/signals/src/core/processor/sandbox.ts +++ b/packages/signals/signals/src/core/processor/sandbox.ts @@ -172,21 +172,15 @@ export type SandboxSettingsConfig = { processSignal: string | undefined edgeFnDownloadURL: string | undefined edgeFnFetchClient?: typeof fetch - /** - * What sandbox strategy to use - * default - use a web worker and regular evaluation - * globalScope - evaluate everything in the global scope -- this avoids CSP errors - * @default 'default' - */ - sandboxStrategy?: 'default' | 'globalScope' + sandboxStrategy: 'iframe' | 'global' } -export type WorkerboxSettingsConfig = Pick< +export type IframeSandboxSettingsConfig = Pick< SandboxSettingsConfig, 'processSignal' | 'edgeFnFetchClient' | 'edgeFnDownloadURL' > -export class WorkerSandboxSettings { +export class IframeSandboxSettings { /** * Should look like: * ```js @@ -196,7 +190,7 @@ export class WorkerSandboxSettings { * ``` */ processSignal: Promise - constructor(settings: WorkerboxSettingsConfig) { + constructor(settings: IframeSandboxSettingsConfig) { const fetch = settings.edgeFnFetchClient ?? globalThis.fetch const processSignalNormalized = settings.processSignal @@ -218,10 +212,10 @@ export interface SignalSandbox { } export class WorkerSandbox implements SignalSandbox { - settings: WorkerSandboxSettings + settings: IframeSandboxSettings jsSandbox: CodeSandbox - constructor(settings: WorkerSandboxSettings) { + constructor(settings: IframeSandboxSettings) { this.settings = settings this.jsSandbox = new JavascriptSandbox() } @@ -280,11 +274,9 @@ const processWithGlobalScopeExecutionEnv = ( const originalAnalytics = g.analytics try { if (g['analytics'] instanceof AnalyticsRuntime) { - console.warn( + throw new Error( 'Invariant: analytics variable was not properly restored on the previous execution. This indicates a concurrency bug' ) - - return } g['analytics'] = analytics @@ -319,6 +311,7 @@ export class GlobalScopeSandbox implements SignalSandbox { htmlScriptLoaded: Promise constructor(settings: GlobalScopeSandboxSettings) { + logger.debug('Initializing global scope sandbox') this.htmlScriptLoaded = loadScript(settings.edgeFnDownloadURL) } diff --git a/packages/signals/signals/src/core/signals/settings.ts b/packages/signals/signals/src/core/signals/settings.ts index a77afdae7..ff30ef0a0 100644 --- a/packages/signals/signals/src/core/signals/settings.ts +++ b/packages/signals/signals/src/core/signals/settings.ts @@ -28,6 +28,7 @@ export type SignalsSettingsConfig = Pick< | 'mutationGenPollInterval' | 'mutationGenObservedAttributes' | 'debug' + | 'sandboxStrategy' > & { signalStorage?: SignalPersistentStorage processSignal?: string @@ -89,6 +90,7 @@ export class SignalGlobalSettings { }, } this.sandbox = { + sandboxStrategy: settings.sandboxStrategy ?? 'iframe', functionHost: settings.functionHost, processSignal: settings.processSignal, edgeFnDownloadURL: undefined, diff --git a/packages/signals/signals/src/plugin/signals-plugin.ts b/packages/signals/signals/src/plugin/signals-plugin.ts index 6d5123300..c4dadec03 100644 --- a/packages/signals/signals/src/plugin/signals-plugin.ts +++ b/packages/signals/signals/src/plugin/signals-plugin.ts @@ -34,24 +34,11 @@ export class SignalsPlugin implements Plugin, SignalsAugmentedFunctionality { Object.assign(window, { SegmentSignalsPlugin: this }) this.signals = new Signals({ - debug: settings.debug, - disableSignalsRedaction: settings.disableSignalsRedaction, - enableSignalsIngestion: settings.enableSignalsIngestion, - flushAt: settings.flushAt, - flushInterval: settings.flushInterval, - functionHost: settings.functionHost, - apiHost: settings.apiHost, - maxBufferSize: settings.maxBufferSize, + ...settings, processSignal: typeof settings.processSignal === 'function' ? settings.processSignal.toString() : settings.processSignal, - networkSignalsAllowSameDomain: settings.networkSignalsAllowSameDomain, - networkSignalsAllowList: settings.networkSignalsAllowList, - networkSignalsDisallowList: settings.networkSignalsDisallowList, - signalStorage: settings.signalStorage, - signalStorageType: settings.signalStorageType, - middleware: settings.middleware, }) logger.debug(`SignalsPlugin v${version} initializing`, { diff --git a/packages/signals/signals/src/types/settings.ts b/packages/signals/signals/src/types/settings.ts index 7c27fc198..6d62f348c 100644 --- a/packages/signals/signals/src/types/settings.ts +++ b/packages/signals/signals/src/types/settings.ts @@ -144,6 +144,14 @@ export interface SignalsPluginSettingsConfig { * (defaultAttributes) => defaultAttributes.filter(attr => attr.toLowerCase() !== 'aria-selected') */ mutationGenObservedAttributes?: (defaultAttributes: string[]) => string[] + + /** + * What sandbox strategy to use + * - global - [EXPERIMENTAL] evaluate everything in the global scope -- use this if you want to avoid CSP errors. + * - iframe - use a web worker and regular evaluation + * @default 'iframe' + */ + sandboxStrategy?: 'iframe' | 'global' } export type RegexLike = RegExp | string From 8a6ce1806bb8672a91aa41764aa87d0d26c8fda7 Mon Sep 17 00:00:00 2001 From: Seth Silesky <5115498+silesky@users.noreply.github.com> Date: Tue, 29 Apr 2025 17:04:19 -0500 Subject: [PATCH 04/16] add event processor --- .../signals/src/core/middleware/event-processor/index.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/signals/signals/src/core/middleware/event-processor/index.ts b/packages/signals/signals/src/core/middleware/event-processor/index.ts index 4d8d51631..f51f52c73 100644 --- a/packages/signals/signals/src/core/middleware/event-processor/index.ts +++ b/packages/signals/signals/src/core/middleware/event-processor/index.ts @@ -24,7 +24,7 @@ export class SignalsEventProcessorSubscriber implements SignalsSubscriber { ) let sandbox: SignalSandbox - console.log('sup') + if (!normalizedEdgeFunctionURL) { console.warn( `No processSignal function found. Have you written a processSignal function on app.segment.com?` From 32222bd2cb033d342ec239c7b6c8e6f866780585 Mon Sep 17 00:00:00 2001 From: Seth Silesky <5115498+silesky@users.noreply.github.com> Date: Tue, 29 Apr 2025 18:00:28 -0500 Subject: [PATCH 05/16] fix tests --- .../__tests__/sandbox-settings.test.ts | 15 +++++------ .../signals/src/core/processor/sandbox.ts | 27 ++++++++++++++----- 2 files changed, 28 insertions(+), 14 deletions(-) diff --git a/packages/signals/signals/src/core/processor/__tests__/sandbox-settings.test.ts b/packages/signals/signals/src/core/processor/__tests__/sandbox-settings.test.ts index 40420a624..f5c5e9c02 100644 --- a/packages/signals/signals/src/core/processor/__tests__/sandbox-settings.test.ts +++ b/packages/signals/signals/src/core/processor/__tests__/sandbox-settings.test.ts @@ -19,16 +19,15 @@ describe(IframeSandboxSettings, () => { expect(await sandboxSettings.processSignal).toEqual(edgeFnResponseBody) }) - test('normalizes edgeFnDownloadURL when functionHost is provided', async () => { - const settings = { + test('should call edgeFnDownloadURL', async () => { + const settings: IframeSandboxSettingsConfig = { ...baseSettings, processSignal: undefined, - functionHost: 'newHost.com', - edgeFnDownloadURL: 'https://original.com/download', + edgeFnDownloadURL: 'https://foo.com/download', } new IframeSandboxSettings(settings) expect(baseSettings.edgeFnFetchClient).toHaveBeenCalledWith( - 'https://newHost.com/download' + 'https://foo.com/download' ) }) @@ -36,14 +35,14 @@ describe(IframeSandboxSettings, () => { const consoleWarnSpy = jest .spyOn(console, 'warn') .mockImplementation(() => {}) - const settings = { + const settings: IframeSandboxSettingsConfig = { ...baseSettings, processSignal: undefined, edgeFnDownloadURL: undefined, } const sandboxSettings = new IframeSandboxSettings(settings) - expect(await sandboxSettings.processSignal).toEqual( - 'globalThis.processSignal = function processSignal() {}' + expect(await sandboxSettings.processSignal).toMatchInlineSnapshot( + `"globalThis.processSignal = function() {}"` ) expect(baseSettings.edgeFnFetchClient).not.toHaveBeenCalled() expect(consoleWarnSpy).toHaveBeenCalledWith( diff --git a/packages/signals/signals/src/core/processor/sandbox.ts b/packages/signals/signals/src/core/processor/sandbox.ts index 2d8a5deb7..f3ae25080 100644 --- a/packages/signals/signals/src/core/processor/sandbox.ts +++ b/packages/signals/signals/src/core/processor/sandbox.ts @@ -180,6 +180,11 @@ export type IframeSandboxSettingsConfig = Pick< 'processSignal' | 'edgeFnFetchClient' | 'edgeFnDownloadURL' > +const consoleWarnProcessSignal = () => + console.warn( + 'processSignal is not defined - have you set up auto-instrumentation on app.segment.com?' + ) + export class IframeSandboxSettings { /** * Should look like: @@ -193,11 +198,21 @@ export class IframeSandboxSettings { constructor(settings: IframeSandboxSettingsConfig) { const fetch = settings.edgeFnFetchClient ?? globalThis.fetch - const processSignalNormalized = settings.processSignal - ? Promise.resolve(settings.processSignal).then( - (str) => `globalThis.processSignal = ${str}` - ) - : fetch(settings.edgeFnDownloadURL!).then((res) => res.text()) + let processSignalNormalized = Promise.resolve( + `globalThis.processSignal = function() {}` + ) + + if (settings.processSignal) { + processSignalNormalized = Promise.resolve(settings.processSignal).then( + (str) => `globalThis.processSignal = ${str}` + ) + } else if (settings.edgeFnDownloadURL) { + processSignalNormalized = fetch(settings.edgeFnDownloadURL!).then((res) => + res.text() + ) + } else { + consoleWarnProcessSignal() + } this.processSignal = processSignalNormalized } @@ -258,7 +273,7 @@ const processWithGlobalScopeExecutionEnv = ( const processSignal: ProcessSignal = g['processSignal'] if (typeof processSignal == 'undefined') { - console.warn('no processSignal function is defined in the global scope') + consoleWarnProcessSignal() return undefined } From 9fb7da2af08849d1ab2c75c1e5d259cd80429607 Mon Sep 17 00:00:00 2001 From: Seth Silesky <5115498+silesky@users.noreply.github.com> Date: Tue, 29 Apr 2025 18:03:52 -0500 Subject: [PATCH 06/16] wip --- .../signals/signals/src/core/processor/sandbox.ts | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/packages/signals/signals/src/core/processor/sandbox.ts b/packages/signals/signals/src/core/processor/sandbox.ts index f3ae25080..3a8645e5b 100644 --- a/packages/signals/signals/src/core/processor/sandbox.ts +++ b/packages/signals/signals/src/core/processor/sandbox.ts @@ -287,18 +287,18 @@ const processWithGlobalScopeExecutionEnv = ( const signals = new WebSignalsRuntime(signalBuffer) const originalAnalytics = g.analytics - try { - if (g['analytics'] instanceof AnalyticsRuntime) { - throw new Error( - 'Invariant: analytics variable was not properly restored on the previous execution. This indicates a concurrency bug' - ) - } + if (originalAnalytics instanceof AnalyticsRuntime) { + throw new Error( + 'Invariant: analytics variable was not properly restored on the previous execution. This indicates a concurrency bug' + ) + } + try { g['analytics'] = analytics - g['signals'] = signals processSignal(signal, { // we eventually want to get rid of globals and processSignal just uses local variables. + // TODO: update processSignal generator to accept params like these for web (mobile currently uses globals for their architecture -- can be changed but hard). analytics: analytics, signals: signals, // constants From cd95f41e0ab0b391b23edc61f3915c3c73ff7c50 Mon Sep 17 00:00:00 2001 From: Seth Silesky <5115498+silesky@users.noreply.github.com> Date: Tue, 29 Apr 2025 18:09:08 -0500 Subject: [PATCH 07/16] update max signal buffer size --- .changeset/empty-eagles-buy.md | 5 +++++ packages/signals/signals/src/core/buffer/index.ts | 2 +- 2 files changed, 6 insertions(+), 1 deletion(-) create mode 100644 .changeset/empty-eagles-buy.md diff --git a/.changeset/empty-eagles-buy.md b/.changeset/empty-eagles-buy.md new file mode 100644 index 000000000..5945ef926 --- /dev/null +++ b/.changeset/empty-eagles-buy.md @@ -0,0 +1,5 @@ +--- +'@segment/analytics-signals': minor +--- + +Update max signals in buffer to 100 diff --git a/packages/signals/signals/src/core/buffer/index.ts b/packages/signals/signals/src/core/buffer/index.ts index a1967d355..8f85c8ad3 100644 --- a/packages/signals/signals/src/core/buffer/index.ts +++ b/packages/signals/signals/src/core/buffer/index.ts @@ -25,7 +25,7 @@ interface IDBPObjectStoreSignals 'readonly' | 'readwrite' | 'versionchange' > {} -const MAX_BUFFER_SIZE_DEFAULT = 50 +const MAX_BUFFER_SIZE_DEFAULT = 100 interface StoreSettings { maxBufferSize?: number From 33205925bc088369224d3dca1d0756ceddc3a424 Mon Sep 17 00:00:00 2001 From: Seth Silesky <5115498+silesky@users.noreply.github.com> Date: Tue, 29 Apr 2025 18:30:41 -0500 Subject: [PATCH 08/16] Update eight-adults-type.md --- .changeset/eight-adults-type.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changeset/eight-adults-type.md b/.changeset/eight-adults-type.md index ea13b0b90..dc3607413 100644 --- a/.changeset/eight-adults-type.md +++ b/.changeset/eight-adults-type.md @@ -2,4 +2,4 @@ '@segment/analytics-signals': minor --- -Add globalscope strategy +Fix CSP errors with sandboxStrategy: global From 4ad4d825ca561b4186648e3ab169fc5c59dda1c8 Mon Sep 17 00:00:00 2001 From: Seth Silesky <5115498+silesky@users.noreply.github.com> Date: Tue, 29 Apr 2025 18:33:01 -0500 Subject: [PATCH 09/16] update sandbox --- packages/signals/signals/src/core/processor/sandbox.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/signals/signals/src/core/processor/sandbox.ts b/packages/signals/signals/src/core/processor/sandbox.ts index 3a8645e5b..bbe9d751a 100644 --- a/packages/signals/signals/src/core/processor/sandbox.ts +++ b/packages/signals/signals/src/core/processor/sandbox.ts @@ -292,6 +292,7 @@ const processWithGlobalScopeExecutionEnv = ( 'Invariant: analytics variable was not properly restored on the previous execution. This indicates a concurrency bug' ) } + const originalSignals = g.signals try { g['analytics'] = analytics @@ -309,10 +310,9 @@ const processWithGlobalScopeExecutionEnv = ( } finally { // restore globals g['analytics'] = originalAnalytics + g['signals'] = originalSignals } - // this seems like it could potentially cause bugs in async environment - g.analytics = originalAnalytics return analytics.getCalls() } From 6c11922a2567fae7fe7bdd4f67f3583b0479bd42 Mon Sep 17 00:00:00 2001 From: Seth Silesky <5115498+silesky@users.noreply.github.com> Date: Tue, 29 Apr 2025 18:34:21 -0500 Subject: [PATCH 10/16] wip --- packages/signals/signals/src/core/processor/sandbox.ts | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/packages/signals/signals/src/core/processor/sandbox.ts b/packages/signals/signals/src/core/processor/sandbox.ts index bbe9d751a..78c9b3e3f 100644 --- a/packages/signals/signals/src/core/processor/sandbox.ts +++ b/packages/signals/signals/src/core/processor/sandbox.ts @@ -341,7 +341,5 @@ export class NoopSandbox implements SignalSandbox { execute(_signal: Signal, _signals: Signal[]) { return Promise.resolve(undefined) } - destroy(): void | Promise { - return - } + destroy(): void {} } From cae66817c8594dda8a48222b2ace20475eb89008 Mon Sep 17 00:00:00 2001 From: Seth Silesky <5115498+silesky@users.noreply.github.com> Date: Wed, 30 Apr 2025 13:21:02 -0500 Subject: [PATCH 11/16] update tests --- .buildkite/pipeline.yml | 1 + .../signals/signals-integration-tests/package.json | 3 ++- .../playwright.global-setup.ts | 6 ++++-- .../src/helpers/base-page-object.ts | 2 ++ .../src/helpers/env-config.ts | 11 +++++++++++ .../src/tests/custom-elements/custom-select.test.ts | 2 +- .../tests/custom-elements/custom-textfield.test.ts | 2 +- .../src/tests/performance/memory-leak.test.ts | 2 +- .../tests/signals-vanilla/all-segment-events.test.ts | 4 ++-- .../src/tests/signals-vanilla/basic.test.ts | 2 +- .../signals-vanilla/button-click-complex.test.ts | 2 +- .../src/tests/signals-vanilla/change-input.test.ts | 2 +- .../src/tests/signals-vanilla/middleware.test.ts | 2 +- .../network-signals-allow-list.test.ts | 2 +- .../signals-vanilla/network-signals-fetch.test.ts | 2 +- .../tests/signals-vanilla/network-signals-xhr.test.ts | 2 +- .../src/tests/signals-vanilla/reset.test.ts | 2 +- .../tests/signals-vanilla/runtime-constants.test.ts | 2 +- .../src/tests/signals-vanilla/signals-find.test.ts | 2 +- .../tests/signals-vanilla/signals-ingestion.test.ts | 2 +- .../tests/signals-vanilla/signals-redaction.test.ts | 2 +- .../tests/signals-vanilla/top-level-metadata.test.ts | 2 +- 22 files changed, 38 insertions(+), 21 deletions(-) create mode 100644 packages/signals/signals-integration-tests/src/helpers/env-config.ts diff --git a/.buildkite/pipeline.yml b/.buildkite/pipeline.yml index d03fd26d5..05bcf542f 100644 --- a/.buildkite/pipeline.yml +++ b/.buildkite/pipeline.yml @@ -84,6 +84,7 @@ steps: - yarn install --immutable - echo "+++ Run Browser integration tests :pray:" - yarn turbo run --filter=@internal/browser-integration-tests test:int + - yarn turbo run --filter=@internal/browser-integration-tests test:int retry: automatic: - exit_status: '*' diff --git a/packages/signals/signals-integration-tests/package.json b/packages/signals/signals-integration-tests/package.json index fc53d72e5..44469e3f9 100644 --- a/packages/signals/signals-integration-tests/package.json +++ b/packages/signals/signals-integration-tests/package.json @@ -8,10 +8,11 @@ "scripts": { ".": "yarn run -T turbo run --filter=@internal/signals-integration-tests...", "build": "webpack", - "test:int": "playwright test", + "test:int": "playwright test && yarn test:global-sandbox", "test:vanilla": "playwright test src/tests/signals-vanilla", "test:perf": "playwright test src/tests/performance", "test:custom": "playwright test src/tests/custom", + "test:global-sandbox": "SANDBOX_STRATEGY=global yarn test:vanilla && SANDBOX_STRATEGY=global yarn test:custom", "watch": "webpack -w", "lint": "yarn concurrently 'yarn:eslint .' 'yarn:tsc --noEmit'", "concurrently": "yarn run -T concurrently", diff --git a/packages/signals/signals-integration-tests/playwright.global-setup.ts b/packages/signals/signals-integration-tests/playwright.global-setup.ts index 6a3fec2b1..310971fe8 100644 --- a/packages/signals/signals-integration-tests/playwright.global-setup.ts +++ b/packages/signals/signals-integration-tests/playwright.global-setup.ts @@ -1,8 +1,10 @@ import type { FullConfig } from '@playwright/test' import { execSync } from 'child_process' +import { envConfig } from './src/helpers/env-config' export default function globalSetup(_cfg: FullConfig) { - console.log('Executing global setup...') + console.log(`Executing playwright.global-setup.ts...\n`) + console.log(`Using envConfig: ${JSON.stringify(envConfig, undefined, 2)}\n`) execSync('yarn build', { stdio: 'inherit' }) - console.log('Finished global setup.') + console.log('Finished global setup.\n') } diff --git a/packages/signals/signals-integration-tests/src/helpers/base-page-object.ts b/packages/signals/signals-integration-tests/src/helpers/base-page-object.ts index de5361aac..97d8c8078 100644 --- a/packages/signals/signals-integration-tests/src/helpers/base-page-object.ts +++ b/packages/signals/signals-integration-tests/src/helpers/base-page-object.ts @@ -7,6 +7,7 @@ import { SignalAPIRequestBuffer, TrackingAPIRequestBuffer, } from './network-utils' +import { envConfig } from './env-config' export class BasePage { protected page!: Page @@ -63,6 +64,7 @@ export class BasePage { await this.page.goto(url, { waitUntil: 'domcontentloaded' }) if (!options.skipSignalsPluginInit) { void this.invokeAnalyticsLoad({ + sandboxStrategy: envConfig.SANDBOX_STRATEGY ?? 'iframe', flushInterval: 500, ...signalSettings, }) diff --git a/packages/signals/signals-integration-tests/src/helpers/env-config.ts b/packages/signals/signals-integration-tests/src/helpers/env-config.ts new file mode 100644 index 000000000..37c2e48ea --- /dev/null +++ b/packages/signals/signals-integration-tests/src/helpers/env-config.ts @@ -0,0 +1,11 @@ +import { SignalsSettingsConfig } from '@segment/analytics-signals/dist/types/core/signals' + +// This is for testing with the global sandbox strategy with an npm script, that executes processSignal in the global scope +// If we change this to be the default, this can be rejiggered +const SANDBOX_STRATEGY = process.env.SANDBOX_STRATEGY as + | SignalsSettingsConfig['sandboxStrategy'] + | undefined + +export const envConfig = { + SANDBOX_STRATEGY, +} diff --git a/packages/signals/signals-integration-tests/src/tests/custom-elements/custom-select.test.ts b/packages/signals/signals-integration-tests/src/tests/custom-elements/custom-select.test.ts index 9851eb719..03b2ed2d7 100644 --- a/packages/signals/signals-integration-tests/src/tests/custom-elements/custom-select.test.ts +++ b/packages/signals/signals-integration-tests/src/tests/custom-elements/custom-select.test.ts @@ -3,7 +3,7 @@ import { waitForCondition } from '../../helpers/playwright-utils' import { IndexPage } from './index-page' import type { SegmentEvent } from '@segment/analytics-next' -const basicEdgeFn = `const processSignal = (signal) => {}` +const basicEdgeFn = `globalThis.processSignal = (signal) => {}` test('Collecting signals whenever a user selects an item', async ({ page }) => { const indexPage = await new IndexPage().loadAndWait(page, basicEdgeFn, { diff --git a/packages/signals/signals-integration-tests/src/tests/custom-elements/custom-textfield.test.ts b/packages/signals/signals-integration-tests/src/tests/custom-elements/custom-textfield.test.ts index 827c8b9cf..b7199fbeb 100644 --- a/packages/signals/signals-integration-tests/src/tests/custom-elements/custom-textfield.test.ts +++ b/packages/signals/signals-integration-tests/src/tests/custom-elements/custom-textfield.test.ts @@ -2,7 +2,7 @@ import { test, expect } from '@playwright/test' import { waitForCondition } from '../../helpers/playwright-utils' import { IndexPage } from './index-page' -const basicEdgeFn = `const processSignal = (signal) => {}` +const basicEdgeFn = `globalThis.processSignal = (signal) => {}` test('Collecting signals whenever a user enters text input and focuses out', async ({ page, diff --git a/packages/signals/signals-integration-tests/src/tests/performance/memory-leak.test.ts b/packages/signals/signals-integration-tests/src/tests/performance/memory-leak.test.ts index a8ecb23e1..2d1a7e891 100644 --- a/packages/signals/signals-integration-tests/src/tests/performance/memory-leak.test.ts +++ b/packages/signals/signals-integration-tests/src/tests/performance/memory-leak.test.ts @@ -16,7 +16,7 @@ declare global { const basicEdgeFn = ` // this is a process signal function - const processSignal = (signal) => { + globalThis.processSignal = (signal) => { if (signal.type === 'interaction') { const eventName = signal.data.eventType + ' ' + '[' + signal.type + ']' analytics.track(eventName, signal.data) diff --git a/packages/signals/signals-integration-tests/src/tests/signals-vanilla/all-segment-events.test.ts b/packages/signals/signals-integration-tests/src/tests/signals-vanilla/all-segment-events.test.ts index 53b45f360..58b3cafdf 100644 --- a/packages/signals/signals-integration-tests/src/tests/signals-vanilla/all-segment-events.test.ts +++ b/packages/signals/signals-integration-tests/src/tests/signals-vanilla/all-segment-events.test.ts @@ -37,7 +37,7 @@ const snapshot = ( test('Segment events', async ({ page }) => { const basicEdgeFn = ` // this is a process signal function - const processSignal = (signal) => { + globalThis.processSignal = (signal) => { if (signal.type === 'interaction' && signal.data.eventType === 'click') { analytics.identify('john', { found: true }) analytics.group('foo', { hello: 'world' }) @@ -65,7 +65,7 @@ test('Should dispatch events from signals that occurred before analytics was ins page, }) => { const edgeFn = ` - const processSignal = (signal) => { + globalThis.processSignal = (signal) => { if (signal.type === 'navigation' && signal.data.action === 'pageLoad') { analytics.page('dispatched from signals - navigation') } diff --git a/packages/signals/signals-integration-tests/src/tests/signals-vanilla/basic.test.ts b/packages/signals/signals-integration-tests/src/tests/signals-vanilla/basic.test.ts index cbb0993ff..ff618682e 100644 --- a/packages/signals/signals-integration-tests/src/tests/signals-vanilla/basic.test.ts +++ b/packages/signals/signals-integration-tests/src/tests/signals-vanilla/basic.test.ts @@ -4,7 +4,7 @@ import { IndexPage } from './index-page' const basicEdgeFn = ` // this is a process signal function - const processSignal = (signal) => { + globalThis.processSignal = (signal) => { if (signal.type === 'interaction') { const eventName = signal.data.eventType + ' ' + '[' + signal.type + ']' analytics.track(eventName, signal.data) diff --git a/packages/signals/signals-integration-tests/src/tests/signals-vanilla/button-click-complex.test.ts b/packages/signals/signals-integration-tests/src/tests/signals-vanilla/button-click-complex.test.ts index 28eb61c32..864dec3b1 100644 --- a/packages/signals/signals-integration-tests/src/tests/signals-vanilla/button-click-complex.test.ts +++ b/packages/signals/signals-integration-tests/src/tests/signals-vanilla/button-click-complex.test.ts @@ -1,7 +1,7 @@ import { test, expect } from '@playwright/test' import { IndexPage } from './index-page' -const basicEdgeFn = `const processSignal = (signal) => {}` +const basicEdgeFn = `globalThis.processSignal = (signal) => {}` let indexPage: IndexPage test.beforeEach(async ({ page }) => { indexPage = await new IndexPage().loadAndWait(page, basicEdgeFn) diff --git a/packages/signals/signals-integration-tests/src/tests/signals-vanilla/change-input.test.ts b/packages/signals/signals-integration-tests/src/tests/signals-vanilla/change-input.test.ts index 94c0044c4..df297f8b6 100644 --- a/packages/signals/signals-integration-tests/src/tests/signals-vanilla/change-input.test.ts +++ b/packages/signals/signals-integration-tests/src/tests/signals-vanilla/change-input.test.ts @@ -2,7 +2,7 @@ import { test, expect } from '@playwright/test' import { waitForCondition } from '../../helpers/playwright-utils' import { IndexPage } from './index-page' -const basicEdgeFn = `const processSignal = (signal) => {}` +const basicEdgeFn = `globalThis.processSignal = (signal) => {}` test('Collecting signals whenever a user enters text input', async ({ page, diff --git a/packages/signals/signals-integration-tests/src/tests/signals-vanilla/middleware.test.ts b/packages/signals/signals-integration-tests/src/tests/signals-vanilla/middleware.test.ts index 3799bc0b5..244a787d7 100644 --- a/packages/signals/signals-integration-tests/src/tests/signals-vanilla/middleware.test.ts +++ b/packages/signals/signals-integration-tests/src/tests/signals-vanilla/middleware.test.ts @@ -1,7 +1,7 @@ import { test, expect } from '@playwright/test' import { IndexPage } from './index-page' -const basicEdgeFn = `const processSignal = (signal) => {}` +const basicEdgeFn = `globalThis.processSignal = (signal) => {}` let indexPage: IndexPage diff --git a/packages/signals/signals-integration-tests/src/tests/signals-vanilla/network-signals-allow-list.test.ts b/packages/signals/signals-integration-tests/src/tests/signals-vanilla/network-signals-allow-list.test.ts index 84323ac04..097813b6e 100644 --- a/packages/signals/signals-integration-tests/src/tests/signals-vanilla/network-signals-allow-list.test.ts +++ b/packages/signals/signals-integration-tests/src/tests/signals-vanilla/network-signals-allow-list.test.ts @@ -1,7 +1,7 @@ import { test, expect } from '@playwright/test' import { IndexPage } from './index-page' -const basicEdgeFn = `const processSignal = (signal) => {}` +const basicEdgeFn = `globalThis.processSignal = (signal) => {}` test('network signals allow and disallow list', async ({ page }) => { const indexPage = await new IndexPage().loadAndWait(page, basicEdgeFn, { diff --git a/packages/signals/signals-integration-tests/src/tests/signals-vanilla/network-signals-fetch.test.ts b/packages/signals/signals-integration-tests/src/tests/signals-vanilla/network-signals-fetch.test.ts index 868b089d4..6d92d67bc 100644 --- a/packages/signals/signals-integration-tests/src/tests/signals-vanilla/network-signals-fetch.test.ts +++ b/packages/signals/signals-integration-tests/src/tests/signals-vanilla/network-signals-fetch.test.ts @@ -2,7 +2,7 @@ import { test, expect } from '@playwright/test' import { commonSignalData } from '../../helpers/fixtures' import { IndexPage } from './index-page' -const basicEdgeFn = `const processSignal = (signal) => {}` +const basicEdgeFn = `globalThis.processSignal = (signal) => {}` test.describe('network signals - fetch', () => { let indexPage: IndexPage diff --git a/packages/signals/signals-integration-tests/src/tests/signals-vanilla/network-signals-xhr.test.ts b/packages/signals/signals-integration-tests/src/tests/signals-vanilla/network-signals-xhr.test.ts index 7fb9b8ecd..13729c583 100644 --- a/packages/signals/signals-integration-tests/src/tests/signals-vanilla/network-signals-xhr.test.ts +++ b/packages/signals/signals-integration-tests/src/tests/signals-vanilla/network-signals-xhr.test.ts @@ -1,7 +1,7 @@ import { test, expect } from '@playwright/test' import { IndexPage } from './index-page' -const basicEdgeFn = `const processSignal = (signal) => {}` +const basicEdgeFn = `globalThis.processSignal = (signal) => {}` test.describe('network signals - XHR', () => { let indexPage: IndexPage diff --git a/packages/signals/signals-integration-tests/src/tests/signals-vanilla/reset.test.ts b/packages/signals/signals-integration-tests/src/tests/signals-vanilla/reset.test.ts index bdeb9aa5f..5c42651c1 100644 --- a/packages/signals/signals-integration-tests/src/tests/signals-vanilla/reset.test.ts +++ b/packages/signals/signals-integration-tests/src/tests/signals-vanilla/reset.test.ts @@ -7,7 +7,7 @@ import { pTimeout } from '@segment/analytics-core' * If a signal is generated, the signal buffer should be reset * when the user clicks on the complex button. */ -const edgeFn = `const processSignal = (signal) => { +const edgeFn = `globalThis.processSignal = (signal) => { // create a custom signal to echo out the current signal buffer if (signal.type === 'userDefined') { analytics.track('current signal buffer', { signalBuffer: signals.signalBuffer }) diff --git a/packages/signals/signals-integration-tests/src/tests/signals-vanilla/runtime-constants.test.ts b/packages/signals/signals-integration-tests/src/tests/signals-vanilla/runtime-constants.test.ts index c0ea50242..644d18366 100644 --- a/packages/signals/signals-integration-tests/src/tests/signals-vanilla/runtime-constants.test.ts +++ b/packages/signals/signals-integration-tests/src/tests/signals-vanilla/runtime-constants.test.ts @@ -2,7 +2,7 @@ import { test, expect } from '@playwright/test' import { IndexPage } from './index-page' const basicEdgeFn = ` - const processSignal = (signal) => { + globalThis.processSignal = (signal) => { // test that constants are properly injected if (typeof EventType !== 'object') { throw new Error('EventType is missing?') diff --git a/packages/signals/signals-integration-tests/src/tests/signals-vanilla/signals-find.test.ts b/packages/signals/signals-integration-tests/src/tests/signals-vanilla/signals-find.test.ts index 5018db7f0..d9099f337 100644 --- a/packages/signals/signals-integration-tests/src/tests/signals-vanilla/signals-find.test.ts +++ b/packages/signals/signals-integration-tests/src/tests/signals-vanilla/signals-find.test.ts @@ -4,7 +4,7 @@ import { IndexPage } from './index-page' const indexPage = new IndexPage() test('should find the most recent signal', async ({ page }) => { - const basicEdgeFn = `const processSignal = (signal) => { + const basicEdgeFn = `globalThis.processSignal = (signal) => { if (signal.type === 'interaction' && signal.data.target.id === 'complex-button') { const mostRecentSignal = signals.find(signal, 'userDefined') if (mostRecentSignal.data.num === 2) { diff --git a/packages/signals/signals-integration-tests/src/tests/signals-vanilla/signals-ingestion.test.ts b/packages/signals/signals-integration-tests/src/tests/signals-vanilla/signals-ingestion.test.ts index e0f7262c0..25f25d4b3 100644 --- a/packages/signals/signals-integration-tests/src/tests/signals-vanilla/signals-ingestion.test.ts +++ b/packages/signals/signals-integration-tests/src/tests/signals-vanilla/signals-ingestion.test.ts @@ -4,7 +4,7 @@ import { waitForCondition } from '../../helpers/playwright-utils' const indexPage = new IndexPage() -const basicEdgeFn = `const processSignal = (signal) => {}` +const basicEdgeFn = `globalThis.processSignal = (signal) => {}` test('debug ingestion disabled and sample rate 0 -> will not send the signal', async ({ page, diff --git a/packages/signals/signals-integration-tests/src/tests/signals-vanilla/signals-redaction.test.ts b/packages/signals/signals-integration-tests/src/tests/signals-vanilla/signals-redaction.test.ts index a03d82b42..88e09b54d 100644 --- a/packages/signals/signals-integration-tests/src/tests/signals-vanilla/signals-redaction.test.ts +++ b/packages/signals/signals-integration-tests/src/tests/signals-vanilla/signals-redaction.test.ts @@ -2,7 +2,7 @@ import { test, expect } from '@playwright/test' import { waitForCondition } from '../../helpers/playwright-utils' import { IndexPage } from './index-page' -const basicEdgeFn = `const processSignal = (signal) => {}` +const basicEdgeFn = `globalThis.processSignal = (signal) => {}` test('redaction enabled -> will XXX the value of text input', async ({ page, diff --git a/packages/signals/signals-integration-tests/src/tests/signals-vanilla/top-level-metadata.test.ts b/packages/signals/signals-integration-tests/src/tests/signals-vanilla/top-level-metadata.test.ts index f956d3e37..02f22caea 100644 --- a/packages/signals/signals-integration-tests/src/tests/signals-vanilla/top-level-metadata.test.ts +++ b/packages/signals/signals-integration-tests/src/tests/signals-vanilla/top-level-metadata.test.ts @@ -4,7 +4,7 @@ import { IndexPage } from './index-page' const basicEdgeFn = ` // this is a process signal function - const processSignal = (signal) => { + globalThis.processSignal = (signal) => { if (signal.type === 'interaction') { analytics.track('hello', { myAnonId: signal.anonymousId, myTimestamp: signal.timestamp }) } From 2870f36ef4263d790627cd0b8b552a763525912d Mon Sep 17 00:00:00 2001 From: Seth Silesky <5115498+silesky@users.noreply.github.com> Date: Wed, 30 Apr 2025 13:23:44 -0500 Subject: [PATCH 12/16] wip --- .../src/helpers/base-page-object.ts | 2 +- .../signals-integration-tests/src/helpers/env-config.ts | 5 ++--- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/packages/signals/signals-integration-tests/src/helpers/base-page-object.ts b/packages/signals/signals-integration-tests/src/helpers/base-page-object.ts index 97d8c8078..13960dec6 100644 --- a/packages/signals/signals-integration-tests/src/helpers/base-page-object.ts +++ b/packages/signals/signals-integration-tests/src/helpers/base-page-object.ts @@ -64,7 +64,7 @@ export class BasePage { await this.page.goto(url, { waitUntil: 'domcontentloaded' }) if (!options.skipSignalsPluginInit) { void this.invokeAnalyticsLoad({ - sandboxStrategy: envConfig.SANDBOX_STRATEGY ?? 'iframe', + sandboxStrategy: envConfig.SANDBOX_STRATEGY, flushInterval: 500, ...signalSettings, }) diff --git a/packages/signals/signals-integration-tests/src/helpers/env-config.ts b/packages/signals/signals-integration-tests/src/helpers/env-config.ts index 37c2e48ea..d21f8efd7 100644 --- a/packages/signals/signals-integration-tests/src/helpers/env-config.ts +++ b/packages/signals/signals-integration-tests/src/helpers/env-config.ts @@ -2,9 +2,8 @@ import { SignalsSettingsConfig } from '@segment/analytics-signals/dist/types/cor // This is for testing with the global sandbox strategy with an npm script, that executes processSignal in the global scope // If we change this to be the default, this can be rejiggered -const SANDBOX_STRATEGY = process.env.SANDBOX_STRATEGY as - | SignalsSettingsConfig['sandboxStrategy'] - | undefined +const SANDBOX_STRATEGY = (process.env.SANDBOX_STRATEGY ?? + 'iframe') as SignalsSettingsConfig['sandboxStrategy'] export const envConfig = { SANDBOX_STRATEGY, From 36b1f371121df28e5ec378a8eff3535fabed0294 Mon Sep 17 00:00:00 2001 From: Seth Silesky <5115498+silesky@users.noreply.github.com> Date: Wed, 30 Apr 2025 13:36:16 -0500 Subject: [PATCH 13/16] update t3ests --- packages/signals/signals-integration-tests/package.json | 4 ++-- .../signals-integration-tests/playwright.global-setup.ts | 7 +++++-- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/packages/signals/signals-integration-tests/package.json b/packages/signals/signals-integration-tests/package.json index 44469e3f9..17b92ea26 100644 --- a/packages/signals/signals-integration-tests/package.json +++ b/packages/signals/signals-integration-tests/package.json @@ -8,11 +8,11 @@ "scripts": { ".": "yarn run -T turbo run --filter=@internal/signals-integration-tests...", "build": "webpack", - "test:int": "playwright test && yarn test:global-sandbox", + "test:int": "playwright test && SKIP_BUILD=true yarn test:global-sandbox", "test:vanilla": "playwright test src/tests/signals-vanilla", "test:perf": "playwright test src/tests/performance", "test:custom": "playwright test src/tests/custom", - "test:global-sandbox": "SANDBOX_STRATEGY=global yarn test:vanilla && SANDBOX_STRATEGY=global yarn test:custom", + "test:global-sandbox": "SANDBOX_STRATEGY=global playwright test src/tests/signals-vanilla src/tests/custom", "watch": "webpack -w", "lint": "yarn concurrently 'yarn:eslint .' 'yarn:tsc --noEmit'", "concurrently": "yarn run -T concurrently", diff --git a/packages/signals/signals-integration-tests/playwright.global-setup.ts b/packages/signals/signals-integration-tests/playwright.global-setup.ts index 310971fe8..2fb62b2f9 100644 --- a/packages/signals/signals-integration-tests/playwright.global-setup.ts +++ b/packages/signals/signals-integration-tests/playwright.global-setup.ts @@ -5,6 +5,9 @@ import { envConfig } from './src/helpers/env-config' export default function globalSetup(_cfg: FullConfig) { console.log(`Executing playwright.global-setup.ts...\n`) console.log(`Using envConfig: ${JSON.stringify(envConfig, undefined, 2)}\n`) - execSync('yarn build', { stdio: 'inherit' }) - console.log('Finished global setup.\n') + if (process.env.SKIP_BUILD !== 'true') { + console.log(`Executing yarn build:\n`) + execSync('yarn build', { stdio: 'inherit' }) + } + console.log('Finished global setup. Should start running tests.\n') } From 83ff15d4b8c81b71563b00c561f83854b734aff5 Mon Sep 17 00:00:00 2001 From: Seth Silesky <5115498+silesky@users.noreply.github.com> Date: Wed, 30 Apr 2025 13:38:24 -0500 Subject: [PATCH 14/16] add env-config.ts --- .../signals-integration-tests/src/helpers/env-config.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/signals/signals-integration-tests/src/helpers/env-config.ts b/packages/signals/signals-integration-tests/src/helpers/env-config.ts index d21f8efd7..880918dc6 100644 --- a/packages/signals/signals-integration-tests/src/helpers/env-config.ts +++ b/packages/signals/signals-integration-tests/src/helpers/env-config.ts @@ -1,9 +1,9 @@ -import { SignalsSettingsConfig } from '@segment/analytics-signals/dist/types/core/signals' +import { SignalsPluginSettingsConfig } from '@segment/analytics-signals' // This is for testing with the global sandbox strategy with an npm script, that executes processSignal in the global scope // If we change this to be the default, this can be rejiggered const SANDBOX_STRATEGY = (process.env.SANDBOX_STRATEGY ?? - 'iframe') as SignalsSettingsConfig['sandboxStrategy'] + 'iframe') as SignalsPluginSettingsConfig['sandboxStrategy'] export const envConfig = { SANDBOX_STRATEGY, From 60f65fac7fa6ccfdd223d3f97a0aaba303b4affb Mon Sep 17 00:00:00 2001 From: Seth Silesky <5115498+silesky@users.noreply.github.com> Date: Wed, 30 Apr 2025 13:44:22 -0500 Subject: [PATCH 15/16] wip --- .buildkite/pipeline.yml | 1 - .eslintrc.js | 1 + 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/.buildkite/pipeline.yml b/.buildkite/pipeline.yml index 05bcf542f..d03fd26d5 100644 --- a/.buildkite/pipeline.yml +++ b/.buildkite/pipeline.yml @@ -84,7 +84,6 @@ steps: - yarn install --immutable - echo "+++ Run Browser integration tests :pray:" - yarn turbo run --filter=@internal/browser-integration-tests test:int - - yarn turbo run --filter=@internal/browser-integration-tests test:int retry: automatic: - exit_status: '*' diff --git a/.eslintrc.js b/.eslintrc.js index 380e87742..bbd7bcadf 100644 --- a/.eslintrc.js +++ b/.eslintrc.js @@ -67,6 +67,7 @@ module.exports = { '@typescript-eslint/no-restricted-imports': [ 'error', { + patterns: [], paths: [ { // Prevent accidental imports from 'lodash' From 098319b75a48ebd37cbe31f1f5d8b8465775e51e Mon Sep 17 00:00:00 2001 From: Seth Silesky <5115498+silesky@users.noreply.github.com> Date: Wed, 30 Apr 2025 13:45:28 -0500 Subject: [PATCH 16/16] wip --- .eslintrc.js | 1 - 1 file changed, 1 deletion(-) diff --git a/.eslintrc.js b/.eslintrc.js index bbd7bcadf..380e87742 100644 --- a/.eslintrc.js +++ b/.eslintrc.js @@ -67,7 +67,6 @@ module.exports = { '@typescript-eslint/no-restricted-imports': [ 'error', { - patterns: [], paths: [ { // Prevent accidental imports from 'lodash'