Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,10 @@

When a root span ends (idle nav spans from `reactNavigationIntegration` / `expoRouterIntegration`, or a user's own `Sentry.startSpan(...)`), the integration writes `turbo_module.<name>.<method>.{call_count,duration_ms,error_count}` attributes plus summary keys (`turbo_module.total_call_count`, `turbo_module.total_duration_ms`, `turbo_module.top_module`). Async calls above `slowCallThresholdMs` (default 500ms) additionally record a `native.turbo_module` breadcrumb. Both surfaces enabled by default; new knobs `enableSpanAttribution`, `slowCallThresholdMs`, `maxTopModulesPerSpan` on `turboModuleContextIntegration`.

### Fixes

- Fix duplicate navigation transaction on Expo Router `withAnchor` navigations ([#6439](https://github.com/getsentry/sentry-react-native/pull/6439))

### Changes

- Expose `instrumentStateGraph` for manual LangGraph instrumentation ([#6520](https://github.com/getsentry/sentry-react-native/pull/6520))
Expand Down
84 changes: 84 additions & 0 deletions packages/core/src/js/tracing/reactnavigation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -237,6 +237,10 @@ export const reactNavigationIntegration = ({

let latestNavigationSpan: Span | undefined;
let latestNavigationSpanNameCustomized: boolean = false;
// Action type of the dispatch that started `latestNavigationSpan`. Tracked
// independently of the (flag-gated) span attribute so the same-route discard
// works even when `useDispatchedActionData` is off.
let latestNavigationActionType: string | undefined;
let navigationProcessingSpan: Span | undefined;
/**
* The first nav span that successfully completed a state change — i.e. the
Expand Down Expand Up @@ -442,6 +446,32 @@ export const reactNavigationIntegration = ({
initialStateHandled = true;
};

/**
* Returns `true` when the given route name is focused at some level of the
* current navigation state — i.e. it is on the chain of active routes from
* the root navigator down to the leaf. Falls back to comparing against the
* leaf route only when the full state is not available.
*/
const isRouteFocused = (routeName: string): boolean => {
try {
const rootState = navigationContainer?.getState();
let currentState: NavigationState | undefined = rootState;
while (currentState) {
const route: NavigationRoute | undefined = currentState.routes[currentState.index ?? 0];
if (route?.name === routeName) {
return true;
}
currentState = route?.state;
}
if (!rootState) {
return navigationContainer?.getCurrentRoute()?.name === routeName;
}
} catch (e) {
debug.warn(`${INTEGRATION_NAME} Failed to read navigation state to check focused route.`, e);
}
return false;
};

/**
* To be called on every React-Navigation action dispatch.
* It does not name the transaction or populate it with route information. Instead, it waits for the state to fully change
Expand Down Expand Up @@ -537,6 +567,37 @@ export const reactNavigationIntegration = ({
return;
}

// A `POP_TO` that both targets an already-focused route AND carries the
// `withAnchor` marker (`params.initial === false`) is not a user-facing
// navigation — it's Expo Router's bookkeeping dispatch, emitted right after
// a `withAnchor` navigation purely to stamp `initial: false` onto the
// destination it just navigated to. Starting a span here would either
// discard the real navigation's in-flight span (state changes are applied
// in a deferred microtask, see #6436) or ship a spurious duplicate
// transaction that steals the real navigation's child spans (#6434).
//
// Requiring the `initial === false` marker (only ever set by `withAnchor`,
// see expo-router's `getNavigationAction`) is what keeps a genuine `popTo`
// to an earlier *same-named* route in the stack (e.g. `[id]` → `[id]`) from
// being wrongly skipped: those carry no `initial` param. The `route.key`
// check in `updateLatestNavigationSpanWithCurrentRoute` backstops any
// bookkeeping dispatch that slips past this filter.
// Driven off `actionType` (parsed unconditionally), not `navigationActionType`,
// so it also applies to the default `expoRouterIntegration` setup where
// `useDispatchedActionData` is off — that is where this bug was reported.
const popToParams = (event?.data?.action?.payload as { params?: { initial?: boolean } } | undefined)?.params;
if (
actionType === 'POP_TO' &&
targetRouteName &&
popToParams?.initial === false &&
isRouteFocused(targetRouteName)
) {
debug.log(
`${INTEGRATION_NAME} POP_TO targets the already focused route ${targetRouteName}, not starting navigation span.`,
);
return;
}
Comment thread
cursor[bot] marked this conversation as resolved.

// Extract route name from dispatch action payload when available
const dispatchedRouteName = useDispatchedActionData ? targetRouteName : undefined;
if (useDispatchedActionData && event && !dispatchedRouteName && !isAppRestart) {
Expand All @@ -562,6 +623,7 @@ export const reactNavigationIntegration = ({
latestNavigationSpanNameCustomized = finalSpanOptions.name !== originalName;

latestNavigationSpan = startGenericIdleNavigationSpan(finalSpanOptions, { ...idleSpanOptions, isAppRestart });
latestNavigationActionType = actionType;
latestNavigationSpan?.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, SPAN_ORIGIN_AUTO_NAVIGATION_REACT_NAVIGATION);
latestNavigationSpan?.setAttribute(SEMANTIC_ATTRIBUTE_NAVIGATION_ACTION_TYPE, navigationActionType);

Expand Down Expand Up @@ -665,6 +727,24 @@ export const reactNavigationIntegration = ({
pushRecentRouteKey(route.key);
latestRoute = route;

// A `POP_TO` span that landed on the route we were already on (same
// `route.key`) was a params-only bookkeeping dispatch — e.g. Expo
// Router's `withAnchor` anchor stamping — that slipped past the
// dispatch-time filter (for instance a nested destination whose payload
// name matches none of the focused route names). Letting the span run
// would ship a spurious duplicate transaction that collects the real
// navigation's in-flight child spans, so discard it unless it carries a
// deep link (#6434). We check `taggedDeepLinkSpans` rather than the
// just-attached result so a span tagged earlier via the synchronous
// late-arrival listener is also preserved. Keyed on `route.key`, this
// never affects a genuine navigation that actually changed the route.
if (!taggedDeepLinkSpans.has(latestNavigationSpan) && latestNavigationActionType === 'POP_TO') {
debug.log(`[${INTEGRATION_NAME}] Discarding POP_TO navigation span that did not change the route.`);
clearStateChangeTimeout();
_discardLatestTransaction();
Comment thread
antonis marked this conversation as resolved.
return undefined;
Comment thread
antonis marked this conversation as resolved.
}

// Clear the latest transaction as it has been handled.
latestNavigationSpan = undefined;
return undefined;
Expand Down Expand Up @@ -777,6 +857,10 @@ export const reactNavigationIntegration = ({
latestNavigationSpan = undefined;
}
if (navigationProcessingSpan) {
// End before dropping the reference so we don't leave an unfinished span
// dangling. It is a child of the (now discarded) navigation span, so it
// is dropped with the transaction rather than sent on its own.
navigationProcessingSpan.end();
navigationProcessingSpan = undefined;
}
};
Expand Down
174 changes: 174 additions & 0 deletions packages/core/test/tracing/reactnavigation.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import {
} from '@sentry/core';

import type { NavigationRoute } from '../../src/js/tracing/reactnavigation';
import type { UnsafeAction } from '../../src/js/vendor/react-navigation/types';

import { nativeFramesIntegration, reactNativeTracingIntegration } from '../../src/js';
import { SPAN_ORIGIN_AUTO_NAVIGATION_REACT_NAVIGATION } from '../../src/js/tracing/origin';
Expand Down Expand Up @@ -1277,6 +1278,179 @@ describe('ReactNavigationInstrumentation', () => {
});
});

describe('withAnchor POP_TO bookkeeping dispatch (#6434)', () => {
// `router.dismissTo('/ScreenB', { withAnchor: true })` navigates to ScreenB
// (a real POP_TO) and, on some expo-router versions, immediately issues a
// second POP_TO to the same route purely to stamp `initial: false`. That
// bookkeeping dispatch carries no state change of its own and must not
// produce a second, spurious navigation transaction.
const realPopTo: UnsafeAction = {
data: {
action: { type: 'POP_TO', payload: { name: 'ScreenB', params: {} } },
noop: false,
stack: undefined,
},
};
const bookkeepingPopTo: UnsafeAction = {
data: {
action: { type: 'POP_TO', payload: { name: 'ScreenB', params: { initial: false } } },
noop: false,
stack: undefined,
},
};

function countTransactionsNamed(name: string): number {
return client.eventQueue.filter(e => e.type === 'transaction' && e.transaction === name).length;
}

test('does not create a second transaction for the bookkeeping dispatch', async () => {
setupTestClient({ useDispatchedActionData: true });
await jest.runOnlyPendingTimers(); // Flush the initial navigation span
client.eventQueue = [];

// Real navigation to ScreenB — resolves via a state change.
mockNavigation.emitWithStateChange(realPopTo, { key: 'screen_b', name: 'ScreenB' });
await jest.runOnlyPendingTimersAsync();

// withAnchor bookkeeping POP_TO to the same route. It only flips
// `params.initial`, which React Navigation reports as a state change to
// the same route key.
mockNavigation.emitWithStateChange(bookkeepingPopTo, { key: 'screen_b', name: 'ScreenB' });
await jest.runOnlyPendingTimersAsync();
await client.flush();

expect(countTransactionsNamed('ScreenB')).toBe(1);
});

test('does not create a second transaction with useDispatchedActionData disabled (default Expo setup)', async () => {
// The guards are driven off `actionType`, not the opt-in flag, so the
// default `expoRouterIntegration` setup (flag off) is fixed too.
setupTestClient({ useDispatchedActionData: false });
await jest.runOnlyPendingTimers();
client.eventQueue = [];

mockNavigation.emitWithStateChange(realPopTo, { key: 'screen_b', name: 'ScreenB' });
await jest.runOnlyPendingTimersAsync();

mockNavigation.emitWithStateChange(bookkeepingPopTo, { key: 'screen_b', name: 'ScreenB' });
await jest.runOnlyPendingTimersAsync();
await client.flush();

expect(client.eventQueue.filter(e => e.type === 'transaction').length).toBe(1);
});

test('still creates a transaction for a genuine popTo navigation', async () => {
setupTestClient({ useDispatchedActionData: true });
await jest.runOnlyPendingTimers(); // Flush the initial navigation span
client.eventQueue = [];

// A real dismissTo/popTo to a different route must still be traced.
mockNavigation.emitWithStateChange(realPopTo, { key: 'screen_b', name: 'ScreenB' });
await jest.runOnlyPendingTimersAsync();
await client.flush();

expect(countTransactionsNamed('ScreenB')).toBe(1);
});

test('still creates a span for a popTo to an earlier same-named route in the stack', async () => {
// Stack with a repeated route name (e.g. Expo Router `[id]`): on Detail(#2),
// popTo the earlier Detail(#1). The target name matches the focused leaf
// name, but the bookkeeping marker (`initial: false`) is absent, so this
// real, key-changing navigation must NOT be filtered.
setupTestClient({ useDispatchedActionData: true });
await jest.runOnlyPendingTimers();
client.eventQueue = [];

mockNavigation.emitWithStateChange(
{ data: { action: { type: 'PUSH', payload: { name: 'Detail' } }, noop: false, stack: undefined } },
{ key: 'detail_2', name: 'Detail' },
);
await jest.runOnlyPendingTimersAsync();
client.eventQueue = [];

mockNavigation.emitWithStateChange(
{ data: { action: { type: 'POP_TO', payload: { name: 'Detail' } }, noop: false, stack: undefined } },
{ key: 'detail_1', name: 'Detail' },
);
await jest.runOnlyPendingTimersAsync();
await client.flush();

expect(countTransactionsNamed('Detail')).toBe(1);
});

test('discards a bookkeeping POP_TO that slips past the dispatch-time filter (same route key)', async () => {
// Payload name matches no focused route (e.g. a parent navigator name),
// so the dispatch-time filter misses, but the state change lands on the
// same route key — the state-change guard must discard the span.
setupTestClient({ useDispatchedActionData: true });
await jest.runOnlyPendingTimers();
client.eventQueue = [];

mockNavigation.emitWithStateChange({
data: {
action: { type: 'POP_TO', payload: { name: '(app)', params: { initial: false } } },
noop: false,
stack: undefined,
},
});
await jest.runOnlyPendingTimersAsync();
await client.flush();

expect(client.eventQueue.filter(e => e.type === 'transaction').length).toBe(0);
});

test('keeps a same-route POP_TO span that carries a deep link', async () => {
// A same-route POP_TO that is actually a deep link to the screen you are
// already on must NOT be discarded by the bookkeeping guard.
setupTestClient({ useDispatchedActionData: true });
await jest.runOnlyPendingTimers();
client.eventQueue = [];

setPendingDeepLink('myapp://initial-screen', 'warm-open');
mockNavigation.emitWithStateChange({
data: {
action: { type: 'POP_TO', payload: { name: '(app)', params: { initial: false } } },
noop: false,
stack: undefined,
},
});
await jest.runOnlyPendingTimersAsync();
await client.flush();

expect(client.event?.contexts?.trace?.data?.['navigation.trigger']).toBe('deeplink');
clearPendingDeepLink();
});

test('a focused-route bookkeeping POP_TO does not tear down an in-flight navigation span', async () => {
setupTestClient({ useDispatchedActionData: true });
await jest.runOnlyPendingTimers();

// Start a real, still-in-flight navigation (dispatch with a route name in
// the payload, but no state change yet).
mockNavigation.emitWithoutStateChange({
data: {
action: { type: 'NAVIGATE', payload: { name: 'New Screen' } },
noop: false,
stack: undefined,
},
});
const activeSpan = getActiveSpan();
expect(activeSpan).toBeDefined();

// The withAnchor bookkeeping POP_TO to the already-focused route must not
// discard that in-flight span via the "noop transaction" branch.
mockNavigation.emitWithoutStateChange({
data: {
action: { type: 'POP_TO', payload: { name: 'Initial Screen', params: { initial: false } } },
noop: false,
stack: undefined,
},
});

expect(getActiveSpan()).toBe(activeSpan);
});
});

test('noop does not remove the previous navigation span from scope', async () => {
setupTestClient({ useDispatchedActionData: true });
Comment thread
antonis marked this conversation as resolved.
await jest.runOnlyPendingTimers(); // Flushes the initial navigation span
Expand Down
Loading