From bec8752cf0ddeced00ea8a570c41864e60b72dcf Mon Sep 17 00:00:00 2001 From: Seth Silesky <5115498+silesky@users.noreply.github.com> Date: Wed, 17 Jul 2024 14:24:27 -0500 Subject: [PATCH 1/3] fix --- packages/signals/signals/src/core/processor/sandbox.ts | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/packages/signals/signals/src/core/processor/sandbox.ts b/packages/signals/signals/src/core/processor/sandbox.ts index 51c2bb3e1..75c214544 100644 --- a/packages/signals/signals/src/core/processor/sandbox.ts +++ b/packages/signals/signals/src/core/processor/sandbox.ts @@ -145,7 +145,12 @@ export class SandboxSettings { : settings.edgeFnDownloadURL if (!edgeFnDownloadURLNormalized && !settings.processSignal) { - throw new Error('edgeFnDownloadURL or processSignal must be defined') + // 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('edgeFnDownloadURL or processSignal must be defined') + return } const fetch = settings.edgeFnFetchClient ?? globalThis.fetch From 11ca952c4ccaee13e61f71e25ff12fa57cbeae1d Mon Sep 17 00:00:00 2001 From: Seth Silesky <5115498+silesky@users.noreply.github.com> Date: Wed, 17 Jul 2024 16:32:50 -0500 Subject: [PATCH 2/3] fix processSignal onboarding issue if no edge fn exists --- .../__tests__/sandbox-settings.test.ts | 54 +++++++++++++++++++ .../signals/src/core/processor/sandbox.ts | 4 +- 2 files changed, 57 insertions(+), 1 deletion(-) create mode 100644 packages/signals/signals/src/core/processor/__tests__/sandbox-settings.test.ts 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 new file mode 100644 index 000000000..b1912ec49 --- /dev/null +++ b/packages/signals/signals/src/core/processor/__tests__/sandbox-settings.test.ts @@ -0,0 +1,54 @@ +import { SandboxSettings, SandboxSettingsConfig } from '../sandbox' + +describe(SandboxSettings, () => { + const edgeFnResponseBody = `function processSignal() { console.log('hello world') }` + const baseSettings: SandboxSettingsConfig = { + functionHost: undefined, + processSignal: undefined, + edgeFnDownloadURL: 'http://example.com/download', + edgeFnFetchClient: jest.fn().mockReturnValue( + Promise.resolve({ + text: () => edgeFnResponseBody, + }) + ), + } + test('initializes with provided settings', async () => { + const sandboxSettings = new SandboxSettings({ ...baseSettings }) + expect(baseSettings.edgeFnFetchClient).toHaveBeenCalledWith( + baseSettings.edgeFnDownloadURL + ) + expect(await sandboxSettings.processSignal).toEqual(edgeFnResponseBody) + }) + + test('normalizes edgeFnDownloadURL when functionHost is provided', async () => { + const settings: SandboxSettingsConfig = { + ...baseSettings, + processSignal: undefined, + functionHost: 'newHost.com', + edgeFnDownloadURL: 'https://original.com/download', + } + new SandboxSettings(settings) + expect(baseSettings.edgeFnFetchClient).toHaveBeenCalledWith( + 'https://newHost.com/download' + ) + }) + + test('creates default processSignal when parameters are missing', async () => { + const consoleWarnSpy = jest + .spyOn(console, 'warn') + .mockImplementation(() => {}) + const settings: SandboxSettingsConfig = { + ...baseSettings, + processSignal: undefined, + edgeFnDownloadURL: undefined, + } + const sandboxSettings = new SandboxSettings(settings) + expect(await sandboxSettings.processSignal).toEqual( + 'globalThis.processSignal = function processSignal() {}' + ) + expect(baseSettings.edgeFnFetchClient).not.toHaveBeenCalled() + expect(consoleWarnSpy).toHaveBeenCalledWith( + expect.stringContaining('processSignal') + ) + }) +}) diff --git a/packages/signals/signals/src/core/processor/sandbox.ts b/packages/signals/signals/src/core/processor/sandbox.ts index 75c214544..e105ffd05 100644 --- a/packages/signals/signals/src/core/processor/sandbox.ts +++ b/packages/signals/signals/src/core/processor/sandbox.ts @@ -149,7 +149,9 @@ export class SandboxSettings { this.processSignal = Promise.resolve( `globalThis.processSignal = function processSignal() {}` ) - console.warn('edgeFnDownloadURL or processSignal must be defined') + console.warn( + `No processSignal function found. Have you written a processSignal function on app.segment.com?` + ) return } From 24afb22d8e806be3d01122e06e770b5470c9244c Mon Sep 17 00:00:00 2001 From: Seth Silesky <5115498+silesky@users.noreply.github.com> Date: Wed, 17 Jul 2024 16:38:31 -0500 Subject: [PATCH 3/3] add changeset --- .changeset/yellow-camels-kiss.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/yellow-camels-kiss.md diff --git a/.changeset/yellow-camels-kiss.md b/.changeset/yellow-camels-kiss.md new file mode 100644 index 000000000..84130ed65 --- /dev/null +++ b/.changeset/yellow-camels-kiss.md @@ -0,0 +1,5 @@ +--- +'@segment/analytics-signals': patch +--- + +Allow collecting signals from sources without an edge function written yet