Skip to content

Commit 934a34a

Browse files
alwxclaude
andcommitted
fix(core): Default-ignore RNSentry in TurboModule aggregate
The previous suppression + drain machinery worked around the flush's own `captureEvent → RNSentry.captureEnvelope` self-noise by drop-listing records around the send. That approach had two follow-on bugs: the blanket drain also discarded real user calls that raced with the flush window, and the async transport `.then()` sometimes fired outside the suppression scope entirely. Move the fix upstream: default `ignoreTurboModules` to `['RNSentry']`. Self-noise never enters the aggregator, so no suppression/drain gymnastics are needed. Users who want RNSentry stats can pass `ignoreTurboModules: []`. Also document the intentional data-loss trade-off when a transaction is dropped by `beforeSendTransaction` (bounded, self-healing on the next transaction or periodic flush). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
1 parent b5d6f2a commit 934a34a

5 files changed

Lines changed: 120 additions & 173 deletions

File tree

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

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

55
import { createSpanJSON } from '../tracing/utils';
66
import {
7-
beginSuppressFirstTurboModuleRecordCallback,
87
drainTurboModuleAggregate,
9-
endSuppressFirstTurboModuleRecordCallback,
108
HISTOGRAM_BUCKET_LABELS,
119
hasTurboModuleAggregateData,
1210
setAggregateRecordingEnabled,
@@ -74,9 +72,14 @@ export interface TurboModuleContextOptions {
7472
aggregateFlushIntervalMs?: number;
7573

7674
/**
77-
* TurboModules whose calls should NOT be counted in the aggregate. Users
78-
* may e.g. want to exclude `RNSentry` itself to keep the signal clean of
79-
* the SDK's own internal calls.
75+
* TurboModules whose calls should NOT be counted in the aggregate.
76+
*
77+
* Default: `['RNSentry']`. The SDK's own transport call
78+
* (`RNSentry.captureEnvelope`) fires from every `captureEvent`, so leaving
79+
* `RNSentry` in the aggregate would (a) pollute app-level TurboModule
80+
* signals with SDK internal noise and (b) allow the periodic flush's own
81+
* `captureEvent` to record back into the aggregator and perpetually re-arm
82+
* the flush timer in idle sessions. Pass `[]` to opt back in.
8083
*
8184
* Note: this does NOT disable wrapping — crashes during those calls still
8285
* get attributed via `contexts.turbo_module`. It only opts the module out
@@ -146,7 +149,7 @@ export const turboModuleContextIntegration = (options: TurboModuleContextOptions
146149

147150
setAggregateRecordingEnabled(enableAggregate);
148151
if (enableAggregate) {
149-
setIgnoredTurboModules(options.ignoreTurboModules);
152+
setIgnoredTurboModules(options.ignoreTurboModules ?? ['RNSentry']);
150153
}
151154
},
152155
setup(client: Client): void {
@@ -202,6 +205,13 @@ export const turboModuleContextIntegration = (options: TurboModuleContextOptions
202205
/**
203206
* Mutates a transaction event in place to add the aggregate breakdown as a
204207
* synthetic child span plus a few headline measurements on the root span.
208+
*
209+
* Draining here runs before `beforeSendTransaction`, so if a user hook drops
210+
* this transaction, the drained batch is lost. Trade-off is intentional:
211+
* peeking without draining would require send-confirmation bookkeeping across
212+
* events and multiple transactions in flight would double-count. Data loss
213+
* from a dropped transaction is bounded (one interval) and self-heals — the
214+
* next transaction or periodic flush picks up fresh activity.
205215
*/
206216
function attachAggregateToTransactionEvent(event: TransactionEvent): void {
207217
const trace = event.contexts?.trace;
@@ -268,16 +278,10 @@ function attachAggregateToTransactionEvent(event: TransactionEvent): void {
268278
* sessions without a transaction still produce a signal. No-op when there's
269279
* nothing to flush.
270280
*
271-
* The `captureEvent` call travels through `RNSentry.captureEnvelope`, which
272-
* is itself a wrapped TurboModule call. Without the suppression scope below,
273-
* the resulting empty→non-empty transition on the aggregator would re-arm
274-
* the flush timer indefinitely in an otherwise-idle session. The release is
275-
* deferred to the next macrotask so the async record fired from the
276-
* transport's `.then()` (after the native side resolves) also lands inside
277-
* the suppression window; the leftover entries are then drained so the next
278-
* real user call registers as an empty→non-empty transition and re-arms
279-
* the periodic timer (otherwise a suppressed record would poison the map
280-
* and the timer would never re-arm again).
281+
* `client.captureEvent` reaches wrapped `RNSentry.captureEnvelope` via the
282+
* native transport — so if `RNSentry` were aggregated, the flush's own send
283+
* would re-arm the lazy timer indefinitely. `ignoreTurboModules` defaults
284+
* to `['RNSentry']` for exactly this reason.
281285
*/
282286
function flushPeriodicAggregate(client: Client): void {
283287
if (!hasTurboModuleAggregateData()) {
@@ -287,35 +291,20 @@ function flushPeriodicAggregate(client: Client): void {
287291
const totals = summarise(snapshot);
288292
const topByTotalMs = [...snapshot].sort((a, b) => b.totalDurationMs - a.totalDurationMs);
289293

290-
beginSuppressFirstTurboModuleRecordCallback();
291-
try {
292-
client.captureEvent?.({
293-
message: 'TurboModule aggregate (periodic)',
294-
level: 'info',
295-
tags: {
296-
'event.kind': 'turbo_modules.aggregate',
297-
},
298-
extra: {
299-
total_call_count: totals.callCount,
300-
total_error_count: totals.errorCount,
301-
total_duration_ms: roundMs(totals.totalDurationMs),
302-
unique_methods: snapshot.length,
303-
modules: topByTotalMs.slice(0, MAX_AGGREGATE_ATTRIBUTE_ROWS).map(serialiseRowAsObject),
304-
},
305-
});
306-
} finally {
307-
setTimeout(() => {
308-
endSuppressFirstTurboModuleRecordCallback();
309-
// Discard whatever landed during the suppression window — those
310-
// records are (in the common case) our own transport's self-noise.
311-
// Leaving them in the aggregator would leave `size > 0`, so the
312-
// next real user call wouldn't be an empty→non-empty transition
313-
// and `onFirstRecordAfterEmpty` would never re-arm the timer.
314-
if (hasTurboModuleAggregateData()) {
315-
drainTurboModuleAggregate();
316-
}
317-
}, 0);
318-
}
294+
client.captureEvent?.({
295+
message: 'TurboModule aggregate (periodic)',
296+
level: 'info',
297+
tags: {
298+
'event.kind': 'turbo_modules.aggregate',
299+
},
300+
extra: {
301+
total_call_count: totals.callCount,
302+
total_error_count: totals.errorCount,
303+
total_duration_ms: roundMs(totals.totalDurationMs),
304+
unique_methods: snapshot.length,
305+
modules: topByTotalMs.slice(0, MAX_AGGREGATE_ATTRIBUTE_ROWS).map(serialiseRowAsObject),
306+
},
307+
});
319308
}
320309

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

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

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

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

Lines changed: 1 addition & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -57,10 +57,6 @@ 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;
6460
// When `false`, `recordTurboModuleCall` is a no-op. The integration flips
6561
// this off when `enableAggregateStats: false` so wrapped TurboModule calls
6662
// don't accumulate into a map that nothing ever drains.
@@ -161,7 +157,7 @@ export function recordTurboModuleCall(args: {
161157
// exists to satisfy `noUncheckedIndexedAccess`.
162158
entry.buckets[bucket] = (entry.buckets[bucket] ?? 0) + 1;
163159

164-
if (wasEmpty && onFirstRecordAfterEmpty && suppressFirstRecordCallbackDepth === 0) {
160+
if (wasEmpty && onFirstRecordAfterEmpty) {
165161
// Don't let a misbehaving observer corrupt the aggregate.
166162
try {
167163
onFirstRecordAfterEmpty();
@@ -171,30 +167,6 @@ export function recordTurboModuleCall(args: {
171167
}
172168
}
173169

174-
/**
175-
* Opens a suppression scope: while at least one is open, the empty→non-empty
176-
* callback registered via {@link setOnFirstTurboModuleRecord} is NOT invoked.
177-
* Records still land in the aggregate as normal.
178-
*
179-
* Used by the periodic flush to break a self-recursion loop: the flush's own
180-
* `captureEvent` call travels through `RNSentry.captureEnvelope`, which is a
181-
* wrapped TurboModule call and would otherwise re-arm the flush timer forever
182-
* in an otherwise-idle session.
183-
*/
184-
export function beginSuppressFirstTurboModuleRecordCallback(): void {
185-
suppressFirstRecordCallbackDepth += 1;
186-
}
187-
188-
/**
189-
* Closes a suppression scope opened by
190-
* {@link beginSuppressFirstTurboModuleRecordCallback}. Idempotent at zero.
191-
*/
192-
export function endSuppressFirstTurboModuleRecordCallback(): void {
193-
if (suppressFirstRecordCallbackDepth > 0) {
194-
suppressFirstRecordCallbackDepth -= 1;
195-
}
196-
}
197-
198170
/**
199171
* Registers a callback fired exactly once when the aggregator transitions
200172
* from empty to non-empty — i.e. when the first record after a drain (or
@@ -265,6 +237,5 @@ export function _resetTurboModuleAggregator(): void {
265237
aggregates.clear();
266238
ignoredModules.clear();
267239
onFirstRecordAfterEmpty = undefined;
268-
suppressFirstRecordCallbackDepth = 0;
269240
recordingEnabled = true;
270241
}

0 commit comments

Comments
 (0)