fix(remix): Avoid rewrapping root loader. (#16136)

Resolves: #16075

We used to allow rewrapping of the root loader when we had a separate
Express adapter, where we could not determine the instrumentation order.
Now that we don't have it anymore, we can remove this. Still, can't
reproduce the original issue running e2e test apps under load. But it's
clear that's what caused it.

Author: onurtemizkan

dev-packages/e2e-tests/test-applications/create-remix-app-v2/package.json

@@ -12,19 +12,19 @@
   },
   "dependencies": {
     "@sentry/remix": "latest || *",
-    "@remix-run/css-bundle": "2.7.2",
-    "@remix-run/node": "2.7.2",
-    "@remix-run/react": "2.7.2",
-    "@remix-run/serve": "2.7.2",
+    "@remix-run/css-bundle": "2.16.5",
+    "@remix-run/node": "2.16.5",
+    "@remix-run/react": "2.16.5",
+    "@remix-run/serve": "2.16.5",
     "isbot": "^3.6.8",
     "react": "^18.2.0",
     "react-dom": "^18.2.0"
   },
   "devDependencies": {
     "@playwright/test": "~1.50.0",
     "@sentry-internal/test-utils": "link:../../../test-utils",
-    "@remix-run/dev": "2.7.2",
-    "@remix-run/eslint-config": "2.7.2",
+    "@remix-run/dev": "2.16.5",
+    "@remix-run/eslint-config": "2.16.5",
     "@sentry/core": "latest || *",
     "@types/react": "^18.0.35",
     "@types/react-dom": "^18.0.11",

dev-packages/e2e-tests/test-applications/remix-hydrogen/tests/server-transactions.test.ts

@@ -40,7 +40,6 @@ test('Sends two linked transactions (server & client) to Sentry', async ({ page
 
   const httpServerTraceId = httpServerTransaction.contexts?.trace?.trace_id;
   const httpServerSpanId = httpServerTransaction.contexts?.trace?.span_id;
-  const loaderSpanId = httpServerTransaction?.spans?.find(span => span.op === 'function.remix.loader')?.span_id;
 
   const pageLoadTraceId = pageloadTransaction.contexts?.trace?.trace_id;
   const pageLoadSpanId = pageloadTransaction.contexts?.trace?.span_id;
@@ -50,7 +49,6 @@ test('Sends two linked transactions (server & client) to Sentry', async ({ page
 
   expect(httpServerTraceId).toBeDefined();
   expect(httpServerSpanId).toBeDefined();
-  expect(loaderSpanId).toBeDefined();
 
   expect(pageLoadTraceId).toEqual(httpServerTraceId);
   expect(pageLoadSpanId).not.toEqual(httpServerSpanId);

packages/remix/src/server/instrumentServer.ts

@@ -364,6 +364,18 @@ function instrumentBuildCallback(
   for (const [id, route] of Object.entries(build.routes)) {
     const wrappedRoute = { ...route, module: { ...route.module } };
 
+    // Entry module should have a loader function to provide `sentry-trace` and `baggage`
+    // They will be available for the root `meta` function as `data.sentryTrace` and `data.sentryBaggage`
+    if (!wrappedRoute.parentId) {
+      if (!wrappedRoute.module.loader) {
+        wrappedRoute.module.loader = () => ({});
+      }
+
+      if (!(wrappedRoute.module.loader as WrappedFunction).__sentry_original__) {
+        fill(wrappedRoute.module, 'loader', makeWrappedRootLoader());
+      }
+    }
+
     const routeAction = wrappedRoute.module.action as undefined | WrappedFunction;
     if (routeAction && !routeAction.__sentry_original__) {
       fill(wrappedRoute.module, 'action', makeWrappedAction(id, options?.instrumentTracing));
@@ -374,17 +386,6 @@ function instrumentBuildCallback(
       fill(wrappedRoute.module, 'loader', makeWrappedLoader(id, options?.instrumentTracing));
     }
 
-    // Entry module should have a loader function to provide `sentry-trace` and `baggage`
-    // They will be available for the root `meta` function as `data.sentryTrace` and `data.sentryBaggage`
-    if (!wrappedRoute.parentId) {
-      if (!wrappedRoute.module.loader) {
-        wrappedRoute.module.loader = () => ({});
-      }
-
-      // We want to wrap the root loader regardless of whether it's already wrapped before.
-      fill(wrappedRoute.module, 'loader', makeWrappedRootLoader());
-    }
-
     routes[id] = wrappedRoute;
   }
 

packages/remix/test/integration/test/server/instrumentation/action.test.ts

@@ -28,15 +28,15 @@ describe('Remix API Actions', () => {
           data: {
             'code.function': 'loader',
             'sentry.op': 'loader.remix',
-            'match.route.id': 'routes/action-json-response.$id',
+            'match.route.id': 'root',
             'match.params.id': '123123',
           },
         },
         {
           data: {
             'code.function': 'loader',
             'sentry.op': 'loader.remix',
-            'match.route.id': 'root',
+            'match.route.id': 'routes/action-json-response.$id',
             'match.params.id': '123123',
           },
         },

packages/remix/test/integration/test/server/instrumentation/loader.test.ts

@@ -242,14 +242,14 @@ describe('Remix API Loaders', () => {
           data: {
             'code.function': 'loader',
             'sentry.op': 'loader.remix',
-            'match.route.id': 'routes/loader-defer-response.$id',
+            'match.route.id': 'root',
           },
         },
         {
           data: {
             'code.function': 'loader',
             'sentry.op': 'loader.remix',
-            'match.route.id': 'root',
+            'match.route.id': 'routes/loader-defer-response.$id',
           },
         },
       ],