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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
- Fix `TypeError` when `showFeedbackForm`/`showFeedbackButton`/`showScreenshotButton` is called before `FeedbackFormProvider` mounts ([#6435](https://github.com/getsentry/sentry-react-native/pull/6435))
- Fix orphaned TTID/TTFD spans in the trace view ([#6437](https://github.com/getsentry/sentry-react-native/pull/6437))
- Fix iOS retain cycle in `RNSentryOnDrawReporterView` leaking TTID/TTFD reporter views and their frame-tracker listeners ([#6449](https://github.com/getsentry/sentry-react-native/pull/6449))
- Fix `expoRouterIntegration` bailing out on cold start when `Sentry.init()` runs before Expo Router's Root Layout mounts โ€” the `store.navigationRef` check is now retried until both the ref and its `.current` are populated ([#6451](https://github.com/getsentry/sentry-react-native/pull/6451))
- Fix `reactNavigationIntegration` reading a stale route from an override provider (e.g. `expoRouterIntegration`), causing navigation transactions to be named/attributed for the previous route ([#6458](https://github.com/getsentry/sentry-react-native/pull/6458))

### Internal
Expand Down
65 changes: 32 additions & 33 deletions packages/core/src/js/tracing/expoRouterIntegration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,49 +37,48 @@ export const expoRouterIntegration = (options: ExpoRouterIntegrationOptions = {}
// expo-router not installed
return;
}
if (!store.navigationRef) {
debug.warn(
`${INTEGRATION_NAME} Found expo-router router-store but it does not expose a \`navigationRef\`. ` +
`This likely means the installed expo-router version is incompatible with this integration.`,
);
return;
}

// reuse the user's reactNavigationIntegration if they registered one manually.
// Otherwise, create and add one.
let reactNavigation = getReactNavigationIntegration(client);
if (!reactNavigation) {
reactNavigation = reactNavigationIntegration(options);
client.addIntegration(reactNavigation);
}

const navigationRef = store.navigationRef;

reactNavigation._setRouteOverrideProvider?.(() => buildExpoRouterRouteOverride(store));

if (navigationRef.current) {
reactNavigation.registerNavigationContainer(navigationRef);
return;
}

// Otherwise, poll until the Root Layout mounts and Expo Router sets `.current`.
// `Sentry.init()` is typically called at module eval time (as the docs recommend),
// which runs before Expo Router's Root Layout mounts. Both `store.navigationRef` and
// `navigationRef.current` may still be undefined here โ€” poll for both to appear.
// Defer adding `reactNavigationIntegration` until we can also register it, so a
// timeout can never leave a non-functional integration attached to the client.
const startedAt = Date.now();

const poll = (): void => {
if (!navigationRef.current) {
if (Date.now() - startedAt >= POLL_MAX_DURATION_MS) {
const navigationRef = store.navigationRef;

if (navigationRef?.current) {
// Reuse the user's reactNavigationIntegration if they registered one manually.
// Otherwise, create and add one.
const existing = getReactNavigationIntegration(client);
const reactNavigation = existing ?? reactNavigationIntegration(options);
if (!existing) {
client.addIntegration(reactNavigation);
}
reactNavigation._setRouteOverrideProvider?.(() => buildExpoRouterRouteOverride(store));
reactNavigation.registerNavigationContainer(navigationRef);
pollTimer = undefined;
return;
}

if (Date.now() - startedAt >= POLL_MAX_DURATION_MS) {
if (!navigationRef) {
debug.warn(
`${INTEGRATION_NAME} Found expo-router router-store but it does not expose a \`navigationRef\`. ` +
`This likely means the installed expo-router version is incompatible with this integration.`,
);
} else {
debug.warn(`${INTEGRATION_NAME} Timed out waiting for Expo Router navigation container.`);
pollTimer = undefined;
return;
}
pollTimer = setTimeout(poll, POLL_INTERVAL_MS);
pollTimer = undefined;
return;
Comment thread
alwx marked this conversation as resolved.
}

reactNavigation?.registerNavigationContainer(navigationRef);
pollTimer = undefined;
pollTimer = setTimeout(poll, POLL_INTERVAL_MS);
};

pollTimer = setTimeout(poll, POLL_INTERVAL_MS);
poll();

client.on('close', () => {
if (pollTimer !== undefined) {
Expand Down
51 changes: 47 additions & 4 deletions packages/core/test/tracing/expoRouterIntegration.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ describe('expoRouterIntegration', () => {
});

describe('expo-router router-store found but navigationRef missing', () => {
it('warns and does not add the integration', () => {
it('does not add the integration and warns after the timeout', () => {
jest.doMock(EXPO_ROUTER_STORE_MODULE, () => ({ store: {} }), { virtual: true });

const { debug } = require('@sentry/core');
Expand All @@ -79,11 +79,49 @@ describe('expoRouterIntegration', () => {
const integ = integration();
integ.afterAllSetup?.(client);

// No warning yet โ€” we keep polling in case the ref shows up later.
expect(warnSpy).not.toHaveBeenCalled();
expect(addIntegration).not.toHaveBeenCalled();

jest.advanceTimersByTime(6_000);

expect(addIntegration).not.toHaveBeenCalled();
expect(warnSpy).toHaveBeenCalledWith(expect.stringContaining('navigationRef'));

warnSpy.mockRestore();
});

it('picks up navigationRef when it appears mid-poll (issue #6431)', () => {
const container = createMockNavigationContainer();
const store: { navigationRef?: { current: MockNavigationContainer | null } } = {};
jest.doMock(EXPO_ROUTER_STORE_MODULE, () => ({ store }), { virtual: true });

const { expoRouterIntegration: integration } = require('../../src/js/tracing/expoRouterIntegration');
const { client, addIntegration } = createMockClient();

const integ = integration();
integ.afterAllSetup?.(client);

// Nothing to attach to yet โ€” reactNavigation must not be added until we see a ref.
expect(addIntegration).not.toHaveBeenCalled();

// Root Layout mounts a bit later: navigationRef appears, then .current gets populated.
jest.advanceTimersByTime(200);
store.navigationRef = { current: null };
jest.advanceTimersByTime(50);

// navigationRef exists but .current is still null โ€” hold off on attaching,
// otherwise a subsequent timeout would leave a non-functional integration behind.
expect(addIntegration).not.toHaveBeenCalled();
expect(container.addListener).not.toHaveBeenCalled();

store.navigationRef.current = container;
jest.advanceTimersByTime(50);

expect(addIntegration).toHaveBeenCalledTimes(1);
expect(container.addListener).toHaveBeenCalledWith('__unsafe_action__', expect.any(Function));
expect(container.addListener).toHaveBeenCalledWith('state', expect.any(Function));
});
});

describe('expo-router installed, navigationRef pre-populated', () => {
Expand Down Expand Up @@ -130,18 +168,20 @@ describe('expoRouterIntegration', () => {
const integ = integration();
integ.afterAllSetup?.(client);

// nothing registered yet
// nothing attached yet โ€” .current is still null
expect(container.addListener).not.toHaveBeenCalled();
expect(addIntegration).toHaveBeenCalledTimes(1);
expect(addIntegration).not.toHaveBeenCalled();

// tick the polling timer once before ref is populated โ€” still no registration
jest.advanceTimersByTime(50);
expect(container.addListener).not.toHaveBeenCalled();
expect(addIntegration).not.toHaveBeenCalled();

// populate the ref and tick again
navigationRef.current = container;
jest.advanceTimersByTime(50);

expect(addIntegration).toHaveBeenCalledTimes(1);
expect(container.addListener).toHaveBeenCalledWith('__unsafe_action__', expect.any(Function));
expect(container.addListener).toHaveBeenCalledWith('state', expect.any(Function));
});
Expand All @@ -157,7 +197,7 @@ describe('expoRouterIntegration', () => {
);

const { expoRouterIntegration: integration } = require('../../src/js/tracing/expoRouterIntegration');
const { client, closeHandlers } = createMockClient();
const { client, addIntegration, closeHandlers } = createMockClient();

const integ = integration();
integ.afterAllSetup?.(client);
Expand All @@ -167,6 +207,9 @@ describe('expoRouterIntegration', () => {

expect(jest.getTimerCount()).toBeLessThan(timersAfterSetup);
expect(closeHandlers.length).toBe(1);
// navigationRef existed the whole time but .current never populated โ€” we must
// not leave a `reactNavigationIntegration` attached that was never registered.
expect(addIntegration).not.toHaveBeenCalled();
});
});

Expand Down
Loading