Skip to content

Commit be24564

Browse files
chargomeclaude
chargome
and
claude
committed
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 c199a22 commit be24564

15 files changed

Lines changed: 388 additions & 17 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');

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

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
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('/', { waitUntil: 'load' });
12+
13+
const runModule = served.find(url => /@remix-run\/ui\/dist\/runtime\/run\.js/.test(decodeURIComponent(url)));
14+
expect(runModule, 'run.js was not served').toBeDefined();
15+
16+
const code = await (await fetch(runModule as string)).text();
17+
expect(code).toMatch(/diagnosticsChannelShim\.js/);
18+
// A CommonJS `require` here would mean the transform emitted the wrong module type.
19+
expect(code).not.toMatch(/\brequire\(/);
20+
});

‎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: 38 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,14 @@ type ModuleLoader = (
2324
) => ModuleLoadResult;
2425

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

@@ -44,6 +49,7 @@ interface CreateAssetServerContext {
4449
const SOURCE_MAPPING_URL_REGEX = /\n\/\/# sourceMappingURL=\S+\s*$/;
4550

4651
const MAX_STAMPED_BODIES = 2000;
52+
const SDK_PACKAGE = '@sentry/remix';
4753

4854
let instrumented = false;
4955

@@ -127,16 +133,44 @@ function withDebugIdOptions(options: AssetServerOptions | undefined): AssetServe
127133
});
128134
}
129135

130-
const loaders = options.scripts?.loaders ?? [];
136+
// Without any allowed package no browser module from `node_modules` is served, so there is nothing
137+
// to transform. With some, the shim has to be allowed too, or its injected import would 404 and
138+
// take the whole client runtime down.
139+
const servesPackages = (options.allowPackages?.length ?? 0) > 0;
140+
const allowPackages =
141+
servesPackages && !options.allowPackages?.includes(SDK_PACKAGE)
142+
? [...(options.allowPackages ?? []), SDK_PACKAGE]
143+
: options.allowPackages;
144+
145+
const shimUrl = servesPackages
146+
? getShimUrl(options.basePath ?? '/', options.rootDir ?? process.cwd(), options.mounts, resolveShimPath())
147+
: undefined;
148+
if (servesPackages && shimUrl === undefined) {
149+
consoleSandbox(() => {
150+
// oxlint-disable-next-line no-console
151+
console.warn(
152+
"[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)`.",
153+
);
154+
});
155+
}
156+
const loaders = (options.scripts?.loaders ?? []).filter(
157+
loader => loader !== debugIdLoader && !isOrchestrionLoader(loader),
158+
);
159+
const external = (options.scripts?.external ?? []).filter(entry => entry !== shimUrl);
131160

132161
return {
133162
...options,
163+
...(allowPackages !== options.allowPackages && { allowPackages }),
134164
// Hidden unless the app chose otherwise, see `stampServedAssets`.
135165
sourceMaps: options.sourceMaps ?? 'external',
136166
scripts: {
137167
...options.scripts,
138-
// Last, so the ID also covers whatever the app's own loaders changed.
139-
loaders: loaders.includes(debugIdLoader) ? loaders : [...loaders, debugIdLoader],
168+
// The transform first, the debug ID last, so the ID covers the transformed module.
169+
loaders:
170+
shimUrl === undefined ? [...loaders, debugIdLoader] : [...loaders, orchestrionLoader(shimUrl), debugIdLoader],
171+
// Kept as a URL by the compiler. A bare specifier would not resolve from inside an instrumented
172+
// package under pnpm.
173+
external: shimUrl === undefined ? external : [shimUrl, ...external],
140174
},
141175
};
142176
}

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

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

78-
const registry = new Map<string, BrowserTracingChannel>();
78+
declare global {
79+
// oxlint-disable-next-line no-var
80+
var __SENTRY_REMIX_DC_REGISTRY__: Map<string, BrowserTracingChannel> | undefined;
81+
}
82+
83+
// Held on `globalThis`, so a second copy of this module loaded under another URL still shares one
84+
// set of subscribers instead of splitting them, which would leave instrumentation silently inert.
85+
const registry = (globalThis.__SENTRY_REMIX_DC_REGISTRY__ ??= new Map<string, BrowserTracingChannel>());
7986

8087
/** Create (or look up) a tracing channel by name. */
8188
export function tracingChannel(name: string): BrowserTracingChannel {

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

Lines changed: 109 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,109 @@
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 relativePath = path.relative(nodeModulesRoot, shimPath);
80+
// Outside the mount, as with a `link:` install or another Windows drive, the asset server cannot
81+
// serve it. Checked before splitting: a drive prefix is only absolute as part of the whole path.
82+
if (relativePath.startsWith('..') || path.isAbsolute(relativePath)) {
83+
return undefined;
84+
}
85+
const encoded = relativePath
86+
.split(path.sep)
87+
.map(segment => encodeURIComponent(segment))
88+
.join('/');
89+
return `${basePath.replace(/\/$/, '')}/${npmMount(mounts)}/${encoded}`;
90+
}
91+
92+
// The default mounts are `{ app: 'app', npm: 'node_modules' }`, but an app can rename them.
93+
function npmMount(mounts: Readonly<Record<string, string>> | undefined): string {
94+
const entry = mounts && Object.entries(mounts).find(([, dir]) => dir === 'node_modules' || dir === './node_modules');
95+
return entry ? entry[0] : 'npm';
96+
}
97+
98+
// The shim is built next to this module, so its path follows from this module's own under any
99+
// install layout. A package self reference would not work: the `./v3` subpaths are import only.
100+
export function resolveShimPath(): string {
101+
let here: string;
102+
/*! rollup-include-cjs-only */
103+
here = __filename;
104+
/*! rollup-include-cjs-only-end */
105+
/*! rollup-include-esm-only */
106+
here = fileURLToPath(import.meta.url);
107+
/*! rollup-include-esm-only-end */
108+
return path.join(path.dirname(here), 'client', 'diagnosticsChannelShim.js');
109+
}

‎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: 76 additions & 6 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,31 @@ 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();
60-
const { receivedOptions } = createAssetServer({ scripts: { loaders: [appLoader], define: { a: 'b' } } });
80+
const { receivedOptions } = createAssetServer({
81+
allowPackages: ['remix'],
82+
scripts: { loaders: [appLoader], define: { a: 'b' } },
83+
});
84+
85+
expect(receivedOptions.scripts).toEqual({
86+
loaders: [appLoader, expect.any(Function), debugIdLoader],
87+
define: { a: 'b' },
88+
external: [expect.stringContaining('/client/diagnosticsChannelShim.js')],
89+
});
90+
});
91+
92+
it('points the transform at the shim by an encoded public URL under the node_modules mount', () => {
93+
const { receivedOptions } = createAssetServer({
94+
allowPackages: ['remix'],
95+
basePath: '/assets',
96+
mounts: { app: 'app', deps: 'node_modules' },
97+
});
6198

62-
expect(receivedOptions.scripts).toEqual({ loaders: [appLoader, debugIdLoader], define: { a: 'b' } });
99+
const [shimUrl] = receivedOptions.scripts.external;
100+
expect(shimUrl).toMatch(/^\/assets\/deps\//);
101+
// `@` must arrive as `%40`: the browser treats the two spellings as different modules.
102+
expect(shimUrl).not.toContain('@');
63103
});
64104

65105
it('does not mutate the options the app passed', () => {
@@ -69,11 +109,41 @@ describe('instrumentAssetServer', () => {
69109
expect(options).toEqual({ basePath: '/assets' });
70110
});
71111

72-
it('adds the loader once when the same options are reused', () => {
73-
const first = createAssetServer({}).receivedOptions;
74-
const { receivedOptions } = createAssetServer(first);
112+
it('serves modules uninstrumented, with a warning, when the shim is outside the node_modules mount', () => {
113+
const warn = vi.spyOn(console, 'warn').mockImplementation(() => {});
114+
// A rootDir the shim cannot be under, as with a `link:` install.
115+
const { receivedOptions } = createAssetServer({
116+
allowPackages: ['remix'],
117+
basePath: '/assets',
118+
rootDir: '/nowhere/near',
119+
});
120+
121+
expect(receivedOptions.scripts.loaders).toEqual([debugIdLoader]);
122+
expect(receivedOptions.scripts.external).toEqual([]);
123+
expect(warn).toHaveBeenCalledWith(expect.stringContaining('outside the asset server'));
124+
warn.mockRestore();
125+
});
126+
127+
it('allows the SDK package, so the injected shim import can be served', () => {
128+
const { receivedOptions } = createAssetServer({ allowPackages: ['remix'] });
75129

130+
expect(receivedOptions.allowPackages).toEqual(['remix', '@sentry/remix']);
131+
});
132+
133+
it('injects nothing when the app serves no packages, since no module could import the shim', () => {
134+
const { receivedOptions } = createAssetServer({ basePath: '/assets' });
135+
136+
expect(receivedOptions.allowPackages).toBeUndefined();
76137
expect(receivedOptions.scripts.loaders).toEqual([debugIdLoader]);
138+
expect(receivedOptions.scripts.external).toEqual([]);
139+
});
140+
141+
it('adds each loader once when the same options are reused', () => {
142+
const first = createAssetServer({ allowPackages: ['remix'] }).receivedOptions;
143+
const { receivedOptions } = createAssetServer(first);
144+
145+
expect(receivedOptions.scripts.loaders).toEqual([expect.any(Function), debugIdLoader]);
146+
expect(receivedOptions.scripts.external).toHaveLength(1);
77147
});
78148
});
79149

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

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+
});

0 commit comments

Comments
 (0)