🐛 [RUMF-1491] fix error when calling fetch with an unexpected value…

… as first parameter (#2061) * 🐛 fix telemetry error when calling fetch(null) * 👌 use the existing `RequestInfo` type Co-authored-by: Bastien Caudan <[email protected]> --------- Co-authored-by: Bastien Caudan <[email protected]>
DataDog · Mar 21, 2023 · 4b34567 · 4b34567
1 parent 6c436f1
commit 4b34567
Show file tree

Hide file tree

Showing 4 changed files with 46 additions and 7 deletions.
diff --git a/packages/core/src/browser/fetchObservable.spec.ts b/packages/core/src/browser/fetchObservable.spec.ts
@@ -7,6 +7,8 @@ import { initFetchObservable } from './fetchObservable'
 
 describe('fetch proxy', () => {
   const FAKE_URL = 'http://fake-url/'
+  const FAKE_RELATIVE_URL = '/fake-path'
+  const NORMALIZED_FAKE_RELATIVE_URL = `${location.origin}/fake-path`
   let fetchStub: (input: RequestInfo, init?: RequestInit) => FetchStubPromise
   let fetchStubManager: FetchStubManager
   let requestsTrackingSubscription: Subscription
@@ -111,6 +113,8 @@ describe('fetch proxy', () => {
     fetchStub(new Request(FAKE_URL, { method: 'PUT' }), { method: 'POST' }).resolveWith({ status: 500 })
     fetchStub(new Request(FAKE_URL), { method: 'POST' }).resolveWith({ status: 500 })
     fetchStub(FAKE_URL, { method: 'POST' }).resolveWith({ status: 500 })
+    fetchStub(null as any).resolveWith({ status: 500 })
+    fetchStub({ method: 'POST' } as any).resolveWith({ status: 500 })
 
     fetchStubManager.whenAllComplete(() => {
       expect(requests[0].method).toEqual('GET')
@@ -119,17 +123,31 @@ describe('fetch proxy', () => {
       expect(requests[3].method).toEqual('POST')
       expect(requests[4].method).toEqual('POST')
       expect(requests[5].method).toEqual('POST')
+      expect(requests[6].method).toEqual('GET')
+      expect(requests[7].method).toEqual('GET')
       done()
     })
   })
 
-  it('should get url from input', (done) => {
+  it('should get the normalized url from input', (done) => {
     fetchStub(FAKE_URL).rejectWith(new Error('fetch error'))
     fetchStub(new Request(FAKE_URL)).rejectWith(new Error('fetch error'))
+    fetchStub(null as any).rejectWith(new Error('fetch error'))
+    fetchStub({
+      toString() {
+        return FAKE_RELATIVE_URL
+      },
+    } as any).rejectWith(new Error('fetch error'))
+    fetchStub(FAKE_RELATIVE_URL).rejectWith(new Error('fetch error'))
+    fetchStub(new Request(FAKE_RELATIVE_URL)).rejectWith(new Error('fetch error'))
 
     fetchStubManager.whenAllComplete(() => {
       expect(requests[0].url).toEqual(FAKE_URL)
       expect(requests[1].url).toEqual(FAKE_URL)
+      expect(requests[2].url).toMatch(/\/null$/)
+      expect(requests[3].url).toEqual(NORMALIZED_FAKE_RELATIVE_URL)
+      expect(requests[4].url).toEqual(NORMALIZED_FAKE_RELATIVE_URL)
+      expect(requests[5].url).toEqual(NORMALIZED_FAKE_RELATIVE_URL)
       done()
     })
   })

diff --git a/packages/core/src/browser/fetchObservable.ts b/packages/core/src/browser/fetchObservable.ts
@@ -8,7 +8,7 @@ import { normalizeUrl } from '../tools/urlPolyfill'
 interface FetchContextBase {
   method: string
   startClocks: ClocksState
-  input: RequestInfo
+  input: unknown
   init?: RequestInit
   url: string
 }
@@ -52,7 +52,7 @@ function createFetchObservable() {
 
           const context = callMonitored(beforeSend, null, [observable, input, init])
           if (context) {
-            responsePromise = originalFetch.call(this, context.input, context.init)
+            responsePromise = originalFetch.call(this, context.input as RequestInfo, context.init)
             callMonitored(afterSend, null, [observable, responsePromise, context])
           } else {
             responsePromise = originalFetch.call(this, input, init)
@@ -68,9 +68,9 @@ function createFetchObservable() {
   return observable
 }
 
-function beforeSend(observable: Observable<FetchContext>, input: RequestInfo, init?: RequestInit) {
-  const method = (init && init.method) || (typeof input === 'object' && input.method) || 'GET'
-  const url = normalizeUrl((typeof input === 'object' && input.url) || (input as string))
+function beforeSend(observable: Observable<FetchContext>, input: unknown, init?: RequestInit) {
+  const method = (init && init.method) || (input instanceof Request && input.method) || 'GET'
+  const url = input instanceof Request ? input.url : normalizeUrl(String(input))
   const startClocks = clocksNow()
 
   const context: FetchStartContext = {

diff --git a/packages/rum-core/src/domain/requestCollection.ts b/packages/rum-core/src/domain/requestCollection.ts
@@ -52,7 +52,7 @@ export interface RequestCompleteEvent {
   traceSampled?: boolean
   xhr?: XMLHttpRequest
   response?: Response
-  input?: RequestInfo
+  input?: unknown
   init?: RequestInit
   error?: Error
 }

diff --git a/packages/rum-core/src/domain/rumEventsCollection/resource/resourceCollection.spec.ts b/packages/rum-core/src/domain/rumEventsCollection/resource/resourceCollection.spec.ts
@@ -7,6 +7,7 @@ import {
   RequestType,
   ResourceType,
 } from '@datadog/browser-core'
+import type { RumFetchResourceEventDomainContext } from '../../../domainContext.types'
 import { createResourceEntry } from '../../../../test/fixtures'
 import type { TestSetupBuilder } from '../../../../test/specHelper'
 import { setup } from '../../../../test/specHelper'
@@ -229,6 +230,26 @@ describe('resourceCollection', () => {
       error: undefined,
     })
   })
+  ;[null, undefined, 42, {}].forEach((input: any) => {
+    it(`should support ${
+      typeof input === 'object' ? JSON.stringify(input) : String(input)
+    } as fetch input parameter`, () => {
+      if (isIE()) {
+        pending('No IE support')
+      }
+      const { lifeCycle, rawRumEvents } = setupBuilder.build()
+      lifeCycle.notify(
+        LifeCycleEventType.REQUEST_COMPLETED,
+        createCompletedRequest({
+          type: RequestType.FETCH,
+          input,
+        })
+      )
+
+      expect(rawRumEvents.length).toBe(1)
+      expect((rawRumEvents[0].domainContext as RumFetchResourceEventDomainContext).requestInput).toBe(input)
+    })
+  })
 
   it('should include the error in failed fetch requests', () => {
     const { lifeCycle, rawRumEvents } = setupBuilder.build()