🐛[RUMF-1423] prevent unexpected behavior when our xhr are reused

packages/core/src/transport/httpRequest.spec.ts

@@ -3,7 +3,7 @@ import type { Request } from '../../test/specHelper'
 import type { EndpointBuilder } from '../domain/configuration'
 import { createEndpointBuilder } from '../domain/configuration'
 import { noop } from '../tools/utils'
-import { createHttpRequest, fetchKeepAliveStrategy } from './httpRequest'
+import { createHttpRequest, fetchKeepAliveStrategy, sendXHR } from './httpRequest'
 import type { HttpRequest } from './httpRequest'
 import { INITIAL_BACKOFF_TIME } from './sendWithRetryStrategy'
 
@@ -112,7 +112,7 @@ describe('httpRequest', () => {
         BATCH_BYTES_LIMIT,
         { data: '{"foo":"bar1"}\n{"foo":"bar2"}', bytesCount: 10 },
         (response) => {
-          expect(response).toEqual({ status: 429, api: 'fetch' })
+          expect(response).toEqual({ status: 429 })
           done()
         }
       )
@@ -135,7 +135,7 @@ describe('httpRequest', () => {
         BATCH_BYTES_LIMIT,
         { data: '{"foo":"bar1"}\n{"foo":"bar2"}', bytesCount: 10 },
         (response) => {
-          expect(response).toEqual({ status: 429, api: 'xhr' })
+          expect(response).toEqual({ status: 429 })
           done()
         }
       )
@@ -153,13 +153,42 @@ describe('httpRequest', () => {
         BATCH_BYTES_LIMIT,
         { data: '{"foo":"bar1"}\n{"foo":"bar2"}', bytesCount: BATCH_BYTES_LIMIT },
         (response) => {
-          expect(response).toEqual({ status: 429, api: 'xhr' })
+          expect(response).toEqual({ status: 429 })
           done()
         }
       )
     })
   })
 
+  describe('sendXhr', () => {
+    it('should prevent third party to trigger callback multiple times', (done) => {
+      const onResponseSpy = jasmine.createSpy('xhrOnResponse')
+      let count = 0
+
+      interceptor.withStubXhr((xhr) => {
+        count++
+        setTimeout(() => {
+          xhr.complete(count === 1 ? 200 : 202)
+          if (count === 1) {
+            // reuse the xhr instance to send another request
+            xhr.open('POST', 'foo')
+            xhr.send()
+          }
+        })
+      })
+
+      sendXHR('foo', '', onResponseSpy)
+
+      setTimeout(() => {
+        expect(onResponseSpy).toHaveBeenCalledTimes(1)
+        expect(onResponseSpy).toHaveBeenCalledWith({
+          status: 200,
+        })
+        done()
+      }, 100)
+    })
+  })
+
   describe('sendOnExit', () => {
     it('should use xhr when sendBeacon is not defined', () => {
       interceptor.withSendBeacon(false)

packages/core/src/transport/httpRequest.ts

@@ -18,7 +18,6 @@ export type HttpRequest = ReturnType<typeof createHttpRequest>
 
 export interface HttpResponse extends Context {
   status: number
-  api?: string
 }
 
 export interface Payload {
@@ -86,7 +85,7 @@ export function fetchKeepAliveStrategy(
   const canUseKeepAlive = isKeepAliveSupported() && bytesCount < bytesLimit
   if (canUseKeepAlive) {
     fetch(url, { method: 'POST', body: data, keepalive: true }).then(
-      monitor((response: Response) => onResponse?.({ status: response.status, api: 'fetch' })),
+      monitor((response: Response) => onResponse?.({ status: response.status })),
       monitor(() => {
         // failed to queue the request
         sendXHR(url, data, onResponse)
@@ -106,14 +105,15 @@ function isKeepAliveSupported() {
   }
 }
 
-function sendXHR(url: string, data: Payload['data'], onResponse?: (r: HttpResponse) => void) {
+export function sendXHR(url: string, data: Payload['data'], onResponse?: (r: HttpResponse) => void) {
   const request = new XMLHttpRequest()
+  const onLoadEnd = monitor(() => {
+    // prevent multiple onResponse callbacks
+    // if the xhr instance is reused by a third party
+    request.removeEventListener('loadend', onLoadEnd)
+    onResponse?.({ status: request.status })
+  })
   request.open('POST', url, true)
+  request.addEventListener('loadend', onLoadEnd)
   request.send(data)
-  request.addEventListener(
-    'loadend',
-    monitor(() => {
-      onResponse?.({ status: request.status, api: 'xhr' })
-    })
-  )
 }

packages/core/src/transport/sendWithRetryStrategy.ts

@@ -1,4 +1,3 @@
-import { addTelemetryDebug } from '../domain/telemetry'
 import type { EndpointType } from '../domain/configuration'
 import { monitor } from '../tools/monitor'
 import type { RawError } from '../tools/error'
@@ -70,22 +69,6 @@ function scheduleRetry(
   setTimeout(
     monitor(() => {
       const payload = state.queuedPayloads.first()
-      if (!payload) {
-        addTelemetryDebug('no payload to retry', {
-          debug: {
-            queue: {
-              size: state.queuedPayloads.size(),
-              is_full: state.queuedPayloads.isFull(),
-              bytes_count: state.queuedPayloads.bytesCount,
-            },
-            transport_status: state.transportStatus,
-            bandwidth: {
-              ongoing_request_count: state.bandwidthMonitor.ongoingRequestCount,
-              ongoing_byte_count: state.bandwidthMonitor.ongoingByteCount,
-            },
-          },
-        })
-      }
       send(payload, state, sendStrategy, {
         onSuccess: () => {
           state.queuedPayloads.dequeue()
@@ -109,16 +92,7 @@ function send(
   { onSuccess, onFailure }: { onSuccess: () => void; onFailure: () => void }
 ) {
   state.bandwidthMonitor.add(payload)
-  const receivedResponses: HttpResponse[] = []
   sendStrategy(payload, (response) => {
-    receivedResponses.push(response)
-    if (receivedResponses.length > 1) {
-      addTelemetryDebug('response already received', {
-        debug: {
-          responses: receivedResponses,
-        },
-      })
-    }
     state.bandwidthMonitor.remove(payload)
     if (!shouldRetryRequest(response)) {
       state.transportStatus = TransportStatus.UP