From 10d7dd02a6b5d2d37de17e9cb16258babcc27b2c Mon Sep 17 00:00:00 2001 From: Seth Silesky <5115498+silesky@users.noreply.github.com> Date: Wed, 30 Oct 2024 13:27:44 -0500 Subject: [PATCH 1/6] wip --- .changeset/little-chefs-hunt.md | 8 ++ .../src/helpers/network-utils.ts | 25 +++++ .../src/tests/signals-vanilla/basic.test.ts | 3 +- .../signals-vanilla/change-input.test.ts | 4 + .../network-signals-fetch.test.ts | 103 +++++++++++------- .../network-signals-xhr.test.ts | 43 +++----- .../signals-vanilla/signals-ingestion.test.ts | 7 +- .../src/web/web-signals-types.ts | 1 + .../src/core/buffer/__tests__/buffer.test.ts | 2 +- .../src/core/client/__tests__/client.test.ts | 2 + .../src/core/client/__tests__/redact.test.ts | 2 + .../src/core/signal-generators/dom-gen.ts | 102 +++++++++++++++-- .../network-gen/__tests__/helpers.test.ts | 7 ++ .../__tests__/network-generator.test.ts | 3 + .../signal-generators/network-gen/helpers.ts | 21 ++-- .../signal-generators/network-gen/index.ts | 16 ++- .../signals/src/core/signals/signals.ts | 3 + .../signals/signals/src/lib/exists/index.ts | 7 ++ .../__tests__/create-network-signal.test.ts | 4 + 19 files changed, 264 insertions(+), 99 deletions(-) create mode 100644 .changeset/little-chefs-hunt.md create mode 100644 packages/signals/signals/src/lib/exists/index.ts diff --git a/.changeset/little-chefs-hunt.md b/.changeset/little-chefs-hunt.md new file mode 100644 index 000000000..41c2bdc50 --- /dev/null +++ b/.changeset/little-chefs-hunt.md @@ -0,0 +1,8 @@ +--- +'@segment/analytics-signals': minor +--- +- Fix runtime errors for submit +- Add better form submit data +- Loosen content-type to parse text/plain +- Tweak disallow list +- Add labels diff --git a/packages/signals/signals-integration-tests/src/helpers/network-utils.ts b/packages/signals/signals-integration-tests/src/helpers/network-utils.ts index d9f0651bf..b9f7e9e57 100644 --- a/packages/signals/signals-integration-tests/src/helpers/network-utils.ts +++ b/packages/signals/signals-integration-tests/src/helpers/network-utils.ts @@ -57,6 +57,30 @@ export class PageNetworkUtils { await req return responseBody } + async makeFileUploadRequest(url: string) { + let normalizeUrl = url + if (url.startsWith('/')) { + normalizeUrl = new URL(url, this.page.url()).href + } + const req = this.page.waitForResponse(normalizeUrl ?? url, { + timeout: this.defaultResponseTimeout, + }) + await this.page.evaluate((_url) => { + const formData = new FormData() + const file = new File(['file content'], 'test.txt', { + type: 'text/plain', + }) + formData.append('file', file) + return fetch(_url, { + method: 'POST', + body: formData, + headers: { + 'Content-Type': 'multipart/form-data', + }, + }) + }, normalizeUrl) + await req + } /** * Make a fetch call in the page context. By default it will POST a JSON object with {foo: 'bar'} */ @@ -64,6 +88,7 @@ export class PageNetworkUtils { url = this.defaultTestApiURL, request: Partial & { contentType?: string + blob?: boolean } = {} ): Promise { let normalizeUrl = url 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 2e1aea0e8..76fd4aa4f 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 @@ -164,10 +164,11 @@ test('navigation signals', async ({ page }) => { // navigate to a new hash { + const flush = indexPage.waitForSignalsApiFlush() await page.evaluate(() => { window.location.hash = '#foo' }) - await indexPage.waitForSignalsApiFlush() + await flush expect(indexPage.signalsAPI.getEvents()).toHaveLength(2) const ev = indexPage.signalsAPI.lastEvent('navigation') expect(ev.properties).toMatchObject({ 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 fb8e9fccc..94c0044c4 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 @@ -39,6 +39,10 @@ test('Collecting signals whenever a user enters text input', async ({ labels: [ { textContent: 'Name:', + id: '', + attributes: { + for: 'name', + }, }, ], name: 'name', 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 8b2739064..34d65cb9d 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 @@ -10,12 +10,60 @@ test.describe('network signals - fetch', () => { indexPage = await new IndexPage().loadAndWait(page, basicEdgeFn) }) - test('should not emit anything if neither request nor response are json', async () => { + test('should not emit non-json requests', async () => { + await indexPage.network.mockTestRoute('http://localhost/upload', { + body: JSON.stringify({ foo: 'test' }), + }) + + await indexPage.network.makeFileUploadRequest('http://localhost/upload') + + await indexPage.waitForSignalsApiFlush() + + const networkEvents = indexPage.signalsAPI.getEvents('network') + + // Check the request + const requests = networkEvents.filter( + (el) => el.properties!.data.action === 'request' + ) + + expect(requests).toHaveLength(0) + }) + + test('should try to parse the body of text/plain requests', async () => { await indexPage.network.mockTestRoute('http://localhost/test', { - body: 'hello', + body: JSON.stringify({ foo: 'test' }), + contentType: 'application/json', + }) + + await indexPage.network.makeFetchCall('http://localhost/test', { + method: 'POST', + body: JSON.stringify({ key: 'value' }), contentType: 'text/plain', }) + await indexPage.waitForSignalsApiFlush() + + const networkEvents = indexPage.signalsAPI.getEvents('network') + + // Check the request + const requests = networkEvents.filter( + (el) => el.properties!.data.action === 'request' + ) + + expect(requests).toHaveLength(1) + expect(requests[0].properties!.data).toMatchObject({ + action: 'request', + url: 'http://localhost/test', + data: { key: 'value' }, + }) + }) + + test('should send the raw string if the request body cannot be parsed as json', async () => { + await indexPage.network.mockTestRoute('http://localhost/test', { + body: JSON.stringify({ foo: 'test' }), + contentType: 'application/json', + }) + await indexPage.network.makeFetchCall('http://localhost/test', { method: 'POST', body: 'hello world', @@ -26,7 +74,17 @@ test.describe('network signals - fetch', () => { const networkEvents = indexPage.signalsAPI.getEvents('network') - expect(networkEvents).toHaveLength(0) + // Check the request + const requests = networkEvents.filter( + (el) => el.properties!.data.action === 'request' + ) + + expect(requests).toHaveLength(1) + expect(requests[0].properties!.data).toMatchObject({ + action: 'request', + url: 'http://localhost/test', + data: 'hello world', + }) }) test('can make a basic json request / not break regular fetch calls', async () => { @@ -112,39 +170,6 @@ test.describe('network signals - fetch', () => { }) }) - test('should emit response but not request if request content-type is not json but response is', async () => { - await indexPage.network.mockTestRoute('http://localhost/test', { - body: JSON.stringify({ foo: 'test' }), - contentType: 'application/json', - }) - - await indexPage.network.makeFetchCall('http://localhost/test', { - method: 'POST', - body: 'hello world', - contentType: 'text/plain', - }) - - await indexPage.waitForSignalsApiFlush() - - const networkEvents = indexPage.signalsAPI.getEvents('network') - - const responses = networkEvents.filter( - (el) => el.properties!.data.action === 'response' - ) - expect(responses).toHaveLength(1) - expect(responses[0].properties!.data).toMatchObject({ - action: 'response', - url: 'http://localhost/test', - data: { foo: 'test' }, - }) - - // Ensure no request was captured - const requests = networkEvents.filter( - (el) => el.properties!.data.action === 'request' - ) - expect(requests).toHaveLength(0) - }) - test.describe('errors', () => { test('will handle a json error response', async () => { await indexPage.network.mockTestRoute('http://localhost/test', { @@ -232,11 +257,7 @@ test.describe('network signals - fetch', () => { status: 400, }) - await indexPage.network.makeFetchCall('http://localhost/test', { - method: 'POST', - body: 'hello world', - contentType: 'text/plain', - }) + await indexPage.network.makeFileUploadRequest('http://localhost/test') await indexPage.waitForSignalsApiFlush() 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 17f20c3e8..7fb9b8ecd 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,6 +1,5 @@ import { test, expect } from '@playwright/test' import { IndexPage } from './index-page' -import { sleep } from '@segment/analytics-core' const basicEdgeFn = `const processSignal = (signal) => {}` @@ -10,27 +9,6 @@ test.describe('network signals - XHR', () => { test.beforeEach(async ({ page }) => { indexPage = await new IndexPage().loadAndWait(page, basicEdgeFn) }) - test('should not emit anything if neither request nor response are json', async () => { - await indexPage.network.mockTestRoute('http://localhost/test', { - body: 'hello', - contentType: 'text/plain', - }) - - await indexPage.network.makeXHRCall('http://localhost/test', { - method: 'POST', - body: 'hello world', - contentType: 'text/plain', - responseType: 'text', - }) - - // Wait for the signals to be flushed - await sleep(300) - - const networkEvents = indexPage.signalsAPI.getEvents('network') - - // Ensure no request or response was captured - expect(networkEvents).toHaveLength(0) - }) test('basic json request / not break XHR', async () => { await indexPage.network.mockTestRoute('http://localhost/test', { @@ -111,7 +89,7 @@ test.describe('network signals - XHR', () => { }) }) - test('should emit response but not request if request content-type is not json but response is', async () => { + test('should emit request content type, even if not json', async () => { await indexPage.network.mockTestRoute('http://localhost/test', { body: JSON.stringify({ foo: 'test' }), contentType: 'application/json', @@ -128,7 +106,18 @@ test.describe('network signals - XHR', () => { const networkEvents = await indexPage.signalsAPI.waitForEvents(1, 'network') - // Check the response (only response should be captured) + // ensure request + const requests = networkEvents.filter( + (el) => el.properties!.data.action === 'request' + ) + expect(requests).toHaveLength(1) + expect(requests[0].properties!.data).toMatchObject({ + action: 'request', + url: 'http://localhost/test', + data: 'hello world', + }) + + // Check the response const responses = networkEvents.filter( (el) => el.properties!.data.action === 'response' ) @@ -138,12 +127,6 @@ test.describe('network signals - XHR', () => { url: 'http://localhost/test', data: { foo: 'test' }, }) - - // Ensure no request was captured - const requests = networkEvents.filter( - (el) => el.properties!.data.action === 'request' - ) - expect(requests).toHaveLength(0) }) test('should parse response if responseType is set to json but response header does not contain application/json', async () => { 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 0e5eddc38..eb1a3e68a 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 @@ -8,12 +8,11 @@ const basicEdgeFn = `const processSignal = (signal) => {}` test('ingestion not enabled -> will not send the signal', async ({ page }) => { await indexPage.loadAndWait(page, basicEdgeFn, { enableSignalsIngestion: false, + flushAt: 1, }) - await indexPage.fillNameInput('John Doe') - await indexPage.waitForSignalsApiFlush().catch(() => { - expect(true).toBe(true) - }) + await page.waitForTimeout(100) + expect(indexPage.signalsAPI.getEvents('interaction')).toHaveLength(0) }) test('ingestion enabled -> will send the signal', async ({ page }) => { diff --git a/packages/signals/signals-runtime/src/web/web-signals-types.ts b/packages/signals/signals-runtime/src/web/web-signals-types.ts index 3649aea41..7747f9e9c 100644 --- a/packages/signals/signals-runtime/src/web/web-signals-types.ts +++ b/packages/signals/signals-runtime/src/web/web-signals-types.ts @@ -68,6 +68,7 @@ interface BaseNetworkData { action: string url: string data: JSONValue + contentType: string } interface NetworkRequestData extends BaseNetworkData { diff --git a/packages/signals/signals/src/core/buffer/__tests__/buffer.test.ts b/packages/signals/signals/src/core/buffer/__tests__/buffer.test.ts index 73970602d..21e4741bd 100644 --- a/packages/signals/signals/src/core/buffer/__tests__/buffer.test.ts +++ b/packages/signals/signals/src/core/buffer/__tests__/buffer.test.ts @@ -16,7 +16,7 @@ describe(getSignalBuffer, () => { it('should add and clear', async () => { const mockSignal = createInteractionSignal({ eventType: 'submit', - submitter: {}, + target: {}, }) await buffer.add(mockSignal) await expect(buffer.getAll()).resolves.toEqual([mockSignal]) diff --git a/packages/signals/signals/src/core/client/__tests__/client.test.ts b/packages/signals/signals/src/core/client/__tests__/client.test.ts index 4990d0574..f619a2fe3 100644 --- a/packages/signals/signals/src/core/client/__tests__/client.test.ts +++ b/packages/signals/signals/src/core/client/__tests__/client.test.ts @@ -44,6 +44,7 @@ describe(SignalsIngestClient, () => { const ctx = await client.send({ type: 'network', data: { + contentType: 'application/json', action: 'request', data: { hello: 'how are you', @@ -58,6 +59,7 @@ describe(SignalsIngestClient, () => { expect(ctx!.event.properties!.data).toMatchInlineSnapshot(` { "action": "request", + "contentType": "application/json", "data": { "hello": "XXX", }, diff --git a/packages/signals/signals/src/core/client/__tests__/redact.test.ts b/packages/signals/signals/src/core/client/__tests__/redact.test.ts index fb5d4982d..d6c4d0a8d 100644 --- a/packages/signals/signals/src/core/client/__tests__/redact.test.ts +++ b/packages/signals/signals/src/core/client/__tests__/redact.test.ts @@ -91,6 +91,7 @@ describe(redactSignalData, () => { it('should redact the values in the "data" property if the type is "network"', () => { const signal = factories.createNetworkSignal( { + contentType: 'application/json', action: 'request', method: 'post', url: 'http://foo.com', @@ -100,6 +101,7 @@ describe(redactSignalData, () => { ) const expected = factories.createNetworkSignal( { + contentType: 'application/json', action: 'request', method: 'post', url: 'http://foo.com', diff --git a/packages/signals/signals/src/core/signal-generators/dom-gen.ts b/packages/signals/signals/src/core/signal-generators/dom-gen.ts index 829478972..aa48957d0 100644 --- a/packages/signals/signals/src/core/signal-generators/dom-gen.ts +++ b/packages/signals/signals/src/core/signal-generators/dom-gen.ts @@ -8,15 +8,31 @@ import { SignalEmitter } from '../emitter' import { SignalGenerator } from './types' interface Label { - textContent?: string + textContent: string + id: string + attributes: Record } + +const parseFormData = (data: FormData): Record => { + return [...data].reduce((acc, [key, value]) => { + if (typeof value === 'string') { + acc[key] = value + } + return acc + }, {} as Record) +} + const parseLabels = ( labels: NodeListOf | null | undefined ): Label[] => { if (!labels) return [] - return [...labels].map((label) => ({ - textContent: label.textContent ?? undefined, - })) + return [...labels] + .map((label) => ({ + id: label.id, + attributes: parseNodeMap(label.attributes), + textContent: label.textContent ? cleanText(label.textContent) : undefined, + })) + .filter((el): el is Label => Boolean(el.textContent)) } const parseNodeMap = (nodeMap: NamedNodeMap): Record => { @@ -34,21 +50,70 @@ export const cleanText = (str: string): string => { .trim() // Trim leading and trailing spaces } -const parseElement = (el: HTMLElement) => { +interface ParsedElementBase { + attributes: Record + classList: string[] + id: string + labels?: Label[] + label?: Label + name: string + nodeName: string + tagName: string + title: string + type: string + value: string + textContent?: string + innerText?: string +} + +interface ParsedSelectElement extends ParsedElementBase { + selectedOptions: { value: string; text: string }[] + selectedIndex: number +} +interface ParsedInputElement extends ParsedElementBase { + checked: boolean +} +interface ParsedMediaElement extends ParsedElementBase { + currentSrc?: string + currentTime?: number + duration: number + ended: boolean + muted: boolean + paused: boolean + playbackRate: number + readyState?: number + src?: string + volume?: number +} + +interface ParsedHTMLFormElement extends ParsedElementBase { + formData: Record +} + +type AnyParsedElement = + | ParsedHTMLFormElement + | ParsedSelectElement + | ParsedInputElement + | ParsedMediaElement + | ParsedElementBase + +const parseElement = (el: HTMLElement): AnyParsedElement => { + const labels = parseLabels((el as HTMLInputElement).labels) const base = { // adding a bunch of fields that are not on _all_ elements, but are on enough that it's useful to have them here. attributes: parseNodeMap(el.attributes), classList: [...el.classList], id: el.id, - labels: parseLabels((el as HTMLInputElement).labels), + labels, + label: labels[0], name: (el as HTMLInputElement).name, nodeName: el.nodeName, tagName: el.tagName, title: el.title, type: (el as HTMLInputElement).type, value: (el as HTMLInputElement).value, - textContent: el.textContent && cleanText(el.textContent), - innerText: el.innerText && cleanText(el.innerText), + textContent: (el.textContent && cleanText(el.textContent)) ?? undefined, + innerText: (el.innerText && cleanText(el.innerText)) ?? undefined, } if (el instanceof HTMLSelectElement) { @@ -79,6 +144,11 @@ const parseElement = (el: HTMLElement) => { src: el.src, volume: el.volume, } + } else if (el instanceof HTMLFormElement) { + return { + ...base, + formData: parseFormData(new FormData(el)), + } } return base } @@ -114,11 +184,18 @@ export class FormSubmitGenerator implements SignalGenerator { id = 'form-submit' register(emitter: SignalEmitter) { const handleSubmit = (ev: SubmitEvent) => { - const target = ev.submitter! + const target = ev.target as HTMLFormElement | null + + if (!target) return + + // reference to the form element that the submit event is being fired at + const submitter = ev.submitter + // If the form is submitted via JavaScript using form.submit(), the submitter property will be null because no specific button/input triggered the submission. emitter.emit( createInteractionSignal({ eventType: 'submit', - submitter: parseElement(target), + target: parseElement(target), + submitter: submitter ? parseElement(submitter) : undefined, }) ) } @@ -131,8 +208,9 @@ export class OnChangeGenerator implements SignalGenerator { id = 'change' register(emitter: SignalEmitter) { const handleChange = (ev: Event) => { - const target = ev.target as HTMLElement - if (target instanceof HTMLInputElement) { + const target = ev.target as HTMLElement | null + if (!target) return + if (target && target instanceof HTMLInputElement) { if (target.type === 'password') { logger.debug('Ignoring change event for input', target) return diff --git a/packages/signals/signals/src/core/signal-generators/network-gen/__tests__/helpers.test.ts b/packages/signals/signals/src/core/signal-generators/network-gen/__tests__/helpers.test.ts index 912a39086..ac9fd839d 100644 --- a/packages/signals/signals/src/core/signal-generators/network-gen/__tests__/helpers.test.ts +++ b/packages/signals/signals/src/core/signal-generators/network-gen/__tests__/helpers.test.ts @@ -99,6 +99,13 @@ describe(containsContentType, () => { 'application/json', ]) ).toBe(true) + + expect( + containsContentType( + new Headers({ 'Content-Type': 'application/json ; charset=utf-8' }), + ['application/json'] + ) + ).toBe(true) }) it('should return false if headers do not contain application/json', () => { diff --git a/packages/signals/signals/src/core/signal-generators/network-gen/__tests__/network-generator.test.ts b/packages/signals/signals/src/core/signal-generators/network-gen/__tests__/network-generator.test.ts index 3232ca79f..8975a12b6 100644 --- a/packages/signals/signals/src/core/signal-generators/network-gen/__tests__/network-generator.test.ts +++ b/packages/signals/signals/src/core/signal-generators/network-gen/__tests__/network-generator.test.ts @@ -146,6 +146,7 @@ describe(NetworkGenerator, () => { { "data": { "action": "response", + "contentType": "application/json", "data": { "data": "test", }, @@ -189,6 +190,7 @@ describe(NetworkGenerator, () => { { "data": { "action": "request", + "contentType": "application/json", "data": { "key": "value", }, @@ -209,6 +211,7 @@ describe(NetworkGenerator, () => { { "data": { "action": "response", + "contentType": "application/json", "data": { "data": "test", }, diff --git a/packages/signals/signals/src/core/signal-generators/network-gen/helpers.ts b/packages/signals/signals/src/core/signal-generators/network-gen/helpers.ts index 196d4ce78..18428ef35 100644 --- a/packages/signals/signals/src/core/signal-generators/network-gen/helpers.ts +++ b/packages/signals/signals/src/core/signal-generators/network-gen/helpers.ts @@ -47,22 +47,29 @@ export const containsContentType = ( // format the content-type header to remove charset -- this is non-standard behavior that is somewhat common // e.g. application/json;charset=utf-8 => application/json const removeCharset = (header: string | null): string | null => - header?.split(';')[0] ?? null + header?.split(';')[0].trim() ?? null - return match.some( - (t) => removeCharset(normalizedHeaders.get('content-type')) === t + return match.some((t) => + removeCharset(normalizedHeaders.get('content-type'))?.includes(t) ) } export const containsJSONContentType = ( - headers: HeadersInit | undefined + Headers: HeadersInit | undefined ): boolean => { - return containsContentType(headers, ['application/json']) + return containsContentType(Headers, [ + 'application/json', + 'application/ld+json', + 'text/json', + ]) } -export function isPlainObject(obj: unknown): obj is Record { +export const containsJSONParseableContentType = ( + headers: HeadersInit | undefined +): boolean => { return ( - Object.prototype.toString.call(obj).slice(8, -1).toLowerCase() === 'object' + containsJSONContentType(headers) || + containsContentType(headers, ['text/plain']) ) } diff --git a/packages/signals/signals/src/core/signal-generators/network-gen/index.ts b/packages/signals/signals/src/core/signal-generators/network-gen/index.ts index c0d09ce5d..8b0de8b5f 100644 --- a/packages/signals/signals/src/core/signal-generators/network-gen/index.ts +++ b/packages/signals/signals/src/core/signal-generators/network-gen/index.ts @@ -12,7 +12,11 @@ import { NetworkRequestHandler, NetworkResponseHandler, } from './network-interceptor' -import { containsJSONContentType } from './helpers' +import { + containsJSONContentType, + containsJSONParseableContentType, + tryJSONParse, +} from './helpers' export type NetworkSettingsConfigSettings = Pick< SignalsSettingsConfig, @@ -52,14 +56,18 @@ export class NetworkGenerator implements SignalGenerator { const createMetadata = () => ({ filters: this.filter.settings.networkSignalsFilterList.getRegexes(), }) + const handleRequest: NetworkRequestHandler = (rq) => { - if (!containsJSONContentType(rq.headers)) { + if (!containsJSONParseableContentType(rq.headers)) { return } if (!rq.url || !this.filter.isAllowed(rq.url)) { return } + + const data = typeof rq.body === 'string' ? tryJSONParse(rq.body) : null + this.emittedRequestIds.push(rq.id) emitter.emit( createNetworkSignal( @@ -67,7 +75,8 @@ export class NetworkGenerator implements SignalGenerator { action: 'request', url: rq.url, method: rq.method || 'GET', - data: rq.body ? JSON.parse(rq.body.toString()) : null, + data, + contentType: rq.headers?.get('content-type') || '', }, createMetadata() ) @@ -101,6 +110,7 @@ export class NetworkGenerator implements SignalGenerator { data: data, ok: rs.ok, status: rs.status, + contentType: rs.headers.get('content-type') || '', }, createMetadata() ) diff --git a/packages/signals/signals/src/core/signals/signals.ts b/packages/signals/signals/src/core/signals/signals.ts index 207ace283..8dcb7e424 100644 --- a/packages/signals/signals/src/core/signals/signals.ts +++ b/packages/signals/signals/src/core/signals/signals.ts @@ -91,6 +91,9 @@ export class Signals implements ISignals { disallowListURLs: [ analyticsService.instance.settings.apiHost, analyticsService.instance.settings.cdnURL, + 'api.segment.io', + 'signals.segment.io', + 'cdn.segment.com', ], sampleRate: analyticsService.instance.settings.cdnSettings diff --git a/packages/signals/signals/src/lib/exists/index.ts b/packages/signals/signals/src/lib/exists/index.ts new file mode 100644 index 000000000..b19ea37ae --- /dev/null +++ b/packages/signals/signals/src/lib/exists/index.ts @@ -0,0 +1,7 @@ +/** + * This type guard can be passed into a function such as native filter + * in order to remove nullish values from a list in a type-safe way. + */ +export const exists = (value: T): value is NonNullable => { + return value != null && value !== undefined +} diff --git a/packages/signals/signals/src/types/__tests__/create-network-signal.test.ts b/packages/signals/signals/src/types/__tests__/create-network-signal.test.ts index 943760653..baa21c407 100644 --- a/packages/signals/signals/src/types/__tests__/create-network-signal.test.ts +++ b/packages/signals/signals/src/types/__tests__/create-network-signal.test.ts @@ -23,6 +23,7 @@ describe(createNetworkSignal, () => { url: 'http://example.com', method: 'post', data: { key: 'value' }, + contentType: 'application/json', } const signal = createNetworkSignal(data, metadata) @@ -31,6 +32,7 @@ describe(createNetworkSignal, () => { { "data": { "action": "request", + "contentType": "application/json", "data": { "key": "value", }, @@ -61,6 +63,7 @@ describe(createNetworkSignal, () => { url: 'http://example.com', ok: true, status: 200, + contentType: 'application/json', data: { key: 'value' }, } @@ -70,6 +73,7 @@ describe(createNetworkSignal, () => { { "data": { "action": "response", + "contentType": "application/json", "data": { "key": "value", }, From 969227d468e444a251840b38694ceae836872752 Mon Sep 17 00:00:00 2001 From: Seth Silesky <5115498+silesky@users.noreply.github.com> Date: Wed, 30 Oct 2024 16:41:12 -0500 Subject: [PATCH 2/6] fix submit data type --- packages/signals/signals-runtime/src/web/web-signals-types.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/signals/signals-runtime/src/web/web-signals-types.ts b/packages/signals/signals-runtime/src/web/web-signals-types.ts index 7747f9e9c..4f0554f22 100644 --- a/packages/signals/signals-runtime/src/web/web-signals-types.ts +++ b/packages/signals/signals-runtime/src/web/web-signals-types.ts @@ -21,7 +21,8 @@ type ClickData = { type SubmitData = { eventType: 'submit' - submitter: SerializedTarget + submitter?: SerializedTarget + target: SerializedTarget } type ChangeData = { From 1275d7ce0a739a7b8d473dce34a045af0e0c5d79 Mon Sep 17 00:00:00 2001 From: Seth Silesky <5115498+silesky@users.noreply.github.com> Date: Wed, 30 Oct 2024 16:42:31 -0500 Subject: [PATCH 3/6] wip --- .../src/tests/signals-vanilla/network-signals-fetch.test.ts | 2 ++ 1 file changed, 2 insertions(+) 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 34d65cb9d..9470866d4 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 @@ -114,6 +114,7 @@ test.describe('network signals - fetch', () => { expect(requests).toHaveLength(1) expect(requests[0].properties!.data).toEqual({ action: 'request', + contentType: 'application/json', url: 'http://localhost/test', method: 'POST', data: { key: 'value' }, @@ -125,6 +126,7 @@ test.describe('network signals - fetch', () => { expect(responses).toHaveLength(1) expect(responses[0].properties!.data).toEqual({ action: 'response', + contentType: 'application/json', url: 'http://localhost/test', data: { foo: 'test' }, status: 200, From 1cac7c30a721ea83517e680b8def46f492103e2b Mon Sep 17 00:00:00 2001 From: Seth Silesky <5115498+silesky@users.noreply.github.com> Date: Wed, 30 Oct 2024 16:44:19 -0500 Subject: [PATCH 4/6] update README --- packages/signals/signals-runtime/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/signals/signals-runtime/README.md b/packages/signals/signals-runtime/README.md index 4c676f88e..87bdf905c 100644 --- a/packages/signals/signals-runtime/README.md +++ b/packages/signals/signals-runtime/README.md @@ -1,5 +1,5 @@ # @segment/analytics-signals-runtime -Encapsults Signals runtime functionality, in order to share logic between the signals plugins for browser and mobile. +Encapsulates Signals runtime functionality, in order to share logic between the signals plugins for browser and mobile. ### Development `yarn build` generate the following artifacts: From 8731dcabfd5013c0af96c7c1a2c8e7eeac278918 Mon Sep 17 00:00:00 2001 From: Seth Silesky <5115498+silesky@users.noreply.github.com> Date: Wed, 30 Oct 2024 16:45:08 -0500 Subject: [PATCH 5/6] wip --- packages/signals/signals-runtime/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/signals/signals-runtime/README.md b/packages/signals/signals-runtime/README.md index 87bdf905c..11236cf86 100644 --- a/packages/signals/signals-runtime/README.md +++ b/packages/signals/signals-runtime/README.md @@ -7,4 +7,4 @@ Encapsulates Signals runtime functionality, in order to share logic between the |--------|-------------| | `/dist/runtime/index.[platform].js`, `/[platform]/get-runtime-code.generated.js` | Exposes `globalThis.Signals` and constants (e.g. `SignalType`), either through the script tag or in the mobile JS engine | | `/dist/editor/[platform]-editor.d.ts.txt` | Type definitions for monaco editor on app.segment.com -| `/dist/esm/index.js` | Entry point for `@segment/analytics-signals` and the segment app, that want to consume the types / runtime code. | \ No newline at end of file +| `/dist/esm/index.js` | Entry point for `@segment/analytics-signals` and the segment app, for consumers that want to consume the types / runtime code as an npm package. | \ No newline at end of file From 178bfd59e22adb36795193c8225d01439e07a3d5 Mon Sep 17 00:00:00 2001 From: Seth Silesky <5115498+silesky@users.noreply.github.com> Date: Thu, 31 Oct 2024 10:10:10 -0500 Subject: [PATCH 6/6] wip --- .../src/test-helpers/mocks/mock-signal-types-web.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/signals/signals-runtime/src/test-helpers/mocks/mock-signal-types-web.ts b/packages/signals/signals-runtime/src/test-helpers/mocks/mock-signal-types-web.ts index 8ca888438..70d247c74 100644 --- a/packages/signals/signals-runtime/src/test-helpers/mocks/mock-signal-types-web.ts +++ b/packages/signals/signals-runtime/src/test-helpers/mocks/mock-signal-types-web.ts @@ -38,6 +38,7 @@ export const mockNetworkSignal: NetworkSignal = { type: 'network', data: { action: 'request', + contentType: 'application/json', url: 'https://api.example.com/data', method: 'GET', data: { key: 'value' },