diff --git a/.changeset/odd-pans-drop.md b/.changeset/odd-pans-drop.md new file mode 100644 index 000000000..b28e4e3f8 --- /dev/null +++ b/.changeset/odd-pans-drop.md @@ -0,0 +1,4 @@ +--- +'@segment/analytics-signals': minor +--- +- normalize XHR URL, http methods, etc 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 721187e03..5190a1e6c 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 @@ -22,6 +22,10 @@ export class BasePage { this.url = 'http://localhost:5432/src/tests' + path } + public origin() { + return new URL(this.page.url()).origin + } + /** * Load and setup routes * and wait for analytics and signals to be initialized @@ -196,6 +200,9 @@ export class BasePage { url = BasePage.defaultTestApiURL, response?: Partial ) { + if (url.startsWith('/')) { + url = new URL(url, this.page.url()).href + } await this.page.route(url, (route) => { return route.fulfill({ contentType: 'application/json', @@ -210,7 +217,11 @@ export class BasePage { url = BasePage.defaultTestApiURL, request?: Partial ): Promise { - const req = this.page.waitForRequest(url) + let normalizeUrl = url + if (url.startsWith('/')) { + normalizeUrl = new URL(url, this.page.url()).href + } + const req = this.page.waitForResponse(normalizeUrl ?? url) await this.page.evaluate( ({ url, request }) => { return fetch(url, { @@ -238,7 +249,11 @@ export class BasePage { responseType: XMLHttpRequestResponseType }> = {} ): Promise { - const req = this.page.waitForRequest(url) + let normalizeUrl = url + if (url.startsWith('/')) { + normalizeUrl = new URL(url, this.page.url()).href + } + const req = this.page.waitForResponse(normalizeUrl ?? url) await this.page.evaluate( ({ url, body, contentType, method, responseType }) => { const xhr = new XMLHttpRequest() 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 0638d7a4c..d6ba9b76f 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 @@ -93,6 +93,55 @@ test.describe('XHR Tests', () => { }) }) + test('works with XHR and relative paths', async () => { + await indexPage.mockTestRoute(`/test`, { + body: JSON.stringify({ foo: 'test' }), + contentType: 'application/json', + }) + + await indexPage.makeXHRCall('/test', { + method: 'POST', + body: JSON.stringify({ key: 'value' }), + responseType: 'json', + contentType: 'application/json', + }) + + // Wait for the signals to be flushed + await indexPage.waitForSignalsApiFlush() + + // Retrieve the batch of events from the signals request + const batch = indexPage.lastSignalsApiReq.postDataJSON() + .batch as SegmentEvent[] + + // Filter out network events + const networkEvents = batch.filter( + (el) => el.properties!.type === '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: `${indexPage.origin()}/test`, + data: { key: 'value' }, + }) + + // Check the response + const responses = networkEvents.filter( + (el) => el.properties!.data.action === 'response' + ) + expect(responses).toHaveLength(1) + expect(responses[0].properties!.data).toMatchObject({ + action: 'response', + url: `${indexPage.origin()}/test`, + data: { foo: 'test' }, + }) + }) + test('should emit response but not request if request content-type is not json but response is', async () => { await indexPage.mockTestRoute('http://localhost/test', { body: JSON.stringify({ foo: 'test' }), 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 08429d399..9301a047b 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 @@ -149,7 +149,7 @@ describe(NetworkGenerator, () => { "data": { "data": "test", }, - "url": "/api", + "url": "http://localhost/api", }, "metadata": { "filters": { @@ -245,6 +245,25 @@ describe(NetworkGenerator, () => { unregister() }) + it('should default to GET method if no method is provided', async () => { + const mockEmitter = { emit: jest.fn() } + const networkGenerator = new TestNetworkGenerator() + const unregister = networkGenerator.register( + mockEmitter as unknown as SignalEmitter + ) + + await window.fetch(`/test`, { + headers: { 'content-type': 'application/json' }, + body: JSON.stringify({ key: 'value' }), + }) + + await sleep(100) + const [first] = mockEmitter.emit.mock.calls + + expect(first[0].data.method).toBe('GET') + unregister() + }) + it('emits signals for same domain if networkSignalsAllowSameDomain = false', async () => { const mockEmitter = { emit: jest.fn() } const networkGenerator = new TestNetworkGenerator({ 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 879078c80..c99658a63 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 @@ -6,7 +6,6 @@ import { import { createNetworkSignal } from '../../../types' import { SignalEmitter } from '../../emitter' import { SignalsSettingsConfig } from '../../signals' -import { normalizeUrl } from '../../../lib/normalize-url' import { SignalGenerator } from '../types' import { NetworkInterceptor, @@ -70,8 +69,8 @@ export class NetworkGenerator implements SignalGenerator { createNetworkSignal( { action: 'request', - url: normalizeUrl(sUrl), - method: rq.method || '', + url: sUrl, + method: rq.method || 'GET', data: JSON.parse(rq.body.toString()), }, createMetadata() @@ -95,7 +94,7 @@ export class NetworkGenerator implements SignalGenerator { createNetworkSignal( { action: 'response', - url: url, + url, data: data, }, createMetadata() @@ -124,7 +123,7 @@ export class NetworkGenerator implements SignalGenerator { createNetworkSignal( { action: 'request', - url: normalizeUrl(sUrl), + url: sUrl, method: rq.method, data: tryParseXHRBody(rq.body), }, 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 new file mode 100644 index 000000000..adfbd2a5e --- /dev/null +++ b/packages/signals/signals/src/types/__tests__/create-network-signal.test.ts @@ -0,0 +1,63 @@ +import { + createNetworkSignal, + NetworkData, + NetworkSignalMetadata, +} from '../signals' +import { normalizeUrl } from '../../lib/normalize-url' + +jest.mock('../../lib/normalize-url', () => ({ + normalizeUrl: jest.fn((url) => url), +})) + +describe(createNetworkSignal, () => { + const metadata: NetworkSignalMetadata = { + filters: { + allowed: ['allowed1', 'allowed2'], + disallowed: ['disallowed1', 'disallowed2'], + }, + } + + it('should create a network signal for a request', () => { + const data: NetworkData = { + action: 'request', + url: 'http://example.com', + method: 'post', + data: { key: 'value' }, + } + + const signal = createNetworkSignal(data, metadata) + + expect(signal).toEqual({ + type: 'network', + data: { + action: 'request', + url: 'http://example.com', + method: 'POST', + data: { key: 'value' }, + }, + metadata, + }) + expect(normalizeUrl).toHaveBeenCalledWith('http://example.com') + }) + + it('should create a network signal for a response', () => { + const data: NetworkData = { + action: 'response', + url: 'http://example.com', + data: { key: 'value' }, + } + + const signal = createNetworkSignal(data, metadata) + + expect(signal).toEqual({ + type: 'network', + data: { + action: 'response', + url: 'http://example.com', + data: { key: 'value' }, + }, + metadata, + }) + expect(normalizeUrl).toHaveBeenCalledWith('http://example.com') + }) +}) diff --git a/packages/signals/signals/src/types/signals.ts b/packages/signals/signals/src/types/signals.ts index 4a6d8921b..ffb5bbc37 100644 --- a/packages/signals/signals/src/types/signals.ts +++ b/packages/signals/signals/src/types/signals.ts @@ -1,4 +1,5 @@ import { JSONValue } from '@segment/analytics-next' +import { normalizeUrl } from '../lib/normalize-url' export type SignalType = | 'navigation' @@ -65,7 +66,7 @@ export type InstrumentationSignal = AppSignal< InstrumentationData > -interface NetworkSignalMetadata { +export interface NetworkSignalMetadata { filters: { allowed: string[] disallowed: string[] @@ -171,7 +172,13 @@ export const createNetworkSignal = ( ): NetworkSignal => { return { type: 'network', - data, + data: { + ...data, + url: normalizeUrl(data.url), + ...(data.action === 'request' + ? { method: data.method.toUpperCase() } + : {}), + }, metadata: metadata, } }