Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,8 @@
},
"scripts": {
"build": "react-router build",
"dev": "NODE_OPTIONS='--import ./instrument.mjs' react-router dev",
"start": "NODE_OPTIONS='--import ./instrument.mjs' react-router-serve ./build/server/index.js",
"dev": "react-router dev",
"start": "react-router-serve ./build/server/index.js",
"proxy": "node start-event-proxy.mjs",
"typecheck": "react-router typegen && tsc",
"clean": "npx rimraf node_modules pnpm-lock.yaml",
Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +1,12 @@
import type { Config } from '@react-router/dev/config';
import { sentryOnBuildEnd } from '@sentry/react-router';

export default {
ssr: true,
prerender: ['/performance/static'],
future: {
v8_middleware: true,
},
// Required for `autoInjectServerInstrumentation`: the auto-injection runs in this build-end hook.
buildEnd: sentryOnBuildEnd,
} satisfies Config;
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,13 @@ export default defineConfig(async config => ({
plugins: [
reactRouter(),
// eslint-disable-next-line @typescript-eslint/no-explicit-any
...((await sentryReactRouter({ sourcemaps: { disable: true } }, config)) as any[]),
...((await sentryReactRouter(
{
sourcemaps: { disable: true },
// Auto-inject server instrumentation into the build output - no `NODE_OPTIONS='--import ...'` needed.
autoInjectServerInstrumentation: true,
},
config,
)) as any[]),
],
}));
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import '../instrument.mjs';
import { createReadableStreamFromReadable } from '@react-router/node';
import * as Sentry from '@sentry/react-router';
import { renderToPipeableStream } from 'react-dom/server';
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
import type { Config } from '@react-router/dev/config';
import { sentryOnBuildEnd } from '@sentry/react-router';

export default {
ssr: true,
buildEnd: sentryOnBuildEnd,
} satisfies Config;
Original file line number Diff line number Diff line change
@@ -1,13 +1,24 @@
import { reactRouter } from '@react-router/dev/vite';
import { sentryReactRouter } from '@sentry/react-router';
import { sentryOrchestrionPlugin } from '@sentry/server-utils/orchestrion/vite';
import { defineConfig } from 'vite';

export default defineConfig({
export default defineConfig(async config => ({
plugins: [
reactRouter(),
// Runs the orchestrion code transform over the SSR server bundle and
// force-bundles the instrumented deps (mysql, ioredis, …) so the
// diagnostics-channel calls are actually injected at build time.
sentryOrchestrionPlugin(),
// Auto-injects `instrument.server.mjs` into the server build output (top-level import),
// so no manual `import '../instrument.server.mjs'` or `--import` flag is needed.
// eslint-disable-next-line @typescript-eslint/no-explicit-any
...((await sentryReactRouter(
{
sourcemaps: { disable: true },
autoInjectServerInstrumentation: true,
},
config,
)) as any[]),
],
});
}));
37 changes: 37 additions & 0 deletions packages/react-router/src/vite/buildEnd/detectDeployTarget.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
import * as fs from 'node:fs';
import * as path from 'node:path';

// Dependencies that signal a Cloudflare Workers build. There is no long-running Node server entry to wrap and
// `Sentry.init` must run inside the worker, so build-time injection must be skipped entirely for these apps.
const CLOUDFLARE_DEPENDENCIES = ['@cloudflare/vite-plugin', 'wrangler', '@react-router/cloudflare'];

/**
* Whether the given set of (dev)dependencies indicates a Cloudflare Workers target. Pure and therefore
* unit-testable without touching the filesystem.
*/
export function isCloudflareTarget(dependencies: Record<string, string | undefined>): boolean {
return CLOUDFLARE_DEPENDENCIES.some(dep => dependencies[dep]);
}

/**
* Detects whether a React Router app targets Cloudflare Workers by reading its `package.json` dependencies.
* On any read/parse error we assume it does not (auto-injection then proceeds and falls back to its own guards).
*
* @param root - The (absolute) project root directory, e.g. Vite's `config.root`.
*/
export function detectCloudflareTarget(root: string): boolean {
try {
const packageJsonPath = path.resolve(root, 'package.json');
const packageJson = JSON.parse(fs.readFileSync(packageJsonPath, 'utf-8')) as {
dependencies?: Record<string, string>;
devDependencies?: Record<string, string>;
};

return isCloudflareTarget({
...packageJson.dependencies,
...packageJson.devDependencies,
});
} catch {
return false;
}
}
39 changes: 39 additions & 0 deletions packages/react-router/src/vite/buildEnd/handleOnBuildEnd.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,48 @@ import SentryCli from '@sentry/cli';
import type { SentryVitePluginOptions } from '@sentry/bundler-plugins/vite';
import { glob } from 'glob';
import type { SentryReactRouterBuildOptions } from '../types';
import { DEFAULT_SERVER_INSTRUMENTATION_FILE, injectServerInstrumentation } from './injectServerInstrumentation';

type BuildEndHook = NonNullable<Config['buildEnd']>;
type BuildEndHookArgs = Parameters<BuildEndHook>[0];

/**
* Auto-injects Sentry server instrumentation into the build output when `autoInjectServerInstrumentation` is enabled.
* Extracted from `sentryOnBuildEnd` to keep that hook's complexity manageable.
*/
async function maybeAutoInjectServerInstrumentation(
sentryConfig: SentryReactRouterBuildOptions,
reactRouterConfig: BuildEndHookArgs['reactRouterConfig'],
viteConfig: BuildEndHookArgs['viteConfig'],
debug: boolean,
): Promise<void> {
if (!sentryConfig.autoInjectServerInstrumentation) {
return;
}

try {
await injectServerInstrumentation({
root: viteConfig.root,
buildDirectory: reactRouterConfig.buildDirectory,
serverBuildFile: reactRouterConfig.serverBuildFile,
serverModuleFormat: reactRouterConfig.serverModuleFormat,
ssr: reactRouterConfig.ssr,
hasServerBundles: !!reactRouterConfig.serverBundles,
serverInstrumentationFile: sentryConfig.serverInstrumentationFile ?? DEFAULT_SERVER_INSTRUMENTATION_FILE,
debug,
});
} catch (error) {
// eslint-disable-next-line no-console
console.error('[Sentry] Could not auto-inject server instrumentation', error);
}
Comment thread
chargome marked this conversation as resolved.
}

function getSentryConfig(viteConfig: unknown): SentryReactRouterBuildOptions {
if (!viteConfig || typeof viteConfig !== 'object' || !('sentryConfig' in viteConfig)) {
// eslint-disable-next-line no-console
console.error('[Sentry] sentryConfig not found - it needs to be passed to vite.config.ts');
// Fall back to an empty config so the build hook degrades gracefully instead of throwing on destructuring.
return {};
}

return (viteConfig as { sentryConfig: SentryReactRouterBuildOptions }).sentryConfig;
Expand Down Expand Up @@ -148,4 +183,8 @@ export const sentryOnBuildEnd: BuildEndHook = async ({ reactRouterConfig, viteCo
console.error('Error deleting files after sourcemap upload:', error);
}
}

// Auto-inject server instrumentation into the built server bundle (after source maps are handled, so we never
// interfere with debug-id injection / upload).
await maybeAutoInjectServerInstrumentation(sentryConfig, reactRouterConfig, viteConfig, debug);
Comment on lines +186 to +189

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: Server instrumentation is injected after source maps are uploaded, causing a 2-line offset in Sentry stack traces for the server entry file.
Severity: MEDIUM

Suggested Fix

Modify the build process to perform the server-side auto-instrumentation before the source maps are generated and uploaded. This will ensure that the source maps accurately reflect the final, instrumented code, preventing line number mismatches in Sentry.

Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.

Location: packages/react-router/src/vite/buildEnd/handleOnBuildEnd.ts#L186-L189

Potential issue: When both `autoInjectServerInstrumentation` and source map uploading
are enabled, the server entry file is modified after its source maps have been uploaded
to Sentry. The `maybeAutoInjectServerInstrumentation` function prepends two lines to the
server entry file, but this happens after `sentry-cli releases uploadSourceMaps` has
already run. As a result, the source maps stored in Sentry do not match the final
production code. This causes all stack traces originating from the server entry file to
have a systematic two-line offset, making debugging significantly more difficult.

Did we get this right? 👍 / 👎 to inform future reviews.

};
Original file line number Diff line number Diff line change
@@ -0,0 +1,154 @@
import * as fs from 'node:fs';
import * as path from 'node:path';
import { consoleSandbox } from '@sentry/core';
import { detectCloudflareTarget } from './detectDeployTarget';

/** Marker written into generated files so re-runs of the build are idempotent and don't double-inject. */
export const SENTRY_AUTO_INJECT_MARKER = '/* @sentry/react-router auto-injected server instrumentation */';

/** Default (relative to project root) path to the user's pre-built ESM server instrumentation file. */
export const DEFAULT_SERVER_INSTRUMENTATION_FILE = './instrument.server.mjs';

const LOG_PREFIX = '[Sentry React Router]';

interface InjectServerInstrumentationOptions {
/** Absolute path to the project root (Vite's `config.root`). */
root: string;
/** Absolute path to the build directory (`reactRouterConfig.buildDirectory`). */
buildDirectory: string;
/** The server build file name (`reactRouterConfig.serverBuildFile`, e.g. `index.js`). */
serverBuildFile: string;
/**
* The server build output format (`reactRouterConfig.serverModuleFormat`, `'esm'` or `'cjs'`). We only inject the
* ESM `import` prefix into ESM builds; CJS builds are skipped with a warning.
*/
serverModuleFormat: 'esm' | 'cjs';
/** Whether SSR is enabled (`reactRouterConfig.ssr`). When `false` there is no server build to wrap. */
ssr: boolean;
/** Whether server bundles are in use (`reactRouterConfig.serverBundles`). Not supported by auto-injection. */
hasServerBundles: boolean;
/** Path (relative to root) to the user's server instrumentation file. */
serverInstrumentationFile: string;
debug: boolean;
}

/**
* Generates the top-level import prefix that is prepended to the server build entry. This loads the Sentry server
* config before the rest of the entry module body runs.
*
* With orchestrion (diagnostics-channel) instrumentation, a top-level import is sufficient: instrumented libraries
* are patched as they are loaded, so there is no need to defer the entry behind a dynamic `import()`.
*/
export function generateTopLevelImportPrefix(instrumentationImportPath: string): string {
return `${SENTRY_AUTO_INJECT_MARKER}\nimport ${JSON.stringify(instrumentationImportPath)};\n`;
}

function log(message: string, debug: boolean): void {
if (!debug) {
return;
}
consoleSandbox(() => {
// eslint-disable-next-line no-console
console.log(`${LOG_PREFIX} ${message}`);
});
}

function warn(message: string): void {
consoleSandbox(() => {
// eslint-disable-next-line no-console
console.warn(`${LOG_PREFIX} ${message}`);
});
}

/**
* Auto-injects Sentry server instrumentation into the React Router server build output, removing the need for a
* manual `NODE_OPTIONS='--import ./instrument.server.mjs'`. Called from `sentryOnBuildEnd` after source map
* handling, so it never interferes with debug-id injection / source map upload.
*
* Gracefully no-ops (with a debug log) for SPA/prerender-only builds and skips (with a warning) for deploy targets
* that aren't supported yet.
*/
export async function injectServerInstrumentation(options: InjectServerInstrumentationOptions): Promise<void> {
const {
root,
buildDirectory,
serverBuildFile,
serverModuleFormat,
ssr,
hasServerBundles,
serverInstrumentationFile,
debug,
} = options;

// SPA / prerender-only builds have no server entry to wrap.
if (!ssr) {
log('`ssr` is disabled (SPA mode) - skipping server instrumentation auto-injection.', debug);
return;
}

// We prepend an ESM `import`, which is invalid in a CJS server build and would crash the server at startup.
if (serverModuleFormat === 'cjs') {
warn(
'`autoInjectServerInstrumentation` only supports ESM server builds (`serverModuleFormat: "esm"`) - skipping. ' +
'Please import your server instrumentation file manually at the top of your server entry instead.',
);
return;
}

if (hasServerBundles) {
warn(
'`autoInjectServerInstrumentation` does not support `serverBundles` yet - skipping. ' +
'Please import your server instrumentation file manually at the top of your server entry instead.',
);
return;
}

if (detectCloudflareTarget(root)) {
log(
'Detected a Cloudflare deploy target - skipping injection. Initialize Sentry inside your worker instead.',
debug,
);
return;
}

// Read the server entry directly rather than `existsSync`-then-read: checking first opens a
// file-system race (the file could change between check and use) and CodeQL flags it.
const serverEntryPath = path.resolve(buildDirectory, 'server', serverBuildFile);
let originalContent: string;
try {
originalContent = fs.readFileSync(serverEntryPath, 'utf-8');
} catch {
warn(`Could not read server build entry at \`${serverEntryPath}\` - skipping auto-injection.`);
return;
}

// Idempotency: if we already injected (e.g. a rebuild without cleaning the output dir), do nothing.
if (originalContent.includes(SENTRY_AUTO_INJECT_MARKER)) {
log('Server build already instrumented - skipping.', debug);
return;
}

// Copy the user's instrumentation file next to the server entry so the build output is self-contained.
// Attempt the copy directly (no prior `existsSync` check) to avoid a check-then-use race.
const copiedInstrumentationFileName = 'instrument.server.mjs';
const copiedInstrumentationPath = path.resolve(buildDirectory, 'server', copiedInstrumentationFileName);
const instrumentationSourcePath = path.resolve(root, serverInstrumentationFile);
try {
fs.copyFileSync(instrumentationSourcePath, copiedInstrumentationPath);
} catch {
warn(
`Could not read server instrumentation file at \`${instrumentationSourcePath}\`. ` +
'Create it (calling `Sentry.init`) or set the `autoInjectServerInstrumentation` option to `false`.',
);
return;
}
const instrumentationImportPath = `./${copiedInstrumentationFileName}`;

fs.writeFileSync(serverEntryPath, generateTopLevelImportPrefix(instrumentationImportPath) + originalContent);
Comment thread
github-advanced-security[bot] marked this conversation as resolved.
Fixed
Comment thread
chargome marked this conversation as resolved.

log(
`Prepended a top-level import of \`${instrumentationImportPath}\` to \`${serverBuildFile}\` so Sentry is ` +
'initialized before the server starts.',
debug,
);
}
20 changes: 20 additions & 0 deletions packages/react-router/src/vite/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -74,4 +74,24 @@ export type SentryReactRouterBuildOptions = BuildTimeOptionsBase &
*/
sourceMapsUploadOptions?: SourceMapsOptions;
// todo(v11): Remove this option (all options already exist in BuildTimeOptionsBase)

/**
* Prepends a top-level import of your server instrumentation file (see {@link serverInstrumentationFile}) to the
* built server entry.
*
* Requires `buildEnd: sentryOnBuildEnd` in `react-router.config.ts`.
* no-ops for CJS/SPA/serverless/Cloudflare targets. Do not also use `--import`, or Sentry initializes twice.
*
* @default false
*/
// todo(v11): Default this to `true`.
autoInjectServerInstrumentation?: boolean;

/**
* Path (relative to the project root) to the server instrumentation file that calls `Sentry.init`.
* Only used when {@link autoInjectServerInstrumentation} is enabled.
*
* @default './instrument.server.mjs'
*/
serverInstrumentationFile?: string;
};
Original file line number Diff line number Diff line change
Expand Up @@ -365,4 +365,16 @@ describe('sentryOnBuildEnd', () => {
url: 'https://custom-instance.ejemplo.es',
});
});

it('does not throw when sentryConfig is missing from viteConfig', async () => {
const config = {
...defaultConfig,
viteConfig: {
build: { sourcemap: true },
} as unknown as TestConfig,
};

// @ts-expect-error - mocking the React config
await expect(sentryOnBuildEnd(config)).resolves.toBeUndefined();
});
});
Loading
Loading