wip

segmentio · Oct 30, 2024 · b5763cc · b5763cc
1 parent 0ed210b
commit b5763cc
Show file tree

Hide file tree

Showing 3 changed files with 23 additions and 52 deletions.
diff --git a/...signals/signals-integration-tests/src/tests/signals-vanilla/network-signals-fetch.test.ts b/...signals/signals-integration-tests/src/tests/signals-vanilla/network-signals-fetch.test.ts
@@ -11,7 +11,7 @@ test.describe('network signals - fetch', () => {
     await indexPage.loadAndWait(page, basicEdgeFn)
   })
 
-  test('should not emit non-json requests without the body', async () => {
+  test('should not emit non-json requests', async () => {
     await indexPage.network.mockTestRoute('http://localhost/upload', {
       body: JSON.stringify({ foo: 'test' }),
     })
@@ -27,12 +27,10 @@ test.describe('network signals - fetch', () => {
       (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()
+    expect(requests).toHaveLength(0)
   })
 
-  test('should handle text/plain', async () => {
+  test('should try to parse the body of text/plain requests', async () => {
     await indexPage.network.mockTestRoute('http://localhost/test', {
       body: JSON.stringify({ foo: 'test' }),
       contentType: 'application/json',
@@ -61,7 +59,7 @@ test.describe('network signals - fetch', () => {
     })
   })
 
-  test('should send the body as string if it cant be parsed to json', async () => {
+  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',
@@ -170,39 +168,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', {
@@ -286,11 +251,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/src/core/signal-generators/network-gen/helpers.ts b/packages/signals/signals/src/core/signal-generators/network-gen/helpers.ts
@@ -54,20 +54,22 @@ export const containsContentType = (
   )
 }
 
-export const containsJSONParseableContentType = (
-  headers: HeadersInit | undefined
+export const containsJSONContentType = (
+  Headers: HeadersInit | undefined
 ): boolean => {
-  return containsContentType(headers, [
+  return containsContentType(Headers, [
     'application/json',
     'application/ld+json',
-    'text/plain',
     'text/json',
   ])
 }
 
-export function isPlainObject(obj: unknown): obj is Record<string, unknown> {
+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
@@ -12,7 +12,11 @@ import {
   NetworkRequestHandler,
   NetworkResponseHandler,
 } from './network-interceptor'
-import { containsJSONParseableContentType, tryJSONParse } from './helpers'
+import {
+  containsJSONContentType,
+  containsJSONParseableContentType,
+  tryJSONParse,
+} from './helpers'
 
 export type NetworkSettingsConfigSettings = Pick<
   SignalsSettingsConfig,
@@ -54,6 +58,10 @@ export class NetworkGenerator implements SignalGenerator {
     })
 
     const handleRequest: NetworkRequestHandler = (rq) => {
+      if (!containsJSONParseableContentType(rq.headers)) {
+        return
+      }
+
       if (!rq.url || !this.filter.isAllowed(rq.url)) {
         return
       }
@@ -79,7 +87,7 @@ export class NetworkGenerator implements SignalGenerator {
       const isSuccessWithNonJSONResponse =
         rs.ok &&
         rs.responseType !== 'json' &&
-        !containsJSONParseableContentType(rs.headers)
+        !containsJSONContentType(rs.headers)
 
       const isErrorButRequestNeverEmittted =
         !rs.ok && !this.emittedRequestIds.includes(rs.req.id)