feat(remix): Run the orchestrion transform on Remix 3 browser modules

dev-packages/e2e-tests/test-applications/remix-v3/app/actions/public/entry.ts

@@ -20,10 +20,6 @@ export const app = run({
   },
 });
 
-// The runtime sends a render error to the event target `run()` returns and does not rethrow, so
-// `window.onerror` never sees it. This listener is the only way the SDK learns about it.
-Sentry.captureRuntimeErrors(app);
-
 function Boom(): () => never {
   return () => {
     throw new Error('Component render failed');

dev-packages/e2e-tests/test-applications/remix-v3/tests/browser-transform.test.ts

@@ -0,0 +1,20 @@
+import { expect, test } from '@playwright/test';
+
+test('the served remix/ui runtime is instrumented', async ({ page, baseURL }) => {
+  const served: string[] = [];
+  page.on('response', response => {
+    if (response.request().resourceType() === 'script') {
+      served.push(response.url());
+    }
+  });
+
+  await page.goto('/', { waitUntil: 'load' });
+
+  const runModule = served.find(url => /@remix-run\/ui\/dist\/runtime\/run\.js/.test(decodeURIComponent(url)));
+  expect(runModule, 'run.js was not served').toBeDefined();
+
+  const code = await (await fetch(runModule as string)).text();
+  expect(code).toMatch(/diagnosticsChannelShim\.js/);
+  // A CommonJS `require` here would mean the transform emitted the wrong module type.
+  expect(code).not.toMatch(/\brequire\(/);
+});

packages/remix/rollup.npm.config.mjs

@@ -63,6 +63,8 @@ export default [
         'src/vite/index.ts',
         'src/v3/index.server.ts',
         'src/v3/index.client.ts',
+        // An explicit entry keeps its default export, which only orchestrion's injected import uses.
+        'src/v3/client/diagnosticsChannelShim.ts',
       ],
       packageSpecificConfig: {
         external: ['react-router', 'react-router-dom', 'react', 'react/jsx-runtime'],

packages/remix/src/v3/assetServer.ts

@@ -2,6 +2,7 @@ import * as diagnosticsChannel from 'node:diagnostics_channel';
 import { consoleSandbox } from '@sentry/core';
 import { remixV3Channels } from '@sentry/server-utils/orchestrion/config';
 import { addDebugIdToSourceMap, findDebugId, getDebugId, injectDebugIdSnippet } from './debugId';
+import { getShimUrl, isOrchestrionLoader, orchestrionLoader, resolveShimPath } from './orchestrionLoader';
 
 // The subset of `@remix-run/assets` types used here. `remix` is an optional peer dependency, so
 // they are restated rather than imported.
@@ -23,10 +24,14 @@ type ModuleLoader = (
 ) => ModuleLoadResult;
 
 interface AssetServerOptions {
+  allowPackages?: readonly string[];
+  basePath?: string;
+  rootDir?: string;
+  mounts?: Readonly<Record<string, string>>;
   // `false` is not in Remix's type, but is how an app tells Sentry it wants no source maps at all,
   // since leaving the option out now means hidden source maps.
   sourceMaps?: 'inline' | 'external' | false;
-  scripts?: { loaders?: readonly ModuleLoader[]; [key: string]: unknown };
+  scripts?: { loaders?: readonly ModuleLoader[]; external?: readonly string[]; [key: string]: unknown };
   [key: string]: unknown;
 }
 
@@ -44,6 +49,7 @@ interface CreateAssetServerContext {
 const SOURCE_MAPPING_URL_REGEX = /\n\/\/# sourceMappingURL=\S+\s*$/;
 
 const MAX_STAMPED_BODIES = 2000;
+const SDK_PACKAGE = '@sentry/remix';
 
 let instrumented = false;
 
@@ -127,16 +133,43 @@ function withDebugIdOptions(options: AssetServerOptions | undefined): AssetServe
     });
   }
 
-  const loaders = options.scripts?.loaders ?? [];
+  // No allowed packages means no `node_modules` module is served, so nothing to transform. Otherwise
+  // the shim must be allowed too, or its injected import 404s and takes the client runtime down.
+  const servesPackages = (options.allowPackages?.length ?? 0) > 0;
+  const allowPackages =
+    servesPackages && !options.allowPackages?.includes(SDK_PACKAGE)
+      ? [...(options.allowPackages ?? []), SDK_PACKAGE]
+      : options.allowPackages;
+
+  const shimUrl = servesPackages
+    ? getShimUrl(options.basePath ?? '/', options.rootDir ?? process.cwd(), options.mounts, resolveShimPath())
+    : undefined;
+  if (servesPackages && shimUrl === undefined) {
+    consoleSandbox(() => {
+      // oxlint-disable-next-line no-console
+      console.warn(
+        "[Sentry] The browser channel shim is outside the asset server's node_modules mount, so browser modules are served without instrumentation. Component render errors need `captureRuntimeErrors(app)`.",
+      );
+    });
+  }
+  const loaders = (options.scripts?.loaders ?? []).filter(
+    loader => loader !== debugIdLoader && !isOrchestrionLoader(loader),
+  );
+  const external = (options.scripts?.external ?? []).filter(entry => entry !== shimUrl);
 
   return {
     ...options,
+    ...(allowPackages !== options.allowPackages && { allowPackages }),
     // Hidden unless the app chose otherwise, see `stampServedAssets`.
     sourceMaps: options.sourceMaps ?? 'external',
     scripts: {
       ...options.scripts,
-      // Last, so the ID also covers whatever the app's own loaders changed.
-      loaders: loaders.includes(debugIdLoader) ? loaders : [...loaders, debugIdLoader],
+      // The transform first, the debug ID last, so the ID covers the transformed module.
+      loaders:
+        shimUrl === undefined ? [...loaders, debugIdLoader] : [...loaders, orchestrionLoader(shimUrl), debugIdLoader],
+      // Kept as a URL by the compiler. A bare specifier would not resolve from inside an instrumented
+      // package under pnpm.
+      external: shimUrl === undefined ? external : [shimUrl, ...external],
     },
   };
 }

packages/remix/src/v3/client/diagnosticsChannelShim.ts

@@ -75,7 +75,15 @@ function createChannel(): Channel {
   };
 }
 
-const registry = new Map<string, BrowserTracingChannel>();
+declare global {
+  // `var`, because only a `var` here becomes a property of `globalThis`.
+  // oxlint-disable-next-line no-var
+  var __SENTRY_REMIX_DC_REGISTRY__: Map<string, BrowserTracingChannel> | undefined;
+}
+
+// On `globalThis`, so a second copy of this module loaded under another URL shares the subscribers
+// instead of splitting them. Keyed by the channel names the transform compiles in, so it stays small.
+const registry = (globalThis.__SENTRY_REMIX_DC_REGISTRY__ ??= new Map<string, BrowserTracingChannel>());
 
 /** Create (or look up) a tracing channel by name. */
 export function tracingChannel(name: string): BrowserTracingChannel {

packages/remix/src/v3/orchestrionLoader.ts

@@ -0,0 +1,114 @@
+import * as path from 'node:path';
+import { fileURLToPath } from 'node:url';
+import { createLoadHookTransform } from '@sentry/server-utils/orchestrion/load-hook';
+import { remixV3Config } from '@sentry/server-utils/orchestrion/config';
+
+/** Node's synchronous `load` hook shape, which `@remix-run/assets` reuses for `scripts.loaders`. */
+export interface ModuleLoadContext {
+  moduleUrl?: string;
+  [key: string]: unknown;
+}
+
+export interface ModuleLoadResult {
+  format: string | null | undefined;
+  shortCircuit?: boolean;
+  source?: string | ArrayBuffer | ArrayBufferView;
+}
+
+export type ModuleLoader = (
+  url: string,
+  context: ModuleLoadContext,
+  nextLoad: (url: string, context?: Partial<ModuleLoadContext>) => ModuleLoadResult,
+) => ModuleLoadResult;
+
+/**
+ * An asset server loader that applies orchestrion's transform to browser modules. Remix 3 has no
+ * bundler, so the asset server's loader chain is the only compile time hook there is.
+ *
+ * `dcModuleUrl`, the shim's public URL, must also be in `scripts.external`, so the compiler leaves
+ * the injected import alone.
+ */
+export function orchestrionLoader(dcModuleUrl: string): ModuleLoader {
+  const cached = loaders.get(dcModuleUrl);
+  if (cached) {
+    return cached;
+  }
+
+  // Only the Remix 3 configs. The others target server packages, and their injected snippet imports
+  // `@sentry/server-utils`, which cannot resolve in a browser.
+  const transform = createLoadHookTransform({ dcModule: dcModuleUrl, instrumentations: remixV3Config });
+
+  const loader: ModuleLoader = (url, context, nextLoad) => {
+    const result = nextLoad(url, context);
+    if (result.format !== 'module' || typeof result.source !== 'string') {
+      return result;
+    }
+
+    // An uninstrumented module is better than one the asset server cannot serve at all.
+    try {
+      const transformed = transform(fileURLToPath(url), result.source);
+      return transformed === undefined ? result : { ...result, source: transformed };
+    } catch {
+      return result;
+    }
+  };
+
+  loaders.set(dcModuleUrl, loader);
+  return loader;
+}
+
+// One loader per shim URL, so options that pass through `withDebugIdOptions` twice keep one copy.
+const loaders = new Map<string, ModuleLoader>();
+
+export function isOrchestrionLoader(loader: unknown): boolean {
+  for (const known of loaders.values()) {
+    if (known === loader) {
+      return true;
+    }
+  }
+  return false;
+}
+
+/**
+ * Public URL of the shipped shim under the asset server's `node_modules` mount, encoded per segment as
+ * the asset server encodes imports. `@sentry` and `%40sentry` are different modules to a browser; a
+ * mismatch loads two shims and instrumentation quietly does nothing.
+ */
+export function getShimUrl(
+  basePath: string,
+  rootDir: string,
+  mounts: Readonly<Record<string, string>> | undefined,
+  shimPath: string,
+): string | undefined {
+  const nodeModulesRoot = path.join(rootDir, 'node_modules');
+  const relativePath = path.relative(nodeModulesRoot, shimPath);
+  // Outside the mount, as with a `link:` install or another Windows drive, the asset server cannot
+  // serve it. Checked before splitting: a drive prefix is only absolute as part of the whole path.
+  if (relativePath.startsWith('..') || path.isAbsolute(relativePath)) {
+    return undefined;
+  }
+  const encoded = relativePath
+    .split(path.sep)
+    .map(segment => encodeURIComponent(segment))
+    .join('/');
+  return `${basePath.replace(/\/$/, '')}/${npmMount(mounts)}/${encoded}`;
+}
+
+// The default mounts are `{ app: 'app', npm: 'node_modules' }`, but an app can rename them.
+function npmMount(mounts: Readonly<Record<string, string>> | undefined): string {
+  const entry = mounts && Object.entries(mounts).find(([, dir]) => dir === 'node_modules' || dir === './node_modules');
+  return entry ? entry[0] : 'npm';
+}
+
+// The shim is built next to this module, so its path follows from this module's own under any
+// install layout. A package self reference would not work: the `./v3` subpaths are import only.
+export function resolveShimPath(): string {
+  let here: string;
+  /*! rollup-include-cjs-only */
+  here = __filename;
+  /*! rollup-include-cjs-only-end */
+  /*! rollup-include-esm-only */
+  here = fileURLToPath(import.meta.url);
+  /*! rollup-include-esm-only-end */
+  return path.join(path.dirname(here), 'client', 'diagnosticsChannelShim.js');
+}

packages/remix/test/v3-exports.test.ts

@@ -63,4 +63,13 @@ describe('Remix 3 exports', () => {
       expect(packageJson.peerDependenciesMeta, `${name} should be optional`).toHaveProperty([name, 'optional'], true);
     }
   });
+
+  it('ships the browser channel shim with the default export the transform imports', () => {
+    // Only the explicit rollup entry keeps the default export the injected import needs. Losing it
+    // fails silently: the build stays green and the browser gets no instrumentation.
+    const shim = fs.readFileSync(path.join(packageRoot, 'build/esm/v3/client/diagnosticsChannelShim.js'), 'utf8');
+
+    // Either spelling rollup may pick for a default export.
+    expect(shim).toMatch(/^export (?:default |\{[^}]*\bas default\b)/m);
+  });
 });

packages/remix/test/v3/assetServer.test.ts

@@ -1,7 +1,27 @@
 import * as diagnosticsChannel from 'node:diagnostics_channel';
 import { remixV3Channels } from '@sentry/server-utils/orchestrion/config';
 import { beforeAll, describe, expect, it, vi } from 'vitest';
+import * as path from 'node:path';
 import { debugIdLoader, instrumentAssetServer } from '../../src/v3/assetServer';
+import type * as OrchestrionLoader from '../../src/v3/orchestrionLoader';
+
+// In the repo the shim sits in `src/`, which no asset server mount could serve. Place it where an
+// installed copy would be, so the URL logic runs the way it does in an app.
+vi.mock('../../src/v3/orchestrionLoader', async importOriginal => ({
+  ...(await importOriginal<typeof OrchestrionLoader>()),
+  resolveShimPath: () =>
+    path.join(
+      process.cwd(),
+      'node_modules',
+      '@sentry',
+      'remix',
+      'build',
+      'esm',
+      'v3',
+      'client',
+      'diagnosticsChannelShim.js',
+    ),
+}));
 import { getDebugId, getDebugIdSnippet } from '../../src/v3/debugId';
 
 type Options = Record<string, any>;
@@ -55,11 +75,31 @@ describe('instrumentAssetServer', () => {
       warn.mockRestore();
     });
 
-    it('adds the debug ID loader after the app loaders', () => {
+    it('adds the transform and then the debug ID loader after the app loaders', () => {
       const appLoader = vi.fn();
-      const { receivedOptions } = createAssetServer({ scripts: { loaders: [appLoader], define: { a: 'b' } } });
+      const { receivedOptions } = createAssetServer({
+        allowPackages: ['remix'],
+        scripts: { loaders: [appLoader], define: { a: 'b' } },
+      });
+
+      expect(receivedOptions.scripts).toEqual({
+        loaders: [appLoader, expect.any(Function), debugIdLoader],
+        define: { a: 'b' },
+        external: [expect.stringContaining('/client/diagnosticsChannelShim.js')],
+      });
+    });
+
+    it('points the transform at the shim by an encoded public URL under the node_modules mount', () => {
+      const { receivedOptions } = createAssetServer({
+        allowPackages: ['remix'],
+        basePath: '/assets',
+        mounts: { app: 'app', deps: 'node_modules' },
+      });
 
-      expect(receivedOptions.scripts).toEqual({ loaders: [appLoader, debugIdLoader], define: { a: 'b' } });
+      const [shimUrl] = receivedOptions.scripts.external;
+      expect(shimUrl).toMatch(/^\/assets\/deps\//);
+      // `@` must arrive as `%40`: the browser treats the two spellings as different modules.
+      expect(shimUrl).not.toContain('@');
     });
 
     it('does not mutate the options the app passed', () => {
@@ -69,11 +109,41 @@ describe('instrumentAssetServer', () => {
       expect(options).toEqual({ basePath: '/assets' });
     });
 
-    it('adds the loader once when the same options are reused', () => {
-      const first = createAssetServer({}).receivedOptions;
-      const { receivedOptions } = createAssetServer(first);
+    it('serves modules uninstrumented, with a warning, when the shim is outside the node_modules mount', () => {
+      const warn = vi.spyOn(console, 'warn').mockImplementation(() => {});
+      // A rootDir the shim cannot be under, as with a `link:` install.
+      const { receivedOptions } = createAssetServer({
+        allowPackages: ['remix'],
+        basePath: '/assets',
+        rootDir: '/nowhere/near',
+      });
+
+      expect(receivedOptions.scripts.loaders).toEqual([debugIdLoader]);
+      expect(receivedOptions.scripts.external).toEqual([]);
+      expect(warn).toHaveBeenCalledWith(expect.stringContaining('outside the asset server'));
+      warn.mockRestore();
+    });
+
+    it('allows the SDK package, so the injected shim import can be served', () => {
+      const { receivedOptions } = createAssetServer({ allowPackages: ['remix'] });
 
+      expect(receivedOptions.allowPackages).toEqual(['remix', '@sentry/remix']);
+    });
+
+    it('injects nothing when the app serves no packages, since no module could import the shim', () => {
+      const { receivedOptions } = createAssetServer({ basePath: '/assets' });
+
+      expect(receivedOptions.allowPackages).toBeUndefined();
       expect(receivedOptions.scripts.loaders).toEqual([debugIdLoader]);
+      expect(receivedOptions.scripts.external).toEqual([]);
+    });
+
+    it('adds each loader once when the same options are reused', () => {
+      const first = createAssetServer({ allowPackages: ['remix'] }).receivedOptions;
+      const { receivedOptions } = createAssetServer(first);
+
+      expect(receivedOptions.scripts.loaders).toEqual([expect.any(Function), debugIdLoader]);
+      expect(receivedOptions.scripts.external).toHaveLength(1);
     });
   });
 

packages/remix/test/v3/diagnosticsChannelShim.test.ts

@@ -0,0 +1,18 @@
+import { describe, expect, it, vi } from 'vitest';
+
+describe('diagnosticsChannelShim', () => {
+  it('shares one registry between two loaded copies of the module', async () => {
+    const first = await import('../../src/v3/client/diagnosticsChannelShim');
+    const seen: unknown[] = [];
+    first.tracingChannel('orchestrion:test:run').subscribe({ end: context => void seen.push(context) });
+
+    // A second module instance, as a browser gets when the same file is loaded under two URLs.
+    vi.resetModules();
+    const second = await import('../../src/v3/client/diagnosticsChannelShim');
+    expect(second.tracingChannel).not.toBe(first.tracingChannel);
+
+    second.tracingChannel('orchestrion:test:run').end.publish({ result: 1 });
+
+    expect(seen).toEqual([{ result: 1 }]);
+  });
+});