From fab911521ce2cc3be765e3953fb962a8e3b1f320 Mon Sep 17 00:00:00 2001 From: Charly Gomez Date: Fri, 2 Oct 2026 10:18:22 +0200 Subject: [PATCH 1/2] feat(remix): Warn when the Remix 3 instrumentation does not apply A module the runtime hook transformed is listed in `__SENTRY_ORCHESTRION__.runtime`. After `init()`, once the app's import graph has loaded, the integration checks that list for the Remix 3 server modules. An installed module that is missing is reported with its reason: imported before the hook was registered, or a version outside the range the transform matches. A hook that could not be registered at all names the Node version. The subscribers that patch a router, request listener or asset server no longer fail silently when the call has a shape they cannot patch. Co-Authored-By: Claude Fable 5.1 --- .../tests/instrumentation-warning.test.ts | 34 ++++ packages/remix/src/v3/assetServer.ts | 9 +- .../src/v3/server/checkInstrumentation.ts | 121 +++++++++++++ packages/remix/src/v3/server/instrument.ts | 9 +- packages/remix/src/v3/server/integration.ts | 6 + .../test/v3/checkInstrumentation.test.ts | 159 ++++++++++++++++++ packages/remix/test/v3/instrument.test.ts | 13 +- packages/remix/test/v3/integration.test.ts | 31 ++++ 8 files changed, 373 insertions(+), 9 deletions(-) create mode 100644 dev-packages/e2e-tests/test-applications/remix-v3/tests/instrumentation-warning.test.ts create mode 100644 packages/remix/src/v3/server/checkInstrumentation.ts create mode 100644 packages/remix/test/v3/checkInstrumentation.test.ts create mode 100644 packages/remix/test/v3/integration.test.ts diff --git a/dev-packages/e2e-tests/test-applications/remix-v3/tests/instrumentation-warning.test.ts b/dev-packages/e2e-tests/test-applications/remix-v3/tests/instrumentation-warning.test.ts new file mode 100644 index 000000000000..fd8065258b09 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/remix-v3/tests/instrumentation-warning.test.ts @@ -0,0 +1,34 @@ +import { spawn } from 'node:child_process'; +import { expect, test } from '@playwright/test'; + +// Started with Remix's own loader instead of the Sentry entry, which is what a setup that forgot to +// switch looks like. The imports are hoisted above `Sentry.init()`, so the hook `init()` registers +// comes too late for every Remix module. +test('warns when the app starts without the Sentry --import entry', async () => { + const app = spawn('node', ['--import', 'remix/node-tsx', 'server.ts'], { + cwd: process.cwd(), + env: { ...process.env, NODE_ENV: 'production', PORT: '3062' }, + }); + + let stderr = ''; + const warned = new Promise(resolve => { + app.stderr.on('data', chunk => { + stderr += String(chunk); + if (stderr.includes('--import @sentry/remix/v3/node')) { + resolve(stderr); + } + }); + }); + + try { + const output = await Promise.race([ + warned, + new Promise((_, reject) => setTimeout(() => reject(new Error(`no warning in:\n${stderr}`)), 15_000)), + ]); + expect(output).toContain( + '[Sentry] Remix 3 is not instrumented: @remix-run/fetch-router was imported before the Sentry module hook was registered', + ); + } finally { + app.kill(); + } +}); diff --git a/packages/remix/src/v3/assetServer.ts b/packages/remix/src/v3/assetServer.ts index 88ce5060662a..593b4cb8873b 100644 --- a/packages/remix/src/v3/assetServer.ts +++ b/packages/remix/src/v3/assetServer.ts @@ -3,6 +3,7 @@ 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'; +import { describeError, warnRemixV3 } from './server/checkInstrumentation'; // The subset of `@remix-run/assets` types used here. `remix` is an optional peer dependency, so // they are restated rather than imported. @@ -82,8 +83,8 @@ export function instrumentAssetServer(): void { const options = context.arguments[0] as AssetServerOptions | undefined; context._sentryHideSourceMaps = options?.sourceMaps === undefined; context.arguments[0] = withDebugIdOptions(options); - } catch { - // Ignored on purpose. + } catch (error) { + warnRemixV3(`Could not configure the Remix 3 asset server for debug IDs (${describeError(error)}).`); } }, end(data) { @@ -92,8 +93,8 @@ export function instrumentAssetServer(): void { if (result) { stampServedAssets(result as AssetServer, { hideSourceMaps: Boolean(_sentryHideSourceMaps) }); } - } catch { - // Ignored on purpose. + } catch (error) { + warnRemixV3(`Could not wrap the Remix 3 asset server for debug IDs (${describeError(error)}).`); } }, asyncStart() {}, diff --git a/packages/remix/src/v3/server/checkInstrumentation.ts b/packages/remix/src/v3/server/checkInstrumentation.ts new file mode 100644 index 000000000000..25c1cb4087ec --- /dev/null +++ b/packages/remix/src/v3/server/checkInstrumentation.ts @@ -0,0 +1,121 @@ +import { existsSync, readFileSync, realpathSync } from 'node:fs'; +import { dirname, join } from 'node:path'; +import { consoleSandbox, GLOBAL_OBJ, parseSemver } from '@sentry/core'; +import { remixV3Config } from '@sentry/server-utils/orchestrion/config'; + +const IMPORT_HINT = 'Start Node with `--import @sentry/remix/v3/node`.'; + +// Every Remix 3 server app creates a router, so this module decides whether the hook was in place. +// The other packages cannot be checked the same way: `remix` depends on all of them, so they look +// installed whether or not the app imports them. +const ANCHOR_MODULE = '@remix-run/fetch-router'; + +const warned = new Set(); + +export function describeError(error: unknown): string { + return error instanceof Error ? error.message : String(error); +} + +/** An always-on warning, deduplicated by message. A user who sees no data will not have `debug` on. */ +export function warnRemixV3(message: string): void { + if (warned.has(message)) { + return; + } + warned.add(message); + consoleSandbox(() => { + // oxlint-disable-next-line no-console + console.warn(`[Sentry] ${message}`); + }); +} + +/** + * Warn when the Remix 3 router module did not go through the runtime hook, with the reason. + * + * A module the hook transformed is listed in `__SENTRY_ORCHESTRION__.runtime`. One that is installed + * but not listed was either imported before the hook was registered, or is outside the version range + * the transform matches. Nothing is reported when it is not installed: the app does not use Remix 3. + */ +export function checkRemixV3Instrumentation(getInstalledVersion = readInstalledVersion): void { + const marker = GLOBAL_OBJ.__SENTRY_ORCHESTRION__; + + if (marker?.runtimeUnavailable) { + warnRemixV3(`Remix 3 is not instrumented: the module hook could not be registered on Node ${process.version}.`); + return; + } + + if (marker?.runtime?.includes(ANCHOR_MODULE)) { + return; + } + + const version = getInstalledVersion(ANCHOR_MODULE); + if (version === undefined) { + return; + } + + const range = getVersionRange(ANCHOR_MODULE); + if (range && !isInRange(version, range)) { + warnRemixV3(`Remix 3 is not instrumented: ${ANCHOR_MODULE}@${version} is outside the supported range ${range}.`); + return; + } + + warnRemixV3( + `Remix 3 is not instrumented: ${ANCHOR_MODULE} was imported before the Sentry module hook was registered. ${IMPORT_HINT}`, + ); +} + +function getVersionRange(name: string): string | undefined { + return remixV3Config.find(config => config.module.name === name)?.module.versionRange; +} + +/** Only the `>=a.b.c =(\d+)\.(\d+)\.(\d+) <(\d+)$/); + const { major, minor, patch } = parseSemver(version); + if (!match || major === undefined || minor === undefined || patch === undefined) { + return true; + } + const [, minMajor = '0', minMinor = '0', minPatch = '0', maxMajor = '0'] = match; + const atLeastMin = + major > Number(minMajor) || + (major === Number(minMajor) && + (minor > Number(minMinor) || (minor === Number(minMinor) && patch >= Number(minPatch)))); + return atLeastMin && major < Number(maxMajor); +} + +/** + * The version of a Remix 3 package as the app resolves it, or `undefined` when it is not installed. + * Looked up from the `remix` package's real location: under pnpm the `@remix-run/*` packages are only + * reachable from there, and resolving from the app would find a hoisted copy of a different version. + * A plain `node_modules` walk rather than `require.resolve`, because `remix` exports no `package.json`. + */ +export function readInstalledVersion(name: string, fromDir = process.cwd()): string | undefined { + try { + const remixPackageJson = findPackageJson('remix', fromDir); + if (!remixPackageJson) { + return undefined; + } + const packageJson = findPackageJson(name, dirname(realpathSync(remixPackageJson))); + if (!packageJson) { + return undefined; + } + const { version } = JSON.parse(readFileSync(packageJson, 'utf8')) as { version?: unknown }; + return typeof version === 'string' ? version : undefined; + } catch { + return undefined; + } +} + +function findPackageJson(name: string, fromDir: string): string | undefined { + let dir = fromDir; + for (;;) { + const candidate = join(dir, 'node_modules', name, 'package.json'); + if (existsSync(candidate)) { + return candidate; + } + const parent = dirname(dir); + if (parent === dir) { + return undefined; + } + dir = parent; + } +} diff --git a/packages/remix/src/v3/server/instrument.ts b/packages/remix/src/v3/server/instrument.ts index d16fff4746af..59b33cf98399 100644 --- a/packages/remix/src/v3/server/instrument.ts +++ b/packages/remix/src/v3/server/instrument.ts @@ -3,6 +3,7 @@ import { createMultiMatcher } from 'remix/route-pattern/match'; import { remixV3Channels } from '@sentry/server-utils/orchestrion/config'; import type { MatcherLike, RequestListenerOptionsLike, RouterOptionsLike } from '../types'; +import { describeError, warnRemixV3 } from './checkInstrumentation'; import { captureRequestError } from './errorFilter'; import { sentryRemixMiddleware } from './middleware'; @@ -47,8 +48,8 @@ function subscribeToCreateRouter(): void { // copy that cannot build a matcher all reach here, so the router is left uninstrumented instead. try { injectRouterMiddleware(ensureOptions(data.arguments)); - } catch { - // Ignored on purpose. + } catch (error) { + warnRemixV3(`Could not add the Sentry middleware to a Remix 3 router (${describeError(error)}).`); } }, end: NOOP, @@ -70,8 +71,8 @@ function subscribeToCreateRequestListener(): void { start(data) { try { injectOnError(ensureOptions(data.arguments, 1)); - } catch { - // Ignored on purpose. + } catch (error) { + warnRemixV3(`Could not hook the Remix 3 request listener's error handler (${describeError(error)}).`); } }, end: NOOP, diff --git a/packages/remix/src/v3/server/integration.ts b/packages/remix/src/v3/server/integration.ts index 3ceabf6acbd4..9ec735312cef 100644 --- a/packages/remix/src/v3/server/integration.ts +++ b/packages/remix/src/v3/server/integration.ts @@ -1,5 +1,6 @@ import { defineIntegration, type IntegrationFn } from '@sentry/core'; +import { checkRemixV3Instrumentation } from './checkInstrumentation'; import { setShouldHandleError, type ShouldHandleError } from './errorFilter'; import { instrumentRemixV3 } from './instrument'; @@ -23,6 +24,11 @@ const _remixV3Integration = ((options: RemixV3IntegrationOptions = {}) => { // `init()`, so `--import @sentry/remix/v3/node` has already subscribed. This covers setups that // register the module hook from `init()` instead. instrumentRemixV3(); + + // Deferred: `init()` can run from an `--import`ed file before the app's own modules load. Once + // the timer fires, the import graph has been evaluated and the hook has seen every module. + const timer = setTimeout(() => checkRemixV3Instrumentation(), 0); + timer.unref?.(); }, }; }) satisfies IntegrationFn; diff --git a/packages/remix/test/v3/checkInstrumentation.test.ts b/packages/remix/test/v3/checkInstrumentation.test.ts new file mode 100644 index 000000000000..221c4c9c1481 --- /dev/null +++ b/packages/remix/test/v3/checkInstrumentation.test.ts @@ -0,0 +1,159 @@ +import * as fs from 'node:fs'; +import * as os from 'node:os'; +import * as path from 'node:path'; +import { GLOBAL_OBJ } from '@sentry/core'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; + +import type * as CheckModule from '../../src/v3/server/checkInstrumentation'; + +type Check = typeof CheckModule.checkRemixV3Instrumentation; + +const INSTALLED: Record = { '@remix-run/fetch-router': '0.21.0' }; + +let check: Check; +let readInstalledVersion: typeof CheckModule.readInstalledVersion; +let warn: ReturnType; + +/** A fresh module per test, since the warning is deduplicated for the life of the process. */ +beforeEach(async () => { + vi.resetModules(); + ({ checkRemixV3Instrumentation: check, readInstalledVersion } = + await import('../../src/v3/server/checkInstrumentation')); + warn = vi.spyOn(console, 'warn').mockImplementation(() => undefined); + GLOBAL_OBJ.__SENTRY_ORCHESTRION__ = { runtime: ['@remix-run/fetch-router'] }; +}); + +afterEach(() => { + warn.mockRestore(); + delete GLOBAL_OBJ.__SENTRY_ORCHESTRION__; +}); + +function installed(name: string): string | undefined { + return INSTALLED[name]; +} + +function warning(): string { + expect(warn).toHaveBeenCalledTimes(1); + return String(warn.mock.calls[0]?.[0]); +} + +describe('checkRemixV3Instrumentation', () => { + it('stays quiet when the hook transformed the router module', () => { + GLOBAL_OBJ.__SENTRY_ORCHESTRION__ = { runtime: ['@remix-run/fetch-router'] }; + + check(installed); + + expect(warn).not.toHaveBeenCalled(); + }); + + it('does not judge the other Remix packages, since remix installs them whether the app uses them or not', () => { + GLOBAL_OBJ.__SENTRY_ORCHESTRION__ = { runtime: ['@remix-run/fetch-router'] }; + + check(() => '0.1.0'); + + expect(warn).not.toHaveBeenCalled(); + }); + + it('reports the Node version when the module hook could not be registered', () => { + GLOBAL_OBJ.__SENTRY_ORCHESTRION__ = { runtimeUnavailable: true }; + + check(installed); + + expect(warning()).toContain(`could not be registered on Node ${process.version}`); + }); + + it('points at --import when the installed router module in range was not transformed', () => { + GLOBAL_OBJ.__SENTRY_ORCHESTRION__ = { runtime: [] }; + + check(installed); + + const message = warning(); + expect(message).toContain('@remix-run/fetch-router was imported before the Sentry module hook'); + expect(message).toContain('--import @sentry/remix/v3/node'); + }); + + it('reports a version outside the supported range as that, not as a missing --import', () => { + GLOBAL_OBJ.__SENTRY_ORCHESTRION__ = { runtime: [] }; + + check(() => '1.0.0'); + + const message = warning(); + expect(message).toContain('@remix-run/fetch-router@1.0.0 is outside the supported range >=0.21.0 <1'); + expect(message).not.toContain('--import'); + }); + + it('stays quiet when Remix 3 is not installed', () => { + GLOBAL_OBJ.__SENTRY_ORCHESTRION__ = { runtime: [] }; + + check(() => undefined); + + expect(warn).not.toHaveBeenCalled(); + }); + + it('treats no marker at all as nothing transformed', () => { + delete GLOBAL_OBJ.__SENTRY_ORCHESTRION__; + + check(installed); + + expect(warning()).toContain('was imported before the Sentry module hook'); + }); + + it('warns once', () => { + GLOBAL_OBJ.__SENTRY_ORCHESTRION__ = { runtime: [] }; + + check(installed); + check(installed); + + expect(warn).toHaveBeenCalledTimes(1); + }); +}); + +describe('readInstalledVersion', () => { + const dirs: string[] = []; + afterEach(() => { + for (const dir of dirs.splice(0)) { + fs.rmSync(dir, { recursive: true, force: true }); + } + }); + + function writePackage(root: string, name: string, version: string): string { + const dir = path.join(root, 'node_modules', name); + fs.mkdirSync(dir, { recursive: true }); + fs.writeFileSync(path.join(dir, 'package.json'), JSON.stringify({ name, version })); + return dir; + } + + it('reads the copy next to the real remix package under pnpm, not a hoisted one', () => { + const app = fs.mkdtempSync(path.join(os.tmpdir(), 'remix-app-')); + dirs.push(app); + // Hoisted at the app root: the wrong copy. + writePackage(app, '@remix-run/fetch-router', '0.1.0'); + // The pnpm store: remix and its own dependency side by side, reached through a symlink. + const store = path.join(app, 'node_modules', '.pnpm', 'remix@3.0.0'); + const realRemix = writePackage(store, 'remix', '3.0.0'); + writePackage(store, '@remix-run/fetch-router', '0.21.0'); + fs.mkdirSync(path.join(app, 'node_modules'), { recursive: true }); + fs.symlinkSync(realRemix, path.join(app, 'node_modules', 'remix'), 'dir'); + + expect(readInstalledVersion('@remix-run/fetch-router', app)).toBe('0.21.0'); + }); + + it('walks up from a nested working directory', () => { + const app = fs.mkdtempSync(path.join(os.tmpdir(), 'remix-app-')); + dirs.push(app); + writePackage(app, 'remix', '3.0.0'); + writePackage(app, '@remix-run/assets', '0.6.0'); + + expect(readInstalledVersion('@remix-run/assets', path.join(app, 'app', 'routes'))).toBe('0.6.0'); + }); + + it('is undefined when remix or the package is not installed', () => { + const app = fs.mkdtempSync(path.join(os.tmpdir(), 'remix-app-')); + dirs.push(app); + + expect(readInstalledVersion('@remix-run/assets', app)).toBeUndefined(); + + writePackage(app, 'remix', '3.0.0'); + expect(readInstalledVersion('@remix-run/assets', app)).toBeUndefined(); + }); +}); diff --git a/packages/remix/test/v3/instrument.test.ts b/packages/remix/test/v3/instrument.test.ts index 1fa4e14d0513..d0d57aa46701 100644 --- a/packages/remix/test/v3/instrument.test.ts +++ b/packages/remix/test/v3/instrument.test.ts @@ -3,7 +3,15 @@ import type * as SentryCore from '@sentry/core'; import { remixV3Channels } from '@sentry/server-utils/orchestrion/config'; import { beforeAll, beforeEach, describe, expect, it, vi } from 'vitest'; +import type * as CheckModule from '../../src/v3/server/checkInstrumentation'; + const captureException = vi.fn(); +const warnRemixV3 = vi.fn(); + +vi.mock('../../src/v3/server/checkInstrumentation', async importOriginal => ({ + ...(await importOriginal()), + warnRemixV3: (...args: unknown[]) => warnRemixV3(...args), +})); // Only `captureException` is replaced; the middleware needs the rest for real. vi.mock('@sentry/core', async importOriginal => ({ @@ -62,7 +70,7 @@ describe('instrumentRemixV3', () => { expect(options.matcher).toBe(appMatcher); }); - it('leaves an options object it cannot write to alone, without crashing the app', async () => { + it('leaves an options object it cannot write to alone and warns, without crashing the app', async () => { // Node rethrows an exception from a channel subscriber as an uncaught exception, so an unguarded // write here would take down an app that runs fine without Sentry. const uncaught: Error[] = []; @@ -80,6 +88,9 @@ describe('instrumentRemixV3', () => { expect(uncaught).toEqual([]); expect(options.middleware).toEqual([]); + expect(warnRemixV3).toHaveBeenCalledWith( + expect.stringContaining('Could not add the Sentry middleware to a Remix 3 router'), + ); }); }); diff --git a/packages/remix/test/v3/integration.test.ts b/packages/remix/test/v3/integration.test.ts new file mode 100644 index 000000000000..308403d83e89 --- /dev/null +++ b/packages/remix/test/v3/integration.test.ts @@ -0,0 +1,31 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; + +const checkRemixV3Instrumentation = vi.fn(); + +vi.mock('../../src/v3/server/checkInstrumentation', () => ({ + checkRemixV3Instrumentation: (...args: unknown[]) => checkRemixV3Instrumentation(...args), + warnRemixV3: vi.fn(), +})); + +const { remixV3Integration } = await import('../../src/v3/server/integration'); + +describe('remixV3Integration', () => { + beforeEach(() => { + vi.useFakeTimers(); + checkRemixV3Instrumentation.mockClear(); + }); + + afterEach(() => { + vi.useRealTimers(); + }); + + it('checks the instrumentation after the current import graph has loaded, not during setup', () => { + remixV3Integration().setupOnce?.(); + + expect(checkRemixV3Instrumentation).not.toHaveBeenCalled(); + + vi.runAllTimers(); + + expect(checkRemixV3Instrumentation).toHaveBeenCalledTimes(1); + }); +}); From 73abd6bfb07dc8d652e1a7d8ee6bb6a734951db6 Mon Sep 17 00:00:00 2001 From: Charly Gomez Date: Fri, 2 Oct 2026 10:34:26 +0200 Subject: [PATCH 2/2] fix(remix): Sandbox the request listener's fallback error log The log stands in for the listener's default handler. Sandboxed so the SDK's console instrumentation does not report the error a second time. Co-Authored-By: Claude Fable 5.1 --- packages/remix/src/v3/server/instrument.ts | 10 +++++++--- packages/remix/test/v3/instrument.test.ts | 2 ++ 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/packages/remix/src/v3/server/instrument.ts b/packages/remix/src/v3/server/instrument.ts index 59b33cf98399..7d18c53fd4eb 100644 --- a/packages/remix/src/v3/server/instrument.ts +++ b/packages/remix/src/v3/server/instrument.ts @@ -1,5 +1,6 @@ import * as diagnosticsChannel from 'node:diagnostics_channel'; import { createMultiMatcher } from 'remix/route-pattern/match'; +import { consoleSandbox } from '@sentry/core'; import { remixV3Channels } from '@sentry/server-utils/orchestrion/config'; import type { MatcherLike, RequestListenerOptionsLike, RouterOptionsLike } from '../types'; @@ -98,9 +99,12 @@ function injectOnError(raw: Record | undefined): void { return appOnError(error); } - // Setting `onError` replaced the listener's default handler, which logs the error. - // oxlint-disable-next-line no-console - console.error(error); + // Setting `onError` replaced the listener's default handler, which logs the error. Sandboxed so + // the SDK's console instrumentation does not report it a second time. + consoleSandbox(() => { + // oxlint-disable-next-line no-console + console.error(error); + }); return undefined; }; } diff --git a/packages/remix/test/v3/instrument.test.ts b/packages/remix/test/v3/instrument.test.ts index d0d57aa46701..bf7b29b1e917 100644 --- a/packages/remix/test/v3/instrument.test.ts +++ b/packages/remix/test/v3/instrument.test.ts @@ -17,6 +17,8 @@ vi.mock('../../src/v3/server/checkInstrumentation', async importOriginal => ({ vi.mock('@sentry/core', async importOriginal => ({ ...(await importOriginal()), captureException: (...args: unknown[]) => captureException(...args), + // Runs the callback as is, so the spy on `console.error` below sees the call. + consoleSandbox: (callback: () => unknown) => callback(), })); const { instrumentRemixV3 } = await import('../../src/v3/server/instrument');