Skip to content

Commit c5d1a0a

Browse files
committed
fix(core): Break TurboModule aggregator self-flush loop
The periodic flush's own `captureEvent` travels through the wrapped `RNSentry.captureEnvelope`, which records back into the aggregator and, because the map was just drained, re-armed the flush timer forever in otherwise-idle sessions. Suppress the empty→non-empty callback around the flush's `captureEvent`, deferring the release to the next macrotask so the async record fired from the transport's `.then()` also lands inside the suppression window.
1 parent a6263e8 commit c5d1a0a

5 files changed

Lines changed: 119 additions & 15 deletions

File tree

packages/core/src/js/integrations/turboModuleContext.ts

Lines changed: 29 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,9 @@ import { debug } from '@sentry/core';
44

55
import { createSpanJSON } from '../tracing/utils';
66
import {
7+
beginSuppressFirstTurboModuleRecordCallback,
78
drainTurboModuleAggregate,
9+
endSuppressFirstTurboModuleRecordCallback,
810
HISTOGRAM_BUCKET_LABELS,
911
hasTurboModuleAggregateData,
1012
setIgnoredTurboModules,
@@ -263,6 +265,14 @@ function attachAggregateToTransactionEvent(event: TransactionEvent): void {
263265
* Emits the current aggregate as a custom Sentry event so long-running
264266
* sessions without a transaction still produce a signal. No-op when there's
265267
* nothing to flush.
268+
*
269+
* The `captureEvent` call travels through `RNSentry.captureEnvelope`, which
270+
* is itself a wrapped TurboModule call. Without the suppression scope below,
271+
* the resulting empty→non-empty transition on the aggregator would re-arm
272+
* the flush timer indefinitely in an otherwise-idle session. The release is
273+
* deferred to the next macrotask so the async record fired from the
274+
* transport's `.then()` (after the native side resolves) also lands inside
275+
* the suppression window.
266276
*/
267277
function flushPeriodicAggregate(client: Client): void {
268278
if (!hasTurboModuleAggregateData()) {
@@ -272,20 +282,25 @@ function flushPeriodicAggregate(client: Client): void {
272282
const totals = summarise(snapshot);
273283
const topByTotalMs = [...snapshot].sort((a, b) => b.totalDurationMs - a.totalDurationMs);
274284

275-
client.captureEvent?.({
276-
message: 'TurboModule aggregate (periodic)',
277-
level: 'info',
278-
tags: {
279-
'event.kind': 'turbo_modules.aggregate',
280-
},
281-
extra: {
282-
total_call_count: totals.callCount,
283-
total_error_count: totals.errorCount,
284-
total_duration_ms: roundMs(totals.totalDurationMs),
285-
unique_methods: snapshot.length,
286-
modules: topByTotalMs.slice(0, MAX_AGGREGATE_ATTRIBUTE_ROWS).map(serialiseRowAsObject),
287-
},
288-
});
285+
beginSuppressFirstTurboModuleRecordCallback();
286+
try {
287+
client.captureEvent?.({
288+
message: 'TurboModule aggregate (periodic)',
289+
level: 'info',
290+
tags: {
291+
'event.kind': 'turbo_modules.aggregate',
292+
},
293+
extra: {
294+
total_call_count: totals.callCount,
295+
total_error_count: totals.errorCount,
296+
total_duration_ms: roundMs(totals.totalDurationMs),
297+
unique_methods: snapshot.length,
298+
modules: topByTotalMs.slice(0, MAX_AGGREGATE_ATTRIBUTE_ROWS).map(serialiseRowAsObject),
299+
},
300+
});
301+
} finally {
302+
setTimeout(endSuppressFirstTurboModuleRecordCallback, 0);
303+
}
289304
}
290305

291306
function summarise(snapshot: ReadonlyArray<TurboModuleAggregate>): {

packages/core/src/js/turbomodule/index.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,9 @@ export {
66
} from './turboModuleTracker';
77
export type { TurboModuleCall, TurboModuleCallKind } from './turboModuleTracker';
88
export {
9+
beginSuppressFirstTurboModuleRecordCallback,
910
drainTurboModuleAggregate,
11+
endSuppressFirstTurboModuleRecordCallback,
1012
HISTOGRAM_BUCKET_LABELS,
1113
HISTOGRAM_BUCKETS_MS,
1214
hasTurboModuleAggregateData,

packages/core/src/js/turbomodule/turboModuleAggregator.ts

Lines changed: 30 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,10 @@ interface MutableAggregate extends TurboModuleAggregate {
5757
const aggregates = new Map<string, MutableAggregate>();
5858
const ignoredModules = new Set<string>();
5959
let onFirstRecordAfterEmpty: (() => void) | undefined;
60+
// Non-zero disables the empty→non-empty callback. The periodic flush uses
61+
// this to prevent the SDK's own transport call (which records itself into
62+
// the aggregator) from re-arming the timer in an idle session.
63+
let suppressFirstRecordCallbackDepth = 0;
6064

6165
function makeKey(name: string, method: string, kind: TurboModuleCallKind): string {
6266
return `${name}|${method}|${kind}`;
@@ -150,7 +154,7 @@ export function recordTurboModuleCall(args: {
150154
// exists to satisfy `noUncheckedIndexedAccess`.
151155
entry.buckets[bucket] = (entry.buckets[bucket] ?? 0) + 1;
152156

153-
if (wasEmpty && onFirstRecordAfterEmpty) {
157+
if (wasEmpty && onFirstRecordAfterEmpty && suppressFirstRecordCallbackDepth === 0) {
154158
// Don't let a misbehaving observer corrupt the aggregate.
155159
try {
156160
onFirstRecordAfterEmpty();
@@ -160,6 +164,30 @@ export function recordTurboModuleCall(args: {
160164
}
161165
}
162166

167+
/**
168+
* Opens a suppression scope: while at least one is open, the empty→non-empty
169+
* callback registered via {@link setOnFirstTurboModuleRecord} is NOT invoked.
170+
* Records still land in the aggregate as normal.
171+
*
172+
* Used by the periodic flush to break a self-recursion loop: the flush's own
173+
* `captureEvent` call travels through `RNSentry.captureEnvelope`, which is a
174+
* wrapped TurboModule call and would otherwise re-arm the flush timer forever
175+
* in an otherwise-idle session.
176+
*/
177+
export function beginSuppressFirstTurboModuleRecordCallback(): void {
178+
suppressFirstRecordCallbackDepth += 1;
179+
}
180+
181+
/**
182+
* Closes a suppression scope opened by
183+
* {@link beginSuppressFirstTurboModuleRecordCallback}. Idempotent at zero.
184+
*/
185+
export function endSuppressFirstTurboModuleRecordCallback(): void {
186+
if (suppressFirstRecordCallbackDepth > 0) {
187+
suppressFirstRecordCallbackDepth -= 1;
188+
}
189+
}
190+
163191
/**
164192
* Registers a callback fired exactly once when the aggregator transitions
165193
* from empty to non-empty — i.e. when the first record after a drain (or
@@ -215,4 +243,5 @@ export function _resetTurboModuleAggregator(): void {
215243
aggregates.clear();
216244
ignoredModules.clear();
217245
onFirstRecordAfterEmpty = undefined;
246+
suppressFirstRecordCallbackDepth = 0;
218247
}

packages/core/test/integrations/turboModuleContext.test.ts

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -196,5 +196,40 @@ describe('turboModuleContextIntegration', () => {
196196
tags: { 'event.kind': 'turbo_modules.aggregate' },
197197
});
198198
});
199+
200+
it('does not re-arm the periodic flush when captureEvent triggers its own TurboModule record', () => {
201+
// Reproduces the self-recursion: `flushPeriodicAggregate` calls
202+
// `client.captureEvent`, whose transport ultimately calls the wrapped
203+
// `RNSentry.captureEnvelope`. That call records back into the
204+
// aggregator; without suppression the empty→non-empty transition
205+
// re-arms the timer, and an idle session loops forever.
206+
jest.useFakeTimers();
207+
const integration = turboModuleContextIntegration();
208+
integration.setupOnce?.();
209+
const client = makeMockClient();
210+
// Simulate the transport wiring: every captureEvent replays the
211+
// TurboModule call the RN transport would have made.
212+
client.captureEvent.mockImplementation(() => {
213+
recordTurboModuleCall({
214+
name: 'RNSentry',
215+
method: 'captureEnvelope',
216+
kind: 'async',
217+
durationMs: 3,
218+
errored: false,
219+
});
220+
return 'event-id';
221+
});
222+
integration.setup?.(client);
223+
224+
recordTurboModuleCall({ name: 'User', method: 'work', kind: 'sync', durationMs: 1, errored: false });
225+
jest.advanceTimersByTime(DEFAULT_AGGREGATE_FLUSH_INTERVAL_MS);
226+
expect(client.captureEvent).toHaveBeenCalledTimes(1);
227+
228+
// Release the deferred suppression and let any additional timers run.
229+
// If the self-noise had re-armed the flush, a second capture would land
230+
// after another interval; the loop would then never terminate.
231+
jest.advanceTimersByTime(DEFAULT_AGGREGATE_FLUSH_INTERVAL_MS * 10);
232+
expect(client.captureEvent).toHaveBeenCalledTimes(1);
233+
});
199234
});
200235
});

packages/core/test/turbomodule/turboModuleAggregator.test.ts

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,11 @@
11
import {
22
_resetTurboModuleAggregator,
3+
beginSuppressFirstTurboModuleRecordCallback,
34
drainTurboModuleAggregate,
5+
endSuppressFirstTurboModuleRecordCallback,
46
recordTurboModuleCall,
57
setIgnoredTurboModules,
8+
setOnFirstTurboModuleRecord,
69
} from '../../src/js/turbomodule/turboModuleAggregator';
710

811
describe('turboModuleAggregator', () => {
@@ -56,4 +59,24 @@ describe('turboModuleAggregator', () => {
5659
expect(snapshot).toHaveLength(1);
5760
expect(snapshot[0]?.name).toBe('Other');
5861
});
62+
63+
it('does not fire the empty→non-empty callback while suppressed', () => {
64+
const cb = jest.fn();
65+
setOnFirstTurboModuleRecord(cb);
66+
67+
beginSuppressFirstTurboModuleRecordCallback();
68+
recordTurboModuleCall({
69+
name: 'RNSentry',
70+
method: 'captureEnvelope',
71+
kind: 'async',
72+
durationMs: 1,
73+
errored: false,
74+
});
75+
expect(cb).not.toHaveBeenCalled();
76+
77+
endSuppressFirstTurboModuleRecordCallback();
78+
drainTurboModuleAggregate();
79+
recordTurboModuleCall({ name: 'User', method: 'work', kind: 'sync', durationMs: 1, errored: false });
80+
expect(cb).toHaveBeenCalledTimes(1);
81+
});
5982
});

0 commit comments

Comments
 (0)