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 24599aee449e..f1e0531c2f63 100644 --- a/packages/remix/package.json +++ b/packages/remix/package.json @@ -89,6 +89,7 @@ "devDependencies": { "@remix-run/node": "^2.17.5", "@remix-run/react": "^2.17.5", + "@remix-run/route-pattern": "^0.24.0", "@remix-run/server-runtime": "^2.17.4", "@types/express": "^4.17.14", "react": "^18.3.1", 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..59d77f99fd19 100644 --- a/packages/remix/src/v3/node.mjs +++ b/packages/remix/src/v3/node.mjs @@ -1,3 +1,20 @@ -// 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(); + +// A failure to load the SDK must not stop the app: this runs before anything of the app has, and an +// uncaught error here means Remix never starts. +try { + const { instrumentRemixV3 } = await import('@sentry/remix/v3'); + instrumentRemixV3(); +} catch (error) { + // oxlint-disable-next-line no-console + console.warn('[Sentry] Could not load @sentry/remix/v3. The app starts without Sentry instrumentation.', error); +} + await import('remix/node-tsx'); diff --git a/packages/remix/src/v3/remix.d.ts b/packages/remix/src/v3/remix.d.ts new file mode 100644 index 000000000000..48197a586fb5 --- /dev/null +++ b/packages/remix/src/v3/remix.d.ts @@ -0,0 +1,10 @@ +// `remix` is an optional peer, so it is not installed here. Only the one subpath the SDK imports is +// declared, with the shape the SDK relies on. Importing it from `remix` rather than from +// `@remix-run/route-pattern` gives the SDK the copy the app's router uses, and adds no dependency for +// Remix 2 apps. +declare module 'remix/route-pattern/match' { + export function createMultiMatcher(): { + add(pattern: unknown, data: unknown): void; + matchAll(url: string | URL): unknown[]; + }; +} diff --git a/packages/remix/src/v3/server/instrument.ts b/packages/remix/src/v3/server/instrument.ts new file mode 100644 index 000000000000..8fc254055588 --- /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/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/remix/vitest.config.unit.ts b/packages/remix/vitest.config.unit.ts index ae119071e469..a6d894d160a0 100644 --- a/packages/remix/vitest.config.unit.ts +++ b/packages/remix/vitest.config.unit.ts @@ -4,6 +4,12 @@ import baseConfig from '../../vite/vite.config'; export default defineConfig({ ...baseConfig, + resolve: { + ...baseConfig.resolve, + // `remix` is an optional peer and not installed here. The SDK imports the matcher through it so an + // app gets the copy its router uses; tests get it from the package that provides it. + alias: { ...baseConfig.resolve?.alias, 'remix/route-pattern/match': '@remix-run/route-pattern/match' }, + }, test: { ...baseConfig.test, include: ['test/**/*.test.ts'], diff --git a/packages/server-utils/src/orchestrion/config/index.ts b/packages/server-utils/src/orchestrion/config/index.ts index 5df88493737a..9ed93c98c0bf 100644 --- a/packages/server-utils/src/orchestrion/config/index.ts +++ b/packages/server-utils/src/orchestrion/config/index.ts @@ -36,6 +36,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 { typesafeConfig } from './typesafe'; @@ -91,6 +92,7 @@ export const SENTRY_INSTRUMENTATIONS: InstrumentationConfig[] = [ ...prismaConfig, ...redisConfig, ...remixConfig, + ...remixV3Config, ...tediousConfig, ...togetherAiConfig, ...typesafeConfig, @@ -193,3 +195,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 2da26a469d9e..86bafcc8fde9 100644 --- a/yarn.lock +++ b/yarn.lock @@ -7578,6 +7578,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"