Skip to content

fix(react-router): Do not re-write origin on router state changes#21056

Merged
mydea merged 7 commits into
developfrom
fn/fix-react-router-pageload-origin-race
May 20, 2026
Merged

fix(react-router): Do not re-write origin on router state changes#21056
mydea merged 7 commits into
developfrom
fn/fix-react-router-pageload-origin-race

Conversation

@mydea
Copy link
Copy Markdown
Member

@mydea mydea commented May 20, 2026

Summary

Closes #20784. Closes #20772.

The router state-change subscription in hydratedRouter.ts was unconditionally re-writing sentry.origin on whatever root span was active at the time. Two side effects:

  1. The flaky CI failure — for static routes (where the parameterized path equals the URL pathname), the pathname === rootSpanName guard didn't filter out the still-active pageload span, so the pageload got tagged with auto.navigation.react_router and produced transactions with op=pageload but origin=auto.navigation.react_router. Dynamic routes hid the bug because the parameterized name no longer equalled the pathname, short-circuiting the block.
  2. An instrumentation-API regression — when the navigation span was created via createClientInstrumentation, its origin is auto.navigation.react_router.instrumentation_api. The subscribe callback's re-write was stripping the .instrumentation_api suffix.

The origin is correctly set once at span creation (in maybeCreateNavigationTransaction / instrumentation API) or once in trySubscribe for the pageload. The subscribe callback only needs to update name + source. Drop the origin write entirely.

Added a regression test asserting the pageload origin is preserved when the subscribe callback fires while the pageload is still the active root, and tightened the existing navigation assertion to check the attribute payload rather than just that setAttributes was called.

Also corrected the react-router-7-framework-instrumentation navigation e2e tests, which were asserting the legacy origin only because of the same bug — React Router 7 actually invokes the instrumentation API navigate hook on the client, so the navigation spans genuinely carry the .instrumentation_api origin (with the legacy subscribe callback still doing parameterization).

mydea and others added 3 commits May 20, 2026 11:51
Closes #20784

The router state-change subscription wrote `sentry.origin:
auto.navigation.react_router` to whatever root span happened to be
active. When the route was static (so the parameterized path equals
the URL pathname), the `pathname === rootSpanName` guard did not
filter out the still-active pageload span — so the pageload got
tagged with the navigation origin, producing transactions with
`op=pageload` but `origin=auto.navigation.react_router`. Dynamic
routes hid the bug because the parameterized name no longer equalled
the pathname, short-circuiting the block.

The origin override here was also incorrect for the instrumentation
API path: that creates the navigation span with
`auto.navigation.react_router.instrumentation_api`, and the subscribe
callback was stripping the `.instrumentation_api` suffix on its way
through.

The origin is correctly set once at span creation (or once in
`trySubscribe` for the pageload). The subscribe callback only needs
to update name + source. Drop the origin write entirely.

Added a regression test asserting the pageload's origin is preserved
when the subscribe callback fires while it's still active, and
tightened the existing navigation assertion to check the attribute
payload rather than just that `setAttributes` was called.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 2 potential issues.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit de12513. Configure here.

Comment thread packages/react-router/test/client/hydratedRouter.test.ts Outdated
Comment thread packages/react-router/test/client/hydratedRouter.test.ts Outdated
mydea and others added 4 commits May 20, 2026 11:54
… 'route')

Source now uses the single-attribute Span#setAttribute API since only
`source` is updated by the router.subscribe callback. The test mocks
need a setAttribute spy, and the regression assertion has to match the
new call shape.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
… for true origin

These tests were asserting `origin: auto.navigation.react_router`, but
that legacy origin was only being produced because of a bug in the
router state-change subscription that unconditionally overwrote the
origin attribute. With `useInstrumentationAPI: true` and the
instrumentation array passed to HydratedRouter, React Router invokes
the navigate hook on the client and the navigation span is genuinely
created via the instrumentation API — so the correct origin is
`auto.navigation.react_router.instrumentation_api`. The subscribe
callback still updates the span name to its parameterized form so
`sentry.source` ends up as `route`.

Renamed the describe block and updated the top-of-file comment to
reflect the new understanding.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@mydea mydea marked this pull request as ready for review May 20, 2026 10:48
@mydea mydea requested a review from a team as a code owner May 20, 2026 10:48
@mydea mydea self-assigned this May 20, 2026
@mydea mydea merged commit bcac1db into develop May 20, 2026
268 of 269 checks passed
@mydea mydea deleted the fn/fix-react-router-pageload-origin-race branch May 20, 2026 13:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment