From b645dabc27c42d3eb0272b2300d10785205e7ae5 Mon Sep 17 00:00:00 2001 From: Benoit Zugmeyer Date: Thu, 7 May 2026 16:35:58 +0200 Subject: [PATCH 1/2] =?UTF-8?q?=F0=9F=90=9B=20fix=20race=20between=20Perfo?= =?UTF-8?q?rmanceObserver=20and=20REQUEST=5FCOMPLETED=20on=20Firefox?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When a fetch resolves, two things happen around the same time: - the browser dispatches a PerformanceResourceTiming entry to our observer - afterSend() resumes from `await responsePromise` and notifies REQUEST_COMPLETED On Firefox the observer task can fire before the resolve microtask runs, so the request isn't in the registry yet when assembleResource() looks it up, and the resource event ends up without handlingStack / requestInit / etc. Defer the registry lookup for request-type entries by REQUEST_MATCHING_DELAY (50ms) so the matching REQUEST_COMPLETED has time to land. assembleResource now takes the resolved request directly instead of pulling it from the registry. Tests drive the delay through mockClock.tick(). --- .../resource/resourceCollection.spec.ts | 11 ++++++--- .../src/domain/resource/resourceCollection.ts | 24 +++++++++++++++---- 2 files changed, 27 insertions(+), 8 deletions(-) diff --git a/packages/rum-core/src/domain/resource/resourceCollection.spec.ts b/packages/rum-core/src/domain/resource/resourceCollection.spec.ts index 80e1868848..419cd1eafc 100644 --- a/packages/rum-core/src/domain/resource/resourceCollection.spec.ts +++ b/packages/rum-core/src/domain/resource/resourceCollection.spec.ts @@ -1,7 +1,7 @@ import type { Duration, MatchOption, RelativeTime, ServerDuration, TaskQueue, TimeStamp } from '@datadog/browser-core' import { createTaskQueue, display, elapsed, RequestType, ResourceType, toServerDuration } from '@datadog/browser-core' -import type { MockTelemetry } from '@datadog/browser-core/test' -import { registerCleanupTask, replaceMockable, startMockTelemetry } from '@datadog/browser-core/test' +import type { Clock, MockTelemetry } from '@datadog/browser-core/test' +import { mockClock, registerCleanupTask, replaceMockable, startMockTelemetry } from '@datadog/browser-core/test' import { collectAndValidateRawRumEvents, createPerformanceEntry, @@ -22,7 +22,7 @@ import { LifeCycle, LifeCycleEventType } from '../lifeCycle' import type { RequestCompleteEvent } from '../requestCollection' import { getDocumentTraceId } from '../tracing/getDocumentTraceId' import { createSpanIdentifier, createTraceIdentifier } from '../tracing/identifier' -import { startResourceCollection } from './resourceCollection' +import { REQUEST_MATCHING_DELAY, startResourceCollection } from './resourceCollection' function buildMatchHeadersForAllUrls(headerNames: MatchOption[]): MatchHeader[] { return headerNames.map((name) => ({ name })) @@ -36,6 +36,7 @@ describe('resourceCollection', () => { let notifyPerformanceEntries: (entries: RumPerformanceEntry[]) => void let rawRumEvents: Array> = [] let taskQueuePushSpy: jasmine.Spy + let clock: Clock function setupResourceCollection(partialConfig: Partial = { trackResources: true }) { const { triggerOnDomLoaded } = mockDocumentReadyState() @@ -55,6 +56,7 @@ describe('resourceCollection', () => { } beforeEach(() => { + clock = mockClock() ;({ notifyPerformanceEntries } = mockPerformanceObserver()) }) @@ -1277,6 +1279,9 @@ describe('resourceCollection', () => { }) function runTasks() { + // Request-type entries are queued through a `setTimeout(…, REQUEST_MATCHING_DELAY)` before + // they reach the task queue — advance past it so they get pushed. + clock.tick(REQUEST_MATCHING_DELAY) taskQueuePushSpy.calls.allArgs().forEach(([task]) => { task() }) diff --git a/packages/rum-core/src/domain/resource/resourceCollection.ts b/packages/rum-core/src/domain/resource/resourceCollection.ts index 4034fc6f04..2c3be8fd3f 100644 --- a/packages/rum-core/src/domain/resource/resourceCollection.ts +++ b/packages/rum-core/src/domain/resource/resourceCollection.ts @@ -1,3 +1,4 @@ +import type { Duration } from '@datadog/browser-core' import { combine, generateUUID, @@ -11,6 +12,7 @@ import { display, addTelemetryDebug, RequestType, + setTimeout, } from '@datadog/browser-core' import type { MatchHeader, RumConfiguration } from '../configuration' import { RumPerformanceEntryType, createPerformanceObservable } from '../../browser/performanceObservable' @@ -36,13 +38,16 @@ import { sanitizeIfLongDataUrl, } from './resourceUtils' import type { ResourceLikeEntry } from './resourceUtils' -import type { RequestRegistry } from './requestRegistry' import { createRequestRegistry } from './requestRegistry' import type { GraphQlMetadata } from './graphql' import { extractGraphQlMetadata, findGraphQlConfiguration } from './graphql' import type { ManualResourceData } from './trackManualResources' import { trackManualResources } from './trackManualResources' +// Delay before looking up the request matching a request-type performance entry. See the call +// site in `startResourceCollection` for the rationale. +export const REQUEST_MATCHING_DELAY = 50 as Duration + export function startResourceCollection(lifeCycle: LifeCycle, configuration: RumConfiguration) { const taskQueue = mockable(createTaskQueue)() const requestRegistry = createRequestRegistry(lifeCycle) @@ -52,12 +57,22 @@ export function startResourceCollection(lifeCycle: LifeCycle, configuration: Rum buffered: true, }).subscribe((entries) => { for (const entry of entries) { - handleResource(() => assembleResource(entry, requestRegistry, configuration)) + if (isResourceEntryRequestType(entry)) { + // The PerformanceObserver callback can fire before the fetch's resolve microtask runs + // (notably on Firefox), so the matching REQUEST_COMPLETED isn't in the registry yet. + // Defer the lookup to give the request time to land before we look it up. + setTimeout( + () => handleResource(() => assembleResource(entry, requestRegistry.getMatchingRequest(entry), configuration)), + REQUEST_MATCHING_DELAY + ) + } else { + handleResource(() => assembleResource(entry, undefined, configuration)) + } } }) const { stop: stopRunOnReadyState } = runOnReadyState(configuration, 'interactive', () => { - handleResource(() => assembleResource(getNavigationEntry(), requestRegistry, configuration)) + handleResource(() => assembleResource(getNavigationEntry(), undefined, configuration)) }) function handleResource(computeRawEvent: () => RawRumEventCollectedData | undefined) { @@ -86,10 +101,9 @@ export function startResourceCollection(lifeCycle: LifeCycle, configuration: Rum function assembleResource( entry: ResourceLikeEntry, - requestRegistry: RequestRegistry, + request: RequestCompleteEvent | undefined, configuration: RumConfiguration ): RawRumEventCollectedData | undefined { - const request = isResourceEntryRequestType(entry) ? requestRegistry.getMatchingRequest(entry) : undefined const tracingInfo = request ? computeRequestTracingInfo(request, configuration) : computeResourceEntryTracingInfo(entry, configuration) From 7636e786815745f416247dc315cfcaceb7ac2efa Mon Sep 17 00:00:00 2001 From: Benoit Zugmeyer Date: Mon, 11 May 2026 11:50:42 +0200 Subject: [PATCH 2/2] add comment about clearTimeout --- packages/rum-core/src/domain/resource/resourceCollection.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/packages/rum-core/src/domain/resource/resourceCollection.ts b/packages/rum-core/src/domain/resource/resourceCollection.ts index 2c3be8fd3f..994aa24f70 100644 --- a/packages/rum-core/src/domain/resource/resourceCollection.ts +++ b/packages/rum-core/src/domain/resource/resourceCollection.ts @@ -61,6 +61,9 @@ export function startResourceCollection(lifeCycle: LifeCycle, configuration: Rum // The PerformanceObserver callback can fire before the fetch's resolve microtask runs // (notably on Firefox), so the matching REQUEST_COMPLETED isn't in the registry yet. // Defer the lookup to give the request time to land before we look it up. + // + // Note: we could clear the timeout on stop(), but this requires a bit of bookkeeping that + // is not necessary right now. We could reevaluate in the future. setTimeout( () => handleResource(() => assembleResource(entry, requestRegistry.getMatchingRequest(entry), configuration)), REQUEST_MATCHING_DELAY