Skip to content
Open
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
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,12 @@
> make sure you follow our [migration guide](https://docs.sentry.io/platforms/react-native/migration/) first.
<!-- prettier-ignore-end -->

## Unreleased

### Fixes

- Fix spurious duplicate navigation transaction created by Expo Router's `withAnchor` bookkeeping `POP_TO` dispatch when `useDispatchedActionData` is enabled ([#6472](https://github.com/getsentry/sentry-react-native/pull/6472))

## 8.19.0

### Features
Expand Down
73 changes: 67 additions & 6 deletions packages/core/src/js/tracing/reactnavigation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -261,7 +261,8 @@ export const reactNavigationIntegration = ({
const spanDispatchSeq = new WeakMap<Span, number>();

/**
* Attempts to attach the pending deep link to the given span.
* Attempts to attach the pending deep link to the given span. Returns `true`
* when the link was attached.
*
* Warm-open links only attach to spans dispatched *after* the link was
* received. This prevents an unrelated, already-in-flight navigation from
Expand All @@ -274,22 +275,22 @@ export const reactNavigationIntegration = ({
* Rejected warm-open links are left in the slot to be picked up by the next
* eligible span.
*/
const applyPendingDeepLinkToSpan = (span: Span, maxAgeMs: number): void => {
const applyPendingDeepLinkToSpan = (span: Span, maxAgeMs: number): boolean => {
const pending = peekPendingDeepLink(maxAgeMs);
if (!pending) {
return;
return false;
}
if (pending.source === 'warm-open') {
const spanSeq = spanDispatchSeq.get(span);
if (spanSeq === undefined || spanSeq <= pending.seq) {
// Span was dispatched before (or at the same tick as) the link arrived
// โ€” it cannot be the navigation the link triggered. Leave the link in
// the slot for the next eligible span.
return;
return false;
}
}
consumePendingDeepLink(maxAgeMs);
tagSpanWithDeepLink(span, pending);
return tagSpanWithDeepLink(span, pending);
};

/**
Expand Down Expand Up @@ -442,6 +443,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 +564,23 @@ export const reactNavigationIntegration = ({
return;
}

// A POP_TO whose target route is already focused is not a user-facing
// navigation โ€” it's a params-only bookkeeping dispatch. Expo Router emits
// exactly this 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 processed in a deferred microtask, see #6436) or
// ship a spurious duplicate transaction that steals the real navigation's
// child spans. A genuine `popTo` is safe from this filter: its target is
// a route *behind* the focused one, and `__unsafe_action__` fires before
// the state change is applied, so the target is never focused here.
if (navigationActionType === 'POP_TO' && targetRouteName && isRouteFocused(targetRouteName)) {
debug.log(
`${INTEGRATION_NAME} POP_TO targets the already focused route ${targetRouteName}, not starting navigation span.`,
);
return;
}

// Extract route name from dispatch action payload when available
const dispatchedRouteName = useDispatchedActionData ? targetRouteName : undefined;
if (useDispatchedActionData && event && !dispatchedRouteName && !isAppRestart) {
Expand Down Expand Up @@ -661,10 +705,27 @@ export const reactNavigationIntegration = ({
// deep link (e.g. deep-linking to the screen you're already on). Make
// sure the pending link still gets attributed before we drop the span
// reference.
applyPendingDeepLinkToSpan(latestNavigationSpan, routeChangeTimeoutMs);
const deepLinkAttached = applyPendingDeepLinkToSpan(latestNavigationSpan, routeChangeTimeoutMs);
pushRecentRouteKey(route.key);
latestRoute = route;

// A POP_TO that landed on the route we were already on was a
// params-only bookkeeping dispatch (e.g. Expo Router's `withAnchor`
// anchor stamping) that slipped past the dispatch-time filter โ€” for
// example 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 a deep link claimed it above.
if (
!deepLinkAttached &&
spanToJSON(latestNavigationSpan).data?.[SEMANTIC_ATTRIBUTE_NAVIGATION_ACTION_TYPE] === 'POP_TO'
) {
debug.log(`[${INTEGRATION_NAME}] Discarding POP_TO navigation span that did not change the route.`);
clearStateChangeTimeout();
_discardLatestTransaction();
return undefined;
}

// Clear the latest transaction as it has been handled.
latestNavigationSpan = undefined;
return undefined;
Expand Down
201 changes: 200 additions & 1 deletion packages/core/test/tracing/reactnavigation.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,11 @@ import { mockAppRegistryIntegration } from '../mocks/appRegistryIntegrationMock'
import { getDefaultTestClientOptions, TestClient } from '../mocks/client';
import { NATIVE } from '../mockWrapper';
import { getDevServer } from './../../src/js/integrations/debugsymbolicatorutils';
import { createMockNavigationAndAttachTo, createMockNavigationWithNestedState } from './reactnavigationutils';
import {
createMockNavigationAndAttachTo,
createMockNavigationWithNestedState,
MockNavigationContainerWithState,
} from './reactnavigationutils';

const dummyRoute = {
name: 'Route',
Expand Down Expand Up @@ -1436,6 +1440,141 @@ describe('ReactNavigationInstrumentation', () => {
expect(client.event?.transaction).toBe('TabScreen');
});

test('POP_TO action to a different route creates a named span', async () => {
mockNavigation.emitWithStateChange(
{
data: {
action: {
type: 'POP_TO',
payload: { name: 'ScreenB' },
},
noop: false,
stack: undefined,
},
},
{ key: 'screen_b', name: 'ScreenB' },
);
jest.runOnlyPendingTimers();

await client.flush();

expect(client.event?.transaction).toBe('ScreenB');
});

test('POP_TO action targeting the focused route does not create a navigation span', async () => {
// Real navigation to New Screen.
mockNavigation.emitWithStateChange(
{
data: {
action: {
type: 'NAVIGATE',
payload: { name: 'New Screen' },
},
noop: false,
stack: undefined,
},
},
{ key: 'new_screen', name: 'New Screen' },
);
await jest.runOnlyPendingTimersAsync();
await client.flush();
expect(client.event?.transaction).toBe('New Screen');
client.event = undefined;

// Expo Router `withAnchor` bookkeeping dispatch: POP_TO stamping
// `initial: false` onto the route that is already focused.
mockNavigation.emitWithStateChange({
data: {
action: {
type: 'POP_TO',
payload: { name: 'New Screen', params: { initial: false } },
},
noop: false,
stack: undefined,
},
});
await jest.runOnlyPendingTimersAsync();
await client.flush();

expect(client.event).toBeUndefined();
});

test('POP_TO action targeting the focused route does not remove the in-flight navigation span', async () => {
mockNavigation.emitNavigationWithoutStateChange();
const activeSpan = getActiveSpan();

mockNavigation.emitWithoutStateChange({
data: {
action: {
type: 'POP_TO',
payload: { name: 'Initial Screen', params: { initial: false } },
},
noop: false,
stack: undefined,
},
});

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

test('POP_TO span that did not change the route is discarded', async () => {
// Payload name matches no focused route (e.g. a nested navigator name),
// so the dispatch-time filter misses, but the state change lands on the
// same route key โ€” the span must be discarded, not left running.
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).toBeUndefined();
});

test('POP_TO span that did not change the route is kept when it carries a deep link', async () => {
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('NAVIGATE landing on the same route still creates a span', async () => {
mockNavigation.emitWithStateChange({
data: {
action: {
type: 'NAVIGATE',
payload: { name: 'Initial Screen' },
},
noop: false,
stack: undefined,
},
});
await jest.runOnlyPendingTimersAsync();
await client.flush();

expect(client.event?.transaction).toBe('Initial Screen');
});

test('cancelled navigation with dispatched route name is discarded', async () => {
mockNavigation.emitWithoutStateChange({
data: {
Expand Down Expand Up @@ -1483,6 +1622,66 @@ describe('ReactNavigationInstrumentation', () => {
});
});

test('POP_TO targeting a focused parent navigator route does not create a navigation span', async () => {
// Expo Router's `withAnchor` bookkeeping POP_TO carries the route name of
// the divergent navigator level, which is often a parent of the leaf
// route โ€” the focused-route check must walk the whole focused chain.
const rNavigation = reactNavigationIntegration({
routeChangeTimeoutMs: 200,
useDispatchedActionData: true,
});

const container = new MockNavigationContainerWithState();
container.currentRoute = { key: 'screen_b', name: 'screenB' };
container.currentState = {
index: 0,
routes: [
{
name: '(app)',
key: 'app',
state: {
index: 1,
routes: [
{ name: 'screenA', key: 'screen_a' },
{ name: 'screenB', key: 'screen_b' },
],
},
},
],
};

const options = getDefaultTestClientOptions({
enableNativeFramesTracking: false,
enableStallTracking: false,
tracesSampleRate: 1.0,
integrations: [rNavigation, reactNativeTracingIntegration()],
enableAppStartTracking: false,
});
client = new TestClient(options);
setCurrentClient(client);
client.init();
rNavigation.registerNavigationContainer(container);

await jest.runOnlyPendingTimersAsync(); // Flush the initial navigation span
client.event = undefined;

container.listeners['__unsafe_action__']({
data: {
action: {
type: 'POP_TO',
payload: { name: '(app)', params: { initial: false } },
},
noop: false,
stack: undefined,
},
});
container.listeners['state']({});
await jest.runOnlyPendingTimersAsync();
await client.flush();

expect(client.event).toBeUndefined();
});

describe('useDispatchedActionData disabled (default) still uses generic span name', () => {
beforeEach(async () => {
setupTestClient({ useDispatchedActionData: false });
Expand Down
Loading