hacky sandbox strategy

Author: silesky

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,

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)

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<void>
   execute(
     signal: Signal,
     signals: Signal[]
@@ -220,10 +226,6 @@ export class WorkerSandbox implements SignalSandbox {
     this.jsSandbox = new JavascriptSandbox()
   }
 
-  isLoaded(): Promise<any> {
-    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<HTMLScriptElement>
-  async isLoaded(): Promise<void> {
-    await this.script
-  }
+  htmlScriptLoaded: Promise<HTMLScriptElement>
+
   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<void> {}
   execute(_signal: Signal, _signals: Signal[]) {
     return Promise.resolve(undefined)
   }