Skip to content

Commit fa120d4

Browse files
committed
Address copilot review
1 parent d017678 commit fa120d4

File tree

1 file changed

+22
-15
lines changed

1 file changed

+22
-15
lines changed

packages/react/src/reactrouter-compat-utils/instrumentation.tsx

Lines changed: 22 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,11 @@ export interface ReactRouterOptions {
101101
/**
102102
* Maximum time (in milliseconds) to wait for lazy routes to load before finalizing span names.
103103
*
104-
* Set to `0` to not wait at all, or `Infinity` to wait indefinitely.
104+
* - Set to `0` to not wait at all (immediate finalization)
105+
* - Set to `Infinity` to wait indefinitely
106+
* - Negative values will fall back to the default
107+
*
108+
* Defaults to 3× the configured `idleTimeout` (default: 3000ms).
105109
*
106110
* @default idleTimeout * 3
107111
*/
@@ -204,11 +208,12 @@ export function updateNavigationSpan(
204208

205209
// Check if this span has already been named to avoid multiple updates
206210
// But allow updates if:
207-
// 1. This is a forced update (e.g., when lazy routes are loaded)
208-
// 2. The current name has wildcards (incomplete parameterization)
211+
// 1. Not yet finalized (!hasBeenNamed)
212+
// 2. Forced update (e.g., when lazy routes are loaded)
213+
// 3. Current name has wildcards (incomplete parameterization - safety valve)
209214
const hasBeenNamed = (activeRootSpan as { __sentry_navigation_name_set__?: boolean })?.__sentry_navigation_name_set__;
210215
const currentNameHasWildcard = currentName && transactionNameHasWildcard(currentName);
211-
const shouldUpdate = forceUpdate || !hasBeenNamed || currentNameHasWildcard;
216+
const shouldUpdate = !hasBeenNamed || forceUpdate || currentNameHasWildcard;
212217

213218
if (shouldUpdate && !spanJson.timestamp) {
214219
// Get fresh branches for the current location with all loaded routes
@@ -226,10 +231,10 @@ export function updateNavigationSpan(
226231
const currentSource = spanJson.data?.[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE];
227232
const isImprovement =
228233
name &&
229-
(!hasBeenNamed || // Span not finalized - accept any name
230-
!currentName || // No current name - always set
231-
(currentNameHasWildcard && source === 'route') || // Wildcard route → better route (MUST stay in route source)
232-
(currentSource !== 'route' && source === 'route')); // URL → route upgrade
234+
(!currentName || // No current name - always set
235+
(!hasBeenNamed && (currentSource !== 'route' || source === 'route')) || // Not finalized - allow unless downgrading route→url
236+
(currentSource !== 'route' && source === 'route') || // URL → route upgrade
237+
(currentSource === 'route' && source === 'route' && currentNameHasWildcard)); // Routebetter route (only if current has wildcard)
233238
if (isImprovement) {
234239
activeRootSpan.updateName(name);
235240
activeRootSpan.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, source);
@@ -521,7 +526,7 @@ export function createReactRouterV6CompatibleTracingIntegration(
521526
DEBUG_BUILD &&
522527
debug.warn('[React Router] lazyRouteTimeout must be a number, falling back to default:', defaultMaxWait);
523528
_lazyRouteTimeout = defaultMaxWait;
524-
} else if (configuredMaxWait < 0 && configuredMaxWait !== Infinity) {
529+
} else if (configuredMaxWait < 0) {
525530
DEBUG_BUILD &&
526531
debug.warn(
527532
'[React Router] lazyRouteTimeout must be non-negative or Infinity, got:',
@@ -871,12 +876,12 @@ function tryUpdateSpanNameBeforeEnd(
871876
// Only update if we have a valid name and it's better than current
872877
// Upgrade conditions:
873878
// 1. No current name exists
874-
// 2. Current name has wildcards and new source is also 'route' (never downgrade route→url)
879+
// 2. Current name has wildcards and new name is non-wildcard route (wildcard resolution)
875880
// 3. Upgrading from non-route source to route source (e.g., URL -> parameterized route)
876881
const isImprovement =
877882
name &&
878883
(!currentName || // No current name - always set
879-
(hasWildcard && source === 'route') || // Wildcard route → better route (MUST stay in route source)
884+
(hasWildcard && source === 'route' && !transactionNameHasWildcard(name)) || // Wildcard → non-wildcard route
880885
(currentSource !== 'route' && source === 'route')); // URL → route upgrade
881886
const spanNotEnded = spanType === 'pageload' || !spanJson.timestamp;
882887

@@ -935,8 +940,8 @@ function patchSpanEnd(
935940
// Don't wait - immediately update and end span
936941
tryUpdateSpanNameBeforeEnd(
937942
span,
938-
spanToJSON(span),
939-
spanToJSON(span).description,
943+
spanJson,
944+
currentName,
940945
location,
941946
routes,
942947
basename,
@@ -958,10 +963,12 @@ function patchSpanEnd(
958963
// Update span name once routes are resolved or timeout expires
959964
waitPromise
960965
.then(() => {
966+
// Re-fetch span state after async wait (span may have been updated)
967+
const updatedSpanJson = spanToJSON(span);
961968
tryUpdateSpanNameBeforeEnd(
962969
span,
963-
spanToJSON(span),
964-
spanToJSON(span).description,
970+
updatedSpanJson,
971+
updatedSpanJson.description,
965972
location,
966973
routes,
967974
basename,

0 commit comments

Comments
 (0)