feat(remix): Report the matched route on Remix 3 HTML responses

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');
+}

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';
@@ -27,7 +28,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 +44,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 +69,22 @@ 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 {
+    response.headers.append('Server-Timing', formatRouteTiming(route));
+  } 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') {

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<string, string> = {}): 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('<html></html>', { 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);
+  });
+});

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();
+  });
+});