From ae0ee84509458eeb0d44caf61500c979bd0025dd Mon Sep 17 00:00:00 2001 From: Charly Gomez Date: Fri, 25 Sep 2026 17:03:46 +0200 Subject: [PATCH] feat(remix): Instrument Remix 3 server requests via fetch-router Patches `createRouter` through orchestrion to prepend a Sentry middleware to every router. The middleware opens no span: Remix 3 serves over `node:http`, so `httpIntegration` has already opened the `http.server` span. It only enriches it with the route, the response status and a low cardinality name. The matcher has to be supplied by the SDK because the router never exposes one, and the request context carries no pattern at middleware entry. Matching happens after router middleware runs and only writes back `params`, so the route is resolved by re-running the match. Co-Authored-By: Claude Opus 5 --- .../remix-v3/app/actions/controller.tsx | 6 ++ .../test-applications/remix-v3/app/router.ts | 6 ++ .../test-applications/remix-v3/app/routes.ts | 2 + .../remix-v3/tests/server-spans.test.ts | 51 +++++++++++ .../remix-v3/tests/smoke.test.ts | 13 --- packages/remix/package.json | 1 + packages/remix/src/v3/index.server.ts | 8 +- packages/remix/src/v3/node.mjs | 14 ++- packages/remix/src/v3/server/instrument.ts | 91 +++++++++++++++++++ packages/remix/src/v3/server/integration.ts | 22 +++++ packages/remix/src/v3/server/middleware.ts | 69 ++++++++++++++ packages/remix/src/v3/server/route.ts | 37 ++++++++ packages/remix/src/v3/server/sdk.ts | 32 +++++++ packages/remix/src/v3/types.ts | 39 ++++++++ packages/remix/test/v3/instrument.test.ts | 70 ++++++++++++++ packages/remix/test/v3/route.test.ts | 81 +++++++++++++++++ .../src/orchestrion/config/index.ts | 4 + .../src/orchestrion/config/remix-v3.ts | 26 ++++++ yarn.lock | 5 + 19 files changed, 559 insertions(+), 18 deletions(-) create mode 100644 dev-packages/e2e-tests/test-applications/remix-v3/tests/server-spans.test.ts create mode 100644 packages/remix/src/v3/server/instrument.ts create mode 100644 packages/remix/src/v3/server/integration.ts create mode 100644 packages/remix/src/v3/server/middleware.ts create mode 100644 packages/remix/src/v3/server/route.ts create mode 100644 packages/remix/src/v3/server/sdk.ts create mode 100644 packages/remix/src/v3/types.ts create mode 100644 packages/remix/test/v3/instrument.test.ts create mode 100644 packages/remix/test/v3/route.test.ts create mode 100644 packages/server-utils/src/orchestrion/config/remix-v3.ts diff --git a/dev-packages/e2e-tests/test-applications/remix-v3/app/actions/controller.tsx b/dev-packages/e2e-tests/test-applications/remix-v3/app/actions/controller.tsx index 9325143b071d..756115577e68 100644 --- a/dev-packages/e2e-tests/test-applications/remix-v3/app/actions/controller.tsx +++ b/dev-packages/e2e-tests/test-applications/remix-v3/app/actions/controller.tsx @@ -30,5 +30,11 @@ export default createController(routes, { home(context) { return context.render(); }, + user(context) { + return Response.json({ id: context.params.id }); + }, + teapot() { + return new Response("I'm a teapot", { status: 418 }); + }, }, }); diff --git a/dev-packages/e2e-tests/test-applications/remix-v3/app/router.ts b/dev-packages/e2e-tests/test-applications/remix-v3/app/router.ts index aef480456066..03724dd7ea27 100644 --- a/dev-packages/e2e-tests/test-applications/remix-v3/app/router.ts +++ b/dev-packages/e2e-tests/test-applications/remix-v3/app/router.ts @@ -19,3 +19,9 @@ export const router = createRouter({ }); router.map(routes, controller); + +// Mounted rather than added to the route map, so the tests cover a route whose pattern carries a mount +// prefix. +router.mount('/api', api => { + api.get('/items/:itemId', context => Response.json({ itemId: context.params.itemId })); +}); diff --git a/dev-packages/e2e-tests/test-applications/remix-v3/app/routes.ts b/dev-packages/e2e-tests/test-applications/remix-v3/app/routes.ts index 1ebc77927332..77b32281374b 100644 --- a/dev-packages/e2e-tests/test-applications/remix-v3/app/routes.ts +++ b/dev-packages/e2e-tests/test-applications/remix-v3/app/routes.ts @@ -3,4 +3,6 @@ import { get, route } from 'remix/routes'; export const routes = route({ assets: get('/assets/*path'), home: '/', + user: get('/users/:id'), + teapot: get('/teapot'), }); diff --git a/dev-packages/e2e-tests/test-applications/remix-v3/tests/server-spans.test.ts b/dev-packages/e2e-tests/test-applications/remix-v3/tests/server-spans.test.ts new file mode 100644 index 000000000000..96e618b7b7f1 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/remix-v3/tests/server-spans.test.ts @@ -0,0 +1,51 @@ +import { expect, test } from '@playwright/test'; +import type { SerializedStreamedSpan } from '@sentry-internal/test-utils'; +import { getSpanOp, waitForStreamedSpan } from '@sentry-internal/test-utils'; + +const APP_NAME = 'remix-v3'; + +/** + * Selecting the span by `http.route` rather than by name is what makes the name assertions below mean + * something: a request that never resolved its route would not match, instead of matching and then + * passing a name check against whatever it happened to be called. + */ +function waitForServerSpan(route: string): Promise { + return waitForStreamedSpan( + APP_NAME, + span => + getSpanOp(span) === 'http.server' && span.is_segment === true && span.attributes?.['http.route']?.value === route, + ); +} + +test('names a parameterized route after its pattern', async ({ baseURL }) => { + const spanPromise = waitForServerSpan('/users/:id'); + + await fetch(`${baseURL}/users/12345`); + + const span = await spanPromise; + expect(span.name).toBe('GET /users/:id'); + // An id in the name would make every request its own transaction. + expect(span.name).not.toContain('12345'); + expect(span.attributes?.['sentry.segment.name.source']?.value).toBe('route'); +}); + +test('includes the mount prefix in the name of a mounted route', async ({ baseURL }) => { + const spanPromise = waitForServerSpan('/api/items/:itemId'); + + await fetch(`${baseURL}/api/items/abc`); + + const span = await spanPromise; + expect(span.name).toBe('GET /api/items/:itemId'); + expect(span.name).not.toContain('abc'); +}); + +test('records the response status', async ({ baseURL }) => { + const spanPromise = waitForServerSpan('/teapot'); + + await fetch(`${baseURL}/teapot`); + + const span = await spanPromise; + expect(span.attributes?.['http.response.status_code']?.value).toBe(418); + // OpenTelemetry leaves a 4xx server span unset, so the error status is the SDK's own doing. + expect(span.status).toBe('error'); +}); diff --git a/dev-packages/e2e-tests/test-applications/remix-v3/tests/smoke.test.ts b/dev-packages/e2e-tests/test-applications/remix-v3/tests/smoke.test.ts index 85e4619e49c4..3b005133749a 100644 --- a/dev-packages/e2e-tests/test-applications/remix-v3/tests/smoke.test.ts +++ b/dev-packages/e2e-tests/test-applications/remix-v3/tests/smoke.test.ts @@ -1,20 +1,7 @@ import { expect, test } from '@playwright/test'; -import { waitForStreamedSpan } from '@sentry-internal/test-utils'; -// There is no instrumentation yet. This app exists so later pull requests add instrumentation and its -// tests together, rather than also introducing a new CI surface. test('the app boots under the Sentry --import entry', async ({ page }) => { await page.goto('/'); await expect(page.locator('#home')).toBeVisible(); }); - -test('Sentry.init from the v3 subpath reports a server span', async ({ baseURL }) => { - // Names are still URL based. This only proves the subpath resolves and the SDK is live. - const spanPromise = waitForStreamedSpan('remix-v3', span => span.is_segment === true); - - await fetch(`${baseURL}/`); - - const span = await spanPromise; - expect(span.attributes?.['http.request.method']?.value).toBe('GET'); -}); diff --git a/packages/remix/package.json b/packages/remix/package.json index 5399dcbd29c3..9289e94a944c 100644 --- a/packages/remix/package.json +++ b/packages/remix/package.json @@ -74,6 +74,7 @@ "access": "public" }, "dependencies": { + "@remix-run/route-pattern": "^0.24.0", "@remix-run/router": "^1.23.4", "@sentry/browser": "11.0.0", "@sentry/bundler-plugins": "11.0.0", diff --git a/packages/remix/src/v3/index.server.ts b/packages/remix/src/v3/index.server.ts index 107a51657aca..870ab16899cb 100644 --- a/packages/remix/src/v3/index.server.ts +++ b/packages/remix/src/v3/index.server.ts @@ -1,4 +1,6 @@ -// Placeholder until the Remix 3 server instrumentation lands. `@sentry/node`'s `init` already emits -// `http.server` spans, so this is useful on its own; route parameterisation and router error capture -// are what is still missing. export * from '@sentry/node'; + +export { getDefaultIntegrations, init } from './server/sdk'; +export { remixV3Integration } from './server/integration'; +export { sentryRemixMiddleware } from './server/middleware'; +export { instrumentRemixV3 } from './server/instrument'; diff --git a/packages/remix/src/v3/node.mjs b/packages/remix/src/v3/node.mjs index 4923bc078d97..2c409d5a996e 100644 --- a/packages/remix/src/v3/node.mjs +++ b/packages/remix/src/v3/node.mjs @@ -1,3 +1,13 @@ -// Replaces `--import remix/node-tsx` rather than adding a second flag. Sentry's module hook is -// registered here once the server instrumentation lands, so for now nothing is instrumented. +// Replaces `--import remix/node-tsx` rather than adding a second flag. +// +// This has to happen here, not in `Sentry.init()`: the module hook must be in place before +// `@remix-run/fetch-router` is imported, and the subscription before `createRouter()` runs, which is +// while the app's own modules are still being imported. +import { registerDiagnosticsChannelInjection } from '@sentry/server-runtime-injection/register'; + +registerDiagnosticsChannelInjection(); + +const { instrumentRemixV3 } = await import('@sentry/remix/v3'); +instrumentRemixV3(); + await import('remix/node-tsx'); diff --git a/packages/remix/src/v3/server/instrument.ts b/packages/remix/src/v3/server/instrument.ts new file mode 100644 index 000000000000..30d012a184fd --- /dev/null +++ b/packages/remix/src/v3/server/instrument.ts @@ -0,0 +1,91 @@ +import * as diagnosticsChannel from 'node:diagnostics_channel'; +import { createMultiMatcher } from '@remix-run/route-pattern/match'; +import { remixV3Channels } from '@sentry/server-utils/orchestrion/config'; + +import type { MatcherLike, RouterOptionsLike } from '../types'; +import { sentryRemixMiddleware } from './middleware'; + +const NOOP = (): void => {}; + +/** The call's arguments, as orchestrion's transform attaches them to a tracing channel context. */ +interface ChannelContext { + arguments: unknown[]; +} + +// Marks an options object already injected into, so the same mutation cannot be applied twice. +const INJECTED = Symbol.for('SentryRemixV3Injected'); + +// `subscribe()` takes a fresh object literal and the channel keys handlers by identity, so nothing else +// stops a second call adding a second set. The documented setup calls this twice, from the `--import` +// entry and from `setupOnce()`. +let subscribed = false; + +/** + * Prepend the Sentry middleware to every router an app builds. + * + * Injection happens on the channel's `start` event, before `createRouter` reads its options. + * Orchestrion's transform collects the arguments into an array and spreads them back into the call, so + * assigning at an index the caller never passed works. That is what covers `createRouter()` with no + * arguments at all. + */ +export function instrumentRemixV3(): void { + if (subscribed || !diagnosticsChannel.tracingChannel) { + return; + } + subscribed = true; + + diagnosticsChannel.tracingChannel(remixV3Channels.REMIX_V3_CREATE_ROUTER).subscribe({ + start(data) { + // Node rethrows anything this handler throws as an uncaught exception, which would kill an app + // that runs fine without Sentry. Frozen options, a non-writable property and a `route-pattern` + // 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. + } + }, + end: NOOP, + asyncStart: NOOP, + asyncEnd: NOOP, + error: NOOP, + }); +} + +/** + * The options object, created when the caller omitted it. `undefined` when the caller passed something + * that is not an options object, which must not be overwritten. + */ +function ensureOptions(args: unknown[]): Record | undefined { + const existing = args[0]; + + if (existing === undefined || existing === null) { + const created: Record = {}; + args[0] = created; + return created; + } + + return typeof existing === 'object' ? (existing as Record) : undefined; +} + +function injectRouterMiddleware(raw: Record | undefined): void { + if (!raw) { + return; + } + + const marker = raw as { [INJECTED]?: boolean }; + if (marker[INJECTED]) { + return; + } + marker[INJECTED] = true; + + const options = raw as RouterOptionsLike; + + // The router never exposes its matcher, and resolving the route pattern needs one, so supplying it is + // the only way to hold a reference. An app that supplied its own keeps it. + const matcher: MatcherLike = options.matcher ?? (createMultiMatcher() as MatcherLike); + options.matcher = matcher; + + // Prepended rather than appended, so the Sentry middleware wraps the app's own. + options.middleware = [sentryRemixMiddleware(matcher), ...(options.middleware ?? [])]; +} diff --git a/packages/remix/src/v3/server/integration.ts b/packages/remix/src/v3/server/integration.ts new file mode 100644 index 000000000000..84af41b67156 --- /dev/null +++ b/packages/remix/src/v3/server/integration.ts @@ -0,0 +1,22 @@ +import { defineIntegration, type IntegrationFn } from '@sentry/core'; + +import { instrumentRemixV3 } from './instrument'; + +const INTEGRATION_NAME = 'RemixV3' as const; + +const _remixV3Integration = (() => { + return { + name: INTEGRATION_NAME, + setupOnce() { + // Usually a no-op: `createRouter()` runs while the app's modules are imported, before any + // `init()`, so `--import @sentry/remix/v3/node` has already subscribed. This covers setups that + // register the module hook from `init()` instead. + instrumentRemixV3(); + }, + }; +}) satisfies IntegrationFn; + +/** + * Names the `http.server` spans `@sentry/node` opens after the matched Remix 3 route. + */ +export const remixV3Integration = defineIntegration(_remixV3Integration); diff --git a/packages/remix/src/v3/server/middleware.ts b/packages/remix/src/v3/server/middleware.ts new file mode 100644 index 000000000000..0409d48f992e --- /dev/null +++ b/packages/remix/src/v3/server/middleware.ts @@ -0,0 +1,69 @@ +import { HTTP_RESPONSE_STATUS_CODE, HTTP_ROUTE } from '@sentry/conventions/attributes'; +import { + getActiveSpan, + getIsolationScope, + getRootSpan, + getSpanStatusFromHttpCode, + INTERNAL_setSegmentNameSourceIfSegment, + type Scope, + updateSpanName, + winterCGRequestToRequestData, +} from '@sentry/core'; + +import type { MatcherLike, MiddlewareLike, NextFunctionLike, RequestContextLike } from '../types'; +import { resolveRoutePattern } from './route'; + +/** + * The middleware orchestrion prepends to every router. + * + * It opens no span. Remix 3 serves over `node:http`, so `httpIntegration` has already opened the + * `http.server` span and forked an isolation scope by the time this runs. Opening another would double + * count every request, so this only enriches what exists, as `@sentry/hono` does. + */ +export function sentryRemixMiddleware(matcher: MatcherLike): MiddlewareLike { + return async function sentryMiddleware(context: RequestContextLike, next: NextFunctionLike): Promise { + const isolationScope = getIsolationScope(); + isolationScope.setSDKProcessingMetadata({ normalizedRequest: winterCGRequestToRequestData(context.request) }); + + // Applied before `next()` so anything captured while the handler runs already carries the route. + applyRoute(isolationScope, matcher, context); + + const response = await next(); + setResponseStatus(response); + + return response; + }; +} + +function applyRoute(isolationScope: Scope, matcher: MatcherLike, context: RequestContextLike): void { + const route = resolveRoutePattern(matcher, context); + if (!route) { + // Nothing matched, so the router falls through to its 404 handler. Leaving the name alone keeps the + // raw URL out of it, which span streaming requires. + return; + } + + const name = `${context.method} ${route}`; + isolationScope.setTransactionName(name); + + const activeSpan = getActiveSpan(); + if (!activeSpan) { + return; + } + + const rootSpan = getRootSpan(activeSpan); + updateSpanName(rootSpan, name); + INTERNAL_setSegmentNameSourceIfSegment(rootSpan, 'route'); + rootSpan.setAttribute(HTTP_ROUTE, route); +} + +function setResponseStatus(response: Response): void { + const activeSpan = getActiveSpan(); + if (!activeSpan || typeof response?.status !== 'number') { + return; + } + + const rootSpan = getRootSpan(activeSpan); + rootSpan.setAttribute(HTTP_RESPONSE_STATUS_CODE, response.status); + rootSpan.setStatus(getSpanStatusFromHttpCode(response.status)); +} diff --git a/packages/remix/src/v3/server/route.ts b/packages/remix/src/v3/server/route.ts new file mode 100644 index 000000000000..89ee52166b87 --- /dev/null +++ b/packages/remix/src/v3/server/route.ts @@ -0,0 +1,37 @@ +import type { MatcherLike, RequestContextLike } from '../types'; + +/** + * Resolve the low cardinality route pattern for a request. + * + * The request context never carries the matched pattern: the router matches after its middleware is + * entered, and only writes `params` back. So this re-runs the match against the router's own matcher, + * applying the router's own selection rules. + */ +export function resolveRoutePattern(matcher: MatcherLike, context: RequestContextLike): string | undefined { + let matches; + try { + matches = matcher.matchAll(context.url); + } catch { + // A matcher from a mismatched `@remix-run/route-pattern` copy can throw on a shape it does not + // recognise. Losing the route name is not worth failing the request over. + return undefined; + } + + let headFallback: string | undefined; + + for (const match of matches) { + const method = match.data?.method; + + if (method === context.method || method === 'ANY') { + return match.data?.pattern?.source; + } + + // A GET route also serves HEAD. The router compares specificity to pick between several; for a + // span name either pattern is equally correct, so the first one wins here. + if (context.method === 'HEAD' && method === 'GET') { + headFallback ??= match.data?.pattern?.source; + } + } + + return headFallback; +} diff --git a/packages/remix/src/v3/server/sdk.ts b/packages/remix/src/v3/server/sdk.ts new file mode 100644 index 000000000000..e375a11361ac --- /dev/null +++ b/packages/remix/src/v3/server/sdk.ts @@ -0,0 +1,32 @@ +import { applySdkMetadata, type Integration } from '@sentry/core'; +import { + getDefaultIntegrations as getNodeDefaultIntegrations, + init as nodeInit, + type NodeClient, + type NodeOptions, +} from '@sentry/node'; + +import { remixV3Integration } from './integration'; + +/** Default integrations for the Remix 3 server SDK. */ +export function getDefaultIntegrations(options: NodeOptions): Integration[] { + return [...getNodeDefaultIntegrations(options), remixV3Integration()]; +} + +/** + * Initialize the Sentry Remix 3 SDK on the server. + * + * Start the app with `--import @sentry/remix/v3/node` so the module hook is registered before + * `@remix-run/fetch-router` is imported. Remix 3 has no build step, so no bundler plugin can apply the + * transform, and imports are hoisted above any `init()` call in the server entry. + */ +export function init(options: NodeOptions): NodeClient | undefined { + const opts = { + ...options, + defaultIntegrations: options.defaultIntegrations ?? getDefaultIntegrations(options), + }; + + applySdkMetadata(opts, 'remix', ['remix', 'node']); + + return nodeInit(opts); +} diff --git a/packages/remix/src/v3/types.ts b/packages/remix/src/v3/types.ts new file mode 100644 index 000000000000..87dff35d797c --- /dev/null +++ b/packages/remix/src/v3/types.ts @@ -0,0 +1,39 @@ +// Structural copies of the `@remix-run/fetch-router` types the SDK touches, so it builds whether or +// not Remix 3 is installed. + +export interface RoutePatternLike { + source: string; +} + +/** What the router stores in its matcher, reachable as `match.data`. */ +export interface RouteEntryLike { + pattern: RoutePatternLike; + method: string; +} + +export interface MatchLike { + data: RouteEntryLike; + params: Record; +} + +/** Only these two, because they are all the router calls on whatever matcher it is given. */ +export interface MatcherLike { + add(pattern: unknown, data: unknown): void; + matchAll(url: string | URL): MatchLike[]; +} + +export interface RequestContextLike { + request: Request; + url: URL; + method: string; + params: Record; +} + +export type NextFunctionLike = () => Promise; + +export type MiddlewareLike = (context: RequestContextLike, next: NextFunctionLike) => Promise | Response; + +export interface RouterOptionsLike { + middleware?: MiddlewareLike[]; + matcher?: MatcherLike; +} diff --git a/packages/remix/test/v3/instrument.test.ts b/packages/remix/test/v3/instrument.test.ts new file mode 100644 index 000000000000..7eddc88c84d3 --- /dev/null +++ b/packages/remix/test/v3/instrument.test.ts @@ -0,0 +1,70 @@ +import { channel } from 'node:diagnostics_channel'; +import { remixV3Channels } from '@sentry/server-utils/orchestrion/config'; +import { beforeAll, describe, expect, it } from 'vitest'; + +import { instrumentRemixV3 } from '../../src/v3/server/instrument'; + +const startChannel = channel(`tracing:${remixV3Channels.REMIX_V3_CREATE_ROUTER}:start`); + +/** What orchestrion's transform publishes: the call's arguments, collected into a real array. */ +function publishCreateRouter(args: unknown[]): void { + startChannel.publish({ arguments: args }); +} + +describe('instrumentRemixV3', () => { + beforeAll(() => { + // Called twice on purpose: the `--import` entry and `setupOnce()` both call it, and a second set + // of channel handlers would inject the middleware twice. + instrumentRemixV3(); + instrumentRemixV3(); + }); + + it('prepends the middleware and supplies a matcher', () => { + const appMiddleware = (): Response => new Response(); + const options: Record = { middleware: [appMiddleware] }; + + publishCreateRouter([options]); + + expect(options.middleware).toEqual([expect.any(Function), appMiddleware]); + expect(options.matcher).toEqual( + expect.objectContaining({ add: expect.any(Function), matchAll: expect.any(Function) }), + ); + }); + + it('creates the options object when createRouter is called with no arguments', () => { + const args: unknown[] = []; + + publishCreateRouter(args); + + expect(args[0]).toEqual(expect.objectContaining({ middleware: [expect.any(Function)] })); + }); + + it('keeps a matcher the app supplied', () => { + const appMatcher = { add: () => {}, matchAll: () => [] }; + const options: Record = { matcher: appMatcher }; + + publishCreateRouter([options]); + + expect(options.matcher).toBe(appMatcher); + }); + + it('leaves an options object it cannot write to alone, 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[] = []; + const onUncaught = (error: Error): void => void uncaught.push(error); + process.on('uncaughtException', onUncaught); + + const options = Object.freeze({ middleware: [] }); + + try { + publishCreateRouter([options]); + await new Promise(resolve => setImmediate(resolve)); + } finally { + process.off('uncaughtException', onUncaught); + } + + expect(uncaught).toEqual([]); + expect(options.middleware).toEqual([]); + }); +}); diff --git a/packages/remix/test/v3/route.test.ts b/packages/remix/test/v3/route.test.ts new file mode 100644 index 000000000000..fa4e6dbb8044 --- /dev/null +++ b/packages/remix/test/v3/route.test.ts @@ -0,0 +1,81 @@ +import { createMultiMatcher } from '@remix-run/route-pattern/match'; +import { RoutePattern } from '@remix-run/route-pattern'; +import { describe, expect, it } from 'vitest'; + +import { resolveRoutePattern } from '../../src/v3/server/route'; +import type { MatcherLike, RequestContextLike } from '../../src/v3/types'; + +/** + * Built with the real matcher rather than a stub, because what is being tested is that the SDK reads + * the same shape the router stores and applies the same selection rules. + */ +function matcherWith(routes: Array<[method: string, pattern: string]>): MatcherLike { + const matcher = createMultiMatcher(); + + for (const [method, source] of routes) { + const pattern = RoutePattern.parse(source); + matcher.add(pattern, { pattern, method }); + } + + return matcher as unknown as MatcherLike; +} + +function contextFor(method: string, url: string): RequestContextLike { + return { request: new Request(url, { method }), url: new URL(url), method, params: {} }; +} + +describe('resolveRoutePattern', () => { + it('returns the pattern rather than the concrete path', () => { + const matcher = matcherWith([['GET', '/users/:id']]); + + expect(resolveRoutePattern(matcher, contextFor('GET', 'http://x/users/12345'))).toBe('/users/:id'); + }); + + it('returns the prefixed pattern for a mounted route', () => { + // `router.mount()` applies its prefix when the route is registered, so a mounted route is already + // stored under its full pattern. + const matcher = matcherWith([['GET', '/api/items/:itemId']]); + + expect(resolveRoutePattern(matcher, contextFor('GET', 'http://x/api/items/abc'))).toBe('/api/items/:itemId'); + }); + + it('skips a route whose method does not match', () => { + // The more specific POST route is matched first, so a resolver that ignored the method would + // return it instead of the GET route the router would actually dispatch to. + const matcher = matcherWith([ + ['POST', '/things'], + ['GET', '/*rest'], + ]); + + expect(resolveRoutePattern(matcher, contextFor('GET', 'http://x/things'))).toBe('/*rest'); + }); + + it('treats an ANY route as a match for every method', () => { + const matcher = matcherWith([['ANY', '/anything']]); + + expect(resolveRoutePattern(matcher, contextFor('DELETE', 'http://x/anything'))).toBe('/anything'); + }); + + it('falls back to a GET route for a HEAD request, as the router does', () => { + const matcher = matcherWith([['GET', '/page']]); + + expect(resolveRoutePattern(matcher, contextFor('HEAD', 'http://x/page'))).toBe('/page'); + }); + + it('returns undefined when nothing matches, so the name is left alone', () => { + const matcher = matcherWith([['GET', '/users/:id']]); + + expect(resolveRoutePattern(matcher, contextFor('GET', 'http://x/nope'))).toBeUndefined(); + }); + + it('returns undefined when the matcher throws', () => { + const broken: MatcherLike = { + add: () => {}, + matchAll: () => { + throw new Error('mismatched route-pattern copy'); + }, + }; + + expect(resolveRoutePattern(broken, contextFor('GET', 'http://x/users/1'))).toBeUndefined(); + }); +}); diff --git a/packages/server-utils/src/orchestrion/config/index.ts b/packages/server-utils/src/orchestrion/config/index.ts index c57412996096..eec81609c6d1 100644 --- a/packages/server-utils/src/orchestrion/config/index.ts +++ b/packages/server-utils/src/orchestrion/config/index.ts @@ -34,6 +34,7 @@ import { postgresJsConfig } from './postgres'; import { prismaConfig } from './prisma'; import { redisConfig } from './redis'; import { remixConfig } from './remix'; +import { remixV3Config } from './remix-v3'; import { tediousConfig } from './tedious'; import { togetherAiConfig } from './together-ai'; import { vercelAiConfig } from './vercel-ai'; @@ -86,6 +87,7 @@ export const SENTRY_INSTRUMENTATIONS: InstrumentationConfig[] = [ ...prismaConfig, ...redisConfig, ...remixConfig, + ...remixV3Config, ...tediousConfig, ...togetherAiConfig, ...vercelAiConfig, @@ -187,3 +189,5 @@ export function withoutInstrumentedExternals( export { nestjsChannels } from './nestjs'; // This is exported so that the remix package can use it to subscribe to the channels. export { remixChannels } from './remix'; +// This is exported so the remix package can subscribe to the Remix 3 channels. +export { remixV3Channels } from './remix-v3'; diff --git a/packages/server-utils/src/orchestrion/config/remix-v3.ts b/packages/server-utils/src/orchestrion/config/remix-v3.ts new file mode 100644 index 000000000000..ee30e3ffdcf7 --- /dev/null +++ b/packages/server-utils/src/orchestrion/config/remix-v3.ts @@ -0,0 +1,26 @@ +import type { InstrumentationConfig } from '../apmTypes'; + +// Remix 3 is a ground up rewrite sharing no modules with Remix 2, so it gets its own config rather +// than a widened `versionRange` on `./remix.ts`. The two never collide: this matches the +// `@remix-run/*` 0.x packages, that one `@remix-run/server-runtime`. +// +// `remix/router` is a one line `export * from '@remix-run/fetch-router'`, so matching the real module +// covers both import styles. +export const remixV3Config: InstrumentationConfig[] = [ + // The only construction point routing needs: `router.mount()` does not create a sub-router, it + // builds a prefixed route builder over the same matcher and dispatch. + { + channelName: 'createRouter', + module: { + name: '@remix-run/fetch-router', + // Still 0.x during the Remix 3 release candidate, so the range is deliberately narrow. + versionRange: '>=0.21.0 <1', + filePath: 'dist/lib/router.js', + }, + functionQuery: { functionName: 'createRouter', kind: 'Sync' }, + }, +]; + +export const remixV3Channels = { + REMIX_V3_CREATE_ROUTER: 'orchestrion:@remix-run/fetch-router:createRouter', +} as const; diff --git a/yarn.lock b/yarn.lock index 2796b4ef923e..da41ee6b64ad 100644 --- a/yarn.lock +++ b/yarn.lock @@ -7622,6 +7622,11 @@ react-router-dom "6.30.4" turbo-stream "2.4.1" +"@remix-run/route-pattern@^0.24.0": + version "0.24.1" + resolved "https://sfw.security.sentry.io/npm/@remix-run/route-pattern/-/route-pattern-0.24.1.tgz#cee709b2f946940777bb4775a8334598f3c4b909" + integrity sha512-HRee7tz2Ct7QdTQGDErHcVTAlXa5BBV6GOLEa/INw21ceTWTWlaU13FJupYVX0k1mh608rod880+SA5NlTPPfw== + "@remix-run/router@1.23.3": version "1.23.3" resolved "https://registry.yarnpkg.com/@remix-run/router/-/router-1.23.3.tgz#957c098d4393d301a8aa7dccf3ef28ea5430e36a"