Skip to content

Commit 03fefc1

Browse files
[skip ci] Merge branch into staging-14
2 parents 821a654 + 3f74789 commit 03fefc1

6 files changed

Lines changed: 88 additions & 8 deletions

File tree

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

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,19 @@ describe('flushController', () => {
100100
})
101101

102102
describe('bytes limit', () => {
103+
it('uses the page exit reason as flush reason for intermediate flushes during page exit', () => {
104+
flushController.notifyBeforeAddMessage(SMALL_MESSAGE_BYTE_COUNT)
105+
flushController.notifyAfterAddMessage()
106+
107+
flushController.preparePageExitFlushObservable.subscribe(() => {
108+
flushController.notifyBeforeAddMessage(BYTES_LIMIT)
109+
})
110+
111+
pageMayExitObservable.notify({ reason: 'before_unload' })
112+
113+
expect(flushSpy.calls.first().args[0].reason).toBe('before_unload')
114+
})
115+
103116
it('notifies when the bytes limit is reached after adding a message', () => {
104117
flushController.notifyBeforeAddMessage(BYTES_LIMIT)
105118
flushController.notifyAfterAddMessage()

packages/core/src/transport/flushController.ts

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,17 @@ interface FlushControllerOptions {
3939
* but relies on invariants described in each method documentation to keep a coherent state.
4040
*/
4141
export function createFlushController({ pageMayExitObservable, sessionExpireObservable }: FlushControllerOptions) {
42-
const pageMayExitSubscription = pageMayExitObservable.subscribe((event) => flush(event.reason))
42+
let forcedFlushReason: FlushReason | undefined
43+
const preparePageExitFlushObservable = new Observable<PageExitReason>()
44+
const pageMayExitSubscription = pageMayExitObservable.subscribe((event) => {
45+
forcedFlushReason = event.reason
46+
try {
47+
preparePageExitFlushObservable.notify(event.reason)
48+
} finally {
49+
forcedFlushReason = undefined
50+
}
51+
flush(event.reason)
52+
})
4353
const sessionExpireSubscription = sessionExpireObservable.subscribe(() => flush('session_expire'))
4454

4555
const flushObservable = new Observable<FlushEvent>(() => () => {
@@ -85,6 +95,7 @@ export function createFlushController({ pageMayExitObservable, sessionExpireObse
8595

8696
return {
8797
flushObservable,
98+
preparePageExitFlushObservable,
8899
get messagesCount() {
89100
return currentMessagesCount
90101
},
@@ -100,7 +111,7 @@ export function createFlushController({ pageMayExitObservable, sessionExpireObse
100111
*/
101112
notifyBeforeAddMessage(estimatedMessageBytesCount: number) {
102113
if (currentBytesCount + estimatedMessageBytesCount >= RECOMMENDED_REQUEST_BYTES_LIMIT) {
103-
flush('bytes_limit')
114+
flush(forcedFlushReason ?? 'bytes_limit')
104115
}
105116
// Consider the message to be added now rather than in `notifyAfterAddMessage`, because if no
106117
// message was added yet and `notifyAfterAddMessage` is called asynchronously, we still want
@@ -123,9 +134,9 @@ export function createFlushController({ pageMayExitObservable, sessionExpireObse
123134
currentBytesCount += messageBytesCountDiff
124135

125136
if (currentMessagesCount >= MESSAGES_LIMIT) {
126-
flush('messages_limit')
137+
flush(forcedFlushReason ?? 'messages_limit')
127138
} else if (currentBytesCount >= RECOMMENDED_REQUEST_BYTES_LIMIT) {
128-
flush('bytes_limit')
139+
flush(forcedFlushReason ?? 'bytes_limit')
129140
}
130141
},
131142

packages/core/test/emulate/mockFlushController.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,12 @@
11
import { Observable } from '../../src/tools/observable'
2+
import type { PageExitReason } from '../../src/browser/pageMayExitObservable'
23
import type { FlushEvent, FlushController, FlushReason } from '../../src/transport'
34

45
export type MockFlushController = ReturnType<typeof createMockFlushController>
56

67
export function createMockFlushController() {
78
const flushObservable = new Observable<FlushEvent>()
9+
const preparePageExitFlushObservable = new Observable<PageExitReason>()
810
let currentMessagesCount = 0
911
let currentBytesCount = 0
1012

@@ -33,6 +35,7 @@ export function createMockFlushController() {
3335
return currentBytesCount
3436
},
3537
flushObservable,
38+
preparePageExitFlushObservable,
3639
notifyFlush(reason: FlushReason = 'bytes_limit') {
3740
if (currentMessagesCount === 0) {
3841
throw new Error(

packages/rum-core/src/boot/startRum.ts

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -88,10 +88,6 @@ export function startRum(
8888
}
8989

9090
const pageMayExitObservable = createPageMayExitObservable(configuration)
91-
const pageMayExitSubscription = pageMayExitObservable.subscribe((event) => {
92-
lifeCycle.notify(LifeCycleEventType.PAGE_MAY_EXIT, event)
93-
})
94-
cleanupTasks.push(() => pageMayExitSubscription.unsubscribe())
9591

9692
const session = !canUseEventBridge()
9793
? startRumSessionManager(configuration, lifeCycle, trackingConsentState)
@@ -106,10 +102,18 @@ export function startRum(
106102
session.expireObservable,
107103
createEncoder
108104
)
105+
const preparePageExitSubscription = batch.flushController.preparePageExitFlushObservable.subscribe((reason) => {
106+
lifeCycle.notify(LifeCycleEventType.PAGE_MAY_EXIT, { reason })
107+
})
108+
cleanupTasks.push(() => preparePageExitSubscription.unsubscribe())
109109
cleanupTasks.push(() => batch.stop())
110110
startCustomerDataTelemetry(telemetry, lifeCycle, batch.flushController.flushObservable)
111111
} else {
112112
startRumEventBridge(lifeCycle)
113+
const pageMayExitSubscription = pageMayExitObservable.subscribe((event) => {
114+
lifeCycle.notify(LifeCycleEventType.PAGE_MAY_EXIT, event)
115+
})
116+
cleanupTasks.push(() => pageMayExitSubscription.unsubscribe())
113117
}
114118

115119
startTrackingConsentContext(hooks, trackingConsentState)

test/e2e/lib/framework/intakeProxyMiddleware.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import type { TelemetryEvent } from '@datadog/browser-core/src/domain/telemetry/
1111
interface BaseIntakeRequest {
1212
isBridge: boolean
1313
encoding: string | null
14+
transport: string | null
1415
}
1516

1617
export type LogsIntakeRequest = {
@@ -56,6 +57,7 @@ interface IntakeRequestInfos {
5657
isBridge: boolean
5758
intakeType: IntakeRequest['intakeType']
5859
encoding: string | null
60+
transport: string | null
5961
}
6062

6163
interface IntakeProxyOptions {
@@ -87,12 +89,14 @@ function computeIntakeRequestInfos(req: express.Request): IntakeRequestInfos {
8789
const { pathname, searchParams } = new URL(ddforward, 'https://example.org')
8890

8991
const encoding = req.headers['content-encoding'] || searchParams.get('dd-evp-encoding')
92+
const transport = searchParams.get('_dd.api')
9093

9194
if (req.query.bridge === 'true') {
9295
const eventType = req.query.event_type
9396
return {
9497
isBridge: true,
9598
encoding,
99+
transport,
96100
intakeType: eventType === 'log' ? 'logs' : eventType === 'record' ? 'replay' : 'rum',
97101
}
98102
}
@@ -108,6 +112,7 @@ function computeIntakeRequestInfos(req: express.Request): IntakeRequestInfos {
108112
return {
109113
isBridge: false,
110114
encoding,
115+
transport,
111116
intakeType,
112117
}
113118
}

test/e2e/scenario/transport.scenario.ts

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,50 @@ import { test, expect } from '@playwright/test'
22
import { createTest } from '../lib/framework'
33

44
test.describe('transport', () => {
5+
test.describe('batch flushing on beforeunload', () => {
6+
createTest('use sendBeacon for a batch flushed by bytes_limit during beforeunload')
7+
// This test reproduces a bug where a batch near the size limit is flushed using fetch
8+
// instead of sendBeacon when the beforeunload event fires.
9+
//
10+
// Scenario:
11+
// 1. A batch is almost full (close to RECOMMENDED_REQUEST_BYTES_LIMIT = 16 KiB)
12+
// 2. beforeunload fires and triggers a final view update
13+
// 3. Adding the view update exceeds the limit → the batch is flushed due to bytes_limit
14+
// 4. BUG: this flush uses fetch instead of sendBeacon, so it may be cancelled during unload
15+
// 5. A new batch is created for the view update and flushed via sendBeacon
16+
//
17+
// Expected: the bytes_limit flush should use sendBeacon when triggered in the context of a
18+
// page exit, so that it is not cancelled by the browser.
19+
.withRum({ telemetrySampleRate: 0 })
20+
.run(async ({ page, flushEvents, intakeRegistry }) => {
21+
// Fill the batch close to the 16 KiB limit using a custom action. The action name is sized
22+
// so that action event is almost at the limit 16KB limit → no flush yet.
23+
await page.evaluate(() => {
24+
window.DD_RUM!.addAction('x'.repeat(15000))
25+
})
26+
27+
// Navigating away fires beforeunload, which triggers a final view update. Adding the view
28+
// update (~2KB) to the near-full batch tips it over the limit and causes a bytes_limit
29+
// flush, which the SDK issues via fetch — and that fetch gets cancelled on page unload.
30+
await flushEvents()
31+
32+
// We expect two last RUM batches:
33+
// 1. The near-full batch containing the large action, flushed due to bytes_limit
34+
// 2. The final view update batch, flushed due to beforeunload
35+
const [penultimateBatch, finalBatch] = intakeRegistry.rumRequests.slice(-2)
36+
37+
// The action event should be present in one of the batches
38+
expect(
39+
penultimateBatch.events.some((e) => e.type === 'action') || finalBatch.events.some((e) => e.type === 'action')
40+
).toBe(true)
41+
42+
// Both batches should use sendBeacon so they are not cancelled during page unload.
43+
// With the bug, penultimateBatch.transport is 'fetch' instead of 'beacon'.
44+
expect(penultimateBatch.transport).toBe('beacon')
45+
expect(finalBatch.transport).toBe('beacon')
46+
})
47+
})
48+
549
test.describe('data compression', () => {
650
createTest('send RUM data compressed')
751
.withRum({

0 commit comments

Comments
 (0)