From 37adee8ce0f3ab68dabfaf1eab6d5784e749a1bd Mon Sep 17 00:00:00 2001 From: Marco Saia Date: Mon, 23 Mar 2026 16:03:41 +0100 Subject: [PATCH 1/3] RUM-15238: fix RUM events associated to prior view --- .../DatadogProvider/Buffer/BufferSingleton.ts | 11 +- .../Buffer/NavigationBuffer.ts | 256 +++++++++++++ .../Buffer/__tests__/BufferSingleton.test.ts | 51 +++ .../Buffer/__tests__/NavigationBuffer.test.ts | 353 ++++++++++++++++++ .../DdRumReactNavigationTracking.test.tsx | 247 +++++++++++- .../DdRumReactNavigationTracking.tsx | 217 ++++++++++- 6 files changed, 1118 insertions(+), 17 deletions(-) create mode 100644 packages/core/src/sdk/DatadogProvider/Buffer/NavigationBuffer.ts create mode 100644 packages/core/src/sdk/DatadogProvider/Buffer/__tests__/NavigationBuffer.test.ts diff --git a/packages/core/src/sdk/DatadogProvider/Buffer/BufferSingleton.ts b/packages/core/src/sdk/DatadogProvider/Buffer/BufferSingleton.ts index 7c84fb910..513fe6a4f 100644 --- a/packages/core/src/sdk/DatadogProvider/Buffer/BufferSingleton.ts +++ b/packages/core/src/sdk/DatadogProvider/Buffer/BufferSingleton.ts @@ -8,23 +8,32 @@ import { getGlobalInstance } from '../../../utils/singletonUtils'; import { BoundedBuffer } from './BoundedBuffer'; import type { DatadogBuffer } from './DatadogBuffer'; +import { NavigationBuffer } from './NavigationBuffer'; import { PassThroughBuffer } from './PassThroughBuffer'; +// IMPORTANT: Keep this key aligned with the react-navigation package const BUFFER_SINGLETON_MODULE = 'com.datadog.reactnative.buffer_singleton'; class _BufferSingleton { private bufferInstance: DatadogBuffer = new BoundedBuffer(); + private navigationBuffer: NavigationBuffer | null = null; getInstance = (): DatadogBuffer => { return BufferSingleton.bufferInstance; }; + getNavigationBuffer = (): NavigationBuffer | null => { + return this.navigationBuffer; + }; + onInitialization = () => { this.bufferInstance.drain(); - this.bufferInstance = new PassThroughBuffer(); + this.navigationBuffer = new NavigationBuffer(new PassThroughBuffer()); + this.bufferInstance = this.navigationBuffer; }; reset = () => { + this.navigationBuffer = null; BufferSingleton.bufferInstance = new BoundedBuffer(); }; } diff --git a/packages/core/src/sdk/DatadogProvider/Buffer/NavigationBuffer.ts b/packages/core/src/sdk/DatadogProvider/Buffer/NavigationBuffer.ts new file mode 100644 index 000000000..a401aaa3a --- /dev/null +++ b/packages/core/src/sdk/DatadogProvider/Buffer/NavigationBuffer.ts @@ -0,0 +1,256 @@ +/* + * Unless explicitly stated otherwise all files in this repository are licensed under the Apache License Version 2.0. + * This product includes software developed at Datadog (https://www.datadoghq.com/). + * Copyright 2016-Present Datadog, Inc. + */ + +import { DatadogBuffer } from './DatadogBuffer'; + +/** + * Safety timeout (ms): auto-drains the buffer if `onStateChange` never fires + * after a navigation dispatch. + */ +export const NAVIGATION_BUFFER_TIMEOUT_MS = 500; + +// TODO: DEBUG LOGGING — remove before shipping +const LOG = (msg: string, ...args: any[]) => + // eslint-disable-next-line no-console + console.log(`[DD NavBuffer] ${new Date().toISOString()} ${msg}`, ...args); + +/** + * An internal `DatadogBuffer` decorator that queues RUM events during a + * navigation transition and flushes them once the new view is confirmed. + * + * **IMPORTANT** + * Any changes to the public methods of this class must be reflected in + * the interface definition of the react-navigation package. + * + * **Lifecycle** + * 1. `startNavigation()` — called when a navigation action is dispatched + * (via the `__unsafe_action__` listener). Starts buffering all incoming + * RUM events and records `navigationStartTime` so the view-start can be + * backdated to the moment the user triggered the navigation. + * A safety timeout (`NAVIGATION_BUFFER_TIMEOUT_MS`) automatically calls + * `endNavigation()` if the state-change callback never fires. + * 2. `prepareEndNavigation()` — called just before `DdRum.startView()`. + * Stops accepting new events into the queue (so `startView` itself passes + * through immediately) but keeps the queue intact and preserves + * `navigationStartTime` for the caller to read. + * 3. `flush()` — called after `startView()` resolves. Drains queued events + * to the inner buffer so they are attributed to the new view. + * 4. `endNavigation()` — stop-and-drain shortcut used by the safety timeout, + * teardown (`stopTrackingViews`), and any path where no `startView` fires + * (background state, predicate returning false, undefined route, etc.). + * + * **Integration point** + * `BufferSingleton.onInitialization()` installs a `NavigationBuffer` wrapping + * a `PassThroughBuffer` as the active SDK buffer. The react-navigation package + * accesses it via `getGlobalInstance` using the shared + * `'com.datadog.reactnative.buffer_singleton'` key — no public export needed. + * + * @internal + */ +export class NavigationBuffer extends DatadogBuffer { + private innerBuffer: DatadogBuffer; + private isNavigating = false; + private callbackQueue: Array<() => void> = []; + private timeoutId: ReturnType | null = null; + private _navigationStartTime: number | null = null; + + /** + * The timestamp (ms since epoch) captured when startNavigation() was called. + * Use this as the timestampMs for DdRum.startView() so the view start reflects + * when the user initiated navigation, not when onStateChange fired. + * Null when no navigation is in progress. + */ + get navigationStartTime(): number | null { + return this._navigationStartTime; + } + + constructor(innerBuffer: DatadogBuffer) { + super(); + this.innerBuffer = innerBuffer; + LOG('constructed', { innerBuffer: innerBuffer.constructor.name }); + } + + addCallback = (callback: () => Promise): Promise => { + if (!this.isNavigating) { + LOG('addCallback → passthrough'); + return this.innerBuffer.addCallback(callback); + } + LOG( + 'addCallback → QUEUED, queueLength now', + this.callbackQueue.length + 1 + ); + this.callbackQueue.push(() => { + this.innerBuffer.addCallback(callback); + }); + return Promise.resolve(); + }; + + addCallbackReturningId = ( + callback: () => Promise + ): Promise => { + if (!this.isNavigating) { + LOG('addCallbackReturningId → passthrough'); + return this.innerBuffer.addCallbackReturningId(callback); + } + LOG( + 'addCallbackReturningId → QUEUED, queueLength now', + this.callbackQueue.length + 1 + ); + return new Promise(resolve => { + this.callbackQueue.push(() => { + this.innerBuffer.addCallbackReturningId(callback).then(resolve); + }); + }); + }; + + addCallbackWithId = ( + callback: (id: string) => Promise, + id: string + ): Promise => { + if (!this.isNavigating) { + LOG('addCallbackWithId → passthrough, id:', id); + return this.innerBuffer.addCallbackWithId(callback, id); + } + LOG( + 'addCallbackWithId → QUEUED, id:', + id, + 'queueLength now', + this.callbackQueue.length + 1 + ); + return new Promise(resolve => { + this.callbackQueue.push(() => { + this.innerBuffer.addCallbackWithId(callback, id).then(resolve); + }); + }); + }; + + drain = (): void => { + LOG( + 'drain() called, queueLength:', + this.callbackQueue.length, + 'isNavigating:', + this.isNavigating + ); + this.flushQueue(); + this.innerBuffer.drain(); + }; + + startNavigation = (): void => { + const wasAlreadyNavigating = this.isNavigating; + if (this.timeoutId !== null) { + clearTimeout(this.timeoutId); + } + // Only capture the start time on the first navigation start; preserve it + // across rapid re-navigations so the timestamp reflects the original intent. + if (!wasAlreadyNavigating) { + this._navigationStartTime = Date.now(); + } + this.isNavigating = true; + this.timeoutId = setTimeout(() => { + LOG( + `timeout fired after ${NAVIGATION_BUFFER_TIMEOUT_MS}ms — calling endNavigation` + ); + this.endNavigation(); + }, NAVIGATION_BUFFER_TIMEOUT_MS); + LOG('startNavigation()', { + wasAlreadyNavigating, + navigationStartTime: this._navigationStartTime, + queueLength: this.callbackQueue.length, + timeoutMs: NAVIGATION_BUFFER_TIMEOUT_MS + }); + }; + + /** + * Stop accepting new events into the buffer and cancel any pending timeout, + * WITHOUT draining the queue. Use this before calling DdRum.startView() so + * that startView() itself passes through immediately. Then call flush() after + * startView resolves to send queued events to the now-active view. + * + * Contrast with endNavigation(), which stops AND drains immediately (used by + * timeout auto-drain and teardown paths). + */ + prepareEndNavigation = (): void => { + if (this.timeoutId !== null) { + clearTimeout(this.timeoutId); + this.timeoutId = null; + } + this.isNavigating = false; + LOG( + 'prepareEndNavigation() — stopped buffering, queue preserved, navigationStartTime:', + this._navigationStartTime, + 'queueLength:', + this.callbackQueue.length + ); + // Note: _navigationStartTime is intentionally kept until flush() so the + // caller can still read it after prepareEndNavigation() returns. + }; + + /** + * Drain the queued events to the inner buffer. Call this after startView() + * resolves to flush buffered events to the new view. + * + * Safe to call when the queue is empty (no-op). + */ + flush = (): void => { + const now = Date.now(); + const lag = + this._navigationStartTime !== null + ? now - this._navigationStartTime + : null; + LOG( + 'flush() called — draining', + this.callbackQueue.length, + 'queued events | navigationStartTime:', + this._navigationStartTime, + '| now:', + now, + '| lag since nav start:', + lag !== null ? `${lag}ms` : 'n/a' + ); + this._navigationStartTime = null; + this.flushQueue(); + }; + + /** + * Stop buffering and drain the queue immediately. Used by: + * - Timeout auto-drain (navigation never completed) + * - Teardown (stopTrackingViews) + */ + endNavigation = (): void => { + if (this.timeoutId !== null) { + clearTimeout(this.timeoutId); + this.timeoutId = null; + } + this.isNavigating = false; + const now = Date.now(); + const lag = + this._navigationStartTime !== null + ? now - this._navigationStartTime + : null; + LOG( + 'endNavigation() — draining', + this.callbackQueue.length, + 'queued events | navigationStartTime:', + this._navigationStartTime, + '| now:', + now, + '| lag since nav start:', + lag !== null ? `${lag}ms` : 'n/a' + ); + this._navigationStartTime = null; + this.flushQueue(); + LOG('endNavigation() done'); + }; + + private flushQueue = (): void => { + const pending = this.callbackQueue; + this.callbackQueue = []; + LOG('flushQueue() executing', pending.length, 'queued callbacks'); + for (const callback of pending) { + callback(); + } + }; +} diff --git a/packages/core/src/sdk/DatadogProvider/Buffer/__tests__/BufferSingleton.test.ts b/packages/core/src/sdk/DatadogProvider/Buffer/__tests__/BufferSingleton.test.ts index 764daed54..c19e16bcd 100644 --- a/packages/core/src/sdk/DatadogProvider/Buffer/__tests__/BufferSingleton.test.ts +++ b/packages/core/src/sdk/DatadogProvider/Buffer/__tests__/BufferSingleton.test.ts @@ -5,6 +5,7 @@ */ import { BufferSingleton } from '../BufferSingleton'; +import { NavigationBuffer } from '../NavigationBuffer'; const flushPromises = () => new Promise(jest.requireActual('timers').setImmediate); @@ -48,4 +49,54 @@ describe('BufferSingleton', () => { expect(callbackWithId).toHaveBeenCalledWith('callbackId'); }); }); + + describe('NavigationBuffer wiring', () => { + afterEach(() => { + BufferSingleton.reset(); + }); + + it('getNavigationBuffer returns null before initialization', () => { + expect(BufferSingleton.getNavigationBuffer()).toBeNull(); + }); + + it('getNavigationBuffer returns NavigationBuffer after initialization', () => { + BufferSingleton.onInitialization(); + const navBuffer = BufferSingleton.getNavigationBuffer(); + expect(navBuffer).toBeInstanceOf(NavigationBuffer); + }); + + it('getInstance returns the NavigationBuffer after initialization', () => { + BufferSingleton.onInitialization(); + const instance = BufferSingleton.getInstance(); + expect(instance).toBeInstanceOf(NavigationBuffer); + }); + + it('NavigationBuffer passes through callbacks after initialization (not navigating)', async () => { + BufferSingleton.onInitialization(); + const cb = jest.fn().mockResolvedValue(undefined); + BufferSingleton.getInstance().addCallback(cb); + expect(cb).toHaveBeenCalledTimes(1); + }); + + it('NavigationBuffer holds callbacks during navigation after initialization', async () => { + BufferSingleton.onInitialization(); + const navBuffer = BufferSingleton.getNavigationBuffer()!; + const cb = jest.fn().mockResolvedValue(undefined); + + navBuffer.startNavigation(); + BufferSingleton.getInstance().addCallback(cb); + expect(cb).not.toHaveBeenCalled(); + + navBuffer.endNavigation(); + expect(cb).toHaveBeenCalledTimes(1); + }); + + it('reset clears navigationBuffer reference', () => { + BufferSingleton.onInitialization(); + expect(BufferSingleton.getNavigationBuffer()).not.toBeNull(); + + BufferSingleton.reset(); + expect(BufferSingleton.getNavigationBuffer()).toBeNull(); + }); + }); }); diff --git a/packages/core/src/sdk/DatadogProvider/Buffer/__tests__/NavigationBuffer.test.ts b/packages/core/src/sdk/DatadogProvider/Buffer/__tests__/NavigationBuffer.test.ts new file mode 100644 index 000000000..d812b4448 --- /dev/null +++ b/packages/core/src/sdk/DatadogProvider/Buffer/__tests__/NavigationBuffer.test.ts @@ -0,0 +1,353 @@ +/* + * Unless explicitly stated otherwise all files in this repository are licensed under the Apache License Version 2.0. + * This product includes software developed at Datadog (https://www.datadoghq.com/). + * Copyright 2016-Present Datadog, Inc. + */ + +import { + NavigationBuffer, + NAVIGATION_BUFFER_TIMEOUT_MS +} from '../NavigationBuffer'; +import { PassThroughBuffer } from '../PassThroughBuffer'; + +const flushPromises = () => + new Promise(jest.requireActual('timers').setImmediate); + +describe('NavigationBuffer', () => { + describe('passthrough when not navigating', () => { + it('forwards addCallback to inner buffer immediately', async () => { + const buffer = new NavigationBuffer(new PassThroughBuffer()); + const cb = jest.fn().mockResolvedValue(undefined); + + await buffer.addCallback(cb); + + expect(cb).toHaveBeenCalledTimes(1); + }); + + it('forwards addCallbackReturningId to inner buffer immediately', async () => { + const buffer = new NavigationBuffer(new PassThroughBuffer()); + + const result = await buffer.addCallbackReturningId(() => + Promise.resolve('realId') + ); + + expect(result).toBe('realId'); + }); + + it('forwards addCallbackWithId to inner buffer immediately', async () => { + const buffer = new NavigationBuffer(new PassThroughBuffer()); + const cb = jest.fn().mockResolvedValue(undefined); + + await buffer.addCallbackWithId(cb, 'someId'); + + expect(cb).toHaveBeenCalledWith('someId'); + }); + }); + + describe('holds events during navigation', () => { + it('queues addCallback during navigation and drains on endNavigation', async () => { + const buffer = new NavigationBuffer(new PassThroughBuffer()); + const cb = jest.fn().mockResolvedValue(undefined); + + buffer.startNavigation(); + buffer.addCallback(cb); + expect(cb).not.toHaveBeenCalled(); + + buffer.endNavigation(); + await flushPromises(); + expect(cb).toHaveBeenCalledTimes(1); + }); + + it('queues addCallbackReturningId during navigation', async () => { + const buffer = new NavigationBuffer(new PassThroughBuffer()); + let resolved = false; + + buffer.startNavigation(); + const idPromise = buffer.addCallbackReturningId(() => + Promise.resolve('nativeId') + ); + idPromise.then(() => { + resolved = true; + }); + + await flushPromises(); + expect(resolved).toBe(false); + + buffer.endNavigation(); + await flushPromises(); + + const id = await idPromise; + expect(id).toBe('nativeId'); + expect(resolved).toBe(true); + }); + + it('queues addCallbackWithId during navigation', async () => { + const buffer = new NavigationBuffer(new PassThroughBuffer()); + const cb = jest.fn().mockResolvedValue(undefined); + + buffer.startNavigation(); + buffer.addCallbackWithId(cb, 'testId'); + expect(cb).not.toHaveBeenCalled(); + + buffer.endNavigation(); + await flushPromises(); + expect(cb).toHaveBeenCalledWith('testId'); + }); + }); + + describe('endNavigation', () => { + it('drains callbacks in FIFO order', async () => { + const buffer = new NavigationBuffer(new PassThroughBuffer()); + const order: number[] = []; + + buffer.startNavigation(); + buffer.addCallback(async () => { + order.push(1); + }); + buffer.addCallback(async () => { + order.push(2); + }); + buffer.addCallback(async () => { + order.push(3); + }); + + buffer.endNavigation(); + await flushPromises(); + + expect(order).toEqual([1, 2, 3]); + }); + + it('is a no-op when not navigating', () => { + const buffer = new NavigationBuffer(new PassThroughBuffer()); + + expect(() => buffer.endNavigation()).not.toThrow(); + }); + }); + + describe('timeout auto-drain', () => { + beforeEach(() => { + jest.useFakeTimers(); + }); + + afterEach(() => { + jest.useRealTimers(); + }); + + it('auto-drains after NAVIGATION_BUFFER_TIMEOUT_MS', async () => { + const buffer = new NavigationBuffer(new PassThroughBuffer()); + const cb = jest.fn().mockResolvedValue(undefined); + + buffer.startNavigation(); + buffer.addCallback(cb); + + jest.advanceTimersByTime(NAVIGATION_BUFFER_TIMEOUT_MS); + await flushPromises(); + + expect(cb).toHaveBeenCalledTimes(1); + }); + + it('does not double-drain if endNavigation called before timeout', async () => { + const buffer = new NavigationBuffer(new PassThroughBuffer()); + const cb = jest.fn().mockResolvedValue(undefined); + + buffer.startNavigation(); + buffer.addCallback(cb); + + buffer.endNavigation(); + await flushPromises(); + expect(cb).toHaveBeenCalledTimes(1); + + jest.advanceTimersByTime(NAVIGATION_BUFFER_TIMEOUT_MS); + await flushPromises(); + expect(cb).toHaveBeenCalledTimes(1); + }); + }); + + describe('rapid navigation', () => { + beforeEach(() => { + jest.useFakeTimers(); + }); + + afterEach(() => { + jest.useRealTimers(); + }); + + it('resets timeout on second startNavigation, preserves queue', async () => { + const buffer = new NavigationBuffer(new PassThroughBuffer()); + const cb1 = jest.fn().mockResolvedValue(undefined); + const cb2 = jest.fn().mockResolvedValue(undefined); + + buffer.startNavigation(); + buffer.addCallback(cb1); + + jest.advanceTimersByTime(400); + + buffer.startNavigation(); + buffer.addCallback(cb2); + + jest.advanceTimersByTime(400); + await flushPromises(); + expect(cb1).not.toHaveBeenCalled(); + expect(cb2).not.toHaveBeenCalled(); + + jest.advanceTimersByTime(100); + await flushPromises(); + expect(cb1).toHaveBeenCalledTimes(1); + expect(cb2).toHaveBeenCalledTimes(1); + }); + + it('drains all events from multiple navigation windows on endNavigation', async () => { + const buffer = new NavigationBuffer(new PassThroughBuffer()); + const cb1 = jest.fn().mockResolvedValue(undefined); + const cb2 = jest.fn().mockResolvedValue(undefined); + + buffer.startNavigation(); + buffer.addCallback(cb1); + + buffer.startNavigation(); + buffer.addCallback(cb2); + + buffer.endNavigation(); + await flushPromises(); + + expect(cb1).toHaveBeenCalledTimes(1); + expect(cb2).toHaveBeenCalledTimes(1); + }); + }); + + describe('drain', () => { + it('flushes navigation queue when called directly', async () => { + const buffer = new NavigationBuffer(new PassThroughBuffer()); + const cb = jest.fn().mockResolvedValue(undefined); + + buffer.startNavigation(); + buffer.addCallback(cb); + + buffer.drain(); + await flushPromises(); + + expect(cb).toHaveBeenCalledTimes(1); + }); + }); + + describe('prepareEndNavigation + flush (two-phase pattern)', () => { + it('prepareEndNavigation stops buffering without draining the queue', async () => { + const buffer = new NavigationBuffer(new PassThroughBuffer()); + const cb = jest.fn().mockResolvedValue(undefined); + + buffer.startNavigation(); + buffer.addCallback(cb); + + buffer.prepareEndNavigation(); + await flushPromises(); + + // Queue is not drained yet + expect(cb).not.toHaveBeenCalled(); + }); + + it('flush drains the queue after prepareEndNavigation', async () => { + const buffer = new NavigationBuffer(new PassThroughBuffer()); + const cb = jest.fn().mockResolvedValue(undefined); + + buffer.startNavigation(); + buffer.addCallback(cb); + + buffer.prepareEndNavigation(); + buffer.flush(); + await flushPromises(); + + expect(cb).toHaveBeenCalledTimes(1); + }); + + it('callbacks added after prepareEndNavigation pass through immediately (not buffered)', async () => { + const buffer = new NavigationBuffer(new PassThroughBuffer()); + const cb = jest.fn().mockResolvedValue(undefined); + + buffer.startNavigation(); + buffer.prepareEndNavigation(); + + // This simulates DdRum.startView() — must pass through, not be queued + await buffer.addCallback(cb); + + expect(cb).toHaveBeenCalledTimes(1); + }); + + it('flush after prepareEndNavigation drains in FIFO order', async () => { + const buffer = new NavigationBuffer(new PassThroughBuffer()); + const order: number[] = []; + + buffer.startNavigation(); + buffer.addCallback(async () => { + order.push(1); + }); + buffer.addCallback(async () => { + order.push(2); + }); + + buffer.prepareEndNavigation(); + // startView equivalent — passes through immediately + await buffer.addCallback(async () => { + order.push(3); + }); + + buffer.flush(); + await flushPromises(); + + // Buffered events (1, 2) flush after the startView equivalent (3) + expect(order).toEqual([3, 1, 2]); + }); + + it('endNavigation called after prepareEndNavigation drains remaining queue', async () => { + const buffer = new NavigationBuffer(new PassThroughBuffer()); + const cb = jest.fn().mockResolvedValue(undefined); + + buffer.startNavigation(); + buffer.addCallback(cb); + buffer.prepareEndNavigation(); + + // Teardown calls endNavigation — should still drain + buffer.endNavigation(); + await flushPromises(); + + expect(cb).toHaveBeenCalledTimes(1); + }); + + it('does not double-drain if flush called after endNavigation', async () => { + const buffer = new NavigationBuffer(new PassThroughBuffer()); + const cb = jest.fn().mockResolvedValue(undefined); + + buffer.startNavigation(); + buffer.addCallback(cb); + buffer.endNavigation(); + await flushPromises(); + expect(cb).toHaveBeenCalledTimes(1); + + buffer.flush(); + await flushPromises(); + expect(cb).toHaveBeenCalledTimes(1); + }); + }); + + describe('ID linking across navigation hold', () => { + it('resolves addCallbackReturningId with the real ID after drain', async () => { + const buffer = new NavigationBuffer(new PassThroughBuffer()); + + buffer.startNavigation(); + const idPromise = buffer.addCallbackReturningId(() => + Promise.resolve('nativeId') + ); + + buffer.endNavigation(); + await flushPromises(); + + const resolvedId = await idPromise; + expect(resolvedId).toBe('nativeId'); + }); + }); + + describe('exported constant', () => { + it('exports NAVIGATION_BUFFER_TIMEOUT_MS as 500', () => { + expect(NAVIGATION_BUFFER_TIMEOUT_MS).toBe(500); + }); + }); +}); diff --git a/packages/react-navigation/src/__tests__/rum/instrumentation/DdRumReactNavigationTracking.test.tsx b/packages/react-navigation/src/__tests__/rum/instrumentation/DdRumReactNavigationTracking.test.tsx index 641042ce9..3a39a20c8 100644 --- a/packages/react-navigation/src/__tests__/rum/instrumentation/DdRumReactNavigationTracking.test.tsx +++ b/packages/react-navigation/src/__tests__/rum/instrumentation/DdRumReactNavigationTracking.test.tsx @@ -55,13 +55,32 @@ jest.mock('react-native', () => { return reactNative; }); +// Mock buffer registered in globalThis so DdRumReactNavigationTracking can +// access it via getGlobalInstance without importing BufferSingleton from core. +const BUFFER_SINGLETON_KEY = 'com.datadog.reactnative.buffer_singleton'; + +const mockNavigationBuffer = { + startNavigation: jest.fn(), + prepareEndNavigation: jest.fn(), + flush: jest.fn(), + endNavigation: jest.fn(), + navigationStartTime: null as number | null +}; + +const mockBufferSingleton = { + getNavigationBuffer: jest.fn(() => mockNavigationBuffer) +}; + +beforeAll(() => { + (globalThis as any)[Symbol.for(BUFFER_SINGLETON_KEY)] = mockBufferSingleton; +}); + jest.mock('@datadog/mobile-react-native', () => { return { DdRum: { - // eslint-disable-next-line @typescript-eslint/no-empty-function - startView: jest.fn().mockImplementation(() => {}), - stopView: jest.fn().mockImplementation(() => {}), - addError: jest.fn().mockImplementation(() => {}) + startView: jest.fn().mockResolvedValue(undefined), + stopView: jest.fn().mockResolvedValue(undefined), + addError: jest.fn().mockResolvedValue(undefined) }, SdkVerbosity: { DEBUG: 'debug', @@ -86,6 +105,10 @@ beforeEach(() => { mocked(AppState.addEventListener).mockClear(); mocked(BackHandler.exitApp).mockClear(); + (mockNavigationBuffer.startNavigation as jest.Mock).mockClear(); + (mockNavigationBuffer.endNavigation as jest.Mock).mockClear(); + mockBufferSingleton.getNavigationBuffer.mockClear(); + // @ts-ignore DdRumReactNavigationTracking._resetInternalStateForTesting(); }); @@ -372,12 +395,12 @@ describe.each([ it('sends a RUM ViewEvent for each when startTrackingViews { multiple navigation containers when first not detached }', async () => { // GIVEN const navigationRef1 = createRef(); - const testUtils1: { getByText } = render( + const testUtils1: { getByText: any } = render( ); const goToAboutButton1 = testUtils1.getByText('Go to About'); const navigationRef2 = createRef(); - const testUtils2: { getByText } = render( + const testUtils2: { getByText: any } = render( ); const goToAboutButton2 = testUtils2.getByText('Go to About'); @@ -406,7 +429,7 @@ describe.each([ it('sends a RUM ViewEvent for each when switching screens { nested navigation containers }', async () => { // GIVEN const navigationRef = createRef(); - const testUtils: { getByText } = render( + const testUtils: { getByText: any } = render( ); DdRumReactNavigationTracking.startTrackingViews( @@ -467,12 +490,12 @@ describe.each([ it('sends a RUM ViewEvent for each when startTrackingViews { multiple navigation containers when first is detached }', async () => { // GIVEN const navigationRef1 = createRef(); - const testUtils1: { getByText } = render( + const testUtils1: { getByText: any } = render( ); const goToAboutButton1 = testUtils1.getByText('Go to About'); const navigationRef2 = createRef(); - const testUtils2: { getByText } = render( + const testUtils2: { getByText: any } = render( ); const goToAboutButton2 = testUtils2.getByText('Go to About'); @@ -519,7 +542,7 @@ describe.each([ ])( 'AppState listener on %s', (reactNativeVersion, AppStateMockVersion) => { - let appStateMock; + let appStateMock: any; beforeEach(() => { AppState.currentState = 'active'; appStateMock = new AppStateMockVersion(); @@ -831,3 +854,207 @@ describe.each([ }); } ); + +describe('Navigation Buffer Integration', () => { + // These tests verify the buffer lifecycle wiring + // They use the v6 navigators as the buffer behavior is version-independent. + const mockNavBuffer = mockNavigationBuffer; + + it('calls startNavigation when dispatch is called directly on the navigation ref', async () => { + // startNavigation is triggered by the patched dispatch on the + // navigation container ref. Screen-level navigation.navigate() + // uses an internal dispatch path that may not hit the container + // ref's dispatch. This test verifies the dispatch patch by + // calling dispatch directly on the container ref. + const navigationRef = createRef(); + render(); + + DdRumReactNavigationTracking.startTrackingViews(navigationRef.current); + (mockNavBuffer.startNavigation as jest.Mock).mockClear(); + (mockNavBuffer.endNavigation as jest.Mock).mockClear(); + + navigationRef.current.dispatch({ + type: 'NAVIGATE', + payload: { name: 'About' } + }); + expect(mockNavBuffer.startNavigation).toHaveBeenCalled(); + }); + + it('calls prepareEndNavigation before startView and flush after it resolves for push navigation', async () => { + const navigationRef = createRef(); + const { getByText } = render( + + ); + + DdRumReactNavigationTracking.startTrackingViews(navigationRef.current); + (mockNavBuffer.prepareEndNavigation as jest.Mock).mockClear(); + (mockNavBuffer.flush as jest.Mock).mockClear(); + + fireEvent.press(getByText('Go to About')); + + // prepareEndNavigation is called synchronously before startView + expect(mockNavBuffer.prepareEndNavigation).toHaveBeenCalled(); + + // Wait for startView promise to resolve, then flush is called + await new Promise(resolve => setTimeout(resolve, 0)); + + expect(mockNavBuffer.flush).toHaveBeenCalled(); + }); + + it('calls endNavigation when viewTrackingPredicate returns false', async () => { + const navigationRef = createRef(); + const { getByText } = render( + + ); + + const viewTrackingPredicate: ViewTrackingPredicate = _route => false; + DdRumReactNavigationTracking.startTrackingViews(navigationRef.current, { + viewTrackingPredicate + }); + (mockNavBuffer.startNavigation as jest.Mock).mockClear(); + (mockNavBuffer.endNavigation as jest.Mock).mockClear(); + + fireEvent.press(getByText('Go to About')); + + // endNavigation called synchronously since view is not tracked + // (startNavigation may not be called here because navigation.navigate() + // uses a screen-level dispatch that may bypass the container ref's patched dispatch) + expect(mockNavBuffer.endNavigation).toHaveBeenCalled(); + }); + + it('handles rapid consecutive navigations via dispatch', async () => { + // Rapid navigations via direct dispatch calls verify the buffer + // correctly handles multiple startNavigation calls. + const navigationRef = createRef(); + render(); + + DdRumReactNavigationTracking.startTrackingViews(navigationRef.current); + (mockNavBuffer.startNavigation as jest.Mock).mockClear(); + (mockNavBuffer.endNavigation as jest.Mock).mockClear(); + + // Trigger two dispatches in quick succession + navigationRef.current.dispatch({ + type: 'NAVIGATE', + payload: { name: 'About' } + }); + navigationRef.current.dispatch({ + type: 'NAVIGATE', + payload: { name: 'Home' } + }); + + // The buffer should handle multiple startNavigation calls gracefully + expect(mockNavBuffer.startNavigation).toHaveBeenCalledTimes(2); + + await new Promise(resolve => setTimeout(resolve, 0)); + expect(mockNavBuffer.flush).toHaveBeenCalled(); + }); + + it('gesture-based back navigation releases buffer via state change listener', async () => { + // Gesture-based back navigation (swipe) may bypass the dispatch patch + // entirely, as React Navigation's gesture handler may use an internal + // dispatch path that our monkey-patch does not intercept. + // + // This test simulates the scenario where startNavigation was called + // via dispatch, then we use goBack() to trigger the state change + // listener path (handleRouteNavigation -> startView -> endNavigation). + // In production, the NavigationBuffer's 500ms timeout is the safety + // net when dispatch is not intercepted. + + const navigationRef = createRef(); + const { getByText } = render( + + ); + + DdRumReactNavigationTracking.startTrackingViews(navigationRef.current); + + // Navigate to About first so we have somewhere to go back from + fireEvent.press(getByText('Go to About')); + await new Promise(resolve => setTimeout(resolve, 0)); + + // Clear mocks after the push navigation settles + (mockNavBuffer.startNavigation as jest.Mock).mockClear(); + (mockNavBuffer.endNavigation as jest.Mock).mockClear(); + + // Simulate a gesture-back: manually call startNavigation (as if dispatch + // was intercepted) then trigger goBack() on the navigation ref. + mockNavBuffer.startNavigation(); + expect(mockNavBuffer.startNavigation).toHaveBeenCalledTimes(1); + + // The state change listener in DdRumReactNavigationTracking calls + // handleRouteNavigation, which calls endNavigation after startView resolves. + if (navigationRef.current?.canGoBack()) { + navigationRef.current.goBack(); + } + + // Wait for startView promise to resolve and endNavigation to be called + await new Promise(resolve => setTimeout(resolve, 0)); + + // flush should have been called via the normal + // handleRouteNavigation -> prepareEndNavigation -> startView -> .then(flush) path + expect(mockNavBuffer.flush).toHaveBeenCalled(); + }); + + it('stopTrackingViews calls endNavigation as teardown', () => { + const navigationRef = createRef(); + render(); + + DdRumReactNavigationTracking.startTrackingViews(navigationRef.current); + (mockNavBuffer.endNavigation as jest.Mock).mockClear(); + + DdRumReactNavigationTracking.stopTrackingViews(navigationRef.current); + + expect(mockNavBuffer.endNavigation).toHaveBeenCalled(); + }); + + it('calls flush even when startView rejects (fail-safe)', async () => { + mocked(DdRum.startView).mockRejectedValueOnce( + new Error('native error') + ); + + const navigationRef = createRef(); + const { getByText } = render( + + ); + + DdRumReactNavigationTracking.startTrackingViews(navigationRef.current); + (mockNavBuffer.prepareEndNavigation as jest.Mock).mockClear(); + (mockNavBuffer.flush as jest.Mock).mockClear(); + + fireEvent.press(getByText('Go to About')); + + // prepareEndNavigation called synchronously before startView + expect(mockNavBuffer.prepareEndNavigation).toHaveBeenCalled(); + + // Wait for rejected promise to settle — flush called as fail-safe + await new Promise(resolve => setTimeout(resolve, 0)); + + expect(mockNavBuffer.flush).toHaveBeenCalled(); + }); +}); + +describe('Regression: Normal Event Flow', () => { + // Verify existing behavior is unchanged + it('existing tests pass with updated mock (startView returns Promise)', async () => { + // This is verified by the entire existing test suite still passing. + // This test explicitly checks that startView being a promise doesn't break the basic flow. + const navigationRef = createRef(); + const { getByText } = render( + + ); + + DdRumReactNavigationTracking.startTrackingViews(navigationRef.current); + + fireEvent.press(getByText('Go to About')); + + expect(DdRum.startView).toHaveBeenCalled(); + }); + + it('buffer singleton registered in globalThis is accessible', () => { + // Verify the globalThis-based registry is wired correctly — the mock + // buffer singleton registered in beforeAll must expose a navigation buffer + // with the expected methods so DdRumReactNavigationTracking can call them. + expect(mockBufferSingleton.getNavigationBuffer()).not.toBeNull(); + expect(mockNavigationBuffer.startNavigation).toBeDefined(); + expect(mockNavigationBuffer.endNavigation).toBeDefined(); + }); +}); diff --git a/packages/react-navigation/src/rum/instrumentation/DdRumReactNavigationTracking.tsx b/packages/react-navigation/src/rum/instrumentation/DdRumReactNavigationTracking.tsx index 1ce16da2d..dfb0a77ce 100644 --- a/packages/react-navigation/src/rum/instrumentation/DdRumReactNavigationTracking.tsx +++ b/packages/react-navigation/src/rum/instrumentation/DdRumReactNavigationTracking.tsx @@ -20,6 +20,10 @@ import { transformViewKey } from './utils'; const REACT_NAVIGATION_TRACKING_MODULE = 'com.datadog.reactnative.rum.react_navigation_tracking'; +// TODO: DEBUG LOGGING — remove before shipping +const LOG = (msg: string, ...args: any[]) => + console.log(`[DD NavTracking] ${msg}`, ...args); + function getGlobalInstance(key: string, objectConstructor: () => T): T { const symbol = Symbol.for(key); const g = (globalThis as unknown) as Record; @@ -30,6 +34,24 @@ function getGlobalInstance(key: string, objectConstructor: () => T): T { return g[symbol] as T; } +// Minimal interface for the NavigationBuffer methods used by this package. +// Accessed via the shared globalThis key so we don't need to import +// BufferSingleton from the core package's public API. +interface INavigationBuffer { + readonly navigationStartTime: number | null; + startNavigation(): void; + prepareEndNavigation(): void; + flush(): void; + endNavigation(): void; +} + +interface IBufferSingleton { + getNavigationBuffer(): INavigationBuffer | null; +} + +// IMPORTANT: Keep this key aligned with core package +const BUFFER_SINGLETON_KEY = 'com.datadog.reactnative.buffer_singleton'; + // AppStateStatus can have values: // 'active' - The app is running in the foreground // 'background' - The app is running in the background. The user is either in another app or on the home screen @@ -42,6 +64,18 @@ export type NavigationTrackingOptions = { viewNamePredicate?: ViewNamePredicate; viewTrackingPredicate?: ViewTrackingPredicate; paramsTrackingPredicate?: ParamsTrackingPredicate; + /** + * When `true` (default), a NavigationBuffer is used to hold RUM events (e.g. resources, + * actions) that fire between a navigation dispatch and the next `onStateChange` callback. + * Those buffered events are then flushed and attributed to the newly-started view, preventing + * them from being attributed to the previous view. + * + * Set to `false` to disable the buffer entirely. Events will pass through immediately without + * any buffering. Use this if the buffer causes unexpected behaviour in your setup. + * + * @default true + */ + useNavigationBuffer?: boolean; }; export type ViewNamePredicate = ( @@ -106,6 +140,8 @@ class RumReactNavigationTracking { private backHandler: NativeEventSubscription | null = null; + private unsafeActionListener: NavigationListener | null = null; + private appStateSubscription?: NativeEventSubscription; private previousAppState: AppStateStatus | undefined; @@ -114,6 +150,8 @@ class RumReactNavigationTracking { private trackingState: 'TRACKING' | 'NOT_TRACKING' = 'NOT_TRACKING'; + private useNavigationBuffer: boolean = true; + /** * @internal * DO NOT USE: This API is for internal testing only. @@ -169,10 +207,13 @@ class RumReactNavigationTracking { const { viewNamePredicate = defaultViewNamePredicate, viewTrackingPredicate = defaultViewTrackingPredicate, - paramsTrackingPredicate = defaultParamsPredicate + paramsTrackingPredicate = defaultParamsPredicate, + useNavigationBuffer = true } = trackingOptions ?? {}; + this.useNavigationBuffer = useNavigationBuffer; if (navigationRef == null) { + LOG('startTrackingViews() — null navigationRef, aborting'); InternalLog.log( this.NULL_NAVIGATION_REF_ERROR_MESSAGE, SdkVerbosity.ERROR @@ -184,11 +225,17 @@ class RumReactNavigationTracking { this.registeredContainer != null && this.registeredContainer !== navigationRef ) { + LOG( + 'startTrackingViews() — another container already registered, ignoring' + ); InternalLog.log( this.NAVIGATION_REF_IN_USE_ERROR_MESSAGE, SdkVerbosity.ERROR ); } else if (this.registeredContainer == null) { + LOG( + 'startTrackingViews() — registering new container, patching dispatch' + ); if (viewNamePredicate) { this.viewNamePredicate = viewNamePredicate; } @@ -202,6 +249,30 @@ class RumReactNavigationTracking { } this.registeredContainer = navigationRef; + // Listen to __unsafe_action__ — fires before state changes and before + // the new screen mounts, so the buffer is active before any useEffect fetches run. + // This catches all navigation (in-screen navigate() calls AND external dispatch()), + // unlike patching navigationRef.dispatch which only catches the latter. + // Only wired when useNavigationBuffer is true. + if (this.useNavigationBuffer) { + this.unsafeActionListener = (event: any) => { + if (event.data?.noop) { + LOG( + '__unsafe_action__ fired but noop=true — skipping startNavigation()' + ); + return; + } + LOG('__unsafe_action__ fired — calling startNavigation()', { + actionType: event.data?.action?.type + }); + this.getNavBuffer()?.startNavigation(); + }; + navigationRef.addListener( + '__unsafe_action__', + this.unsafeActionListener + ); + } + const listener = this.resolveNavigationStateChangeListener(); navigationRef.addListener('state', listener); @@ -221,9 +292,23 @@ class RumReactNavigationTracking { * @param navigationRef the reference to the real NavigationContainer. */ stopTrackingViews(navigationRef: NavigationContainerRef | null): void { + LOG('stopTrackingViews() called'); this.navigationTimeline?.addStopTrackingEvent(); this.previousRoute = undefined; if (navigationRef != null) { + // Remove __unsafe_action__ listener and force-flush buffer + LOG( + 'stopTrackingViews() — removing __unsafe_action__ listener and calling endNavigation() force-flush' + ); + if (this.unsafeActionListener) { + navigationRef.removeListener( + '__unsafe_action__', + this.unsafeActionListener + ); + this.unsafeActionListener = null; + } + this.getNavBuffer()?.endNavigation(); + if (this.navigationStateChangeListener) { navigationRef.removeListener( 'state', @@ -255,16 +340,25 @@ class RumReactNavigationTracking { } _resetInternalStateForTesting(): void { + if (this.unsafeActionListener && this.registeredContainer) { + this.registeredContainer.removeListener( + '__unsafe_action__', + this.unsafeActionListener + ); + } this._navigationTimeline = undefined; this.registeredContainer = null; this.navigationStateChangeListener = null; + this.unsafeActionListener = null; this.previousRoute = undefined; this.backHandler = null; this.appStateSubscription = undefined; this.previousAppState = undefined; this.previousRouteKey = undefined; this.trackingState = 'NOT_TRACKING'; + this.useNavigationBuffer = true; this.resetPredicates(); + this.getNavBuffer()?.endNavigation(); } private resetPredicates() { @@ -273,23 +367,44 @@ class RumReactNavigationTracking { this.viewTrackingPredicate = defaultViewTrackingPredicate; } + private getNavBuffer(): INavigationBuffer | null | undefined { + if (!this.useNavigationBuffer) { + return undefined; + } + const symbol = Symbol.for(BUFFER_SINGLETON_KEY); + const singleton = (globalThis as any)[symbol] as + | IBufferSingleton + | undefined; + return singleton?.getNavigationBuffer() ?? null; + } + private handleRouteNavigation( route: Route | undefined, appStateStatus: AppStateStatus, stateEvent: StateEvent | undefined ) { if (route === undefined || route === null) { + LOG('handleRouteNavigation() — route is undefined/null, dropping'); InternalLog.log( this.ROUTE_UNDEFINED_NAVIGATION_WARNING_MESSAGE, SdkVerbosity.WARN ); // RUMM-1400 in some cases the route seem to be undefined + // Still drain the buffer so events are never held indefinitely + this.getNavBuffer()?.endNavigation(); return; } const key = route.key; const screenName = this.viewNamePredicate(route, route.name); const customKey = transformViewKey(key, screenName); + LOG('handleRouteNavigation()', { + routeName: route.name, + key, + screenName, + appStateStatus + }); + if (key != null && screenName != null) { // On iOS, the app can start in either "active", "background" or "unknown" state if (appStateStatus !== 'background') { @@ -305,14 +420,85 @@ class RumReactNavigationTracking { } ); if (this.viewTrackingPredicate(route)) { + // Stop buffering BEFORE startView so the startView call + // itself passes through the NavigationBuffer immediately (not queued). + // Then flush queued events AFTER startView resolves so they are + // attributed to the now-active view. + const navBuffer = this.getNavBuffer(); + // Capture the navigation start timestamp BEFORE prepareEndNavigation + // clears it, so we can backdate the view start to when navigation began. + const navigationStartTime = + navBuffer?.navigationStartTime ?? undefined; + const onStateChangeTime = Date.now(); + const onStateChangeLag = + navigationStartTime !== undefined + ? onStateChangeTime - navigationStartTime + : null; + navBuffer?.prepareEndNavigation(); + + LOG( + `handleRouteNavigation() — viewTrackingPredicate=true | navigationStartTime: ${ + navigationStartTime !== undefined + ? new Date(navigationStartTime).toISOString() + : 'n/a' + } | onStateChange fired at: ${new Date( + onStateChangeTime + ).toISOString()} | gap (dispatch→onStateChange): ${ + onStateChangeLag !== null + ? `${onStateChangeLag}ms` + : 'n/a (no dispatch intercepted)' + } | calling DdRum.startView(${customKey}, ${screenName})` + ); const params = this.paramsTrackingPredicate(route); - if (params) { - DdRum.startView(customKey, screenName, { params }); - } else { - DdRum.startView(customKey, screenName); - } + const context = params ? { params } : undefined; + const startViewPromise = + navigationStartTime !== undefined + ? DdRum.startView( + customKey, + screenName, + context ?? {}, + navigationStartTime + ) + : context + ? DdRum.startView(customKey, screenName, context) + : DdRum.startView(customKey, screenName); + + startViewPromise + .then(() => { + LOG( + 'handleRouteNavigation() — startView resolved → calling flush() to drain buffered events' + ); + navBuffer?.flush(); + }) + .catch((err: any) => { + LOG( + 'handleRouteNavigation() — startView REJECTED → calling flush() fail-safe', + err + ); + // Fail-safe: always release buffered events + navBuffer?.flush(); + }); + } else { + // view not tracked — drain buffer immediately (no startView to wait for) + LOG( + 'handleRouteNavigation() — viewTrackingPredicate=false (view not tracked) → calling endNavigation() to drain buffer' + ); + this.getNavBuffer()?.endNavigation(); } + } else { + // App is in background — no startView, drain buffer immediately + LOG( + 'handleRouteNavigation() — skipped (appState is background) → draining buffer' + ); + this.getNavBuffer()?.endNavigation(); } + } else { + // key or screenName is null — no startView, drain buffer immediately + LOG( + 'handleRouteNavigation() — key or screenName is null, skipping startView', + { key, screenName } + ); + this.getNavBuffer()?.endNavigation(); } this.previousRouteKey = route.key; @@ -366,6 +552,11 @@ class RumReactNavigationTracking { if (this.navigationStateChangeListener == null) { this.navigationStateChangeListener = () => { const route = this.registeredContainer?.getCurrentRoute(); + LOG('navigationStateChangeListener fired', { + routeName: route?.name, + routeKey: route?.key, + previousRouteKey: this.previousRouteKey + }); const newRouteStateEvent = this.navigationTimeline?.addNewRouteEvent( this.previousRouteKey, @@ -373,6 +564,9 @@ class RumReactNavigationTracking { ); if (route === undefined) { + LOG( + 'navigationStateChangeListener — route undefined, dropping' + ); InternalLog.log( this.ROUTE_UNDEFINED_NAVIGATION_WARNING_MESSAGE, SdkVerbosity.WARN @@ -382,6 +576,9 @@ class RumReactNavigationTracking { // Route already tracked if (this.previousRoute === route) { + LOG( + 'navigationStateChangeListener — same route as previousRoute, discarding' + ); this.navigationTimeline?.addNavigationStateEvent( 'DISCARDED', route.name, @@ -394,6 +591,9 @@ class RumReactNavigationTracking { return; } + LOG( + 'navigationStateChangeListener — new route detected, forwarding to handleRouteNavigation' + ); this.handleRouteNavigation( route, AppState.currentState, @@ -409,8 +609,13 @@ class RumReactNavigationTracking { private appStateListener: AppStateListener = ( appStateStatus: AppStateStatus ) => { + LOG('appStateListener fired', { + appStateStatus, + previousAppState: this.previousAppState + }); const currentRoute = this.registeredContainer?.getCurrentRoute(); if (currentRoute === undefined || currentRoute === null) { + LOG('appStateListener — could not determine route, dropping'); InternalLog.log( `We could not determine the route when changing the application state to: ${appStateStatus}. No RUM View event will be sent in this case.`, SdkVerbosity.ERROR From 3f0b66455729d0635c65506424d058ff96ffd2f5 Mon Sep 17 00:00:00 2001 From: Marco Saia Date: Mon, 23 Mar 2026 16:56:32 +0100 Subject: [PATCH 2/3] chore: removed NavigationBuffer internal logs --- .../Buffer/NavigationBuffer.ts | 76 ---------------- .../DdRumReactNavigationTracking.tsx | 87 +------------------ 2 files changed, 1 insertion(+), 162 deletions(-) diff --git a/packages/core/src/sdk/DatadogProvider/Buffer/NavigationBuffer.ts b/packages/core/src/sdk/DatadogProvider/Buffer/NavigationBuffer.ts index a401aaa3a..6e69a6f46 100644 --- a/packages/core/src/sdk/DatadogProvider/Buffer/NavigationBuffer.ts +++ b/packages/core/src/sdk/DatadogProvider/Buffer/NavigationBuffer.ts @@ -12,11 +12,6 @@ import { DatadogBuffer } from './DatadogBuffer'; */ export const NAVIGATION_BUFFER_TIMEOUT_MS = 500; -// TODO: DEBUG LOGGING — remove before shipping -const LOG = (msg: string, ...args: any[]) => - // eslint-disable-next-line no-console - console.log(`[DD NavBuffer] ${new Date().toISOString()} ${msg}`, ...args); - /** * An internal `DatadogBuffer` decorator that queues RUM events during a * navigation transition and flushes them once the new view is confirmed. @@ -70,18 +65,12 @@ export class NavigationBuffer extends DatadogBuffer { constructor(innerBuffer: DatadogBuffer) { super(); this.innerBuffer = innerBuffer; - LOG('constructed', { innerBuffer: innerBuffer.constructor.name }); } addCallback = (callback: () => Promise): Promise => { if (!this.isNavigating) { - LOG('addCallback → passthrough'); return this.innerBuffer.addCallback(callback); } - LOG( - 'addCallback → QUEUED, queueLength now', - this.callbackQueue.length + 1 - ); this.callbackQueue.push(() => { this.innerBuffer.addCallback(callback); }); @@ -92,13 +81,8 @@ export class NavigationBuffer extends DatadogBuffer { callback: () => Promise ): Promise => { if (!this.isNavigating) { - LOG('addCallbackReturningId → passthrough'); return this.innerBuffer.addCallbackReturningId(callback); } - LOG( - 'addCallbackReturningId → QUEUED, queueLength now', - this.callbackQueue.length + 1 - ); return new Promise(resolve => { this.callbackQueue.push(() => { this.innerBuffer.addCallbackReturningId(callback).then(resolve); @@ -111,15 +95,8 @@ export class NavigationBuffer extends DatadogBuffer { id: string ): Promise => { if (!this.isNavigating) { - LOG('addCallbackWithId → passthrough, id:', id); return this.innerBuffer.addCallbackWithId(callback, id); } - LOG( - 'addCallbackWithId → QUEUED, id:', - id, - 'queueLength now', - this.callbackQueue.length + 1 - ); return new Promise(resolve => { this.callbackQueue.push(() => { this.innerBuffer.addCallbackWithId(callback, id).then(resolve); @@ -128,12 +105,6 @@ export class NavigationBuffer extends DatadogBuffer { }; drain = (): void => { - LOG( - 'drain() called, queueLength:', - this.callbackQueue.length, - 'isNavigating:', - this.isNavigating - ); this.flushQueue(); this.innerBuffer.drain(); }; @@ -150,17 +121,8 @@ export class NavigationBuffer extends DatadogBuffer { } this.isNavigating = true; this.timeoutId = setTimeout(() => { - LOG( - `timeout fired after ${NAVIGATION_BUFFER_TIMEOUT_MS}ms — calling endNavigation` - ); this.endNavigation(); }, NAVIGATION_BUFFER_TIMEOUT_MS); - LOG('startNavigation()', { - wasAlreadyNavigating, - navigationStartTime: this._navigationStartTime, - queueLength: this.callbackQueue.length, - timeoutMs: NAVIGATION_BUFFER_TIMEOUT_MS - }); }; /** @@ -178,12 +140,6 @@ export class NavigationBuffer extends DatadogBuffer { this.timeoutId = null; } this.isNavigating = false; - LOG( - 'prepareEndNavigation() — stopped buffering, queue preserved, navigationStartTime:', - this._navigationStartTime, - 'queueLength:', - this.callbackQueue.length - ); // Note: _navigationStartTime is intentionally kept until flush() so the // caller can still read it after prepareEndNavigation() returns. }; @@ -195,21 +151,6 @@ export class NavigationBuffer extends DatadogBuffer { * Safe to call when the queue is empty (no-op). */ flush = (): void => { - const now = Date.now(); - const lag = - this._navigationStartTime !== null - ? now - this._navigationStartTime - : null; - LOG( - 'flush() called — draining', - this.callbackQueue.length, - 'queued events | navigationStartTime:', - this._navigationStartTime, - '| now:', - now, - '| lag since nav start:', - lag !== null ? `${lag}ms` : 'n/a' - ); this._navigationStartTime = null; this.flushQueue(); }; @@ -225,30 +166,13 @@ export class NavigationBuffer extends DatadogBuffer { this.timeoutId = null; } this.isNavigating = false; - const now = Date.now(); - const lag = - this._navigationStartTime !== null - ? now - this._navigationStartTime - : null; - LOG( - 'endNavigation() — draining', - this.callbackQueue.length, - 'queued events | navigationStartTime:', - this._navigationStartTime, - '| now:', - now, - '| lag since nav start:', - lag !== null ? `${lag}ms` : 'n/a' - ); this._navigationStartTime = null; this.flushQueue(); - LOG('endNavigation() done'); }; private flushQueue = (): void => { const pending = this.callbackQueue; this.callbackQueue = []; - LOG('flushQueue() executing', pending.length, 'queued callbacks'); for (const callback of pending) { callback(); } diff --git a/packages/react-navigation/src/rum/instrumentation/DdRumReactNavigationTracking.tsx b/packages/react-navigation/src/rum/instrumentation/DdRumReactNavigationTracking.tsx index dfb0a77ce..2799e77e8 100644 --- a/packages/react-navigation/src/rum/instrumentation/DdRumReactNavigationTracking.tsx +++ b/packages/react-navigation/src/rum/instrumentation/DdRumReactNavigationTracking.tsx @@ -20,10 +20,6 @@ import { transformViewKey } from './utils'; const REACT_NAVIGATION_TRACKING_MODULE = 'com.datadog.reactnative.rum.react_navigation_tracking'; -// TODO: DEBUG LOGGING — remove before shipping -const LOG = (msg: string, ...args: any[]) => - console.log(`[DD NavTracking] ${msg}`, ...args); - function getGlobalInstance(key: string, objectConstructor: () => T): T { const symbol = Symbol.for(key); const g = (globalThis as unknown) as Record; @@ -213,7 +209,6 @@ class RumReactNavigationTracking { this.useNavigationBuffer = useNavigationBuffer; if (navigationRef == null) { - LOG('startTrackingViews() — null navigationRef, aborting'); InternalLog.log( this.NULL_NAVIGATION_REF_ERROR_MESSAGE, SdkVerbosity.ERROR @@ -225,17 +220,11 @@ class RumReactNavigationTracking { this.registeredContainer != null && this.registeredContainer !== navigationRef ) { - LOG( - 'startTrackingViews() — another container already registered, ignoring' - ); InternalLog.log( this.NAVIGATION_REF_IN_USE_ERROR_MESSAGE, SdkVerbosity.ERROR ); } else if (this.registeredContainer == null) { - LOG( - 'startTrackingViews() — registering new container, patching dispatch' - ); if (viewNamePredicate) { this.viewNamePredicate = viewNamePredicate; } @@ -257,14 +246,8 @@ class RumReactNavigationTracking { if (this.useNavigationBuffer) { this.unsafeActionListener = (event: any) => { if (event.data?.noop) { - LOG( - '__unsafe_action__ fired but noop=true — skipping startNavigation()' - ); return; } - LOG('__unsafe_action__ fired — calling startNavigation()', { - actionType: event.data?.action?.type - }); this.getNavBuffer()?.startNavigation(); }; navigationRef.addListener( @@ -292,14 +275,9 @@ class RumReactNavigationTracking { * @param navigationRef the reference to the real NavigationContainer. */ stopTrackingViews(navigationRef: NavigationContainerRef | null): void { - LOG('stopTrackingViews() called'); this.navigationTimeline?.addStopTrackingEvent(); this.previousRoute = undefined; if (navigationRef != null) { - // Remove __unsafe_action__ listener and force-flush buffer - LOG( - 'stopTrackingViews() — removing __unsafe_action__ listener and calling endNavigation() force-flush' - ); if (this.unsafeActionListener) { navigationRef.removeListener( '__unsafe_action__', @@ -384,7 +362,6 @@ class RumReactNavigationTracking { stateEvent: StateEvent | undefined ) { if (route === undefined || route === null) { - LOG('handleRouteNavigation() — route is undefined/null, dropping'); InternalLog.log( this.ROUTE_UNDEFINED_NAVIGATION_WARNING_MESSAGE, SdkVerbosity.WARN @@ -398,13 +375,6 @@ class RumReactNavigationTracking { const screenName = this.viewNamePredicate(route, route.name); const customKey = transformViewKey(key, screenName); - LOG('handleRouteNavigation()', { - routeName: route.name, - key, - screenName, - appStateStatus - }); - if (key != null && screenName != null) { // On iOS, the app can start in either "active", "background" or "unknown" state if (appStateStatus !== 'background') { @@ -429,26 +399,8 @@ class RumReactNavigationTracking { // clears it, so we can backdate the view start to when navigation began. const navigationStartTime = navBuffer?.navigationStartTime ?? undefined; - const onStateChangeTime = Date.now(); - const onStateChangeLag = - navigationStartTime !== undefined - ? onStateChangeTime - navigationStartTime - : null; navBuffer?.prepareEndNavigation(); - LOG( - `handleRouteNavigation() — viewTrackingPredicate=true | navigationStartTime: ${ - navigationStartTime !== undefined - ? new Date(navigationStartTime).toISOString() - : 'n/a' - } | onStateChange fired at: ${new Date( - onStateChangeTime - ).toISOString()} | gap (dispatch→onStateChange): ${ - onStateChangeLag !== null - ? `${onStateChangeLag}ms` - : 'n/a (no dispatch intercepted)' - } | calling DdRum.startView(${customKey}, ${screenName})` - ); const params = this.paramsTrackingPredicate(route); const context = params ? { params } : undefined; const startViewPromise = @@ -465,39 +417,22 @@ class RumReactNavigationTracking { startViewPromise .then(() => { - LOG( - 'handleRouteNavigation() — startView resolved → calling flush() to drain buffered events' - ); navBuffer?.flush(); }) - .catch((err: any) => { - LOG( - 'handleRouteNavigation() — startView REJECTED → calling flush() fail-safe', - err - ); + .catch(() => { // Fail-safe: always release buffered events navBuffer?.flush(); }); } else { // view not tracked — drain buffer immediately (no startView to wait for) - LOG( - 'handleRouteNavigation() — viewTrackingPredicate=false (view not tracked) → calling endNavigation() to drain buffer' - ); this.getNavBuffer()?.endNavigation(); } } else { // App is in background — no startView, drain buffer immediately - LOG( - 'handleRouteNavigation() — skipped (appState is background) → draining buffer' - ); this.getNavBuffer()?.endNavigation(); } } else { // key or screenName is null — no startView, drain buffer immediately - LOG( - 'handleRouteNavigation() — key or screenName is null, skipping startView', - { key, screenName } - ); this.getNavBuffer()?.endNavigation(); } @@ -552,21 +487,12 @@ class RumReactNavigationTracking { if (this.navigationStateChangeListener == null) { this.navigationStateChangeListener = () => { const route = this.registeredContainer?.getCurrentRoute(); - LOG('navigationStateChangeListener fired', { - routeName: route?.name, - routeKey: route?.key, - previousRouteKey: this.previousRouteKey - }); - const newRouteStateEvent = this.navigationTimeline?.addNewRouteEvent( this.previousRouteKey, route?.key ); if (route === undefined) { - LOG( - 'navigationStateChangeListener — route undefined, dropping' - ); InternalLog.log( this.ROUTE_UNDEFINED_NAVIGATION_WARNING_MESSAGE, SdkVerbosity.WARN @@ -576,9 +502,6 @@ class RumReactNavigationTracking { // Route already tracked if (this.previousRoute === route) { - LOG( - 'navigationStateChangeListener — same route as previousRoute, discarding' - ); this.navigationTimeline?.addNavigationStateEvent( 'DISCARDED', route.name, @@ -591,9 +514,6 @@ class RumReactNavigationTracking { return; } - LOG( - 'navigationStateChangeListener — new route detected, forwarding to handleRouteNavigation' - ); this.handleRouteNavigation( route, AppState.currentState, @@ -609,13 +529,8 @@ class RumReactNavigationTracking { private appStateListener: AppStateListener = ( appStateStatus: AppStateStatus ) => { - LOG('appStateListener fired', { - appStateStatus, - previousAppState: this.previousAppState - }); const currentRoute = this.registeredContainer?.getCurrentRoute(); if (currentRoute === undefined || currentRoute === null) { - LOG('appStateListener — could not determine route, dropping'); InternalLog.log( `We could not determine the route when changing the application state to: ${appStateStatus}. No RUM View event will be sent in this case.`, SdkVerbosity.ERROR From 5c925f3356c3d1cf932146275fcde754094873d8 Mon Sep 17 00:00:00 2001 From: Marco Saia Date: Mon, 23 Mar 2026 17:42:54 +0100 Subject: [PATCH 3/3] RUM-15238: handle back-to-back navigation --- .../Buffer/NavigationBuffer.ts | 36 +++++++++++--- .../Buffer/__tests__/NavigationBuffer.test.ts | 49 +++++++++++++++++++ 2 files changed, 79 insertions(+), 6 deletions(-) diff --git a/packages/core/src/sdk/DatadogProvider/Buffer/NavigationBuffer.ts b/packages/core/src/sdk/DatadogProvider/Buffer/NavigationBuffer.ts index 6e69a6f46..b3cb87731 100644 --- a/packages/core/src/sdk/DatadogProvider/Buffer/NavigationBuffer.ts +++ b/packages/core/src/sdk/DatadogProvider/Buffer/NavigationBuffer.ts @@ -49,6 +49,12 @@ export class NavigationBuffer extends DatadogBuffer { private innerBuffer: DatadogBuffer; private isNavigating = false; private callbackQueue: Array<() => void> = []; + // Snapshot of callbackQueue taken at prepareEndNavigation() time. + // Kept separate so that a back-to-back navigation that calls startNavigation() + // before flush() runs cannot mix its events into the pending flush for the + // previous view. flush() drains only this snapshot; new events from the + // subsequent navigation accumulate in the fresh callbackQueue. + private _pendingFlushQueue: Array<() => void> = []; private timeoutId: ReturnType | null = null; private _navigationStartTime: number | null = null; @@ -105,7 +111,7 @@ export class NavigationBuffer extends DatadogBuffer { }; drain = (): void => { - this.flushQueue(); + this.drainAllQueues(); this.innerBuffer.drain(); }; @@ -140,6 +146,12 @@ export class NavigationBuffer extends DatadogBuffer { this.timeoutId = null; } this.isNavigating = false; + // Snapshot the current queue so flush() only drains events that belong + // to this navigation. Events arriving after this point (e.g. from a + // back-to-back navigation that re-enables buffering) go into the fresh + // callbackQueue and will not be included in the upcoming flush(). + this._pendingFlushQueue = this.callbackQueue; + this.callbackQueue = []; // Note: _navigationStartTime is intentionally kept until flush() so the // caller can still read it after prepareEndNavigation() returns. }; @@ -152,7 +164,11 @@ export class NavigationBuffer extends DatadogBuffer { */ flush = (): void => { this._navigationStartTime = null; - this.flushQueue(); + const toFlush = this._pendingFlushQueue; + this._pendingFlushQueue = []; + for (const callback of toFlush) { + callback(); + } }; /** @@ -167,13 +183,21 @@ export class NavigationBuffer extends DatadogBuffer { } this.isNavigating = false; this._navigationStartTime = null; - this.flushQueue(); + // Drain both queues: _pendingFlushQueue may be non-empty if a + // prepareEndNavigation() was followed by a back-to-back navigation + // before flush() ran. + this.drainAllQueues(); }; - private flushQueue = (): void => { - const pending = this.callbackQueue; + private drainAllQueues = (): void => { + const pendingFlush = this._pendingFlushQueue; + this._pendingFlushQueue = []; + for (const callback of pendingFlush) { + callback(); + } + const queued = this.callbackQueue; this.callbackQueue = []; - for (const callback of pending) { + for (const callback of queued) { callback(); } }; diff --git a/packages/core/src/sdk/DatadogProvider/Buffer/__tests__/NavigationBuffer.test.ts b/packages/core/src/sdk/DatadogProvider/Buffer/__tests__/NavigationBuffer.test.ts index d812b4448..cdbd497a2 100644 --- a/packages/core/src/sdk/DatadogProvider/Buffer/__tests__/NavigationBuffer.test.ts +++ b/packages/core/src/sdk/DatadogProvider/Buffer/__tests__/NavigationBuffer.test.ts @@ -326,6 +326,55 @@ describe('NavigationBuffer', () => { await flushPromises(); expect(cb).toHaveBeenCalledTimes(1); }); + + it('back-to-back nav: flush only drains events from the first navigation', async () => { + const buffer = new NavigationBuffer(new PassThroughBuffer()); + const cbA1 = jest.fn().mockResolvedValue(undefined); + const cbA2 = jest.fn().mockResolvedValue(undefined); + + // First navigation + buffer.startNavigation(); + buffer.addCallback(cbA1); + buffer.prepareEndNavigation(); + + // Second navigation fires before flush() runs + buffer.startNavigation(); + buffer.addCallback(cbA2); + + // Flush for first navigation — only A1 should drain + buffer.flush(); + await flushPromises(); + expect(cbA1).toHaveBeenCalledTimes(1); + expect(cbA2).not.toHaveBeenCalled(); + + // Complete second navigation + buffer.prepareEndNavigation(); + buffer.flush(); + await flushPromises(); + expect(cbA2).toHaveBeenCalledTimes(1); + }); + + it('endNavigation after prepareEndNavigation drains both pending and queued events', async () => { + const buffer = new NavigationBuffer(new PassThroughBuffer()); + const cbA1 = jest.fn().mockResolvedValue(undefined); + const cbA2 = jest.fn().mockResolvedValue(undefined); + + // First navigation + buffer.startNavigation(); + buffer.addCallback(cbA1); + buffer.prepareEndNavigation(); + + // Second navigation starts before flush() + buffer.startNavigation(); + buffer.addCallback(cbA2); + + // Teardown / timeout path — endNavigation drains everything + buffer.endNavigation(); + await flushPromises(); + + expect(cbA1).toHaveBeenCalledTimes(1); + expect(cbA2).toHaveBeenCalledTimes(1); + }); }); describe('ID linking across navigation hold', () => {