Skip to content

Commit b48cc8b

Browse files
chargomeclaude
andcommitted
feat(remix): Run the orchestrion transform on Remix 3 browser modules
Remix 3 has no bundler, so the build time plugin the other SDKs use has nothing to attach to. The asset server compiles each module on request through a loader chain with Node's `load` hook shape, and that is the only compile time hook the browser pipeline has. This adds a loader there that applies orchestrion's transform, and patches `run` in `@remix-run/ui` so component render errors are captured with no application code. `@sentry/server-utils` gains a `load-hook` entry for callers with a file path and its source and no bundler in between. It is compiled into that package's build like the webpack loader, because the `@apm-js-collab` packages are bundled devDependencies and not resolvable on user installs. This keeps `@sentry/remix` free of the transformer as a dependency. The transform imports the browser `diagnostics_channel` shim by public URL, percent encoded per segment the way the asset server encodes rewritten imports, and the URL is listed in `scripts.external`. Two spellings of that URL are two modules to a browser, so a mismatch would load two shims with two subscriber registries and no instrumentation. The shim is now an explicit rollup entry: nothing in the module graph uses its default export, so a non entry module would have it tree shaken away, which the injected import needs. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
1 parent 9fa7790 commit b48cc8b

15 files changed

Lines changed: 352 additions & 15 deletions

File tree

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

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -20,10 +20,6 @@ export const app = run({
2020
},
2121
});
2222

23-
// The runtime sends a render error to the event target `run()` returns and does not rethrow, so
24-
// `window.onerror` never sees it. This listener is the only way the SDK learns about it.
25-
Sentry.captureRuntimeErrors(app);
26-
2723
function Boom(): () => never {
2824
return () => {
2925
throw new Error('Component render failed');
Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
import { expect, test } from '@playwright/test';
2+
3+
test('the served remix/ui runtime is instrumented', async ({ page, baseURL }) => {
4+
const served: string[] = [];
5+
page.on('response', response => {
6+
if (response.request().resourceType() === 'script') {
7+
served.push(response.url());
8+
}
9+
});
10+
11+
await page.goto('/');
12+
await page.waitForLoadState('networkidle');
13+
14+
const runModule = served.find(url => /@remix-run\/ui\/dist\/runtime\/run\.js/.test(decodeURIComponent(url)));
15+
expect(runModule, 'run.js was not served').toBeDefined();
16+
17+
const code = await (await fetch(runModule as string)).text();
18+
expect(code).toMatch(/diagnosticsChannelShim\.js/);
19+
// A CommonJS `require` here would mean the transform emitted the wrong module type.
20+
expect(code).not.toMatch(/\brequire\(/);
21+
});

‎packages/remix/rollup.npm.config.mjs‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,8 @@ export default [
6363
'src/vite/index.ts',
6464
'src/v3/index.server.ts',
6565
'src/v3/index.client.ts',
66+
// An explicit entry keeps its default export, which only orchestrion's injected import uses.
67+
'src/v3/client/diagnosticsChannelShim.ts',
6668
],
6769
packageSpecificConfig: {
6870
external: ['react-router', 'react-router-dom', 'react', 'react/jsx-runtime'],

‎packages/remix/src/v3/assetServer.ts‎

Lines changed: 30 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import * as diagnosticsChannel from 'node:diagnostics_channel';
22
import { consoleSandbox } from '@sentry/core';
33
import { remixV3Channels } from '@sentry/server-utils/orchestrion/config';
44
import { addDebugIdToSourceMap, findDebugId, getDebugId, injectDebugIdSnippet } from './debugId';
5+
import { getShimUrl, isOrchestrionLoader, orchestrionLoader, resolveShimPath } from './orchestrionLoader';
56

67
// The subset of `@remix-run/assets` types used here. `remix` is an optional peer dependency, so
78
// they are restated rather than imported.
@@ -23,10 +24,13 @@ type ModuleLoader = (
2324
) => ModuleLoadResult;
2425

2526
interface AssetServerOptions {
27+
basePath?: string;
28+
rootDir?: string;
29+
mounts?: Readonly<Record<string, string>>;
2630
// `false` is not in Remix's type, but is how an app tells Sentry it wants no source maps at all,
2731
// since leaving the option out now means hidden source maps.
2832
sourceMaps?: 'inline' | 'external' | false;
29-
scripts?: { loaders?: readonly ModuleLoader[]; [key: string]: unknown };
33+
scripts?: { loaders?: readonly ModuleLoader[]; external?: readonly string[]; [key: string]: unknown };
3034
[key: string]: unknown;
3135
}
3236

@@ -127,16 +131,38 @@ function withDebugIdOptions(options: AssetServerOptions | undefined): AssetServe
127131
});
128132
}
129133

130-
const loaders = options.scripts?.loaders ?? [];
134+
const shimUrl = getShimUrl(
135+
options.basePath ?? '/',
136+
options.rootDir ?? process.cwd(),
137+
options.mounts,
138+
resolveShimPath(),
139+
);
140+
if (shimUrl === undefined) {
141+
// A 404 on the injected import would take the whole client runtime down.
142+
consoleSandbox(() => {
143+
// oxlint-disable-next-line no-console
144+
console.warn(
145+
"[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)`.",
146+
);
147+
});
148+
}
149+
const loaders = (options.scripts?.loaders ?? []).filter(
150+
loader => loader !== debugIdLoader && !isOrchestrionLoader(loader),
151+
);
152+
const external = (options.scripts?.external ?? []).filter(entry => entry !== shimUrl);
131153

132154
return {
133155
...options,
134156
// Hidden unless the app chose otherwise, see `stampServedAssets`.
135157
sourceMaps: options.sourceMaps ?? 'external',
136158
scripts: {
137159
...options.scripts,
138-
// Last, so the ID also covers whatever the app's own loaders changed.
139-
loaders: loaders.includes(debugIdLoader) ? loaders : [...loaders, debugIdLoader],
160+
// The transform first, the debug ID last, so the ID covers the transformed module.
161+
loaders:
162+
shimUrl === undefined ? [...loaders, debugIdLoader] : [...loaders, orchestrionLoader(shimUrl), debugIdLoader],
163+
// Kept as a URL by the compiler. A bare specifier would not resolve from inside an instrumented
164+
// package under pnpm.
165+
external: shimUrl === undefined ? external : [shimUrl, ...external],
140166
},
141167
};
142168
}

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

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,13 @@ function createChannel(): Channel {
7575
};
7676
}
7777

78-
const registry = new Map<string, BrowserTracingChannel>();
78+
// Held on `globalThis`, so a second copy of this module loaded under another URL still shares one
79+
// set of subscribers instead of splitting them, which would leave instrumentation silently inert.
80+
const REGISTRY_KEY = '__SENTRY_REMIX_DC_REGISTRY__';
81+
const registry: Map<string, BrowserTracingChannel> = ((globalThis as Record<string, unknown>)[REGISTRY_KEY] ??= new Map<
82+
string,
83+
BrowserTracingChannel
84+
>()) as Map<string, BrowserTracingChannel>;
7985

8086
/** Create (or look up) a tracing channel by name. */
8187
export function tracingChannel(name: string): BrowserTracingChannel {
Lines changed: 105 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,105 @@
1+
import * as path from 'node:path';
2+
import { fileURLToPath } from 'node:url';
3+
import { createLoadHookTransform } from '@sentry/server-utils/orchestrion/load-hook';
4+
import { remixV3Config } from '@sentry/server-utils/orchestrion/config';
5+
6+
/** Node's synchronous `load` hook shape, which `@remix-run/assets` reuses for `scripts.loaders`. */
7+
export interface ModuleLoadContext {
8+
moduleUrl?: string;
9+
[key: string]: unknown;
10+
}
11+
12+
export interface ModuleLoadResult {
13+
format: string | null | undefined;
14+
shortCircuit?: boolean;
15+
source?: string | ArrayBuffer | ArrayBufferView;
16+
}
17+
18+
export type ModuleLoader = (
19+
url: string,
20+
context: ModuleLoadContext,
21+
nextLoad: (url: string, context?: Partial<ModuleLoadContext>) => ModuleLoadResult,
22+
) => ModuleLoadResult;
23+
24+
/**
25+
* An asset server loader that applies orchestrion's transform to browser modules. Remix 3 has no
26+
* bundler, so the asset server's loader chain is the only compile time hook there is.
27+
*
28+
* `dcModuleUrl`, the shim's public URL, must also be in `scripts.external`, so the compiler leaves
29+
* the injected import alone.
30+
*/
31+
export function orchestrionLoader(dcModuleUrl: string): ModuleLoader {
32+
const cached = loaders.get(dcModuleUrl);
33+
if (cached) {
34+
return cached;
35+
}
36+
37+
// Only the Remix 3 configs. The others target server packages, and their injected snippet imports
38+
// `@sentry/server-utils`, which cannot resolve in a browser.
39+
const transform = createLoadHookTransform({ dcModule: dcModuleUrl, instrumentations: remixV3Config });
40+
41+
const loader: ModuleLoader = (url, context, nextLoad) => {
42+
const result = nextLoad(url, context);
43+
if (result.format !== 'module' || typeof result.source !== 'string') {
44+
return result;
45+
}
46+
47+
const transformed = transform(fileURLToPath(url), result.source);
48+
return transformed === undefined ? result : { ...result, source: transformed };
49+
};
50+
51+
loaders.set(dcModuleUrl, loader);
52+
return loader;
53+
}
54+
55+
// One loader per shim URL, so options that pass through `withDebugIdOptions` twice keep one copy.
56+
const loaders = new Map<string, ModuleLoader>();
57+
58+
export function isOrchestrionLoader(loader: unknown): boolean {
59+
for (const known of loaders.values()) {
60+
if (known === loader) {
61+
return true;
62+
}
63+
}
64+
return false;
65+
}
66+
67+
/**
68+
* Public URL of the shipped shim under the asset server's `node_modules` mount, percent encoded per
69+
* segment as the asset server encodes rewritten imports. `@sentry` and `%40sentry` are two different
70+
* modules to a browser, so a mismatch loads two shims and instrumentation quietly does nothing.
71+
*/
72+
export function getShimUrl(
73+
basePath: string,
74+
rootDir: string,
75+
mounts: Readonly<Record<string, string>> | undefined,
76+
shimPath: string,
77+
): string | undefined {
78+
const nodeModulesRoot = path.join(rootDir, 'node_modules');
79+
const relative = path.relative(nodeModulesRoot, shimPath).split(path.sep);
80+
// Outside the mount, as with a `link:` install, the asset server cannot serve it.
81+
if (relative[0] === '..' || path.isAbsolute(relative[0] ?? '')) {
82+
return undefined;
83+
}
84+
const encoded = relative.map(segment => encodeURIComponent(segment)).join('/');
85+
return `${basePath.replace(/\/$/, '')}/${npmMount(mounts)}/${encoded}`;
86+
}
87+
88+
// The default mounts are `{ app: 'app', npm: 'node_modules' }`, but an app can rename them.
89+
function npmMount(mounts: Readonly<Record<string, string>> | undefined): string {
90+
const entry = mounts && Object.entries(mounts).find(([, dir]) => dir === 'node_modules' || dir === './node_modules');
91+
return entry ? entry[0] : 'npm';
92+
}
93+
94+
// The shim is built next to this module, so its path follows from this module's own under any
95+
// install layout. A package self reference would not work: the `./v3` subpaths are import only.
96+
export function resolveShimPath(): string {
97+
let here: string;
98+
/*! rollup-include-cjs-only */
99+
here = __filename;
100+
/*! rollup-include-cjs-only-end */
101+
/*! rollup-include-esm-only */
102+
here = fileURLToPath(import.meta.url);
103+
/*! rollup-include-esm-only-end */
104+
return path.join(path.dirname(here), 'client', 'diagnosticsChannelShim.js');
105+
}

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

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,4 +63,13 @@ describe('Remix 3 exports', () => {
6363
expect(packageJson.peerDependenciesMeta, `${name} should be optional`).toHaveProperty([name, 'optional'], true);
6464
}
6565
});
66+
67+
it('ships the browser channel shim with the default export the transform imports', () => {
68+
// Only the explicit rollup entry keeps the default export the injected import needs. Losing it
69+
// fails silently: the build stays green and the browser gets no instrumentation.
70+
const shim = fs.readFileSync(path.join(packageRoot, 'build/esm/v3/client/diagnosticsChannelShim.js'), 'utf8');
71+
72+
// Either spelling rollup may pick for a default export.
73+
expect(shim).toMatch(/^export (?:default |\{[^}]*\bas default\b)/m);
74+
});
6675
});

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

Lines changed: 52 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,27 @@
11
import * as diagnosticsChannel from 'node:diagnostics_channel';
22
import { remixV3Channels } from '@sentry/server-utils/orchestrion/config';
33
import { beforeAll, describe, expect, it, vi } from 'vitest';
4+
import * as path from 'node:path';
45
import { debugIdLoader, instrumentAssetServer } from '../../src/v3/assetServer';
6+
import type * as OrchestrionLoader from '../../src/v3/orchestrionLoader';
7+
8+
// In the repo the shim sits in `src/`, which no asset server mount could serve. Place it where an
9+
// installed copy would be, so the URL logic runs the way it does in an app.
10+
vi.mock('../../src/v3/orchestrionLoader', async importOriginal => ({
11+
...(await importOriginal<typeof OrchestrionLoader>()),
12+
resolveShimPath: () =>
13+
path.join(
14+
process.cwd(),
15+
'node_modules',
16+
'@sentry',
17+
'remix',
18+
'build',
19+
'esm',
20+
'v3',
21+
'client',
22+
'diagnosticsChannelShim.js',
23+
),
24+
}));
525
import { getDebugId, getDebugIdSnippet } from '../../src/v3/debugId';
626

727
type Options = Record<string, any>;
@@ -55,11 +75,27 @@ describe('instrumentAssetServer', () => {
5575
warn.mockRestore();
5676
});
5777

58-
it('adds the debug ID loader after the app loaders', () => {
78+
it('adds the transform and then the debug ID loader after the app loaders', () => {
5979
const appLoader = vi.fn();
6080
const { receivedOptions } = createAssetServer({ scripts: { loaders: [appLoader], define: { a: 'b' } } });
6181

62-
expect(receivedOptions.scripts).toEqual({ loaders: [appLoader, debugIdLoader], define: { a: 'b' } });
82+
expect(receivedOptions.scripts).toEqual({
83+
loaders: [appLoader, expect.any(Function), debugIdLoader],
84+
define: { a: 'b' },
85+
external: [expect.stringContaining('/client/diagnosticsChannelShim.js')],
86+
});
87+
});
88+
89+
it('points the transform at the shim by an encoded public URL under the node_modules mount', () => {
90+
const { receivedOptions } = createAssetServer({
91+
basePath: '/assets',
92+
mounts: { app: 'app', deps: 'node_modules' },
93+
});
94+
95+
const [shimUrl] = receivedOptions.scripts.external;
96+
expect(shimUrl).toMatch(/^\/assets\/deps\//);
97+
// `@` must arrive as `%40`: the browser treats the two spellings as different modules.
98+
expect(shimUrl).not.toContain('@');
6399
});
64100

65101
it('does not mutate the options the app passed', () => {
@@ -69,11 +105,23 @@ describe('instrumentAssetServer', () => {
69105
expect(options).toEqual({ basePath: '/assets' });
70106
});
71107

72-
it('adds the loader once when the same options are reused', () => {
108+
it('serves modules uninstrumented, with a warning, when the shim is outside the node_modules mount', () => {
109+
const warn = vi.spyOn(console, 'warn').mockImplementation(() => {});
110+
// A rootDir the shim cannot be under, as with a `link:` install.
111+
const { receivedOptions } = createAssetServer({ basePath: '/assets', rootDir: '/nowhere/near' });
112+
113+
expect(receivedOptions.scripts.loaders).toEqual([debugIdLoader]);
114+
expect(receivedOptions.scripts.external).toEqual([]);
115+
expect(warn).toHaveBeenCalledWith(expect.stringContaining('outside the asset server'));
116+
warn.mockRestore();
117+
});
118+
119+
it('adds each loader once when the same options are reused', () => {
73120
const first = createAssetServer({}).receivedOptions;
74121
const { receivedOptions } = createAssetServer(first);
75122

76-
expect(receivedOptions.scripts.loaders).toEqual([debugIdLoader]);
123+
expect(receivedOptions.scripts.loaders).toEqual([expect.any(Function), debugIdLoader]);
124+
expect(receivedOptions.scripts.external).toHaveLength(1);
77125
});
78126
});
79127

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
import { describe, expect, it, vi } from 'vitest';
2+
3+
describe('diagnosticsChannelShim', () => {
4+
it('shares one registry between two loaded copies of the module', async () => {
5+
const first = await import('../../src/v3/client/diagnosticsChannelShim');
6+
const seen: unknown[] = [];
7+
first.tracingChannel('orchestrion:test:run').subscribe({ end: context => void seen.push(context) });
8+
9+
// A second module instance, as a browser gets when the same file is loaded under two URLs.
10+
vi.resetModules();
11+
const second = await import('../../src/v3/client/diagnosticsChannelShim');
12+
expect(second.tracingChannel).not.toBe(first.tracingChannel);
13+
14+
second.tracingChannel('orchestrion:test:run').end.publish({ result: 1 });
15+
16+
expect(seen).toEqual([{ result: 1 }]);
17+
});
18+
});

‎packages/server-utils/package.json‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,11 @@
5151
"import": "./build/esm/orchestrion/bundler/webpack-loader.js",
5252
"require": "./build/cjs/orchestrion/bundler/webpack-loader.js"
5353
},
54+
"./orchestrion/load-hook": {
55+
"types": "./build/types/orchestrion/bundler/load-hook.d.ts",
56+
"import": "./build/esm/orchestrion/bundler/load-hook.js",
57+
"require": "./build/cjs/orchestrion/bundler/load-hook.js"
58+
},
5459
"./orchestrion/esbuild": {
5560
"types": "./build/types/orchestrion/bundler/esbuild.d.ts",
5661
"import": "./build/esm/orchestrion/bundler/esbuild.js",
@@ -81,6 +86,9 @@
8186
],
8287
"orchestrion/bun": [
8388
"build/types/orchestrion/bundler/bun.d.ts"
89+
],
90+
"orchestrion/load-hook": [
91+
"build/types/orchestrion/bundler/load-hook.d.ts"
8492
]
8593
}
8694
},

0 commit comments

Comments
 (0)