Skip to content

Commit f2fc75c

Browse files
chargomeclaude
andauthored
feat(remix): Report the matched route on Remix 3 HTML responses (#25052)
The browser has no route table: `remix/component` never matches routes client side and exposes no matched pattern. The server 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, the same channel the Remix 2 SDK and Nitro use for trace data. Nothing reads it yet; the browser SDK does in the next PRs of this stack. Part of #24872 --------- Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
1 parent 95c1146 commit f2fc75c

4 files changed

Lines changed: 153 additions & 10 deletions

File tree

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
// The `Server-Timing` entry that carries the matched route pattern from the server to the browser.
2+
// Shared by the server middleware, which writes it, and the browser SDK, which reads it, so the two
3+
// cannot drift. Nothing in here may pull server code into the client bundle.
4+
5+
export const ROUTE_TIMING_NAME = 'sentry-route';
6+
7+
/** The entry for a route, quoted per the header grammar. */
8+
export function formatRouteTiming(route: string): string {
9+
return `${ROUTE_TIMING_NAME};desc="${route.replace(/["\\]/g, '\\$&')}"`;
10+
}
11+
12+
/** The route from a raw `Server-Timing` header value, or `undefined` when the entry is absent. */
13+
export function parseRouteTiming(header: string | null | undefined): string | undefined {
14+
// Everything between the quotes, where a backslash escapes the next character.
15+
const quoted = header?.match(new RegExp(`(?:^|,)\\s*${ROUTE_TIMING_NAME};desc="((?:[^"\\\\]|\\\\.)*)"`))?.[1];
16+
return quoted?.replace(/\\(.)/g, '$1');
17+
}

‎packages/remix/src/v3/server/middleware.ts‎

Lines changed: 28 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import {
1010
winterCGRequestToRequestData,
1111
} from '@sentry/core';
1212

13+
import { formatRouteTiming } from '../routeTiming';
1314
import type { MatcherLike, MiddlewareLike, NextFunctionLike, RequestContextLike } from '../types';
1415
import { captureRequestError } from './errorFilter';
1516
import { resolveRoutePattern } from './route';
@@ -27,7 +28,10 @@ export function sentryRemixMiddleware(matcher: MatcherLike): MiddlewareLike {
2728
isolationScope.setSDKProcessingMetadata({ normalizedRequest: winterCGRequestToRequestData(context.request) });
2829

2930
// Applied before `next()` so anything captured while the handler runs already carries the route.
30-
applyRoute(isolationScope, matcher, context);
31+
const route = resolveRoutePattern(matcher, context);
32+
if (route) {
33+
applyRoute(isolationScope, route, context.method);
34+
}
3135

3236
let response;
3337
try {
@@ -40,20 +44,18 @@ export function sentryRemixMiddleware(matcher: MatcherLike): MiddlewareLike {
4044
}
4145

4246
setResponseStatus(response);
47+
if (route) {
48+
addRouteHeader(response, context.request, route);
49+
}
4350

4451
return response;
4552
};
4653
}
4754

48-
function applyRoute(isolationScope: Scope, matcher: MatcherLike, context: RequestContextLike): void {
49-
const route = resolveRoutePattern(matcher, context);
50-
if (!route) {
51-
// Nothing matched, so the router falls through to its 404 handler. Leaving the name alone keeps the
52-
// raw URL out of it, which span streaming requires.
53-
return;
54-
}
55-
56-
const name = `${context.method} ${route}`;
55+
// When nothing matched, the router falls through to its 404 handler. Leaving the name alone keeps the
56+
// raw URL out of it, which span streaming requires.
57+
function applyRoute(isolationScope: Scope, route: string, method: string): void {
58+
const name = `${method} ${route}`;
5759
isolationScope.setTransactionName(name);
5860

5961
const activeSpan = getActiveSpan();
@@ -67,6 +69,22 @@ function applyRoute(isolationScope: Scope, matcher: MatcherLike, context: Reques
6769
rootSpan.setAttribute(HTTP_ROUTE, route);
6870
}
6971

72+
/**
73+
* Tells the browser SDK which route served a document, so it can name its page load and navigation
74+
* spans after the pattern. The browser has no route table of its own. Only HTML responses carry it,
75+
* which is what document loads and the runtime's frame fetches ask for.
76+
*/
77+
function addRouteHeader(response: Response, request: Request, route: string): void {
78+
if (!request.headers.get('accept')?.includes('text/html')) {
79+
return;
80+
}
81+
try {
82+
response.headers.append('Server-Timing', formatRouteTiming(route));
83+
} catch {
84+
// Immutable headers, e.g. a response passed through from `fetch()`.
85+
}
86+
}
87+
7088
function setResponseStatus(response: Response): void {
7189
const activeSpan = getActiveSpan();
7290
if (!activeSpan || typeof response?.status !== 'number') {
Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,83 @@
1+
import { describe, expect, it } from 'vitest';
2+
3+
import { sentryRemixMiddleware } from '../../src/v3/server/middleware';
4+
import type { MatcherLike, RequestContextLike } from '../../src/v3/types';
5+
6+
/** A matcher that resolves every URL to the one pattern, or to nothing. */
7+
function matcherFor(pattern: string | undefined): MatcherLike {
8+
return {
9+
add: () => {},
10+
matchAll: () => (pattern ? [{ data: { pattern: { source: pattern }, method: 'GET' }, params: {} }] : []),
11+
};
12+
}
13+
14+
function contextFor(url: string, headers: Record<string, string> = {}): RequestContextLike {
15+
return { request: new Request(url, { headers }), url: new URL(url), method: 'GET', params: {} };
16+
}
17+
18+
describe('sentryRemixMiddleware', () => {
19+
it('reports the route on HTML responses through Server-Timing', async () => {
20+
const middleware = sentryRemixMiddleware(matcherFor('/users/:id'));
21+
22+
const response = await middleware(
23+
contextFor('http://x/users/12345', { accept: 'text/html' }),
24+
async () => new Response('<html></html>', { headers: { 'content-type': 'text/html' } }),
25+
);
26+
27+
expect(response.headers.get('server-timing')).toBe('sentry-route;desc="/users/:id"');
28+
});
29+
30+
it('quotes a pattern that contains header-breaking characters', async () => {
31+
const middleware = sentryRemixMiddleware(matcherFor('/files/:name("a\\b")'));
32+
33+
const response = await middleware(
34+
contextFor('http://x/files/x', { accept: 'text/html' }),
35+
async () => new Response(''),
36+
);
37+
38+
expect(response.headers.get('server-timing')).toBe('sentry-route;desc="/files/:name(\\"a\\\\b\\")"');
39+
});
40+
41+
it('keeps an existing Server-Timing header', async () => {
42+
const middleware = sentryRemixMiddleware(matcherFor('/'));
43+
44+
const response = await middleware(
45+
contextFor('http://x/', { accept: 'text/html' }),
46+
async () => new Response('', { headers: { 'server-timing': 'db;dur=5' } }),
47+
);
48+
49+
expect(response.headers.get('server-timing')).toBe('db;dur=5, sentry-route;desc="/"');
50+
});
51+
52+
it('adds nothing to a response the browser did not ask for as a document', async () => {
53+
const middleware = sentryRemixMiddleware(matcherFor('/api/items/:itemId'));
54+
55+
const response = await middleware(
56+
contextFor('http://x/api/items/1', { accept: 'application/json' }),
57+
async () => new Response('{}'),
58+
);
59+
60+
expect(response.headers.has('server-timing')).toBe(false);
61+
});
62+
63+
it('adds nothing when no route matched', async () => {
64+
const middleware = sentryRemixMiddleware(matcherFor(undefined));
65+
66+
const response = await middleware(
67+
contextFor('http://x/nope', { accept: 'text/html' }),
68+
async () => new Response('Not Found', { status: 404 }),
69+
);
70+
71+
expect(response.headers.has('server-timing')).toBe(false);
72+
});
73+
74+
it('leaves a response with immutable headers alone, without failing the request', async () => {
75+
const middleware = sentryRemixMiddleware(matcherFor('/'));
76+
77+
// `Response.error()` is the one constructor that yields immutable headers, like a passed through
78+
// `fetch()` response does.
79+
const response = await middleware(contextFor('http://x/', { accept: 'text/html' }), async () => Response.error());
80+
81+
expect(response.headers.has('server-timing')).toBe(false);
82+
});
83+
});
Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
import { describe, expect, it } from 'vitest';
2+
3+
import { formatRouteTiming, parseRouteTiming } from '../../src/v3/routeTiming';
4+
5+
describe('route timing entry', () => {
6+
it('round trips a route', () => {
7+
expect(parseRouteTiming(formatRouteTiming('/users/:id'))).toBe('/users/:id');
8+
});
9+
10+
it('round trips a route with header-breaking characters', () => {
11+
const route = '/files/:name("a\\b")';
12+
13+
expect(formatRouteTiming(route)).toBe('sentry-route;desc="/files/:name(\\"a\\\\b\\")"');
14+
expect(parseRouteTiming(formatRouteTiming(route))).toBe(route);
15+
});
16+
17+
it('finds the entry among others', () => {
18+
expect(parseRouteTiming(`db;dur=5, ${formatRouteTiming('/')}, cache;desc="hit"`)).toBe('/');
19+
});
20+
21+
it('is undefined without the entry', () => {
22+
expect(parseRouteTiming('db;dur=5')).toBeUndefined();
23+
expect(parseRouteTiming(null)).toBeUndefined();
24+
});
25+
});

0 commit comments

Comments
 (0)