diff --git a/CHANGELOG.md b/CHANGELOG.md index f444667332..781f0e6865 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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 diff --git a/packages/core/src/js/tracing/expoRouterIntegration.ts b/packages/core/src/js/tracing/expoRouterIntegration.ts index 58df6f0056..7b2ceba316 100644 --- a/packages/core/src/js/tracing/expoRouterIntegration.ts +++ b/packages/core/src/js/tracing/expoRouterIntegration.ts @@ -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; } - reactNavigation?.registerNavigationContainer(navigationRef); - pollTimer = undefined; + pollTimer = setTimeout(poll, POLL_INTERVAL_MS); }; - pollTimer = setTimeout(poll, POLL_INTERVAL_MS); + poll(); client.on('close', () => { if (pollTimer !== undefined) { diff --git a/packages/core/test/tracing/expoRouterIntegration.test.ts b/packages/core/test/tracing/expoRouterIntegration.test.ts index 13d1d7cc3a..4e30b2eb58 100644 --- a/packages/core/test/tracing/expoRouterIntegration.test.ts +++ b/packages/core/test/tracing/expoRouterIntegration.test.ts @@ -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'); @@ -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', () => { @@ -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)); }); @@ -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); @@ -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(); }); });