Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[nextjs] Fix transaction name getting lost when hitting _error page #5826

Closed
lobsterkatie opened this issue Sep 27, 2022 · 2 comments
Closed
Labels
Feature: Spans Package: nextjs Issues related to the Sentry Nextjs SDK Type: Bug

Comments

@lobsterkatie
Copy link
Member

In the nextjs SDK, when a user hits the _error page, the original transaction name gets overwritten with _error. This causes two problems:

  1. It makes it so a user would not be able to tell that they have a particularly buggy route, since every time it errored, it would show up in the Sentry UI filed under _error rather than the real route name.

  2. The DSC can get propagated with the original transaction name, but that then doesn't match up with the eventual transaction name.

We should prevent the original name from getting overwritten.

(This is current as of 7.13.0.)

@lobsterkatie lobsterkatie added this to the NextJS Improvements milestone Sep 27, 2022
@lobsterkatie lobsterkatie changed the title [nextjs] Transaction name issues when hitting _error page [nextjs] Fix transaction name getting lost when hitting _error page Sep 27, 2022
@lforst
Copy link
Member

lforst commented Sep 27, 2022

We should rephrase this issue so that it's clear that this only affects frontend pageload transactions (which get named _error in the case an error occurs in the backend). The backend transaction is still named and parameterized fine.

I believe we even had this behavior before we made any recent Next.js SDK changes.

@lforst
Copy link
Member

lforst commented Sep 27, 2022

The DSC can get propagated with the original transaction name, but that then doesn't match up with the eventual transaction name.

That is correct behaviour. The frontend must pick up the transaction name from the backend and put it into DSC no matter what. If there is no backend transaction (and inherently no backend DSC) the SDK currently propagates _error which behavior-wise is probably even fine because all the requests that we wanna trace are kicked off by the _error page and not by the page that is in window.location.pathname.

I actually think the todo I left is wrong. It doesn't matter that DSC and transaction name are different. This only matters in the head SDK which is the Next.js data fetcher instrumentation in our case and that works fine:

// `nextData.page` always contains the parameterized route - except for when an error occurs in a data fetching
// function, then it is "/_error", but that isn't a problem since users know which route threw by looking at the
// parent transaction
// TODO: Actually this is a problem (even though it is not that big), because the DSC and the transaction payload will contain
// a different transaction name. Maybe we can fix this. Idea: Also send transaction name via pageProps when available.

What we should discuss is if we even want the pageload transaction to be named after the requested route.

But I should definitely change that todo again.

@HazAT HazAT closed this as completed Jan 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature: Spans Package: nextjs Issues related to the Sentry Nextjs SDK Type: Bug
Projects
None yet
Development

No branches or pull requests

3 participants