Fix race condition and improve navigation state management

Author: onurtemizkan

packages/remix/src/client/performance.tsx

@@ -14,6 +14,7 @@ import * as React from 'react';
 import { DEBUG_BUILD } from '../utils/debug-build';
 import { hasManifest, maybeParameterizeRemixRoute } from './remixRouteParameterization';
 import {
+  clearNavigationTraceCache,
   getMetaTagTraceContext,
   getNavigationTraceContext,
   getNavigationTraceContextAsync,
@@ -93,6 +94,14 @@ function getTransactionNameAndSource(
 // Flag to prevent async callback from creating a span after navigation has occurred
 let pageloadSpanStarted = false;
 
+/**
+ * Resets navigation state and trace cache when starting a new navigation.
+ */
+function resetNavigationState(): void {
+  clearNavigationTraceCache();
+  pageloadSpanStarted = true;
+}
+
 export function startPageloadSpan(client: Client): void {
   const initPathName = getInitPathName();
 
@@ -230,8 +239,8 @@ export function withSentry<P extends Record<string, unknown>, R extends React.Co
       }
 
       if (_instrumentNavigation && matches?.length) {
-        // Mark pageload as started to prevent async callback from firing after navigation
-        pageloadSpanStarted = true;
+        // Reset navigation state when starting a new navigation
+        resetNavigationState();
 
         if (activeRootSpan) {
           activeRootSpan.end();

packages/remix/src/client/serverTimingTracePropagation.ts

@@ -72,7 +72,7 @@ function parseServerTimingTrace(serverTiming: readonly PerformanceServerTiming[]
 
   const traceparentData = extractTraceparentData(sentryTrace);
   if (!traceparentData?.traceId || !traceparentData?.parentSpanId) {
-    DEBUG_BUILD && debug.warn('[Server-Timing] Invalid sentry-trace format:', sentryTrace);
+    DEBUG_BUILD && debug.warn('Invalid sentry-trace format:', sentryTrace);
     return null;
   }
 
@@ -117,17 +117,15 @@ function tryGetNavigationTraceContext(): NavigationTraceResult {
 }
 
 /**
- * Get trace context from the initial navigation (page load).
- * Reads the Server-Timing header from the navigation performance entry.
- * Results are cached after first successful retrieval.
+ * Get trace context from Server-Timing header synchronously. Results are cached.
  */
 export function getNavigationTraceContext(): ServerTimingTraceContext | null {
   if (navigationTraceCache !== undefined) {
     return navigationTraceCache;
   }
 
   if (!isServerTimingSupported()) {
-    DEBUG_BUILD && debug.log('[Server-Timing] Server-Timing API not supported');
+    DEBUG_BUILD && debug.log('Server-Timing API not supported');
     navigationTraceCache = null;
     return null;
   }
@@ -147,10 +145,8 @@ export function getNavigationTraceContext(): ServerTimingTraceContext | null {
 }
 
 /**
- * Get trace context from navigation with retry mechanism.
- * Useful during SDK init when browser may not have finished processing headers.
- *
- * @returns Cleanup function to cancel pending retries (e.g., on navigation)
+ * Get trace context from Server-Timing header with retry mechanism for early SDK initialization.
+ * Returns a cleanup function to cancel pending retries.
  */
 export function getNavigationTraceContextAsync(
   callback: (trace: ServerTimingTraceContext | null) => void,
@@ -167,7 +163,7 @@ export function getNavigationTraceContextAsync(
   }
 
   if (!isServerTimingSupported()) {
-    DEBUG_BUILD && debug.log('[Server-Timing] Server-Timing API not supported');
+    DEBUG_BUILD && debug.log('Server-Timing API not supported');
     navigationTraceCache = null;
     callback(null);
     return () => {
@@ -187,21 +183,27 @@ export function getNavigationTraceContextAsync(
 
     switch (result.status) {
       case 'unavailable':
-        navigationTraceCache = null;
-        callback(null);
+        if (!state.cancelled) {
+          navigationTraceCache = null;
+          callback(null);
+        }
         return;
       case 'pending':
         if (attempts < maxAttempts) {
           setTimeout(tryGet, delayMs);
           return;
         }
-        DEBUG_BUILD && debug.warn('[Server-Timing] Max retry attempts reached, trace context unavailable');
-        navigationTraceCache = null;
-        callback(null);
+        DEBUG_BUILD && debug.warn('Max retry attempts reached, trace context unavailable');
+        if (!state.cancelled) {
+          navigationTraceCache = null;
+          callback(null);
+        }
         return;
       case 'available':
-        navigationTraceCache = result.data;
-        callback(result.data);
+        if (!state.cancelled) {
+          navigationTraceCache = result.data;
+          callback(result.data);
+        }
     }
   };
 
@@ -213,8 +215,7 @@ export function getNavigationTraceContextAsync(
 }
 
 /**
- * Get trace context from meta tags.
- * Looks for `<meta name="sentry-trace">` and `<meta name="baggage">` tags.
+ * Get trace context from meta tags as a fallback for browsers without Server-Timing support.
  */
 export function getMetaTagTraceContext(): ServerTimingTraceContext | null {
   if (typeof WINDOW === 'undefined' || !WINDOW.document) {
@@ -233,7 +234,7 @@ export function getMetaTagTraceContext(): ServerTimingTraceContext | null {
 
     const traceparentData = extractTraceparentData(sentryTrace);
     if (!traceparentData?.traceId || !traceparentData?.parentSpanId) {
-      DEBUG_BUILD && debug.warn('[Server-Timing] Invalid sentry-trace format in meta tag:', sentryTrace);
+      DEBUG_BUILD && debug.warn('Invalid sentry-trace format in meta tag:', sentryTrace);
       return null;
     }
 
@@ -246,7 +247,10 @@ export function getMetaTagTraceContext(): ServerTimingTraceContext | null {
   }
 }
 
-/** @internal */
+/**
+ * Resets the navigation trace cache for fresh retrieval.
+ * @internal
+ */
 export function clearNavigationTraceCache(): void {
   navigationTraceCache = undefined;
 }

packages/remix/src/server/instrumentServer.ts

@@ -103,9 +103,9 @@ export function wrapHandleErrorWithSentry(
 }
 
 /**
- * Get trace context for injection into loader response data (for meta tags).
- * Returns empty object when Server-Timing headers are available, as they take priority.
- * Only provides trace data for meta tags as a fallback mechanism.
+ * Get trace context for meta tag injection. Returns empty object when Server-Timing
+ * headers will be used (active span in Node.js/Cloudflare), as Server-Timing takes
+ * priority over meta tags for trace propagation.
  */
 function getTraceAndBaggage(): {
   sentryTrace?: string;
@@ -117,8 +117,9 @@ function getTraceAndBaggage(): {
   if (isNodeEnv() || isCloudflareEnv()) {
     const activeSpan = getActiveSpan();
     if (activeSpan) {
-      // Server-Timing header will be available, skip meta tag injection
-      DEBUG_BUILD && debug.log('[getTraceAndBaggage] Skipping meta tag injection - Server-Timing header will be used');
+      // Active span exists - Server-Timing header will be injected by makeWrappedDocumentRequestFunction.
+      // Return empty to avoid duplicate trace context in meta tags.
+      DEBUG_BUILD && debug.log('Skipping meta tag injection - Server-Timing header will be used');
       return {};
     }
 
@@ -130,15 +131,15 @@ function getTraceAndBaggage(): {
 
     if (propagationContext.traceId && spanId) {
       const fallbackTrace = generateSentryTraceHeader(propagationContext.traceId, spanId, propagationContext.sampled);
-      DEBUG_BUILD && debug.log('[getTraceAndBaggage] Using meta tags fallback - no active span for Server-Timing');
+      DEBUG_BUILD && debug.log('Using meta tags fallback - no active span for Server-Timing');
 
       return {
         sentryTrace: fallbackTrace,
         sentryBaggage: traceData.baggage,
       };
     }
 
-    DEBUG_BUILD && debug.log('[getTraceAndBaggage] No valid trace context available');
+    DEBUG_BUILD && debug.log('No valid trace context available');
     return {};
   }
 

packages/remix/src/server/serverTimingTracePropagation.ts

@@ -33,8 +33,8 @@ export function isCloudflareEnv(): boolean {
 }
 
 /**
- * Generate Server-Timing header value containing Sentry trace context.
- * Enables trace propagation from server to client via the Performance API.
+ * Generate a Server-Timing header value containing Sentry trace context.
+ * Called automatically by instrumented `handleDocumentRequest`.
  */
 export function generateSentryServerTimingHeader(options: ServerTimingTraceOptions = {}): string | null {
   // Only generate on server environments
@@ -62,8 +62,6 @@ export function generateSentryServerTimingHeader(options: ServerTimingTraceOptio
     // Get DSC from span and ensure trace_id consistency
     const dsc = getDynamicSamplingContextFromSpan(span);
 
-    // Build baggage string, ensuring trace_id matches the span's trace_id
-    // The DSC may have a different trace_id if it was frozen from an earlier context
     const baggageEntries: string[] = [];
     for (const [key, value] of Object.entries(dsc)) {
       if (value) {
@@ -98,7 +96,7 @@ export function generateSentryServerTimingHeader(options: ServerTimingTraceOptio
 }
 
 /**
- * Merge Sentry Server-Timing with an existing Server-Timing header value.
+ * Merge Sentry trace context with an existing Server-Timing header value.
  */
 export function mergeSentryServerTimingHeader(
   existingHeader: string | null | undefined,
@@ -146,8 +144,8 @@ export function injectServerTimingHeaderValue(response: Response, serverTimingVa
 }
 
 /**
- * Add Sentry trace context to Response headers via Server-Timing.
- * Returns a new Response with the header added.
+ * Add Sentry trace context to a Response via the Server-Timing header.
+ * Returns a new Response with the header added (original is not modified).
  */
 export function addSentryServerTimingHeader(response: Response, options?: ServerTimingTraceOptions): Response {
   const sentryTiming = generateSentryServerTimingHeader(options);