fix: add chunkResponseStatus tag (#6509)

* fix: add chunkResponseStatus tag

* add-tests

* fix tests

* tests

* description

* comments

* comment

* move

* comment

* lint

src/setupTests.ts

@@ -20,6 +20,10 @@ if (typeof global.TextEncoder === 'undefined') {
 
 global.ResizeObserver = ResizeObserver
 
+// Sets origin to the production origin, because some tests depend on this.
+// This prevents each test file from needing to set this manually.
+global.origin = 'https://app.uniswap.org'
+
 global.matchMedia =
   global.matchMedia ||
   function () {

src/tracing/errors.test.ts

-import { ErrorEvent } from '@sentry/types'
+import { ErrorEvent, Event } from '@sentry/types'
 
-import { filterKnownErrors } from './errors'
+import { beforeSend, filterKnownErrors } from './errors'
 
 Object.defineProperty(window.performance, 'getEntriesByType', {
   writable: true,
   value: jest.fn(),
 })
 
+describe('beforeSend', () => {
+  const ERROR = {} as ErrorEvent
+
+  describe('chunkResponseStatus', () => {
+    afterEach(() => {
+      jest.restoreAllMocks()
+    })
+
+    it('handles when matching JS file not found', () => {
+      jest.spyOn(window.performance, 'getEntriesByType').mockReturnValue([])
+      const originalException = new Error(
+        'Loading chunk 20 failed. (error: https://app.uniswap.org/static/js/20.d55382e0.chunk.js)'
+      )
+      expect((beforeSend(ERROR, { originalException }) as Event).tags).toBeUndefined()
+    })
+
+    it('handles when matching CSS file not found', () => {
+      jest.spyOn(window.performance, 'getEntriesByType').mockReturnValue([])
+      const originalException = new Error(
+        'Loading chunk 20 failed. (error: https://app.uniswap.org/static/js/20.d55382e0.chunk.js)'
+      )
+      expect((beforeSend(ERROR, { originalException }) as Event).tags).toBeUndefined()
+    })
+
+    it('handles when performance is undefined', () => {
+      window.performance = undefined as any
+      const originalException = new Error('Loading CSS chunk 12 failed. (./static/css/12.d5b3cfe3.chunk.css)')
+      expect((beforeSend(ERROR, { originalException }) as Event).tags).toBeUndefined()
+    })
+
+    it('adds status for a matching JS file', () => {
+      jest.spyOn(window.performance, 'getEntriesByType').mockReturnValue([
+        {
+          name: 'https://app.uniswap.org/static/js/20.d55382e0.chunk.js',
+          responseStatus: 499,
+        } as PerformanceEntry,
+      ])
+      const originalException = new Error(
+        'Loading chunk 20 failed. (error: https://app.uniswap.org/static/js/20.d55382e0.chunk.js)'
+      )
+      expect((beforeSend(ERROR, { originalException }) as Event).tags?.chunkResponseStatus).toBe(499)
+    })
+
+    it('adds status for a matching CSS file', () => {
+      jest.spyOn(window.performance, 'getEntriesByType').mockReturnValue([
+        {
+          name: 'https://app.uniswap.org/static/css/12.d5b3cfe3.chunk.css',
+          responseStatus: 200,
+        } as PerformanceEntry,
+      ])
+      const originalException = new Error('Loading CSS chunk 12 failed. (./static/css/12.d5b3cfe3.chunk.css)')
+      expect((beforeSend(ERROR, { originalException }) as Event).tags?.chunkResponseStatus).toBe(200)
+    })
+  })
+})
+
 describe('filterKnownErrors', () => {
   const ERROR = {} as ErrorEvent
   it('propagates an error', () => {
@@ -72,113 +128,6 @@ describe('filterKnownErrors', () => {
     })
   })
 
-  describe('chunk errors', () => {
-    afterEach(() => {
-      jest.restoreAllMocks()
-    })
-
-    it('filters 499 error coded chunk error', () => {
-      jest.spyOn(window.performance, 'getEntriesByType').mockReturnValue([
-        {
-          name: 'https://app.uniswap.org/static/js/20.d55382e0.chunk.js',
-          responseStatus: 499,
-        } as PerformanceEntry,
-      ])
-      const originalException = new Error(
-        'Loading chunk 20 failed. (error: https://app.uniswap.org/static/js/20.d55382e0.chunk.js)'
-      )
-      expect(filterKnownErrors(ERROR, { originalException })).toBeNull()
-    })
-
-    it('filters 499 error coded chunk timeout', () => {
-      jest.spyOn(window.performance, 'getEntriesByType').mockReturnValue([
-        {
-          name: 'https://app.uniswap.org/static/js/20.d55382e0.chunk.js',
-          responseStatus: 499,
-        } as PerformanceEntry,
-      ])
-      const originalException = new Error(
-        'Loading chunk 20 failed. (timeout: https://app.uniswap.org/static/js/20.d55382e0.chunk.js)'
-      )
-      expect(filterKnownErrors(ERROR, { originalException })).toBeNull()
-    })
-
-    it('filters 499 error coded chunk missing', () => {
-      jest.spyOn(window.performance, 'getEntriesByType').mockReturnValue([
-        {
-          name: 'https://app.uniswap.org/static/js/20.d55382e0.chunk.js',
-          responseStatus: 499,
-        } as PerformanceEntry,
-      ])
-      const originalException = new Error(
-        'Loading chunk 20 failed. (missing: https://app.uniswap.org/static/js/20.d55382e0.chunk.js)'
-      )
-      expect(filterKnownErrors(ERROR, { originalException })).toBeNull()
-    })
-
-    it('filters 499 error coded CSS chunk error', () => {
-      jest.spyOn(window.performance, 'getEntriesByType').mockReturnValue([
-        {
-          name: 'https://app.uniswap.org/static/css/12.d5b3cfe3.chunk.css',
-          responseStatus: 499,
-        } as PerformanceEntry,
-      ])
-      const originalException = new Error('Loading CSS chunk 12 failed. (./static/css/12.d5b3cfe3.chunk.css)')
-      expect(filterKnownErrors(ERROR, { originalException })).toBeNull()
-    })
-
-    it('keeps error when status is different than 499', () => {
-      jest.spyOn(window.performance, 'getEntriesByType').mockReturnValue([
-        {
-          name: 'https://app.uniswap.org/static/js/20.d55382e0.chunk.js',
-          responseStatus: 400,
-        } as PerformanceEntry,
-      ])
-      const originalException = new Error(
-        'Loading chunk 20 failed. (error: https://app.uniswap.org/static/js/20.d55382e0.chunk.js)'
-      )
-      expect(filterKnownErrors(ERROR, { originalException })).not.toBeNull()
-    })
-
-    it('keeps CSS error when status is different than 499', () => {
-      jest.spyOn(window.performance, 'getEntriesByType').mockReturnValue([
-        {
-          name: 'https://app.uniswap.org/static/css/12.d5b3cfe3.chunk.css',
-          responseStatus: 400,
-        } as PerformanceEntry,
-      ])
-      const originalException = new Error('Loading CSS chunk 12 failed. (./static/css/12.d5b3cfe3.chunk.css)')
-      expect(filterKnownErrors(ERROR, { originalException })).not.toBeNull()
-    })
-
-    it('filters out error when resource is missing', () => {
-      jest.spyOn(window.performance, 'getEntriesByType').mockReturnValue([])
-      const originalException = new Error(
-        'Loading chunk 20 failed. (error: https://app.uniswap.org/static/js/20.d55382e0.chunk.js)'
-      )
-      expect(filterKnownErrors(ERROR, { originalException })).toBeNull()
-    })
-
-    it('filters out error when performance is undefined', () => {
-      const originalException = new Error(
-        'Loading chunk 20 failed. (error: https://app.uniswap.org/static/js/20.d55382e0.chunk.js)'
-      )
-      expect(filterKnownErrors(ERROR, { originalException })).toBeNull()
-    })
-
-    it('filters out error when responseStatus is undefined', () => {
-      jest.spyOn(window.performance, 'getEntriesByType').mockReturnValue([
-        {
-          name: 'https://app.uniswap.org/static/js/20.d55382e0.chunk.js',
-        } as PerformanceEntry,
-      ])
-      const originalException = new Error(
-        'Loading chunk 20 failed. (error: https://app.uniswap.org/static/js/20.d55382e0.chunk.js)'
-      )
-      expect(filterKnownErrors(ERROR, { originalException })).toBeNull()
-    })
-  })
-
   describe('Content Security Policy', () => {
     it('filters unsafe-eval evaluate errors', () => {
       const originalException = new Error(

src/tracing/errors.ts

@@ -9,32 +9,58 @@ declare global {
   }
 }
 
+export function beforeSend(event: ErrorEvent, hint: EventHint) {
+  updateRequestUrl(event)
+  addChunkResponseStatusTag(event, hint)
+  return filterKnownErrors(event, hint)
+}
+
 /** Identifies ethers request errors (as thrown by {@type import(@ethersproject/web).fetchJson}). */
 function isEthersRequestError(error: Error): error is Error & { requestBody: string } {
   return 'requestBody' in error && typeof (error as unknown as Record<'requestBody', unknown>).requestBody === 'string'
 }
 
-export function beforeSend(event: ErrorEvent, hint: EventHint) {
-  // Since the interface currently uses HashRouter, URLs will have a # before the path.
-  // This leads to issues when we send the URL into Sentry, as the path gets parsed as a "fragment".
-  // Instead, this logic removes the # part of the URL.
-  // See https://romain-clement.net/articles/sentry-url-fragments/#url-fragments
+// Since the interface currently uses HashRouter, URLs will have a # before the path.
+// This leads to issues when we send the URL into Sentry, as the path gets parsed as a "fragment".
+// Instead, this logic removes the # part of the URL.
+// See https://romain-clement.net/articles/sentry-url-fragments/#url-fragments
+function updateRequestUrl(event: ErrorEvent) {
   if (event.request?.url) {
     event.request.url = event.request.url.replace('/#', '')
   }
+}
 
-  return filterKnownErrors(event, hint)
+// If a request fails due to a chunk error, this looks for that asset in the performance entries.
+// If found, it adds a tag to the event with the response status of the chunk request.
+function addChunkResponseStatusTag(event: ErrorEvent, hint: EventHint) {
+  const error = hint.originalException
+  if (error instanceof Error) {
+    let asset: string | undefined
+    if (error.message.match(/Loading chunk \d+ failed\. \(([a-zA-Z]+): .+\.chunk\.js\)/)) {
+      asset = error.message.match(/https?:\/\/.+?\.chunk\.js/)?.[0]
+    }
+
+    if (error.message.match(/Loading CSS chunk \d+ failed\. \(.+\.chunk\.css\)/)) {
+      const relativePath = error.message.match(/\/static\/css\/.*\.chunk\.css/)?.[0]
+      asset = `${window.origin}${relativePath}`
+    }
+
+    if (asset) {
+      const status = getChunkResponseStatus(asset)
+      if (status) {
+        if (!event.tags) {
+          event.tags = {}
+        }
+        event.tags.chunkResponseStatus = status
+      }
+    }
+  }
 }
 
-function shouldFilterChunkError(asset?: string) {
+function getChunkResponseStatus(asset?: string): number | undefined {
   const entries = [...(performance?.getEntriesByType('resource') ?? [])]
   const resource = entries?.find(({ name }) => name === asset)
-  const status = resource?.responseStatus
-
-  // If the status if 499, then we ignore.
-  // If there's no status (meaning the browser doesn't support `responseStatus`) then we also ignore.
-  // These errors are likely also 499 errors, and we can catch any spikes in non-499 chunk errors via other browsers.
-  return !status || status === 499
+  return resource?.responseStatus
 }
 
 /**
@@ -57,20 +83,6 @@ export const filterKnownErrors: Required<ClientOptions>['beforeSend'] = (event:
     // If the error is based on a user rejecting, it should not be considered an exception.
     if (didUserReject(error)) return null
 
-    // This ignores 499 errors, which are caused by Cloudflare when a request is cancelled.
-    // CF claims that some number of these is expected, and that they should be ignored.
-    // See https://groups.google.com/a/uniswap.org/g/cloudflare-eng/c/t3xvAiJFujY.
-    if (error.message.match(/Loading chunk \d+ failed\. \(([a-zA-Z]+): .+\.chunk\.js\)/)) {
-      const asset = error.message.match(/https?:\/\/.+?\.chunk\.js/)?.[0]
-      if (shouldFilterChunkError(asset)) return null
-    }
-
-    if (error.message.match(/Loading CSS chunk \d+ failed\. \(.+\.chunk\.css\)/)) {
-      const relativePath = error.message.match(/\/static\/css\/.*\.chunk\.css/)?.[0]
-      const asset = `https://app.uniswap.org${relativePath}`
-      if (shouldFilterChunkError(asset)) return null
-    }
-
     // This is caused by HTML being returned for a chunk from Cloudflare.
     // Usually, it's the result of a 499 exception right before it, which should be handled.
     // Therefore, this can be ignored.