Skip to content

Conversation

onurtemizkan
Copy link
Collaborator

@onurtemizkan onurtemizkan commented Sep 5, 2025

Removes recently introduced special-casing lazy-route -> lazy-route transaction name updates.
This completes the fix in #17438.
E2E tests are updated to replicate a similar case in the reproduction here: #17417

Also manually tested on the reproduction.

Copy link
Contributor

github-actions bot commented Sep 5, 2025

size-limit report 📦

Path Size % Change Change
@sentry/browser 24.17 kB - -
@sentry/browser - with treeshaking flags 22.75 kB - -
@sentry/browser (incl. Tracing) 40.14 kB - -
@sentry/browser (incl. Tracing, Replay) 78.5 kB - -
@sentry/browser (incl. Tracing, Replay) - with treeshaking flags 68.26 kB - -
@sentry/browser (incl. Tracing, Replay with Canvas) 83.18 kB - -
@sentry/browser (incl. Tracing, Replay, Feedback) 95.38 kB - -
@sentry/browser (incl. Feedback) 40.91 kB - -
@sentry/browser (incl. sendFeedback) 28.82 kB - -
@sentry/browser (incl. FeedbackAsync) 33.77 kB - -
@sentry/react 25.89 kB - -
@sentry/react (incl. Tracing) 42.11 kB - -
@sentry/vue 28.66 kB - -
@sentry/vue (incl. Tracing) 41.95 kB - -
@sentry/svelte 24.2 kB - -
CDN Bundle 25.76 kB - -
CDN Bundle (incl. Tracing) 40.01 kB - -
CDN Bundle (incl. Tracing, Replay) 76.29 kB - -
CDN Bundle (incl. Tracing, Replay, Feedback) 81.75 kB - -
CDN Bundle - uncompressed 75.2 kB - -
CDN Bundle (incl. Tracing) - uncompressed 118.31 kB - -
CDN Bundle (incl. Tracing, Replay) - uncompressed 233.4 kB - -
CDN Bundle (incl. Tracing, Replay, Feedback) - uncompressed 246.16 kB - -
@sentry/nextjs (client) 44.14 kB - -
@sentry/sveltekit (client) 40.58 kB - -
@sentry/node-core 49.66 kB - -
@sentry/node 150.7 kB - -
@sentry/node - without tracing 92.23 kB - -
@sentry/aws-serverless 105.56 kB - -

View base workflow run

@onurtemizkan onurtemizkan force-pushed the onur/remove-handleexistingnavigation branch from 1ba555f to 0a34f8b Compare September 5, 2025 12:21
@onurtemizkan onurtemizkan marked this pull request as ready for review September 5, 2025 12:26
@chargome chargome requested a review from s1gr1d September 5, 2025 12:29
cursor[bot]

This comment was marked as outdated.

cursor[bot]

This comment was marked as outdated.

handleExistingNavigationSpan(activeSpan, spanJson, name, source, isLazyRouteContext);
} else {
createNewNavigationSpan(client, name, source, version, isLazyRouteContext);
if (!isAlreadyInNavigationSpan) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

q: Can you explain the change of this check?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, the first block of this condition was updating the span name depending on the context of a lazy-route. That was the reason for the faulty transaction name updates (as the previous navigation transaction's name / route was leaking into the current one).

Turns out it's unnecessary (and also regressed the bug we were trying to fix).

Removing this whole handleExistingNavigationSpan logic (with its lazy-route context checks) fixed the issue, without breaking anything else. We still need to check if we are already inside a navigation span, for the instrumentation cross-usage to prevent nesting / duplication.

@s1gr1d s1gr1d enabled auto-merge (squash) September 9, 2025 07:30
@s1gr1d s1gr1d merged commit 38cc574 into develop Sep 9, 2025
36 checks passed
@s1gr1d s1gr1d deleted the onur/remove-handleexistingnavigation branch September 9, 2025 07:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants