From 12759adc63691bf44feb4ba708cb68020a0dad98 Mon Sep 17 00:00:00 2001 From: Charly Gomez Date: Mon, 5 Oct 2026 14:47:00 +0200 Subject: [PATCH 1/2] feat(remix): Report the matched route on Remix 3 HTML responses The browser has no route table: the runtime never matches routes client side. The middleware already resolves the pattern for the server span, so it now also reports it on HTML responses through a `Server-Timing: sentry-route` entry, for the browser SDK to name its spans from. Co-Authored-By: Claude Fable 5.1 --- packages/remix/src/v3/server/middleware.ts | 39 +++++++--- packages/remix/test/v3/middleware.test.ts | 83 ++++++++++++++++++++++ 2 files changed, 112 insertions(+), 10 deletions(-) create mode 100644 packages/remix/test/v3/middleware.test.ts diff --git a/packages/remix/src/v3/server/middleware.ts b/packages/remix/src/v3/server/middleware.ts index 541adc696214..bdfcbea083da 100644 --- a/packages/remix/src/v3/server/middleware.ts +++ b/packages/remix/src/v3/server/middleware.ts @@ -27,7 +27,10 @@ export function sentryRemixMiddleware(matcher: MatcherLike): MiddlewareLike { 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 route = resolveRoutePattern(matcher, context); + if (route) { + applyRoute(isolationScope, route, context.method); + } let response; try { @@ -40,20 +43,18 @@ export function sentryRemixMiddleware(matcher: MatcherLike): MiddlewareLike { } setResponseStatus(response); + if (route) { + addRouteHeader(response, context.request, route); + } 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}`; +// When nothing matched, the router falls through to its 404 handler. Leaving the name alone keeps the +// raw URL out of it, which span streaming requires. +function applyRoute(isolationScope: Scope, route: string, method: string): void { + const name = `${method} ${route}`; isolationScope.setTransactionName(name); const activeSpan = getActiveSpan(); @@ -67,6 +68,24 @@ function applyRoute(isolationScope: Scope, matcher: MatcherLike, context: Reques rootSpan.setAttribute(HTTP_ROUTE, route); } +/** + * Tells the browser SDK which route served a document, so it can name its page load and navigation + * spans after the pattern. The browser has no route table of its own. Only HTML responses carry it, + * which is what document loads and the runtime's frame fetches ask for. + */ +function addRouteHeader(response: Response, request: Request, route: string): void { + if (!request.headers.get('accept')?.includes('text/html')) { + return; + } + try { + // Quoted string per the header grammar; the browser unquotes it for the timing entry. + const quoted = route.replace(/["\\]/g, '\\$&'); + response.headers.append('Server-Timing', `sentry-route;desc="${quoted}"`); + } catch { + // Immutable headers, e.g. a response passed through from `fetch()`. + } +} + function setResponseStatus(response: Response): void { const activeSpan = getActiveSpan(); if (!activeSpan || typeof response?.status !== 'number') { diff --git a/packages/remix/test/v3/middleware.test.ts b/packages/remix/test/v3/middleware.test.ts new file mode 100644 index 000000000000..88c8f1fc42db --- /dev/null +++ b/packages/remix/test/v3/middleware.test.ts @@ -0,0 +1,83 @@ +import { describe, expect, it } from 'vitest'; + +import { sentryRemixMiddleware } from '../../src/v3/server/middleware'; +import type { MatcherLike, RequestContextLike } from '../../src/v3/types'; + +/** A matcher that resolves every URL to the one pattern, or to nothing. */ +function matcherFor(pattern: string | undefined): MatcherLike { + return { + add: () => {}, + matchAll: () => (pattern ? [{ data: { pattern: { source: pattern }, method: 'GET' }, params: {} }] : []), + }; +} + +function contextFor(url: string, headers: Record = {}): RequestContextLike { + return { request: new Request(url, { headers }), url: new URL(url), method: 'GET', params: {} }; +} + +describe('sentryRemixMiddleware', () => { + it('reports the route on HTML responses through Server-Timing', async () => { + const middleware = sentryRemixMiddleware(matcherFor('/users/:id')); + + const response = await middleware( + contextFor('http://x/users/12345', { accept: 'text/html' }), + async () => new Response('', { headers: { 'content-type': 'text/html' } }), + ); + + expect(response.headers.get('server-timing')).toBe('sentry-route;desc="/users/:id"'); + }); + + it('quotes a pattern that contains header-breaking characters', async () => { + const middleware = sentryRemixMiddleware(matcherFor('/files/:name("a\\b")')); + + const response = await middleware( + contextFor('http://x/files/x', { accept: 'text/html' }), + async () => new Response(''), + ); + + expect(response.headers.get('server-timing')).toBe('sentry-route;desc="/files/:name(\\"a\\\\b\\")"'); + }); + + it('keeps an existing Server-Timing header', async () => { + const middleware = sentryRemixMiddleware(matcherFor('/')); + + const response = await middleware( + contextFor('http://x/', { accept: 'text/html' }), + async () => new Response('', { headers: { 'server-timing': 'db;dur=5' } }), + ); + + expect(response.headers.get('server-timing')).toBe('db;dur=5, sentry-route;desc="/"'); + }); + + it('adds nothing to a response the browser did not ask for as a document', async () => { + const middleware = sentryRemixMiddleware(matcherFor('/api/items/:itemId')); + + const response = await middleware( + contextFor('http://x/api/items/1', { accept: 'application/json' }), + async () => new Response('{}'), + ); + + expect(response.headers.has('server-timing')).toBe(false); + }); + + it('adds nothing when no route matched', async () => { + const middleware = sentryRemixMiddleware(matcherFor(undefined)); + + const response = await middleware( + contextFor('http://x/nope', { accept: 'text/html' }), + async () => new Response('Not Found', { status: 404 }), + ); + + expect(response.headers.has('server-timing')).toBe(false); + }); + + it('leaves a response with immutable headers alone, without failing the request', async () => { + const middleware = sentryRemixMiddleware(matcherFor('/')); + + // `Response.error()` is the one constructor that yields immutable headers, like a passed through + // `fetch()` response does. + const response = await middleware(contextFor('http://x/', { accept: 'text/html' }), async () => Response.error()); + + expect(response.headers.has('server-timing')).toBe(false); + }); +}); From 2d6bc7d918301ed2e27204418518e627c2937cbd Mon Sep 17 00:00:00 2001 From: Charly Gomez Date: Tue, 6 Oct 2026 08:35:18 +0200 Subject: [PATCH 2/2] ref(remix): Share the route timing entry between server and client One module formats and parses the `Server-Timing` entry, so the two sides cannot drift. Co-Authored-By: Claude Fable 5.1 --- packages/remix/src/v3/routeTiming.ts | 17 +++++++++++++++ packages/remix/src/v3/server/middleware.ts | 5 ++--- packages/remix/test/v3/routeTiming.test.ts | 25 ++++++++++++++++++++++ 3 files changed, 44 insertions(+), 3 deletions(-) create mode 100644 packages/remix/src/v3/routeTiming.ts create mode 100644 packages/remix/test/v3/routeTiming.test.ts diff --git a/packages/remix/src/v3/routeTiming.ts b/packages/remix/src/v3/routeTiming.ts new file mode 100644 index 000000000000..483a867ed8af --- /dev/null +++ b/packages/remix/src/v3/routeTiming.ts @@ -0,0 +1,17 @@ +// The `Server-Timing` entry that carries the matched route pattern from the server to the browser. +// Shared by the server middleware, which writes it, and the browser SDK, which reads it, so the two +// cannot drift. Nothing in here may pull server code into the client bundle. + +export const ROUTE_TIMING_NAME = 'sentry-route'; + +/** The entry for a route, quoted per the header grammar. */ +export function formatRouteTiming(route: string): string { + return `${ROUTE_TIMING_NAME};desc="${route.replace(/["\\]/g, '\\$&')}"`; +} + +/** The route from a raw `Server-Timing` header value, or `undefined` when the entry is absent. */ +export function parseRouteTiming(header: string | null | undefined): string | undefined { + // Everything between the quotes, where a backslash escapes the next character. + const quoted = header?.match(new RegExp(`(?:^|,)\\s*${ROUTE_TIMING_NAME};desc="((?:[^"\\\\]|\\\\.)*)"`))?.[1]; + return quoted?.replace(/\\(.)/g, '$1'); +} diff --git a/packages/remix/src/v3/server/middleware.ts b/packages/remix/src/v3/server/middleware.ts index bdfcbea083da..7ae9dc989362 100644 --- a/packages/remix/src/v3/server/middleware.ts +++ b/packages/remix/src/v3/server/middleware.ts @@ -10,6 +10,7 @@ import { winterCGRequestToRequestData, } from '@sentry/core'; +import { formatRouteTiming } from '../routeTiming'; import type { MatcherLike, MiddlewareLike, NextFunctionLike, RequestContextLike } from '../types'; import { captureRequestError } from './errorFilter'; import { resolveRoutePattern } from './route'; @@ -78,9 +79,7 @@ function addRouteHeader(response: Response, request: Request, route: string): vo return; } try { - // Quoted string per the header grammar; the browser unquotes it for the timing entry. - const quoted = route.replace(/["\\]/g, '\\$&'); - response.headers.append('Server-Timing', `sentry-route;desc="${quoted}"`); + response.headers.append('Server-Timing', formatRouteTiming(route)); } catch { // Immutable headers, e.g. a response passed through from `fetch()`. } diff --git a/packages/remix/test/v3/routeTiming.test.ts b/packages/remix/test/v3/routeTiming.test.ts new file mode 100644 index 000000000000..4e681b686a87 --- /dev/null +++ b/packages/remix/test/v3/routeTiming.test.ts @@ -0,0 +1,25 @@ +import { describe, expect, it } from 'vitest'; + +import { formatRouteTiming, parseRouteTiming } from '../../src/v3/routeTiming'; + +describe('route timing entry', () => { + it('round trips a route', () => { + expect(parseRouteTiming(formatRouteTiming('/users/:id'))).toBe('/users/:id'); + }); + + it('round trips a route with header-breaking characters', () => { + const route = '/files/:name("a\\b")'; + + expect(formatRouteTiming(route)).toBe('sentry-route;desc="/files/:name(\\"a\\\\b\\")"'); + expect(parseRouteTiming(formatRouteTiming(route))).toBe(route); + }); + + it('finds the entry among others', () => { + expect(parseRouteTiming(`db;dur=5, ${formatRouteTiming('/')}, cache;desc="hit"`)).toBe('/'); + }); + + it('is undefined without the entry', () => { + expect(parseRouteTiming('db;dur=5')).toBeUndefined(); + expect(parseRouteTiming(null)).toBeUndefined(); + }); +});