Signals: always include headers object (#1192)

Author: silesky

.changeset/dull-monkeys-matter.md

@@ -0,0 +1,5 @@
+---
+'@segment/analytics-signals': patch
+---
+
+Always include headers in network interceptor, even if empty

packages/signals/signals/src/core/signal-generators/network-gen/__tests__/network-interceptor.test.ts

@@ -1,4 +1,8 @@
-import { NetworkInterceptor } from '../network-interceptor'
+import {
+  NetworkInterceptor,
+  NetworkRequestHandler,
+  NetworkResponseHandler,
+} from '../network-interceptor'
 import { Response } from 'node-fetch'
 import { EventEmitter } from 'events'
 
@@ -9,10 +13,11 @@ describe(NetworkInterceptor, () => {
     interceptor.cleanup()
   })
 
+  const mockRequestHandler: jest.MockedFn<NetworkRequestHandler> = jest.fn()
+  const mockResponseHandler: jest.MockedFn<NetworkResponseHandler> = jest.fn()
+
   it('should intercept fetch requests and responses', async () => {
     interceptor = new NetworkInterceptor()
-    const mockRequestHandler = jest.fn()
-    const mockResponseHandler = jest.fn()
     const mockResponse = new Response(JSON.stringify({ data: 'test' }), {
       headers: { 'Content-Type': 'application/json' },
     })
@@ -51,27 +56,36 @@ describe(NetworkInterceptor, () => {
 
   // Very primitive mock for XMLHttpRequest -- better tests are at the integration level
   it('should intercept XHR requests and responses', async () => {
-    const mockRequestHandler = jest.fn()
-    const mockResponseHandler = jest.fn()
+    interface XMLHttpRequestMock {
+      open: XMLHttpRequest['open']
+      send: XMLHttpRequest['send']
+      setRequestHeader: XMLHttpRequest['setRequestHeader']
+      getAllResponseHeaders: XMLHttpRequest['getAllResponseHeaders']
+      addEventListener: XMLHttpRequest['addEventListener']
+      onreadystatechange: XMLHttpRequest['onreadystatechange']
+    }
 
-    class MockXMLHttpRequest {
+    class MockXMLHttpRequest implements XMLHttpRequestMock {
       UNSENT = 0
       OPENED = 1
       HEADERS_RECEIVED = 2
       LOADING = 3
       DONE = 4
 
-      private _emitter = new EventEmitter()
+      private _emitter: EventEmitter
       public readyState = 0
       public status = 0
       public responseText = ''
-      public onreadystatechange: (() => void) | null = null
       public responseURL = ''
-      public _responseMethod = ''
-      public _responseHeaders = ''
+      private _responseHeaders = ''
+      public onreadystatechange: () => void = () => undefined
+      constructor() {
+        this._emitter = new EventEmitter().on('readystatechange', () => {
+          this.onreadystatechange()
+        })
+      }
 
-      open(method: string, url: string) {
-        this._responseMethod = method
+      open(_method: string, url: string) {
         this.responseURL = url
       }
 
@@ -91,17 +105,23 @@ describe(NetworkInterceptor, () => {
           this.status = 200
           this.responseText = JSON.stringify({ data: 'test' })
           this.responseURL = 'http://example.com'
-          this._responseHeaders = 'Content-Type: application/json'
+          this._responseHeaders =
+            [
+              'content-type: application/json; charset=utf-8',
+              'cache-control: max-age=3600',
+              'x-content-type-options: nosniff',
+              'date: Mon, 18 Nov 2000 12:00:00 GMT',
+            ].join('\r\n') + '\r\n' // trailing CRLF to be realistic
+
           this._emitter.emit('readystatechange')
-          if (this.onreadystatechange) {
-            this.onreadystatechange()
-          }
         }, 40)
       }
 
-      setRequestHeader = jest.fn()
+      setRequestHeader() {
+        // no-op
+      }
 
-      addEventListener(event: string, listener: () => void) {
+      addEventListener(event: string, listener: (ev: any) => void) {
         this._emitter.on(event, listener)
       }
 
@@ -121,15 +141,24 @@ describe(NetworkInterceptor, () => {
 
     const xhr = new XMLHttpRequest()
     xhr.open('POST', 'http://example.com')
+    xhr.setRequestHeader('accept', 'application/json')
+    xhr.setRequestHeader('x-something-else', 'foo')
     xhr.send()
 
     await new Promise((resolve) => setTimeout(resolve, 100))
 
     expect(mockRequestHandler).toHaveBeenCalled()
+    const request = mockRequestHandler.mock.calls[0][0]
+    expect(request.headers).toBeInstanceOf(Headers)
+    expect(request.headers!.get('accept')).toBe('application/json')
+    expect(request.headers!.get('x-something-else')).toBe('foo')
     expect(mockResponseHandler).toHaveBeenCalled()
     const response = mockResponseHandler.mock.calls[0][0]
     expect(response.headers).toBeInstanceOf(Headers)
-    expect(response.headers.get('content-type')).toBe('application/json')
+    expect(response.headers.get('content-type')).toBe(
+      'application/json; charset=utf-8'
+    )
+    expect(response.headers.get('date')).toBe('Mon, 18 Nov 2000 12:00:00 GMT')
     expect(response.url).toBe('http://example.com')
     expect(response.status).toBe(200)
   })

packages/signals/signals/src/core/signal-generators/network-gen/network-interceptor.ts

@@ -28,7 +28,7 @@ export interface NetworkInterceptorRequest {
   method: HTTPMethod
   contentType: string | undefined
   body: string | undefined
-  headers: Headers | undefined
+  headers: Headers
   id: string
 }
 
@@ -61,7 +61,7 @@ const createInterceptorRequest = ({
 }): NetworkInterceptorRequest => ({
   url: url.toString(),
   method: method,
-  headers,
+  headers: headers ?? new Headers(),
   contentType: headers?.get('content-type') ?? undefined,
   body: typeof body == 'string' ? body : undefined,
   id,