beforeNavigate isn't called for ReactRouterV6

[yarinsa]

Is there an existing issue for this?

• [X] I have checked for existing issues https://github.com/getsentry/sentry-javascript/issues
• [X] I have reviewed the documentation https://docs.sentry.io/
• [X] I am using the latest SDK release https://github.com/getsentry/sentry-javascript/releases

How do you use Sentry?

Sentry Saas (sentry.io)

Which package are you using?

@sentry/react

SDK Version

7.11.0

Framework Version

React 17.0.2

Link to Sentry event

No response

Steps to Reproduce

1. Setup sentry instrumentation with ReactRouterV6 according to the docs
2. Implement some dummy function for "beforeNavigate" on BrowserTracing options

    integrations: [
      new BrowserTracing({
        routingInstrumentation: Sentry.reactRouterV6Instrumentation(
          useEffect,
          useLocation,
          useNavigationType,
          createRoutesFromChildren,
          matchRoutes,
        ),
        beforeNavigate: (ctx) => {
          console.log(ctx);
          return ctx;
        },
      }),
    ],

3. Go into the browser and navigate across different routes

Expected Result

console would print the ctx of each navigation

Actual Result

Except pageload - initial navigation , nothing happens.

Also tried to blend into "matchRoutes" function using hoisting. Also tried to capture the event in "beforeSend" Seems like the events doesn't go trough there. Trying to enrich the events with the route context.

[AbhiPrasad]

Hey, thanks for writing in. Could you enable debug mode by supplying debug: true as an option to the SDK init? What logs out when you turn on debug mode?

[yarinsa]

Sentry Logger [log]: [Tracing] Starting 'http.client' span on transaction '/setup' (b153127e151021d2).
logger.ts:77 Sentry Logger [log]: [Tracing] Finishing 'http.client' span on transaction '/setup' (b153127e151021d2).
instrument.ts:123 IdleTransaction {traceId: '6ec990e72481440781756c2284ce6aab', spanId: 'b153127e151021d2', startTimestamp: 1660815344.4526, tags: {…}, data: {…}, …}activities: {}data: {}endTimestamp: 1660815345.4535metadata: {source: 'url', baggage: Array(3), spanMetadata: {…}, transactionSampling: {…}}op: "pageload"sampled: falsespanId: "b153127e151021d2"startTimestamp: 1660815344.4526tags: {routing.instrumentation: 'react-router-v6'}traceId: "6ec990e72481440781756c2284ce6aab"transaction: IdleTransaction {traceId: '6ec990e72481440781756c2284ce6aab', spanId: 'b153127e151021d2', startTimestamp: 1660815344.4526, tags: {…}, data: {…}, …}_beforeFinishCallbacks: [ƒ]_finalTimeout: 30000_finished: true_heartbeatCounter: 0_hub: Hub {_version: 4, _stack: Array(1)}_idleHub: Hub {_version: 4, _stack: Array(1)}_idleTimeout: 1000_idleTimeoutID: 3_measurements: {}_name: "/setup"_onScope: true_trimEnd: truename: (...)[[Prototype]]: Transaction
logger.ts:77 Sentry Logger [log]: [Tracing] No active IdleTransaction
logger.ts:77 Sentry Logger [log]: [Tracing] Discarding transaction because its trace was not chosen to be sampled.
logger.ts:77 Sentry Logger [log]: Adding outcome: "sample_rate:transaction"
logger.ts:77 Sentry Logger [log]: [Tracing] Finishing 'http.client' span on transaction '/setup' (b153127e151021d2).
instrument.ts:123 undefined
logger.ts:77 Sentry Logger [log]: [Tracing] Finishing 'http.client' span on transaction '/setup' (b153127e151021d2).

That's on page load, which is also the only case beforeNavigate is getting called. When switching routes nothing goes to the console

https://user-images.githubusercontent.com/44482362/185363393-7c178d98-f2c6-49dd-aebc-4c048d22720b.mov

[AbhiPrasad]

Thank you for the video!

This is the logic for how navigations are generated:

https://github.com/getsentry/sentry-javascript/blob/5172a7deef55fb9e339c9129553e5d50f0321b1c/packages/react/src/reactrouterv6.tsx#L181-L205

So there's a couple things here: a) the useEffect is not being triggered b) the navigationType is not PUSH or POP.

Perhaps we have to adjust the dependency array of the useEffect here.

[yarinsa]

@AbhiPrasad Thank you! Is this on your roadmap? When can we expect it to be resolved?

[AbhiPrasad]

Could you check what is talked about in https://github.com/getsentry/sentry-javascript/issues/5555 could resolve your issue?

[yarinsa]

This is our entry file. We have a "bootstrap" function and we are running Sentry first like it says in the docs

[ymatusevich]

We stuck with the same issue. Hope the solution or fix will be found soon.

[yarinsa]

@AbhiPrasad If I understand correctly the we should "Start Transaction" for each route navigation (similar to page load). I was trying to implement something similar with my own code, but it's seems like the scope that I am setting getting lost in the global Sentry object. I was trying to do something like:

export const handleRouteChange = (routes, location) => {
 // Close existing transaction if exists
  const transaction = Sentry.getCurrentHub().getScope().getTransaction();
  transaction?.finish();

  const route = matchRoutes(routes, location.pathname)?.[0];

  const nextTransaction = Sentry.startTransaction({
    name: route.pathname,
  });

  Sentry.getCurrentHub().withScope((scope) => {
    scope.setSpan(nextTransaction);
    scope.setExtra('route', route); // Used to identify my scope for debugging
  });
};

Then in other place on my app I did something like

    const existingTransaction = Sentry.getCurrentHub().getScope()?.getTransaction();

    // Create a transaction if one does not exist in order to work around
    // https://github.com/getsentry/sentry-javascript/issues/3169
    // https://github.com/getsentry/sentry-javascript/issues/4072
    const transaction =
      existingTransaction ??
      Sentry.startTransaction({
        name: `API Request(${config?.data?.serviceId}): ${config?.data?.endpoint ?? config.url}`,
      });
      
    Sentry.getCurrentHub().configureScope((scope) => scope.setSpan(transaction));

That's in order to attach the network error to the transaction. But existingTransaction is undefined.

I ended up creating a "single" transaction for every network endpoint& request which misses the whole idea of transactions., which feels strongly wrong. When can we expect a solution for something so basic?

[AbhiPrasad]

If I understand correctly the we should "Start Transaction" for each route navigation

You shouldn't need to do this, we should be automatically creating these transactions. That's the bug that needs to be fixed. I do have some bandwidth to look at this - are you able to share some kind of repro of this problem so I can take a look?

[yarinsa]

Mmmm , it's pretty basic. Start create react app, add the react-router integration and have debug true, in the sentry init. I am willing to go on a call,

[AbhiPrasad]

Ah ok, was able to reproduce with https://stackblitz.com/edit/github-7tjxzc-gabqis?file=src/App.tsx. Will take a look!

[AbhiPrasad]

Ah ok - seems like I fell into the same thing that was addressed here: https://github.com/getsentry/sentry-javascript/issues/5555#issuecomment-1234315339

See our docs update: https://github.com/getsentry/sentry-docs/pull/5505/files

You have to make sure that Sentry.init is being called before you wrap your routes, otherwise things won't work properly.

I updated the example to move the logic into App.tsx, and everything works properly. This API doesn't feel ideal to me though, so I'm gonna try and update it. https://stackblitz.com/edit/github-7tjxzc-gabqis

You can confirm this by turning on debug mode, and searching for the string reactRouterV6Instrumentation was unable to wrap Routes because of one or more missing parameters.

[yarinsa]

@AbhiPrasad I red trough it and even had a comment in my code. My problem was that I had file called 'sentry.tsx' which contains:

export const initSentry =() => { Sentry.init() }

export const SentryRoutes = Sentry.withSentryReactRouterV6Routing(Routes);

The init is called in a function so the wrapping of HOC is actually happening first.

I highly suggest to create some promise based logic for this

[AbhiPrasad]

I highly suggest to create some promise based logic for this

We might just eliminate this problem entirely by supporting the new APIs in 6.4 - https://github.com/getsentry/sentry-javascript/issues/5872

[yarinsa]

Seems like it was solved in 7.15.0 (and we also fixed our code). Therefore closing the issue :) Thanks @AbhiPrasad