fix(remix): Record the route under the URL that served it, and forget failed navigations

A redirected frame fetch answers from another URL; recording its route
under the requested path would misname every later visit there. A fetch
that failed never reported back, so its entry stayed pending for good.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

Author: chargome

packages/remix/src/v3/client/browserTracingIntegration.ts

@@ -83,8 +83,9 @@ function instrumentNavigationApi(client: Client): void {
 
   // Navigation spans still waiting for their route, which arrives with the response to the runtime's
   // fetch of the destination. Keyed by pathname: a second navigation before the first response must
-  // not lose either span.
+  // not lose either span. Bounded, because a fetch that fails never reports back for its entry.
   const pending = new Map<string, { span: Span; url: string }>();
+  const MAX_PENDING = 20;
 
   navigation.addEventListener('navigate', event => {
     const url = event.destination?.url;
@@ -111,12 +112,19 @@ function instrumentNavigationApi(client: Client): void {
     );
 
     if (span && !route) {
+      // A repeat of the same path replaces its entry; the Navigation API aborted the earlier intercept.
+      pending.delete(pathname);
       pending.set(pathname, { span, url });
+      while (pending.size > MAX_PENDING) {
+        pending.delete(pending.keys().next().value as string);
+      }
     }
   });
 
   addFetchInstrumentationHandler(({ fetchData, response }) => {
     if (pending.size === 0 || !response) {
+      // A failed fetch does not settle the entry: the Navigation API aborts the previous intercept when
+      // a new navigation starts, and that rejection lands after the new one re-queued the same path.
       return;
     }
     // The runtime fetches the destination itself. Any other fetch during the navigation is the app's.
@@ -127,8 +135,11 @@ function instrumentNavigationApi(client: Client): void {
     }
     pending.delete(pathname);
 
-    recordRoute(client, pathname, parseRouteTiming(response.headers.get('server-timing')));
-    const route = resolveRoute(waiting.url, client);
+    // The route belongs to the URL that answered. After a redirect that is not the one requested, and
+    // recording it under the requested path would misname every later visit there.
+    const servedPathname = pathnameOf(response.url, WINDOW.location?.href) || pathname;
+    recordRoute(client, servedPathname, parseRouteTiming(response.headers.get('server-timing')));
+    const route = resolveRoute(response.url || waiting.url, client);
     if (route) {
       applyRoute(waiting.span, route);
     }

packages/remix/test/v3/browserTracingIntegration.test.ts

@@ -11,7 +11,13 @@ type FakeSpan = ReturnType<typeof fakeSpan>;
 
 let navigationSpan: FakeSpan | undefined;
 let activeSpan: FakeSpan | undefined;
-let fetchHandler: ((data: { fetchData: { url: string }; response?: { headers: Headers } }) => void) | undefined;
+let fetchHandler:
+  | ((data: {
+      fetchData: { url: string };
+      endTimestamp?: number;
+      response?: { url: string; headers: Headers };
+    }) => void)
+  | undefined;
 
 const startBrowserTracingNavigationSpan = vi.fn(() => navigationSpan);
 const upstreamIntegration = { name: 'BrowserTracing', afterAllSetup: vi.fn() };
@@ -57,13 +63,22 @@ vi.mock('@sentry/core', async importOriginal => ({
 const { browserTracingIntegration } = await import('../../src/v3/client/browserTracingIntegration');
 
 /** A response to the runtime's frame fetch, as the fetch instrumentation reports it. */
-function frameResponse(url: string, serverTiming?: string) {
+function frameResponse(url: string, serverTiming?: string, servedUrl = url) {
   return {
     fetchData: { url },
-    response: { headers: new Headers(serverTiming ? { 'server-timing': serverTiming } : {}) },
+    endTimestamp: 1,
+    response: {
+      url: new URL(servedUrl, 'https://app.test').href,
+      headers: new Headers(serverTiming ? { 'server-timing': serverTiming } : {}),
+    },
   };
 }
 
+/** A frame fetch that failed: settled, but with no response. */
+function failedFrameFetch(url: string) {
+  return { fetchData: { url }, endTimestamp: 1 };
+}
+
 interface FakeNavigateEvent {
   canIntercept: boolean;
   info?: unknown;
@@ -224,6 +239,53 @@ describe('browserTracingIntegration', () => {
     expect(navigationSpan?.updateName).toHaveBeenCalledWith('/files/:name("a\\b")');
   });
 
+  it('records a redirected response under the path that served it, not the one requested', () => {
+    const navigate = setup();
+    navigate({ canIntercept: true, destination: { url: 'https://app.test/old/5' } });
+    activeSpan = navigationSpan;
+
+    fetchHandler?.(frameResponse('/old/5', 'sentry-route;desc="/users/:id"', '/users/5'));
+
+    expect(recorded.get('/users/5')).toBe('/users/:id');
+    expect(recorded.has('/old/5')).toBe(false);
+    expect(navigationSpan?.updateName).toHaveBeenCalledWith('/users/:id');
+  });
+
+  it('still names a repeated navigation after the aborted first attempt reports its failure', () => {
+    const first = fakeSpan('navigation');
+    const second = fakeSpan('navigation');
+    const navigate = setup();
+    navigationSpan = first;
+    navigate({ canIntercept: true, destination: { url: 'https://app.test/users/1' } });
+    navigationSpan = second;
+    navigate({ canIntercept: true, destination: { url: 'https://app.test/users/1' } });
+    activeSpan = second;
+
+    // The Navigation API aborted the first intercept; its rejection arrives after the second queued.
+    fetchHandler?.(failedFrameFetch('/users/1'));
+    fetchHandler?.(frameResponse('/users/1', 'sentry-route;desc="/users/:id"'));
+
+    expect(second.updateName).toHaveBeenCalledWith('/users/:id');
+    expect(first.updateName).not.toHaveBeenCalled();
+  });
+
+  it('keeps the pending set bounded when navigations never get a response', () => {
+    const navigate = setup();
+    for (let i = 0; i < 30; i++) {
+      navigationSpan = fakeSpan('navigation');
+      navigate({ canIntercept: true, destination: { url: `https://app.test/never/${i}` } });
+    }
+    const last = navigationSpan;
+    activeSpan = last;
+
+    // The oldest entries were dropped; the newest still resolves.
+    fetchHandler?.(frameResponse('/never/0', 'sentry-route;desc="/never/:n"'));
+    fetchHandler?.(frameResponse('/never/29', 'sentry-route;desc="/never/:n"'));
+
+    expect(last?.updateName).toHaveBeenCalledWith('/never/:n');
+    expect(recorded.has('/never/0')).toBe(false);
+  });
+
   it("ignores the app's own fetches during a navigation", () => {
     const navigate = setup();
     navigate({ canIntercept: true, destination: { url: 'https://app.test/users/12345' } });