wip

Author: silesky

packages/signals/signals-integration-tests/src/helpers/network-utils.ts

@@ -56,13 +56,38 @@ 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'}
    */
   async makeFetchCall(
     url = this.defaultTestApiURL,
     request: Partial<RequestInit> & {
       contentType?: string
+      blob?: boolean
     } = {}
   ): Promise<any> {
     let normalizeUrl = url

packages/signals/signals-integration-tests/src/tests/signals-vanilla/network-signals-fetch.test.ts

@@ -11,12 +11,62 @@ test.describe('network signals - fetch', () => {
     await indexPage.loadAndWait(page, basicEdgeFn)
   })
 
-  test('should not emit anything if neither request nor response are json', async () => {
+  test('should not emit non-json requests without the body', 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(1)
+    expect(requests[0].properties!.data.url).toBe('http://localhost/upload')
+    expect(requests[0].properties!.data.data).toBeNull()
+  })
+
+  test('should handle text/plain', 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 body as string if it cant be parsed to 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',
@@ -27,7 +77,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 () => {

packages/signals/signals-integration-tests/src/tests/signals-vanilla/network-signals-xhr.test.ts

@@ -116,7 +116,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',
@@ -134,7 +134,18 @@ test.describe('network signals - XHR', () => {
 
     const networkEvents = indexPage.signalsAPI.getEvents('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'
     )
@@ -144,12 +155,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 () => {

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', () => {

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

@@ -142,10 +142,29 @@ describe(NetworkGenerator, () => {
     await sleep(100)
     expect(mockEmitter.emit.mock.calls).toMatchInlineSnapshot(`
       [
+        [
+          {
+            "data": {
+              "action": "request",
+              "contentType": "text/html",
+              "data": "hello world",
+              "method": "GET",
+              "url": "http://localhost/api",
+            },
+            "metadata": {
+              "filters": {
+                "allowed": [],
+                "disallowed": [],
+              },
+            },
+            "type": "network",
+          },
+        ],
         [
           {
             "data": {
               "action": "response",
+              "contentType": "application/json",
               "data": {
                 "data": "test",
               },
@@ -189,6 +208,7 @@ describe(NetworkGenerator, () => {
       {
         "data": {
           "action": "request",
+          "contentType": "application/json",
           "data": {
             "key": "value",
           },
@@ -209,6 +229,7 @@ describe(NetworkGenerator, () => {
       {
         "data": {
           "action": "response",
+          "contentType": "application/json",
           "data": {
             "data": "test",
           },

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

@@ -47,17 +47,22 @@ 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 = (
+export const containsJSONParseableContentType = (
   headers: HeadersInit | undefined
 ): boolean => {
-  return containsContentType(headers, ['application/json'])
+  return containsContentType(headers, [
+    'application/json',
+    'application/ld+json',
+    'text/plain',
+    'text/json',
+  ])
 }
 
 export function isPlainObject(obj: unknown): obj is Record<string, unknown> {

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

@@ -12,7 +12,7 @@ import {
   NetworkRequestHandler,
   NetworkResponseHandler,
 } from './network-interceptor'
-import { containsJSONContentType } from './helpers'
+import { containsJSONParseableContentType, tryJSONParse } from './helpers'
 
 export type NetworkSettingsConfigSettings = Pick<
   SignalsSettingsConfig,
@@ -52,22 +52,23 @@ export class NetworkGenerator implements SignalGenerator {
     const createMetadata = () => ({
       filters: this.filter.settings.networkSignalsFilterList.getRegexes(),
     })
-    const handleRequest: NetworkRequestHandler = (rq) => {
-      if (!containsJSONContentType(rq.headers)) {
-        return
-      }
 
+    const handleRequest: NetworkRequestHandler = (rq) => {
       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(
           {
             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()
         )
@@ -78,7 +79,7 @@ export class NetworkGenerator implements SignalGenerator {
       const isSuccessWithNonJSONResponse =
         rs.ok &&
         rs.responseType !== 'json' &&
-        !containsJSONContentType(rs.headers)
+        !containsJSONParseableContentType(rs.headers)
 
       const isErrorButRequestNeverEmittted =
         !rs.ok && !this.emittedRequestIds.includes(rs.req.id)
@@ -101,6 +102,7 @@ export class NetworkGenerator implements SignalGenerator {
             data: data,
             ok: rs.ok,
             status: rs.status,
+            contentType: rs.headers.get('content-type') || '',
           },
           createMetadata()
         )

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

packages/signals/signals/src/types/signals.ts

@@ -84,13 +84,15 @@ interface NetworkRequestData extends BaseNetworkData {
   action: 'request'
   url: string
   method: string
+  contentType: string
 }
 
 interface NetworkResponseData extends BaseNetworkData {
   action: 'response'
   url: string
   status: number
   ok: boolean
+  contentType: string
 }
 
 export type NetworkData = NetworkRequestData | NetworkResponseData