React Router browser tracing - Lazy imported routes with suspense start transaction spans with wrong path

[peteragarclover]

Is there an existing issue for this?

• I have checked for existing issues https://github.com/getsentry/sentry-javascript/issues
• I have reviewed the documentation https://docs.sentry.io/
• I am using the latest SDK release https://github.com/getsentry/sentry-javascript/releases

How do you use Sentry?

Self-hosted/on-premise

Which SDK are you using?

@sentry/react

SDK Version

8.47.0

Framework Version

React 18.3.1

Link to Sentry event

No response

Reproduction Example/SDK Setup

Init sentry with React Router browser tracing integration.

Sentry.init({
  dsn,
  release,
  environment,
  attachStacktrace: true,
  tracePropagationTargets,
  integrations: [
    Sentry.reactRouterV7BrowserTracingIntegration({
      useEffect,
      useLocation,
      useNavigationType,
      createRoutesFromChildren,
      matchRoutes,
    }),
  ],
  tracesSampleRate,
});

Configure React Router with Sentry wrapper. But Lazy import routes, e.g. something like:

// user routes bundle loaded lazily
const UserRoutes = lazy(() => import("./UserRoutes"));

const Router: FC = () => {
  const { user } = useContext(UserContext);

  const sentryCreateBrowserRouter: typeof createBrowserRouter =
    Sentry.wrapCreateBrowserRouterV7(createBrowserRouter);

  const router = sentryCreateBrowserRouter(
    createRoutesFromElements(
      <>
        <Route path="login" element={<Login />} />
        <Route
          path="*"
          element={
            user ? (
              // All other routes for logged in users
              <Suspense fallback={<LoadingPage />}>
                <UserRoutes />
              </Suspense>
            ) : (
              // All other routes when not logged in, just navigate back to login page
              <Navigate to="/login" />
            )
          }
        />
      </>,
    ),
  );

Steps to Reproduce

1. Configured React Router with Sentry Wrappers
2. Lazy load routes
3. Navigate to "/" (This initializes Sentry's route change subscriptions using routes available on initial page load)
4. Navigate to lazy loaded routes, e.g. "/profile"

Expected Result

Navigation transaction span started using path "/profile"

Actual Result

Navigation transaction span started using path "/*"

This appears to be because the routes are only extracted once on mount:

sentry-javascript/packages/react/src/reactrouterv6-compat-utils.tsx

Lines 602 to 631 in 9f74bc9

	_useEffect(
	() => {
	const routes = _createRoutesFromChildren(props.children) as RouteObject[];
	if (isMountRenderPass.current) {
	routes.forEach(route => {
	const extractedChildRoutes = getChildRoutesRecursively(route);
	extractedChildRoutes.forEach(r => {
	allRoutes.add(r);
	});
	});
	updatePageloadTransaction(getActiveRootSpan(), location, routes, undefined, undefined, Array.from(allRoutes));
	isMountRenderPass.current = false;
	} else {
	handleNavigation({
	location,
	routes,
	navigationType,
	version,
	allRoutes: Array.from(allRoutes),
	});
	}
	},
	// `props.children` is purposely not included in the dependency array, because we do not want to re-run this effect
	// when the children change. We only want to start transactions when the location or navigation type change.
	[location, navigationType],
	);

And even though additional routes are lazy loaded, and the route name is correctly passed from the navigation event, when the name is normalized, it can't find the route, so falls back to the parent catch-all "/*"

sentry-javascript/packages/react/src/reactrouterv6-compat-utils.tsx

Lines 333 to 342 in 9f74bc9

	const isInDescendantRoute = locationIsInsideDescendantRoute(location, allRoutes || routes);
	if (isInDescendantRoute) {
	name = prefixWithSlash(rebuildRoutePathFromAllRoutes(allRoutes || routes, location));
	source = 'route';
	}
	if (!isInDescendantRoute || !name) {
	[name, source] = getNormalizedName(routes, location, branches, basename);
	}

[s1gr1d]

Hello, thanks for writing in and already trying to investigate this! We'll take a look at this.

However, if you are interested in contributing, feel free to let me know and open a PR! :)

Just for clarification: the first link to the code you posted is probably not the reason, as this is not the function which is used when setting up the routes with JSX. This would be this function:

sentry-javascript/packages/react/src/reactrouterv6-compat-utils.tsx

Line 67 in 8e53415

export function createV6CompatibleWrapCreateBrowserRouter<

[onurtemizkan]

Thanks for the issue report @peteragarclover!

#15039 will add support for lazy-loaded components/pages on navigations.

However, adding support for lazy-loaded routes is a bit more tricky, as we are currently unable to access the lazy-loaded sub-Routes, without breaking lazy-loading.

I opened an issue to discuss how we can implement that - #15040

[peteragarclover]

Thank you so much for the prompt update on this team 😍. Keen to try it out. Can I ask what release to expect this in? Will there be a minor, or should I wait for 9?

Also, I didn't spot any documentation updates in the PR, but am I right in that the only change I will need to make, is for each Routes that is loaded lazily, they need to be wrapped like this?

const SentryRoutes = Sentry.withSentryReactRouterV6Routing(Routes);

sentry-javascript/dev-packages/e2e-tests/test-applications/react-create-browser-router/src/pages/LazyLoadedUser.tsx

Lines 5 to 10 in 2e708dc

	const SentryRoutes = Sentry.withSentryReactRouterV6Routing(Routes);
	const InnerRoute = React.lazy(() => import('./LazyLoadedInnerRoute'));
	const LazyLoadedUser = () => {
	return (
	<SentryRoutes>

[peteragarclover]

Actually, I'm a little confused by the test examples. LazyLoadedUser is already lazy loaded with a dynamic import

sentry-javascript/dev-packages/e2e-tests/test-applications/react-create-browser-router/src/index.tsx

Line 45 in 2e708dc

const LazyLoadedUser = lazy(() => import('./pages/LazyLoadedUser'));

So, does the InnerRoute also need to be lazy loaded, or is that just a deeper test case? 🤔

Ideally for us, it would just be LazyLoadedUser that was lazy imported, since that will result in a single bundle. No need for bundle splitting to create bundles for each individual inner route within LazyLoadedUser... but maybe I am completely misunderstanding 😅

[onurtemizkan]

@peteragarclover, that's just a deeper test case, making sure we're not losing the context in nested descendant lazy-routes.

It should work out of the box for your case, without any configuration updates.

Please let us know if you bump into any further issues.