From 4b34567b20ef245519396bf1de04b07d00654a3a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt?= Date: Tue, 21 Mar 2023 18:00:34 +0100 Subject: [PATCH] =?UTF-8?q?=F0=9F=90=9B=20[RUMF-1491]=20fix=20error=20when?= =?UTF-8?q?=20calling=20`fetch`=20with=20an=20unexpected=20value=20as=20fi?= =?UTF-8?q?rst=20parameter=20(#2061)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * 🐛 fix telemetry error when calling fetch(null) * 👌 use the existing `RequestInfo` type Co-authored-by: Bastien Caudan <1331991+bcaudan@users.noreply.github.com> --------- Co-authored-by: Bastien Caudan <1331991+bcaudan@users.noreply.github.com> --- .../core/src/browser/fetchObservable.spec.ts | 20 +++++++++++++++++- packages/core/src/browser/fetchObservable.ts | 10 ++++----- .../rum-core/src/domain/requestCollection.ts | 2 +- .../resource/resourceCollection.spec.ts | 21 +++++++++++++++++++ 4 files changed, 46 insertions(+), 7 deletions(-) diff --git a/packages/core/src/browser/fetchObservable.spec.ts b/packages/core/src/browser/fetchObservable.spec.ts index 4fff7bbcc9..4c6f760ac2 100644 --- 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 index 23c53c6a1c..13498e618d 100644 --- 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, 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, 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 index feb38cdd32..0e895e1df8 100644 --- 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 index 14b60cc527..a3981a11dd 100644 --- 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()