From 99881bd3080b10d9d62a1bdd236b3bdbc881d46b Mon Sep 17 00:00:00 2001 From: Aymeric Mortemousque Date: Tue, 25 May 2021 15:42:18 +0200 Subject: [PATCH 01/20] =?UTF-8?q?=E2=99=BB=EF=B8=8F=20=F0=9F=90=9B=20Clear?= =?UTF-8?q?=20event=20listeners=20on=20XMLHttpRequest?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- packages/core/src/browser/xhrProxy.ts | 97 ++++++++++++++------------- 1 file changed, 52 insertions(+), 45 deletions(-) diff --git a/packages/core/src/browser/xhrProxy.ts b/packages/core/src/browser/xhrProxy.ts index 0afb30a84b..00d0056c85 100644 --- a/packages/core/src/browser/xhrProxy.ts +++ b/packages/core/src/browser/xhrProxy.ts @@ -3,7 +3,7 @@ import { Duration, elapsed, relativeNow, RelativeTime, ClocksState, clocksNow, t import { normalizeUrl } from '../tools/urlPolyfill' interface BrowserXHR extends XMLHttpRequest { - _datadog_xhr: XhrStartContext + _datadog_xhr?: XhrStartContext } export interface XhrProxy< @@ -19,6 +19,8 @@ export interface XhrStartContext { url: string startTime: RelativeTime // deprecated startClocks: ClocksState + isAborted: boolean + hasBeenReported: boolean /** * allow clients to enhance the context @@ -30,7 +32,6 @@ export interface XhrCompleteContext extends XhrStartContext { duration: Duration status: number response: string | undefined - isAborted: boolean } let xhrProxySingleton: XhrProxy | undefined @@ -77,66 +78,72 @@ function proxyXhr() { // eslint-disable-next-line @typescript-eslint/unbound-method originalXhrAbort = XMLHttpRequest.prototype.abort + function reportXhr(this: BrowserXHR, clearListeners: () => void) { + clearListeners() + if (this._datadog_xhr!.hasBeenReported) { + return + } + this._datadog_xhr!.hasBeenReported = true + + const xhrCompleteContext: XhrCompleteContext = { + ...this._datadog_xhr!, + duration: elapsed(this._datadog_xhr!.startClocks.timeStamp, timeStampNow()), + response: this.response as string | undefined, + status: this.status, + } + + onRequestCompleteCallbacks.forEach((callback) => callback(xhrCompleteContext)) + } + XMLHttpRequest.prototype.open = function (this: BrowserXHR, method: string, url: string) { callMonitored(() => { // WARN: since this data structure is tied to the instance, it is shared by both logs and rum // and can be used by different code versions depending on customer setup // so it should stay compatible with older versions - ;(this._datadog_xhr as Pick) = { + this._datadog_xhr = { method, url: normalizeUrl(url), - } + } as XhrStartContext }) return originalXhrOpen.apply(this, arguments as any) } XMLHttpRequest.prototype.send = function (this: BrowserXHR) { callMonitored(() => { - if (this._datadog_xhr) { - const xhrPendingContext = this._datadog_xhr as XhrStartContext & - Pick - xhrPendingContext.startTime = relativeNow() - xhrPendingContext.startClocks = clocksNow() - xhrPendingContext.isAborted = false - - const originalOnreadystatechange = this.onreadystatechange - - this.onreadystatechange = function () { - if (this.readyState === XMLHttpRequest.DONE) { - // Try to report the XHR as soon as possible, because the XHR may be mutated by the - // application during a future event. For example, Angular is calling .abort() on - // completed requests during a onreadystatechange event, so the status becomes '0' - // before the request is collected. - callMonitored(reportXhr) - } - - if (originalOnreadystatechange) { - originalOnreadystatechange.apply(this, arguments as any) - } - } - - let hasBeenReported = false - - const reportXhr = () => { - if (hasBeenReported) { - return - } - hasBeenReported = true - - const xhrCompleteContext: XhrCompleteContext = { - ...xhrPendingContext, - duration: elapsed(xhrPendingContext.startClocks.timeStamp, timeStampNow()), - response: this.response as string | undefined, - status: this.status, - } + if (!this._datadog_xhr) { + return + } + this._datadog_xhr = { + ...this._datadog_xhr, + startTime: relativeNow(), + startClocks: clocksNow(), + isAborted: false, + hasBeenReported: false, + } - onRequestCompleteCallbacks.forEach((callback) => callback(xhrCompleteContext)) + const originalOnreadystatechange = this.onreadystatechange + const onreadystatechange = function (this: BrowserXHR) { + if (this.readyState === XMLHttpRequest.DONE) { + // Try to report the XHR as soon as possible, because the XHR may be mutated by the + // application during a future event. For example, Angular is calling .abort() on + // completed requests during a onreadystatechange event, so the status becomes '0' + // before the request is collected. + callMonitored(reportXhr.bind(this, clearListeners)) } - this.addEventListener('loadend', monitor(reportXhr)) - - beforeSendCallbacks.forEach((callback) => callback(xhrPendingContext, this)) + if (originalOnreadystatechange) { + originalOnreadystatechange.apply(this, arguments as any) + } } + const clearListeners = () => { + this.removeEventListener('loadend', onLoadend) + this.onreadystatechange = originalOnreadystatechange + } + const onLoadend = monitor(reportXhr.bind(this, clearListeners)) + this.onreadystatechange = onreadystatechange + this.addEventListener('loadend', onLoadend) + + beforeSendCallbacks.forEach((callback) => callback(this._datadog_xhr!, this)) }) return originalXhrSend.apply(this, arguments as any) From fd3c9dcf3ce72152826d760e3c40a4b1471c898c Mon Sep 17 00:00:00 2001 From: Aymeric Mortemousque Date: Tue, 25 May 2021 15:43:56 +0200 Subject: [PATCH 02/20] =?UTF-8?q?=E2=9C=85=20Add=20test=20for=20multi=20re?= =?UTF-8?q?quests=20with=20XMLHttpRequest?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- packages/core/src/browser/xhrProxy.spec.ts | 22 ++++++++++++++++++++++ packages/core/test/specHelper.ts | 4 ++++ 2 files changed, 26 insertions(+) diff --git a/packages/core/src/browser/xhrProxy.spec.ts b/packages/core/src/browser/xhrProxy.spec.ts index b2cf132aba..d95d05eec6 100644 --- a/packages/core/src/browser/xhrProxy.spec.ts +++ b/packages/core/src/browser/xhrProxy.spec.ts @@ -209,4 +209,26 @@ describe('xhr proxy', () => { }, }) }) + + it('should track only the second request (the first request is canceled)', (done) => { + const onLoad = jasmine.createSpy() + withXhr({ + setup(xhr) { + xhr.addEventListener('load', onLoad) + xhr.open('GET', '/first') + xhr.send() + xhr.open('GET', '/second') + xhr.send() + xhr.complete(200, 'ok') + }, + onComplete() { + const request = getRequest(0) + expect(onLoad).toHaveBeenCalledTimes(1) + expect(request.method).toBe('GET') + expect(request.url).toContain('/second') + expect(request.status).toBe(200) + done() + }, + }) + }) }) diff --git a/packages/core/test/specHelper.ts b/packages/core/test/specHelper.ts index c3831c6af2..8ff8767803 100644 --- a/packages/core/test/specHelper.ts +++ b/packages/core/test/specHelper.ts @@ -181,6 +181,10 @@ class StubXhr { this.fakeEventTarget.addEventListener(name, callback) } + removeEventListener(name: string, callback: () => void) { + this.fakeEventTarget.removeEventListener(name, callback) + } + private dispatchEvent(name: string) { this.fakeEventTarget.dispatchEvent(createNewEvent(name)) } From b4ebff77b16e6e5036d4fdf2f81a3d96b722306f Mon Sep 17 00:00:00 2001 From: Aymeric Mortemousque Date: Wed, 26 May 2021 17:56:35 +0200 Subject: [PATCH 03/20] Add a third state interface for open xhr --- packages/core/src/browser/xhrProxy.ts | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/packages/core/src/browser/xhrProxy.ts b/packages/core/src/browser/xhrProxy.ts index 00d0056c85..1abbf926c6 100644 --- a/packages/core/src/browser/xhrProxy.ts +++ b/packages/core/src/browser/xhrProxy.ts @@ -2,8 +2,8 @@ import { monitor, callMonitored } from '../domain/internalMonitoring' import { Duration, elapsed, relativeNow, RelativeTime, ClocksState, clocksNow, timeStampNow } from '../tools/timeUtils' import { normalizeUrl } from '../tools/urlPolyfill' -interface BrowserXHR extends XMLHttpRequest { - _datadog_xhr?: XhrStartContext +interface BrowserXHR extends XMLHttpRequest { + _datadog_xhr?: T } export interface XhrProxy< @@ -14,14 +14,16 @@ export interface XhrProxy< onRequestComplete: (callback: (context: CompleteContext) => void) => void } -export interface XhrStartContext { +export interface XhrOpenContext { method: string url: string +} + +export interface XhrStartContext extends XhrOpenContext { startTime: RelativeTime // deprecated startClocks: ClocksState isAborted: boolean hasBeenReported: boolean - /** * allow clients to enhance the context */ @@ -78,7 +80,7 @@ function proxyXhr() { // eslint-disable-next-line @typescript-eslint/unbound-method originalXhrAbort = XMLHttpRequest.prototype.abort - function reportXhr(this: BrowserXHR, clearListeners: () => void) { + function reportXhr(this: BrowserXHR, clearListeners: () => void) { clearListeners() if (this._datadog_xhr!.hasBeenReported) { return @@ -95,7 +97,7 @@ function proxyXhr() { onRequestCompleteCallbacks.forEach((callback) => callback(xhrCompleteContext)) } - XMLHttpRequest.prototype.open = function (this: BrowserXHR, method: string, url: string) { + XMLHttpRequest.prototype.open = function (this: BrowserXHR, method: string, url: string) { callMonitored(() => { // WARN: since this data structure is tied to the instance, it is shared by both logs and rum // and can be used by different code versions depending on customer setup @@ -103,7 +105,7 @@ function proxyXhr() { this._datadog_xhr = { method, url: normalizeUrl(url), - } as XhrStartContext + } }) return originalXhrOpen.apply(this, arguments as any) } From 6f83a4c027c12a8f115dd071f7ba43d39dafd27c Mon Sep 17 00:00:00 2001 From: Aymeric Mortemousque Date: Thu, 27 May 2021 19:43:46 +0200 Subject: [PATCH 04/20] =?UTF-8?q?=E2=99=BB=EF=B8=8F=20=20Move=20reportXhr?= =?UTF-8?q?=20out=20of=20=20XMLHttpRequest.send?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- packages/core/src/browser/xhrProxy.ts | 148 +++++++++++++------------- 1 file changed, 74 insertions(+), 74 deletions(-) diff --git a/packages/core/src/browser/xhrProxy.ts b/packages/core/src/browser/xhrProxy.ts index 1abbf926c6..7080c36844 100644 --- a/packages/core/src/browser/xhrProxy.ts +++ b/packages/core/src/browser/xhrProxy.ts @@ -48,7 +48,16 @@ export function startXhrProxy< CompleteContext extends XhrCompleteContext = XhrCompleteContext >(): XhrProxy { if (!xhrProxySingleton) { - proxyXhr() + // eslint-disable-next-line @typescript-eslint/unbound-method + originalXhrOpen = XMLHttpRequest.prototype.open + // eslint-disable-next-line @typescript-eslint/unbound-method + originalXhrSend = XMLHttpRequest.prototype.send + // eslint-disable-next-line @typescript-eslint/unbound-method + originalXhrAbort = XMLHttpRequest.prototype.abort + XMLHttpRequest.prototype.open = openXhr + XMLHttpRequest.prototype.send = sendXhr + XMLHttpRequest.prototype.abort = abortXhr + xhrProxySingleton = { beforeSend(callback: (context: XhrStartContext, xhr: XMLHttpRequest) => void) { beforeSendCallbacks.push(callback) @@ -72,91 +81,82 @@ export function resetXhrProxy() { } } -function proxyXhr() { - // eslint-disable-next-line @typescript-eslint/unbound-method - originalXhrOpen = XMLHttpRequest.prototype.open - // eslint-disable-next-line @typescript-eslint/unbound-method - originalXhrSend = XMLHttpRequest.prototype.send - // eslint-disable-next-line @typescript-eslint/unbound-method - originalXhrAbort = XMLHttpRequest.prototype.abort - - function reportXhr(this: BrowserXHR, clearListeners: () => void) { - clearListeners() - if (this._datadog_xhr!.hasBeenReported) { - return +function openXhr(this: BrowserXHR, method: string, url: string) { + callMonitored(() => { + // WARN: since this data structure is tied to the instance, it is shared by both logs and rum + // and can be used by different code versions depending on customer setup + // so it should stay compatible with older versions + this._datadog_xhr = { + method, + url: normalizeUrl(url), } - this._datadog_xhr!.hasBeenReported = true + }) + return originalXhrOpen.apply(this, arguments as any) +} - const xhrCompleteContext: XhrCompleteContext = { - ...this._datadog_xhr!, - duration: elapsed(this._datadog_xhr!.startClocks.timeStamp, timeStampNow()), - response: this.response as string | undefined, - status: this.status, +function sendXhr(this: BrowserXHR) { + callMonitored(() => { + if (!this._datadog_xhr) { + return + } + this._datadog_xhr = { + ...this._datadog_xhr, + startTime: relativeNow(), + startClocks: clocksNow(), + isAborted: false, + hasBeenReported: false, } - onRequestCompleteCallbacks.forEach((callback) => callback(xhrCompleteContext)) - } + const originalOnreadystatechange = this.onreadystatechange + const onreadystatechange = function (this: BrowserXHR) { + if (this.readyState === XMLHttpRequest.DONE) { + // Try to report the XHR as soon as possible, because the XHR may be mutated by the + // application during a future event. For example, Angular is calling .abort() on + // completed requests during a onreadystatechange event, so the status becomes '0' + // before the request is collected. + onEnd() + } - XMLHttpRequest.prototype.open = function (this: BrowserXHR, method: string, url: string) { - callMonitored(() => { - // WARN: since this data structure is tied to the instance, it is shared by both logs and rum - // and can be used by different code versions depending on customer setup - // so it should stay compatible with older versions - this._datadog_xhr = { - method, - url: normalizeUrl(url), + if (originalOnreadystatechange) { + originalOnreadystatechange.apply(this, arguments as any) } + } + + const onEnd = monitor(() => { + this.removeEventListener('loadend', onEnd) + this.onreadystatechange = originalOnreadystatechange + reportXhr(this) }) - return originalXhrOpen.apply(this, arguments as any) - } + this.onreadystatechange = onreadystatechange + this.addEventListener('loadend', onEnd) - XMLHttpRequest.prototype.send = function (this: BrowserXHR) { - callMonitored(() => { - if (!this._datadog_xhr) { - return - } - this._datadog_xhr = { - ...this._datadog_xhr, - startTime: relativeNow(), - startClocks: clocksNow(), - isAborted: false, - hasBeenReported: false, - } + beforeSendCallbacks.forEach((callback) => callback(this._datadog_xhr!, this)) + }) - const originalOnreadystatechange = this.onreadystatechange - const onreadystatechange = function (this: BrowserXHR) { - if (this.readyState === XMLHttpRequest.DONE) { - // Try to report the XHR as soon as possible, because the XHR may be mutated by the - // application during a future event. For example, Angular is calling .abort() on - // completed requests during a onreadystatechange event, so the status becomes '0' - // before the request is collected. - callMonitored(reportXhr.bind(this, clearListeners)) - } - - if (originalOnreadystatechange) { - originalOnreadystatechange.apply(this, arguments as any) - } - } - const clearListeners = () => { - this.removeEventListener('loadend', onLoadend) - this.onreadystatechange = originalOnreadystatechange - } - const onLoadend = monitor(reportXhr.bind(this, clearListeners)) - this.onreadystatechange = onreadystatechange - this.addEventListener('loadend', onLoadend) + return originalXhrSend.apply(this, arguments as any) +} - beforeSendCallbacks.forEach((callback) => callback(this._datadog_xhr!, this)) - }) +function abortXhr(this: BrowserXHR) { + callMonitored(() => { + if (this._datadog_xhr) { + this._datadog_xhr.isAborted = true + } + }) + return originalXhrAbort.apply(this, arguments as any) +} - return originalXhrSend.apply(this, arguments as any) +function reportXhr(xhr: BrowserXHR) { + if (xhr._datadog_xhr!.hasBeenReported) { + return } + xhr._datadog_xhr!.hasBeenReported = true - XMLHttpRequest.prototype.abort = function (this: BrowserXHR) { - callMonitored(() => { - if (this._datadog_xhr) { - this._datadog_xhr.isAborted = true - } - }) - return originalXhrAbort.apply(this, arguments as any) + const xhrCompleteContext: XhrCompleteContext = { + ...xhr._datadog_xhr!, + duration: elapsed(xhr._datadog_xhr!.startClocks.timeStamp, timeStampNow()), + response: xhr.response as string | undefined, + status: xhr.status, } + + onRequestCompleteCallbacks.forEach((callback) => callback(xhrCompleteContext)) } From cb09c8068c737496d49ab0920e9f838307992cf4 Mon Sep 17 00:00:00 2001 From: Aymeric Mortemousque Date: Thu, 27 May 2021 19:48:12 +0200 Subject: [PATCH 05/20] =?UTF-8?q?=E2=9C=85=20=20Test=20for=20XHRs=20with?= =?UTF-8?q?=20same=20XMLHttpRequest=20instance?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- test/e2e/scenario/rum/resources.scenario.ts | 50 +++++++++++++++++++++ 1 file changed, 50 insertions(+) diff --git a/test/e2e/scenario/rum/resources.scenario.ts b/test/e2e/scenario/rum/resources.scenario.ts index 082c33945c..db01456e86 100644 --- a/test/e2e/scenario/rum/resources.scenario.ts +++ b/test/e2e/scenario/rum/resources.scenario.ts @@ -201,6 +201,56 @@ describe('rum resources', () => { expect(resourceEvent?.resource.status_code).toBe(0) }) }) + + describe('support XHRs with same XMLHttpRequest instance', () => { + createTest('track only second XHR (first is canceled by the browser)') + .withRum() + .withSetup(bundleSetup) + .run(async ({ events }) => { + await browserExecuteAsync((done) => { + const xhr = new XMLHttpRequest() + xhr.open('GET', '/ok?duration=200&call=1') + xhr.send() + xhr.open('GET', '/ok?duration=100&call=2') + xhr.send() + setTimeout(() => { + done(undefined) + }, 200) + }) + + await flushEvents() + + const resourceEvents = events.rumResources.filter((event) => event.resource.type === 'xhr') + expect(resourceEvents.length).toEqual(1) + expect(events.rumErrors.length).toBe(0) + expect(resourceEvents[0]?.resource.status_code).toBe(200) + }) + + createTest('track both XHRs when the second wait for the respond of the first') + .withRum() + .withSetup(bundleSetup) + .run(async ({ events }) => { + await browserExecuteAsync((done) => { + const xhr = new XMLHttpRequest() + const triggerSecondCall = () => { + xhr.removeEventListener('loadend', triggerSecondCall) + xhr.addEventListener('loadend', () => done(undefined)) + xhr.open('GET', '/ok?duration=100&call=2') + xhr.send() + } + xhr.addEventListener('loadend', triggerSecondCall) + xhr.open('GET', '/ok?duration=100&call=1') + xhr.send() + }) + + await flushEvents() + + const resourceEvents = events.rumResources.filter((event) => event.resource.type === 'xhr') + expect(resourceEvents.length).toEqual(2) + expect(events.rumErrors.length).toBe(0) + expect(resourceEvents.map((resourceEvent) => resourceEvent.resource.status_code)).toEqual([200, 200]) + }) + }) }) function expectToHaveValidTimings(resourceEvent: RumResourceEvent) { From 61775faf796a7cca5ddcf8ce8deb0d2b7c20a198 Mon Sep 17 00:00:00 2001 From: Aymeric Mortemousque Date: Tue, 1 Jun 2021 17:22:15 +0200 Subject: [PATCH 06/20] =?UTF-8?q?=F0=9F=91=8C=20use=20event=20listener=20f?= =?UTF-8?q?or=20readystatechange?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- packages/core/src/browser/xhrProxy.ts | 66 ++++++++++++--------------- 1 file changed, 29 insertions(+), 37 deletions(-) diff --git a/packages/core/src/browser/xhrProxy.ts b/packages/core/src/browser/xhrProxy.ts index 7080c36844..c98c8fbb63 100644 --- a/packages/core/src/browser/xhrProxy.ts +++ b/packages/core/src/browser/xhrProxy.ts @@ -1,4 +1,4 @@ -import { monitor, callMonitored } from '../domain/internalMonitoring' +import { callMonitored } from '../domain/internalMonitoring' import { Duration, elapsed, relativeNow, RelativeTime, ClocksState, clocksNow, timeStampNow } from '../tools/timeUtils' import { normalizeUrl } from '../tools/urlPolyfill' @@ -23,7 +23,6 @@ export interface XhrStartContext extends XhrOpenContext { startTime: RelativeTime // deprecated startClocks: ClocksState isAborted: boolean - hasBeenReported: boolean /** * allow clients to enhance the context */ @@ -90,6 +89,12 @@ function openXhr(this: BrowserXHR, method: string, url: string) method, url: normalizeUrl(url), } + + // To avoid adding listeners each time 'open()' is called, + // we take advantage of the automatic listener discarding if multiple identical EventListeners are registered + // https://developer.mozilla.org/en-US/docs/Web/API/EventTarget/addEventListener#multiple_identical_event_listeners + this.addEventListener('readystatechange', onReadyStateChange) + this.addEventListener('loadend', reportXhr) }) return originalXhrOpen.apply(this, arguments as any) } @@ -104,32 +109,8 @@ function sendXhr(this: BrowserXHR) { startTime: relativeNow(), startClocks: clocksNow(), isAborted: false, - hasBeenReported: false, - } - - const originalOnreadystatechange = this.onreadystatechange - const onreadystatechange = function (this: BrowserXHR) { - if (this.readyState === XMLHttpRequest.DONE) { - // Try to report the XHR as soon as possible, because the XHR may be mutated by the - // application during a future event. For example, Angular is calling .abort() on - // completed requests during a onreadystatechange event, so the status becomes '0' - // before the request is collected. - onEnd() - } - - if (originalOnreadystatechange) { - originalOnreadystatechange.apply(this, arguments as any) - } } - const onEnd = monitor(() => { - this.removeEventListener('loadend', onEnd) - this.onreadystatechange = originalOnreadystatechange - reportXhr(this) - }) - this.onreadystatechange = onreadystatechange - this.addEventListener('loadend', onEnd) - beforeSendCallbacks.forEach((callback) => callback(this._datadog_xhr!, this)) }) @@ -145,18 +126,29 @@ function abortXhr(this: BrowserXHR) { return originalXhrAbort.apply(this, arguments as any) } -function reportXhr(xhr: BrowserXHR) { - if (xhr._datadog_xhr!.hasBeenReported) { - return +function onReadyStateChange(this: BrowserXHR) { + if (this.readyState === XMLHttpRequest.DONE) { + // Try to report the XHR as soon as possible, because the XHR may be mutated by the + // application during a future event. For example, Angular is calling .abort() on + // completed requests during a onreadystatechange event, so the status becomes '0' + // before the request is collected. + // https://github.com/angular/angular/blob/master/packages/common/http/src/xhr.ts + reportXhr.call(this) } - xhr._datadog_xhr!.hasBeenReported = true +} - const xhrCompleteContext: XhrCompleteContext = { - ...xhr._datadog_xhr!, - duration: elapsed(xhr._datadog_xhr!.startClocks.timeStamp, timeStampNow()), - response: xhr.response as string | undefined, - status: xhr.status, - } +function reportXhr(this: BrowserXHR) { + callMonitored(() => { + const xhrCompleteContext: XhrCompleteContext = { + ...this._datadog_xhr!, + duration: elapsed(this._datadog_xhr!.startClocks.timeStamp, timeStampNow()), + response: this.response as string | undefined, + status: this.status, + } - onRequestCompleteCallbacks.forEach((callback) => callback(xhrCompleteContext)) + onRequestCompleteCallbacks.forEach((callback) => callback(xhrCompleteContext)) + // Unsubscribe to avoid being reported twice + this.removeEventListener('readystatechange', onReadyStateChange) + this.removeEventListener('loadend', reportXhr) + }) } From d0f2a7769a5fe36b2cc0025a91f2b38ac32bfb62 Mon Sep 17 00:00:00 2001 From: Aymeric Mortemousque Date: Tue, 1 Jun 2021 17:23:36 +0200 Subject: [PATCH 07/20] Update StubXhr to also stub listeners logic --- packages/core/test/specHelper.ts | 28 ++++++++++++++++++---------- 1 file changed, 18 insertions(+), 10 deletions(-) diff --git a/packages/core/test/specHelper.ts b/packages/core/test/specHelper.ts index 8ff8767803..90c601dc05 100644 --- a/packages/core/test/specHelper.ts +++ b/packages/core/test/specHelper.ts @@ -135,11 +135,7 @@ class StubXhr { public onreadystatechange: () => void = noop private hasEnded = false - private fakeEventTarget: HTMLDivElement - - constructor() { - this.fakeEventTarget = document.createElement('div') - } + private listeners: { [k: string]: Array<() => void> } = {} /* eslint-disable @typescript-eslint/no-empty-function,@typescript-eslint/no-unused-vars */ open(method: string, url: string) {} @@ -154,7 +150,7 @@ class StubXhr { } this.hasEnded = true this.readyState = XMLHttpRequest.DONE - this.onreadystatechange() + this.dispatchEvent('readystatechange') this.dispatchEvent('abort') this.dispatchEvent('loadend') } @@ -167,7 +163,8 @@ class StubXhr { this.response = response this.status = status this.readyState = XMLHttpRequest.DONE - this.onreadystatechange() + this.dispatchEvent('readystatechange') + if (status >= 200 && status < 500) { this.dispatchEvent('load') } @@ -178,15 +175,26 @@ class StubXhr { } addEventListener(name: string, callback: () => void) { - this.fakeEventTarget.addEventListener(name, callback) + if (!this.listeners[name]) { + this.listeners[name] = [] + } + + this.listeners[name].push(callback) } removeEventListener(name: string, callback: () => void) { - this.fakeEventTarget.removeEventListener(name, callback) + if (!this.listeners[name]) { + throw new Error(`Can't remove a listener. Event "${name}" doesn't exits.`) + } + + this.listeners[name] = this.listeners[name].filter((listener) => listener !== callback) } private dispatchEvent(name: string) { - this.fakeEventTarget.dispatchEvent(createNewEvent(name)) + if (!this.listeners[name]) { + return + } + this.listeners[name].forEach((listener) => listener.call(this)) } } From c567f7e61c9b3581ea2426170b81494d1dc72a91 Mon Sep 17 00:00:00 2001 From: Aymeric Mortemousque Date: Tue, 1 Jun 2021 17:25:05 +0200 Subject: [PATCH 08/20] =?UTF-8?q?=E2=9C=85=20=20Update=20xhr=20test=20to?= =?UTF-8?q?=20match=20the=20Angular=20use=20case?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- packages/core/src/browser/xhrProxy.spec.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/core/src/browser/xhrProxy.spec.ts b/packages/core/src/browser/xhrProxy.spec.ts index d95d05eec6..759708ab7d 100644 --- a/packages/core/src/browser/xhrProxy.spec.ts +++ b/packages/core/src/browser/xhrProxy.spec.ts @@ -110,9 +110,9 @@ describe('xhr proxy', () => { const onReadyStateChange = jasmine.createSpy() withXhr({ setup(xhr) { - xhr.onreadystatechange = onReadyStateChange - xhr.addEventListener('load', () => xhr.abort()) xhr.open('GET', '/ok') + xhr.addEventListener('readystatechange', onReadyStateChange) + xhr.addEventListener('load', () => xhr.abort()) xhr.send() xhr.complete(200, 'ok') }, From e14e35a2cc99bd9233743a87fd05a0c413ef0f53 Mon Sep 17 00:00:00 2001 From: Aymeric Mortemousque Date: Thu, 3 Jun 2021 21:13:33 +0200 Subject: [PATCH 09/20] =?UTF-8?q?=F0=9F=91=8C=20StubXhr=20support=20readys?= =?UTF-8?q?tatechange=20attribute=20and=20evt?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- packages/core/test/specHelper.ts | 21 ++++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/packages/core/test/specHelper.ts b/packages/core/test/specHelper.ts index 90c601dc05..df7d9a6f43 100644 --- a/packages/core/test/specHelper.ts +++ b/packages/core/test/specHelper.ts @@ -133,12 +133,14 @@ class StubXhr { public status: number | undefined = undefined public readyState: number = XMLHttpRequest.UNSENT public onreadystatechange: () => void = noop - + public listeners: { [k: string]: Array<() => void> } = {} + private isOnreadystatechangeAttributeCallFirst = true private hasEnded = false - private listeners: { [k: string]: Array<() => void> } = {} /* eslint-disable @typescript-eslint/no-empty-function,@typescript-eslint/no-unused-vars */ - open(method: string, url: string) {} + open(method: string, url: string) { + this.hasEnded = false + } send() {} /* eslint-enable @typescript-eslint/no-empty-function,@typescript-eslint/no-unused-vars */ @@ -163,7 +165,14 @@ class StubXhr { this.response = response this.status = status this.readyState = XMLHttpRequest.DONE - this.dispatchEvent('readystatechange') + + if (this.isOnreadystatechangeAttributeCallFirst) { + this.onreadystatechange() + this.dispatchEvent('readystatechange') + } else { + this.dispatchEvent('readystatechange') + this.onreadystatechange() + } if (status >= 200 && status < 500) { this.dispatchEvent('load') @@ -178,7 +187,9 @@ class StubXhr { if (!this.listeners[name]) { this.listeners[name] = [] } - + if (name === 'readystatechange' && this.onreadystatechange === noop) { + this.isOnreadystatechangeAttributeCallFirst = false + } this.listeners[name].push(callback) } From 3a49a3e0d4096f67b905a0fb278019f74541196b Mon Sep 17 00:00:00 2001 From: Aymeric Mortemousque Date: Thu, 3 Jun 2021 21:15:39 +0200 Subject: [PATCH 10/20] =?UTF-8?q?=E2=9C=85=20=20Test=20helper=20withXhr=20?= =?UTF-8?q?trigger=20complete=20manually?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- packages/core/test/specHelper.ts | 22 +++++++++++++++------- 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/packages/core/test/specHelper.ts b/packages/core/test/specHelper.ts index df7d9a6f43..f101c6f5a3 100644 --- a/packages/core/test/specHelper.ts +++ b/packages/core/test/specHelper.ts @@ -243,17 +243,25 @@ export function stubXhr() { export function withXhr({ setup, onComplete, + completionMode = 'automatic', }: { - setup: (xhr: StubXhr) => void + setup: (xhr: StubXhr, complete: (xhr: StubXhr) => void) => void onComplete: (xhr: XMLHttpRequest) => void + completionMode?: 'manual' | 'automatic' }) { const xhr = new XMLHttpRequest() - xhr.addEventListener('loadend', () => { - setTimeout(() => { - onComplete(xhr) - }) - }) - setup((xhr as unknown) as StubXhr) + if (completionMode === 'automatic') { + const loadendHandler = () => { + xhr.removeEventListener('loadend', loadendHandler) + setTimeout(() => { + onComplete(xhr) + }) + } + xhr.addEventListener('loadend', loadendHandler) + } + const complete = (xhr: StubXhr) => onComplete((xhr as unknown) as XMLHttpRequest) + + setup((xhr as unknown) as StubXhr, complete) } export function setPageVisibility(visibility: 'visible' | 'hidden') { From 96ea908d434ab33c031780120a8d2365f486dd6b Mon Sep 17 00:00:00 2001 From: Aymeric Mortemousque Date: Thu, 3 Jun 2021 21:18:49 +0200 Subject: [PATCH 11/20] =?UTF-8?q?=F0=9F=91=8C=20Cover=20onreadystage=20bin?= =?UTF-8?q?ding=20before=20open?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- packages/core/src/browser/xhrProxy.spec.ts | 115 +++++++++++++++++---- packages/core/src/browser/xhrProxy.ts | 27 ++++- 2 files changed, 122 insertions(+), 20 deletions(-) diff --git a/packages/core/src/browser/xhrProxy.spec.ts b/packages/core/src/browser/xhrProxy.spec.ts index 759708ab7d..4150ea5543 100644 --- a/packages/core/src/browser/xhrProxy.spec.ts +++ b/packages/core/src/browser/xhrProxy.spec.ts @@ -106,18 +106,22 @@ describe('xhr proxy', () => { }) }) - it('should track successful request aborted', (done) => { - const onReadyStateChange = jasmine.createSpy() + it('should track successful request aborted when onreadystatechange is overridden before open', (done) => { withXhr({ setup(xhr) { + xhr.onreadystatechange = () => { + if (xhr.readyState === XMLHttpRequest.DONE) { + xhr.abort() + } + } + spyOn(xhr, 'onreadystatechange').and.callThrough() xhr.open('GET', '/ok') - xhr.addEventListener('readystatechange', onReadyStateChange) - xhr.addEventListener('load', () => xhr.abort()) xhr.send() xhr.complete(200, 'ok') }, onComplete(xhr) { const request = getRequest(0) + expect(completeSpy.calls.count()).toBe(1) expect(request.method).toBe('GET') expect(request.url).toContain('/ok') expect(request.response).toBe('ok') @@ -126,7 +130,37 @@ describe('xhr proxy', () => { expect(request.duration).toEqual(jasmine.any(Number)) expect(request.isAborted).toBe(false) expect(xhr.status).toBe(0) - expect(onReadyStateChange).toHaveBeenCalled() + expect(xhr.onreadystatechange).toHaveBeenCalledTimes(1) + expect() + done() + }, + }) + }) + + it('should track successful request aborted when onreadystatechange overridden after open', (done) => { + withXhr({ + setup(xhr) { + xhr.open('GET', '/ok') + xhr.onreadystatechange = () => { + if (xhr.readyState === XMLHttpRequest.DONE) { + xhr.abort() + } + } + spyOn(xhr, 'onreadystatechange').and.callThrough() + xhr.send() + xhr.complete(200, 'ok') + }, + onComplete(xhr) { + const request = getRequest(0) + expect(request.method).toBe('GET') + expect(request.url).toContain('/ok') + expect(request.response).toBe('ok') + expect(request.status).toBe(200) + expect(request.startTime).toEqual(jasmine.any(Number)) + expect(request.duration).toEqual(jasmine.any(Number)) + expect(request.isAborted).toBe(false) + expect(xhr.status).toBe(0) + expect(xhr.onreadystatechange).toHaveBeenCalledTimes(1) done() }, }) @@ -154,7 +188,29 @@ describe('xhr proxy', () => { }) }) - it('should track request with onreadystatechange overridden', (done) => { + it('should track request with onreadystatechange overridden before open', (done) => { + withXhr({ + setup(xhr) { + xhr.onreadystatechange = () => undefined + xhr.open('GET', '/ok') + xhr.send() + xhr.complete(200, 'ok') + }, + onComplete() { + const request = getRequest(0) + expect(request.method).toBe('GET') + expect(request.url).toContain('/ok') + expect(request.response).toBe('ok') + expect(request.status).toBe(200) + expect(request.isAborted).toBe(false) + expect(request.startTime).toEqual(jasmine.any(Number)) + expect(request.duration).toEqual(jasmine.any(Number)) + done() + }, + }) + }) + + it('should track request with onreadystatechange overridden after open', (done) => { withXhr({ setup(xhr) { xhr.open('GET', '/ok') @@ -194,7 +250,7 @@ describe('xhr proxy', () => { }) }) - it('should should not break xhr opened before the instrumentation', (done) => { + it('should not break xhr opened before the instrumentation', (done) => { resetXhrProxy() withXhr({ setup(xhr) { @@ -210,23 +266,44 @@ describe('xhr proxy', () => { }) }) - it('should track only the second request (the first request is canceled)', (done) => { - const onLoad = jasmine.createSpy() + it('should track multiple requests with the same xhr instance', (done) => { + let listeners: { [k: string]: Array<() => void> } withXhr({ - setup(xhr) { + completionMode: 'manual', + setup(xhr, complete) { + const onLoad = () => { + xhr.removeEventListener('load', onLoad) + xhr.open('GET', '/ok') + xhr.send() + xhr.complete(400, 'ok') + } + xhr.onreadystatechange = jasmine.createSpy() xhr.addEventListener('load', onLoad) - xhr.open('GET', '/first') - xhr.send() - xhr.open('GET', '/second') + xhr.open('GET', '/ok') xhr.send() xhr.complete(200, 'ok') + listeners = xhr.listeners + complete(xhr) }, - onComplete() { - const request = getRequest(0) - expect(onLoad).toHaveBeenCalledTimes(1) - expect(request.method).toBe('GET') - expect(request.url).toContain('/second') - expect(request.status).toBe(200) + onComplete(xhr) { + const firstRequest = getRequest(1) + expect(firstRequest.method).toBe('GET') + expect(firstRequest.status).toBe(400) + expect(firstRequest.isAborted).toBe(false) + expect(firstRequest.startTime).toEqual(jasmine.any(Number)) + expect(firstRequest.duration).toEqual(jasmine.any(Number)) + + const secondRequest = getRequest(0) + expect(secondRequest.method).toBe('GET') + expect(secondRequest.status).toBe(200) + expect(secondRequest.isAborted).toBe(false) + expect(secondRequest.startTime).toEqual(jasmine.any(Number)) + expect(secondRequest.duration).toEqual(jasmine.any(Number)) + + expect(xhr.onreadystatechange).toHaveBeenCalledTimes(2) + expect(listeners.load.length).toBe(0) + expect(listeners.loadend.length).toBe(0) + expect(listeners.readystatechange.length).toBe(0) done() }, }) diff --git a/packages/core/src/browser/xhrProxy.ts b/packages/core/src/browser/xhrProxy.ts index c98c8fbb63..6e608634d1 100644 --- a/packages/core/src/browser/xhrProxy.ts +++ b/packages/core/src/browser/xhrProxy.ts @@ -17,6 +17,7 @@ export interface XhrProxy< export interface XhrOpenContext { method: string url: string + originalOnReadyStateChange: ((this: XMLHttpRequest, ev: Event) => any) | null } export interface XhrStartContext extends XhrOpenContext { @@ -88,13 +89,19 @@ function openXhr(this: BrowserXHR, method: string, url: string) this._datadog_xhr = { method, url: normalizeUrl(url), + // Keep track of the user onreadystatechange to be able to call it after us + originalOnReadyStateChange: getOriginalOnReadyStateChange(this), } // To avoid adding listeners each time 'open()' is called, // we take advantage of the automatic listener discarding if multiple identical EventListeners are registered // https://developer.mozilla.org/en-US/docs/Web/API/EventTarget/addEventListener#multiple_identical_event_listeners - this.addEventListener('readystatechange', onReadyStateChange) this.addEventListener('loadend', reportXhr) + + // Bind readystatechange with addEventListener and attribute affectation + // to ensure not to be overridden after or before the xhr open is called + this.addEventListener('readystatechange', onReadyStateChange) + this.onreadystatechange = onReadyStateChangeFromProperty }) return originalXhrOpen.apply(this, arguments as any) } @@ -126,6 +133,15 @@ function abortXhr(this: BrowserXHR) { return originalXhrAbort.apply(this, arguments as any) } +function onReadyStateChangeFromProperty(this: BrowserXHR) { + onReadyStateChange.call(this) + + const originalOnReadyStateChange = getOriginalOnReadyStateChange(this) + if (originalOnReadyStateChange) { + originalOnReadyStateChange.apply(this, arguments as any) + } +} + function onReadyStateChange(this: BrowserXHR) { if (this.readyState === XMLHttpRequest.DONE) { // Try to report the XHR as soon as possible, because the XHR may be mutated by the @@ -150,5 +166,14 @@ function reportXhr(this: BrowserXHR) { // Unsubscribe to avoid being reported twice this.removeEventListener('readystatechange', onReadyStateChange) this.removeEventListener('loadend', reportXhr) + this.onreadystatechange = getOriginalOnReadyStateChange(this) }) } + +function getOriginalOnReadyStateChange(xhr: BrowserXHR | BrowserXHR) { + // Check if the onreadystatechange has changed between the open() and the async onreadystatechange callback + // and get xhr.onreadystatechange instead of originalOnreadystatechange + return xhr.onreadystatechange !== onReadyStateChangeFromProperty + ? xhr.onreadystatechange + : xhr._datadog_xhr?.originalOnReadyStateChange ?? null +} From ccb7343bf3b29c12f3b92d3995df19cb9b0e1b05 Mon Sep 17 00:00:00 2001 From: Aymeric Mortemousque Date: Fri, 4 Jun 2021 11:06:28 +0200 Subject: [PATCH 12/20] Fix multiple request xhr test --- packages/core/src/browser/xhrProxy.spec.ts | 18 +++++++++++------- packages/core/test/specHelper.ts | 10 ++++++---- 2 files changed, 17 insertions(+), 11 deletions(-) diff --git a/packages/core/src/browser/xhrProxy.spec.ts b/packages/core/src/browser/xhrProxy.spec.ts index 4150ea5543..9e3a0c9e37 100644 --- a/packages/core/src/browser/xhrProxy.spec.ts +++ b/packages/core/src/browser/xhrProxy.spec.ts @@ -271,31 +271,35 @@ describe('xhr proxy', () => { withXhr({ completionMode: 'manual', setup(xhr, complete) { + const secondOnload = () => { + xhr.removeEventListener('load', secondOnload) + complete(xhr) + } const onLoad = () => { xhr.removeEventListener('load', onLoad) - xhr.open('GET', '/ok') + xhr.addEventListener('load', secondOnload) + xhr.open('GET', '/ok?request=1') xhr.send() xhr.complete(400, 'ok') } xhr.onreadystatechange = jasmine.createSpy() xhr.addEventListener('load', onLoad) - xhr.open('GET', '/ok') + xhr.open('GET', '/ok?request=1') xhr.send() xhr.complete(200, 'ok') listeners = xhr.listeners - complete(xhr) }, onComplete(xhr) { - const firstRequest = getRequest(1) + const firstRequest = getRequest(0) expect(firstRequest.method).toBe('GET') - expect(firstRequest.status).toBe(400) + expect(firstRequest.status).toBe(200) expect(firstRequest.isAborted).toBe(false) expect(firstRequest.startTime).toEqual(jasmine.any(Number)) expect(firstRequest.duration).toEqual(jasmine.any(Number)) - const secondRequest = getRequest(0) + const secondRequest = getRequest(1) expect(secondRequest.method).toBe('GET') - expect(secondRequest.status).toBe(200) + expect(secondRequest.status).toBe(400) expect(secondRequest.isAborted).toBe(false) expect(secondRequest.startTime).toEqual(jasmine.any(Number)) expect(secondRequest.duration).toEqual(jasmine.any(Number)) diff --git a/packages/core/test/specHelper.ts b/packages/core/test/specHelper.ts index f101c6f5a3..7b05a6846e 100644 --- a/packages/core/test/specHelper.ts +++ b/packages/core/test/specHelper.ts @@ -250,16 +250,18 @@ export function withXhr({ completionMode?: 'manual' | 'automatic' }) { const xhr = new XMLHttpRequest() + const complete = (xhr: StubXhr | XMLHttpRequest) => { + setTimeout(() => { + onComplete((xhr as unknown) as XMLHttpRequest) + }) + } if (completionMode === 'automatic') { const loadendHandler = () => { xhr.removeEventListener('loadend', loadendHandler) - setTimeout(() => { - onComplete(xhr) - }) + complete(xhr) } xhr.addEventListener('loadend', loadendHandler) } - const complete = (xhr: StubXhr) => onComplete((xhr as unknown) as XMLHttpRequest) setup((xhr as unknown) as StubXhr, complete) } From dfb256faeb985946dbfe5b7b7f0f25abebd67a11 Mon Sep 17 00:00:00 2001 From: Aymeric Mortemousque Date: Fri, 4 Jun 2021 11:07:30 +0200 Subject: [PATCH 13/20] Fix infinite loop in case of DD_RUM and DD_LOGS --- packages/core/src/browser/xhrProxy.ts | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/packages/core/src/browser/xhrProxy.ts b/packages/core/src/browser/xhrProxy.ts index 6e608634d1..1db9f0562e 100644 --- a/packages/core/src/browser/xhrProxy.ts +++ b/packages/core/src/browser/xhrProxy.ts @@ -101,7 +101,7 @@ function openXhr(this: BrowserXHR, method: string, url: string) // Bind readystatechange with addEventListener and attribute affectation // to ensure not to be overridden after or before the xhr open is called this.addEventListener('readystatechange', onReadyStateChange) - this.onreadystatechange = onReadyStateChangeFromProperty + this.onreadystatechange = datadogOnReadyStateChangeFromProperty }) return originalXhrOpen.apply(this, arguments as any) } @@ -133,7 +133,8 @@ function abortXhr(this: BrowserXHR) { return originalXhrAbort.apply(this, arguments as any) } -function onReadyStateChangeFromProperty(this: BrowserXHR) { +// prefix the function name to avoid overlap with the end user for getOriginalOnReadyStateChange() +function datadogOnReadyStateChangeFromProperty(this: BrowserXHR) { onReadyStateChange.call(this) const originalOnReadyStateChange = getOriginalOnReadyStateChange(this) @@ -173,7 +174,9 @@ function reportXhr(this: BrowserXHR) { function getOriginalOnReadyStateChange(xhr: BrowserXHR | BrowserXHR) { // Check if the onreadystatechange has changed between the open() and the async onreadystatechange callback // and get xhr.onreadystatechange instead of originalOnreadystatechange - return xhr.onreadystatechange !== onReadyStateChangeFromProperty + // In the case where DD_RUM and DD_LOGS are in the page the comparison by reference miss + // therefore we check the function name to avoid recursive calls + return xhr.onreadystatechange?.name !== datadogOnReadyStateChangeFromProperty?.name ? xhr.onreadystatechange : xhr._datadog_xhr?.originalOnReadyStateChange ?? null } From ef3b2354fe3d136d0fed0c97523f5f8206646634 Mon Sep 17 00:00:00 2001 From: Aymeric Mortemousque Date: Fri, 4 Jun 2021 12:04:15 +0200 Subject: [PATCH 14/20] Create functionName helper for IE --- packages/core/src/browser/xhrProxy.ts | 3 ++- packages/core/src/tools/utils.ts | 11 +++++++++++ 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/packages/core/src/browser/xhrProxy.ts b/packages/core/src/browser/xhrProxy.ts index 1db9f0562e..a2398a7555 100644 --- a/packages/core/src/browser/xhrProxy.ts +++ b/packages/core/src/browser/xhrProxy.ts @@ -1,3 +1,4 @@ +import { functionName } from '..' import { callMonitored } from '../domain/internalMonitoring' import { Duration, elapsed, relativeNow, RelativeTime, ClocksState, clocksNow, timeStampNow } from '../tools/timeUtils' import { normalizeUrl } from '../tools/urlPolyfill' @@ -176,7 +177,7 @@ function getOriginalOnReadyStateChange(xhr: BrowserXHR | Browser // and get xhr.onreadystatechange instead of originalOnreadystatechange // In the case where DD_RUM and DD_LOGS are in the page the comparison by reference miss // therefore we check the function name to avoid recursive calls - return xhr.onreadystatechange?.name !== datadogOnReadyStateChangeFromProperty?.name + return functionName(xhr.onreadystatechange) !== functionName(datadogOnReadyStateChangeFromProperty) ? xhr.onreadystatechange : xhr._datadog_xhr?.originalOnReadyStateChange ?? null } diff --git a/packages/core/src/tools/utils.ts b/packages/core/src/tools/utils.ts index 8a0f78c8e4..c33e56ddd8 100644 --- a/packages/core/src/tools/utils.ts +++ b/packages/core/src/tools/utils.ts @@ -242,6 +242,17 @@ export function mapValues(object: { [key: string]: A }, fn: (arg: A) => B) return newObject } +export function functionName(func: ((...args: any[]) => any) | null | undefined) { + if (!func) { + return + } + if (func.name) { + return func.name + } + const result = /^function\s*([^\s(]+)/.exec(func.toString()) + return result ? result[1] : undefined +} + /** * inspired by https://mathiasbynens.be/notes/globalthis */ From 9903554a5a0d346c195048767f98e23ee7cf7164 Mon Sep 17 00:00:00 2001 From: Aymeric Mortemousque Date: Fri, 4 Jun 2021 18:08:43 +0200 Subject: [PATCH 15/20] Rollback to previous implementation --- packages/core/src/browser/xhrProxy.spec.ts | 32 +------ packages/core/src/browser/xhrProxy.ts | 95 ++++++++------------- packages/core/src/tools/utils.ts | 11 --- rum-events-format | 2 +- test/e2e/scenario/rum/resources.scenario.ts | 25 +----- 5 files changed, 39 insertions(+), 126 deletions(-) diff --git a/packages/core/src/browser/xhrProxy.spec.ts b/packages/core/src/browser/xhrProxy.spec.ts index 9e3a0c9e37..fc9f0a028e 100644 --- a/packages/core/src/browser/xhrProxy.spec.ts +++ b/packages/core/src/browser/xhrProxy.spec.ts @@ -106,7 +106,7 @@ describe('xhr proxy', () => { }) }) - it('should track successful request aborted when onreadystatechange is overridden before open', (done) => { + it('should track successful request aborted', (done) => { withXhr({ setup(xhr) { xhr.onreadystatechange = () => { @@ -137,35 +137,6 @@ describe('xhr proxy', () => { }) }) - it('should track successful request aborted when onreadystatechange overridden after open', (done) => { - withXhr({ - setup(xhr) { - xhr.open('GET', '/ok') - xhr.onreadystatechange = () => { - if (xhr.readyState === XMLHttpRequest.DONE) { - xhr.abort() - } - } - spyOn(xhr, 'onreadystatechange').and.callThrough() - xhr.send() - xhr.complete(200, 'ok') - }, - onComplete(xhr) { - const request = getRequest(0) - expect(request.method).toBe('GET') - expect(request.url).toContain('/ok') - expect(request.response).toBe('ok') - expect(request.status).toBe(200) - expect(request.startTime).toEqual(jasmine.any(Number)) - expect(request.duration).toEqual(jasmine.any(Number)) - expect(request.isAborted).toBe(false) - expect(xhr.status).toBe(0) - expect(xhr.onreadystatechange).toHaveBeenCalledTimes(1) - done() - }, - }) - }) - it('should track aborted requests', (done) => { withXhr({ setup(xhr) { @@ -307,7 +278,6 @@ describe('xhr proxy', () => { expect(xhr.onreadystatechange).toHaveBeenCalledTimes(2) expect(listeners.load.length).toBe(0) expect(listeners.loadend.length).toBe(0) - expect(listeners.readystatechange.length).toBe(0) done() }, }) diff --git a/packages/core/src/browser/xhrProxy.ts b/packages/core/src/browser/xhrProxy.ts index a2398a7555..55c317aea1 100644 --- a/packages/core/src/browser/xhrProxy.ts +++ b/packages/core/src/browser/xhrProxy.ts @@ -1,5 +1,4 @@ -import { functionName } from '..' -import { callMonitored } from '../domain/internalMonitoring' +import { monitor, callMonitored } from '../domain/internalMonitoring' import { Duration, elapsed, relativeNow, RelativeTime, ClocksState, clocksNow, timeStampNow } from '../tools/timeUtils' import { normalizeUrl } from '../tools/urlPolyfill' @@ -18,7 +17,6 @@ export interface XhrProxy< export interface XhrOpenContext { method: string url: string - originalOnReadyStateChange: ((this: XMLHttpRequest, ev: Event) => any) | null } export interface XhrStartContext extends XhrOpenContext { @@ -90,19 +88,7 @@ function openXhr(this: BrowserXHR, method: string, url: string) this._datadog_xhr = { method, url: normalizeUrl(url), - // Keep track of the user onreadystatechange to be able to call it after us - originalOnReadyStateChange: getOriginalOnReadyStateChange(this), } - - // To avoid adding listeners each time 'open()' is called, - // we take advantage of the automatic listener discarding if multiple identical EventListeners are registered - // https://developer.mozilla.org/en-US/docs/Web/API/EventTarget/addEventListener#multiple_identical_event_listeners - this.addEventListener('loadend', reportXhr) - - // Bind readystatechange with addEventListener and attribute affectation - // to ensure not to be overridden after or before the xhr open is called - this.addEventListener('readystatechange', onReadyStateChange) - this.onreadystatechange = datadogOnReadyStateChangeFromProperty }) return originalXhrOpen.apply(this, arguments as any) } @@ -119,6 +105,34 @@ function sendXhr(this: BrowserXHR) { isAborted: false, } + let hasBeenReported = false + const originalOnreadystatechange = this.onreadystatechange + const onreadystatechange = function (this: BrowserXHR) { + if (this.readyState === XMLHttpRequest.DONE) { + // Try to report the XHR as soon as possible, because the XHR may be mutated by the + // application during a future event. For example, Angular is calling .abort() on + // completed requests during a onreadystatechange event, so the status becomes '0' + // before the request is collected. + onEnd() + } + + if (originalOnreadystatechange) { + originalOnreadystatechange.apply(this, arguments as any) + } + } + + const onEnd = monitor(() => { + this.removeEventListener('loadend', onEnd) + this.onreadystatechange = originalOnreadystatechange + if (hasBeenReported) { + return + } + hasBeenReported = true + reportXhr(this) + }) + this.onreadystatechange = onreadystatechange + this.addEventListener('loadend', onEnd) + beforeSendCallbacks.forEach((callback) => callback(this._datadog_xhr!, this)) }) @@ -134,50 +148,13 @@ function abortXhr(this: BrowserXHR) { return originalXhrAbort.apply(this, arguments as any) } -// prefix the function name to avoid overlap with the end user for getOriginalOnReadyStateChange() -function datadogOnReadyStateChangeFromProperty(this: BrowserXHR) { - onReadyStateChange.call(this) - - const originalOnReadyStateChange = getOriginalOnReadyStateChange(this) - if (originalOnReadyStateChange) { - originalOnReadyStateChange.apply(this, arguments as any) - } -} - -function onReadyStateChange(this: BrowserXHR) { - if (this.readyState === XMLHttpRequest.DONE) { - // Try to report the XHR as soon as possible, because the XHR may be mutated by the - // application during a future event. For example, Angular is calling .abort() on - // completed requests during a onreadystatechange event, so the status becomes '0' - // before the request is collected. - // https://github.com/angular/angular/blob/master/packages/common/http/src/xhr.ts - reportXhr.call(this) +function reportXhr(xhr: BrowserXHR) { + const xhrCompleteContext: XhrCompleteContext = { + ...xhr._datadog_xhr!, + duration: elapsed(xhr._datadog_xhr!.startClocks.timeStamp, timeStampNow()), + response: xhr.response as string | undefined, + status: xhr.status, } -} - -function reportXhr(this: BrowserXHR) { - callMonitored(() => { - const xhrCompleteContext: XhrCompleteContext = { - ...this._datadog_xhr!, - duration: elapsed(this._datadog_xhr!.startClocks.timeStamp, timeStampNow()), - response: this.response as string | undefined, - status: this.status, - } - - onRequestCompleteCallbacks.forEach((callback) => callback(xhrCompleteContext)) - // Unsubscribe to avoid being reported twice - this.removeEventListener('readystatechange', onReadyStateChange) - this.removeEventListener('loadend', reportXhr) - this.onreadystatechange = getOriginalOnReadyStateChange(this) - }) -} -function getOriginalOnReadyStateChange(xhr: BrowserXHR | BrowserXHR) { - // Check if the onreadystatechange has changed between the open() and the async onreadystatechange callback - // and get xhr.onreadystatechange instead of originalOnreadystatechange - // In the case where DD_RUM and DD_LOGS are in the page the comparison by reference miss - // therefore we check the function name to avoid recursive calls - return functionName(xhr.onreadystatechange) !== functionName(datadogOnReadyStateChangeFromProperty) - ? xhr.onreadystatechange - : xhr._datadog_xhr?.originalOnReadyStateChange ?? null + onRequestCompleteCallbacks.forEach((callback) => callback(xhrCompleteContext)) } diff --git a/packages/core/src/tools/utils.ts b/packages/core/src/tools/utils.ts index c33e56ddd8..8a0f78c8e4 100644 --- a/packages/core/src/tools/utils.ts +++ b/packages/core/src/tools/utils.ts @@ -242,17 +242,6 @@ export function mapValues(object: { [key: string]: A }, fn: (arg: A) => B) return newObject } -export function functionName(func: ((...args: any[]) => any) | null | undefined) { - if (!func) { - return - } - if (func.name) { - return func.name - } - const result = /^function\s*([^\s(]+)/.exec(func.toString()) - return result ? result[1] : undefined -} - /** * inspired by https://mathiasbynens.be/notes/globalthis */ diff --git a/rum-events-format b/rum-events-format index a37c41a4ac..8a0a90c5ff 160000 --- a/rum-events-format +++ b/rum-events-format @@ -1 +1 @@ -Subproject commit a37c41a4ac1aa3bfdc8d1fcecb35e4d1e07adddc +Subproject commit 8a0a90c5ffa020b56b62d89c78dc70420981c712 diff --git a/test/e2e/scenario/rum/resources.scenario.ts b/test/e2e/scenario/rum/resources.scenario.ts index db01456e86..a6025d91db 100644 --- a/test/e2e/scenario/rum/resources.scenario.ts +++ b/test/e2e/scenario/rum/resources.scenario.ts @@ -203,30 +203,7 @@ describe('rum resources', () => { }) describe('support XHRs with same XMLHttpRequest instance', () => { - createTest('track only second XHR (first is canceled by the browser)') - .withRum() - .withSetup(bundleSetup) - .run(async ({ events }) => { - await browserExecuteAsync((done) => { - const xhr = new XMLHttpRequest() - xhr.open('GET', '/ok?duration=200&call=1') - xhr.send() - xhr.open('GET', '/ok?duration=100&call=2') - xhr.send() - setTimeout(() => { - done(undefined) - }, 200) - }) - - await flushEvents() - - const resourceEvents = events.rumResources.filter((event) => event.resource.type === 'xhr') - expect(resourceEvents.length).toEqual(1) - expect(events.rumErrors.length).toBe(0) - expect(resourceEvents[0]?.resource.status_code).toBe(200) - }) - - createTest('track both XHRs when the second wait for the respond of the first') + createTest('track XHRs when calling requests one after another') .withRum() .withSetup(bundleSetup) .run(async ({ events }) => { From 3bdd0a03c5dd525de92f5f1302deb1f611ccea09 Mon Sep 17 00:00:00 2001 From: Aymeric Mortemousque Date: Tue, 8 Jun 2021 10:10:37 +0200 Subject: [PATCH 16/20] =?UTF-8?q?=F0=9F=91=8C=20Fix=20xhr=20tests?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- packages/core/src/browser/xhrProxy.spec.ts | 1 - packages/core/test/specHelper.ts | 75 ++++++++++------------ 2 files changed, 35 insertions(+), 41 deletions(-) diff --git a/packages/core/src/browser/xhrProxy.spec.ts b/packages/core/src/browser/xhrProxy.spec.ts index fc9f0a028e..5e3b9b40e6 100644 --- a/packages/core/src/browser/xhrProxy.spec.ts +++ b/packages/core/src/browser/xhrProxy.spec.ts @@ -131,7 +131,6 @@ describe('xhr proxy', () => { expect(request.isAborted).toBe(false) expect(xhr.status).toBe(0) expect(xhr.onreadystatechange).toHaveBeenCalledTimes(1) - expect() done() }, }) diff --git a/packages/core/test/specHelper.ts b/packages/core/test/specHelper.ts index 7b05a6846e..a5758274fb 100644 --- a/packages/core/test/specHelper.ts +++ b/packages/core/test/specHelper.ts @@ -128,21 +128,47 @@ export interface FetchStubPromise extends Promise { rejectWith: (error: Error) => void } -class StubXhr { +class StubEventEmitter { + public listeners: { [k: string]: Array<() => void> } = {} + + addEventListener(name: string, callback: () => void) { + if (!this.listeners[name]) { + this.listeners[name] = [] + } + + this.listeners[name].push(callback) + } + + removeEventListener(name: string, callback: () => void) { + if (!this.listeners[name]) { + throw new Error(`Can't remove a listener. Event "${name}" doesn't exits.`) + } + + this.listeners[name] = this.listeners[name].filter((listener) => listener !== callback) + } + + protected dispatchEvent(name: string) { + if (!this.listeners[name]) { + return + } + this.listeners[name].forEach((listener) => listener.call(this)) + } +} + +class StubXhr extends StubEventEmitter { public response: string | undefined = undefined public status: number | undefined = undefined public readyState: number = XMLHttpRequest.UNSENT public onreadystatechange: () => void = noop - public listeners: { [k: string]: Array<() => void> } = {} - private isOnreadystatechangeAttributeCallFirst = true + private hasEnded = false /* eslint-disable @typescript-eslint/no-empty-function,@typescript-eslint/no-unused-vars */ open(method: string, url: string) { this.hasEnded = false } + send() {} - /* eslint-enable @typescript-eslint/no-empty-function,@typescript-eslint/no-unused-vars */ abort() { this.status = 0 @@ -152,7 +178,7 @@ class StubXhr { } this.hasEnded = true this.readyState = XMLHttpRequest.DONE - this.dispatchEvent('readystatechange') + this.onreadystatechange() this.dispatchEvent('abort') this.dispatchEvent('loadend') } @@ -166,13 +192,7 @@ class StubXhr { this.status = status this.readyState = XMLHttpRequest.DONE - if (this.isOnreadystatechangeAttributeCallFirst) { - this.onreadystatechange() - this.dispatchEvent('readystatechange') - } else { - this.dispatchEvent('readystatechange') - this.onreadystatechange() - } + this.onreadystatechange() if (status >= 200 && status < 500) { this.dispatchEvent('load') @@ -182,31 +202,6 @@ class StubXhr { } this.dispatchEvent('loadend') } - - addEventListener(name: string, callback: () => void) { - if (!this.listeners[name]) { - this.listeners[name] = [] - } - if (name === 'readystatechange' && this.onreadystatechange === noop) { - this.isOnreadystatechangeAttributeCallFirst = false - } - this.listeners[name].push(callback) - } - - removeEventListener(name: string, callback: () => void) { - if (!this.listeners[name]) { - throw new Error(`Can't remove a listener. Event "${name}" doesn't exits.`) - } - - this.listeners[name] = this.listeners[name].filter((listener) => listener !== callback) - } - - private dispatchEvent(name: string) { - if (!this.listeners[name]) { - return - } - this.listeners[name].forEach((listener) => listener.call(this)) - } } export function createNewEvent(eventName: string, properties: { [name: string]: unknown } = {}) { @@ -250,15 +245,15 @@ export function withXhr({ completionMode?: 'manual' | 'automatic' }) { const xhr = new XMLHttpRequest() - const complete = (xhr: StubXhr | XMLHttpRequest) => { + const complete = () => { setTimeout(() => { - onComplete((xhr as unknown) as XMLHttpRequest) + onComplete(xhr) }) } if (completionMode === 'automatic') { const loadendHandler = () => { xhr.removeEventListener('loadend', loadendHandler) - complete(xhr) + complete() } xhr.addEventListener('loadend', loadendHandler) } From f0d1d2d5dcb767c8430e90c609e6f2bd6efbc8ae Mon Sep 17 00:00:00 2001 From: Aymeric Mortemousque Date: Tue, 8 Jun 2021 10:59:12 +0200 Subject: [PATCH 17/20] =?UTF-8?q?=F0=9F=91=8C=20Ensure=20to=20call=20overr?= =?UTF-8?q?idden=20onreadystatechange?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- packages/core/src/browser/xhrProxy.spec.ts | 10 ++++++---- packages/core/src/browser/xhrProxy.ts | 5 ++++- test/e2e/scenario/rum/resources.scenario.ts | 5 ++++- 3 files changed, 14 insertions(+), 6 deletions(-) diff --git a/packages/core/src/browser/xhrProxy.spec.ts b/packages/core/src/browser/xhrProxy.spec.ts index 5e3b9b40e6..7e078888d1 100644 --- a/packages/core/src/browser/xhrProxy.spec.ts +++ b/packages/core/src/browser/xhrProxy.spec.ts @@ -161,12 +161,12 @@ describe('xhr proxy', () => { it('should track request with onreadystatechange overridden before open', (done) => { withXhr({ setup(xhr) { - xhr.onreadystatechange = () => undefined + xhr.onreadystatechange = jasmine.createSpy() xhr.open('GET', '/ok') xhr.send() xhr.complete(200, 'ok') }, - onComplete() { + onComplete(xhr) { const request = getRequest(0) expect(request.method).toBe('GET') expect(request.url).toContain('/ok') @@ -175,6 +175,7 @@ describe('xhr proxy', () => { expect(request.isAborted).toBe(false) expect(request.startTime).toEqual(jasmine.any(Number)) expect(request.duration).toEqual(jasmine.any(Number)) + expect(xhr.onreadystatechange).toHaveBeenCalled() done() }, }) @@ -185,10 +186,10 @@ describe('xhr proxy', () => { setup(xhr) { xhr.open('GET', '/ok') xhr.send() - xhr.onreadystatechange = () => undefined + xhr.onreadystatechange = jasmine.createSpy() xhr.complete(200, 'ok') }, - onComplete() { + onComplete(xhr) { const request = getRequest(0) expect(request.method).toBe('GET') expect(request.url).toContain('/ok') @@ -197,6 +198,7 @@ describe('xhr proxy', () => { expect(request.isAborted).toBe(false) expect(request.startTime).toEqual(jasmine.any(Number)) expect(request.duration).toEqual(jasmine.any(Number)) + expect(xhr.onreadystatechange).toHaveBeenCalled() done() }, }) diff --git a/packages/core/src/browser/xhrProxy.ts b/packages/core/src/browser/xhrProxy.ts index 55c317aea1..90306c0837 100644 --- a/packages/core/src/browser/xhrProxy.ts +++ b/packages/core/src/browser/xhrProxy.ts @@ -123,7 +123,10 @@ function sendXhr(this: BrowserXHR) { const onEnd = monitor(() => { this.removeEventListener('loadend', onEnd) - this.onreadystatechange = originalOnreadystatechange + // if the onreadystatechange hasn't been overridden by the user after the send() + if (this.onreadystatechange === onreadystatechange) { + this.onreadystatechange = originalOnreadystatechange + } if (hasBeenReported) { return } diff --git a/test/e2e/scenario/rum/resources.scenario.ts b/test/e2e/scenario/rum/resources.scenario.ts index a6025d91db..6cece11a69 100644 --- a/test/e2e/scenario/rum/resources.scenario.ts +++ b/test/e2e/scenario/rum/resources.scenario.ts @@ -225,7 +225,10 @@ describe('rum resources', () => { const resourceEvents = events.rumResources.filter((event) => event.resource.type === 'xhr') expect(resourceEvents.length).toEqual(2) expect(events.rumErrors.length).toBe(0) - expect(resourceEvents.map((resourceEvent) => resourceEvent.resource.status_code)).toEqual([200, 200]) + expect(resourceEvents[0].resource.url).toContain('/ok?duration=100&call=1') + expect(resourceEvents[0].resource.status_code).toEqual(200) + expect(resourceEvents[1].resource.url).toContain('/ok?duration=100&call=2') + expect(resourceEvents[1].resource.status_code).toEqual(200) }) }) }) From 20bc6343148fa5894daec25efd7e78caa1ad71e3 Mon Sep 17 00:00:00 2001 From: Aymeric Mortemousque Date: Tue, 8 Jun 2021 20:44:07 +0200 Subject: [PATCH 18/20] =?UTF-8?q?=F0=9F=91=8C=20Remove=20manual=20xhr=20te?= =?UTF-8?q?st?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- packages/core/src/browser/xhrProxy.spec.ts | 8 ++++---- packages/core/test/specHelper.ts | 18 +++++------------- 2 files changed, 9 insertions(+), 17 deletions(-) diff --git a/packages/core/src/browser/xhrProxy.spec.ts b/packages/core/src/browser/xhrProxy.spec.ts index 7e078888d1..b7acb85e29 100644 --- a/packages/core/src/browser/xhrProxy.spec.ts +++ b/packages/core/src/browser/xhrProxy.spec.ts @@ -241,16 +241,14 @@ describe('xhr proxy', () => { it('should track multiple requests with the same xhr instance', (done) => { let listeners: { [k: string]: Array<() => void> } withXhr({ - completionMode: 'manual', - setup(xhr, complete) { + setup(xhr) { const secondOnload = () => { xhr.removeEventListener('load', secondOnload) - complete(xhr) } const onLoad = () => { xhr.removeEventListener('load', onLoad) xhr.addEventListener('load', secondOnload) - xhr.open('GET', '/ok?request=1') + xhr.open('GET', '/ok?request=2') xhr.send() xhr.complete(400, 'ok') } @@ -264,6 +262,7 @@ describe('xhr proxy', () => { onComplete(xhr) { const firstRequest = getRequest(0) expect(firstRequest.method).toBe('GET') + expect(firstRequest.url).toContain('/ok?request=1') expect(firstRequest.status).toBe(200) expect(firstRequest.isAborted).toBe(false) expect(firstRequest.startTime).toEqual(jasmine.any(Number)) @@ -271,6 +270,7 @@ describe('xhr proxy', () => { const secondRequest = getRequest(1) expect(secondRequest.method).toBe('GET') + expect(secondRequest.url).toContain('/ok?request=2') expect(secondRequest.status).toBe(400) expect(secondRequest.isAborted).toBe(false) expect(secondRequest.startTime).toEqual(jasmine.any(Number)) diff --git a/packages/core/test/specHelper.ts b/packages/core/test/specHelper.ts index a5758274fb..b6946bc6c5 100644 --- a/packages/core/test/specHelper.ts +++ b/packages/core/test/specHelper.ts @@ -238,27 +238,19 @@ export function stubXhr() { export function withXhr({ setup, onComplete, - completionMode = 'automatic', }: { - setup: (xhr: StubXhr, complete: (xhr: StubXhr) => void) => void + setup: (xhr: StubXhr) => void onComplete: (xhr: XMLHttpRequest) => void - completionMode?: 'manual' | 'automatic' }) { const xhr = new XMLHttpRequest() - const complete = () => { + const loadend = () => { + xhr.removeEventListener('loadend', loadend) setTimeout(() => { onComplete(xhr) }) } - if (completionMode === 'automatic') { - const loadendHandler = () => { - xhr.removeEventListener('loadend', loadendHandler) - complete() - } - xhr.addEventListener('loadend', loadendHandler) - } - - setup((xhr as unknown) as StubXhr, complete) + xhr.addEventListener('loadend', loadend) + setup((xhr as unknown) as StubXhr) } export function setPageVisibility(visibility: 'visible' | 'hidden') { From c365895d899b32cc6026186af098af24fded5c6c Mon Sep 17 00:00:00 2001 From: Aymeric Mortemousque Date: Tue, 8 Jun 2021 20:44:37 +0200 Subject: [PATCH 19/20] =?UTF-8?q?=F0=9F=91=8C=20Keep=20same=20=5Fdatadog?= =?UTF-8?q?=5Fxhr=20ref=20in=20sendXhr?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- packages/core/src/browser/xhrProxy.ts | 28 +++++++++++++-------------- 1 file changed, 13 insertions(+), 15 deletions(-) diff --git a/packages/core/src/browser/xhrProxy.ts b/packages/core/src/browser/xhrProxy.ts index 90306c0837..531c60e86b 100644 --- a/packages/core/src/browser/xhrProxy.ts +++ b/packages/core/src/browser/xhrProxy.ts @@ -1,8 +1,8 @@ -import { monitor, callMonitored } from '../domain/internalMonitoring' +import { callMonitored, monitor } from '../domain/internalMonitoring' import { Duration, elapsed, relativeNow, RelativeTime, ClocksState, clocksNow, timeStampNow } from '../tools/timeUtils' import { normalizeUrl } from '../tools/urlPolyfill' -interface BrowserXHR extends XMLHttpRequest { +interface BrowserXHR extends XMLHttpRequest { _datadog_xhr?: T } @@ -93,21 +93,19 @@ function openXhr(this: BrowserXHR, method: string, url: string) return originalXhrOpen.apply(this, arguments as any) } -function sendXhr(this: BrowserXHR) { +function sendXhr(this: BrowserXHR) { callMonitored(() => { if (!this._datadog_xhr) { return } - this._datadog_xhr = { - ...this._datadog_xhr, - startTime: relativeNow(), - startClocks: clocksNow(), - isAborted: false, - } + + this._datadog_xhr.startTime = relativeNow() + this._datadog_xhr.startClocks = clocksNow() + this._datadog_xhr.isAborted = false let hasBeenReported = false const originalOnreadystatechange = this.onreadystatechange - const onreadystatechange = function (this: BrowserXHR) { + const onreadystatechange = function (this: BrowserXHR) { if (this.readyState === XMLHttpRequest.DONE) { // Try to report the XHR as soon as possible, because the XHR may be mutated by the // application during a future event. For example, Angular is calling .abort() on @@ -131,7 +129,7 @@ function sendXhr(this: BrowserXHR) { return } hasBeenReported = true - reportXhr(this) + reportXhr(this, this._datadog_xhr!) }) this.onreadystatechange = onreadystatechange this.addEventListener('loadend', onEnd) @@ -142,7 +140,7 @@ function sendXhr(this: BrowserXHR) { return originalXhrSend.apply(this, arguments as any) } -function abortXhr(this: BrowserXHR) { +function abortXhr(this: BrowserXHR) { callMonitored(() => { if (this._datadog_xhr) { this._datadog_xhr.isAborted = true @@ -151,10 +149,10 @@ function abortXhr(this: BrowserXHR) { return originalXhrAbort.apply(this, arguments as any) } -function reportXhr(xhr: BrowserXHR) { +function reportXhr(xhr: BrowserXHR, pendingContext: XhrStartContext) { const xhrCompleteContext: XhrCompleteContext = { - ...xhr._datadog_xhr!, - duration: elapsed(xhr._datadog_xhr!.startClocks.timeStamp, timeStampNow()), + ...pendingContext, + duration: elapsed(pendingContext.startClocks.timeStamp, timeStampNow()), response: xhr.response as string | undefined, status: xhr.status, } From 185cb707fe8729aadc233bcc97edcaf2575ae3ec Mon Sep 17 00:00:00 2001 From: Aymeric Mortemousque Date: Wed, 9 Jun 2021 17:48:49 +0200 Subject: [PATCH 20/20] =?UTF-8?q?=F0=9F=91=8C=20Simplify=20reportXhr?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- packages/core/src/browser/xhrProxy.ts | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/packages/core/src/browser/xhrProxy.ts b/packages/core/src/browser/xhrProxy.ts index 531c60e86b..816fca5d4a 100644 --- a/packages/core/src/browser/xhrProxy.ts +++ b/packages/core/src/browser/xhrProxy.ts @@ -129,7 +129,7 @@ function sendXhr(this: BrowserXHR) { return } hasBeenReported = true - reportXhr(this, this._datadog_xhr!) + reportXhr(this as BrowserXHR) }) this.onreadystatechange = onreadystatechange this.addEventListener('loadend', onEnd) @@ -149,13 +149,10 @@ function abortXhr(this: BrowserXHR) { return originalXhrAbort.apply(this, arguments as any) } -function reportXhr(xhr: BrowserXHR, pendingContext: XhrStartContext) { - const xhrCompleteContext: XhrCompleteContext = { - ...pendingContext, - duration: elapsed(pendingContext.startClocks.timeStamp, timeStampNow()), - response: xhr.response as string | undefined, - status: xhr.status, - } +function reportXhr(xhr: BrowserXHR) { + xhr._datadog_xhr!.duration = elapsed(xhr._datadog_xhr!.startClocks.timeStamp, timeStampNow()) + xhr._datadog_xhr!.response = xhr.response as string | undefined + xhr._datadog_xhr!.status = xhr.status - onRequestCompleteCallbacks.forEach((callback) => callback(xhrCompleteContext)) + onRequestCompleteCallbacks.forEach((callback) => callback({ ...xhr._datadog_xhr! })) }