XHR Network signals URL normalization (#1152)

Author: silesky

.changeset/odd-pans-drop.md

@@ -0,0 +1,4 @@
+---
+'@segment/analytics-signals': minor
+---
+- normalize XHR URL, http methods, etc

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<FulfillOptions>
   ) {
+    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<RequestInit>
   ): Promise<void> {
-    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<void> {
-    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()

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' }),

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({

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),
           },

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')
+  })
+})

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,
   }
 }