getsentry / sentry-javascript

Official Sentry SDKs for JavaScript
https://sentry.io
MIT License
7.97k stars 1.57k forks source link

Next.js v14.2 links don't respond to clicks when Sentry is enabled #11796

Closed rvanlaarhoven closed 6 months ago

rvanlaarhoven commented 6 months ago

Is there an existing issue for this?

How do you use Sentry?

Sentry Saas (sentry.io)

Which SDK are you using?

@sentry/nextjs

SDK Version

7.112.2

Framework Version

Nextjs 14.2.3 & 14.3.0-canary.23

Link to Sentry event

No response

SDK Setup

No response

Steps to Reproduce

After upgrading Next.js from v14.1.3 to v14.2.3 (and v14.3.0-canary) I noticed that some links in my app didn't respond to clicks anymore and resulted in a full page crash several seconds after the clicks. In production, not in development.

After some digging I found that it only happens when @sentry/nextjs is enabled, and it only seems to happen on links to pages that use a server-side fetch to an external api (It was a Neon database in my case, but replaced it with a random free rest api in the reproduction below). It looks like it's caused by setting the __SENTRY_DEBUG__ & __SENTRY_TRACING__ flags to false in my next.config.js (as suggested here).

I created a reproduction repo & test environment here:

Expected Result

Enabling Sentry should not have a negative effect when clicking links.

Actual Result

See "steps to reproduce" section.

lforst commented 6 months ago

Hi, I just took a look at this. Thanks for reporting in any case!

So I can see that your deployed application is broken. When I click the first link, everything works fine. If I do a browser-back and then click on a link, it won't navigate anymore.

However, I ran the reproduction locally with next build && next start and everything worked perfectly fine. Then I also deployed the app to Vercel and I couldn't reproduce the problem.

Did you configure anything differently in the test environment you linked? Is your Vercel instance configured in a special way? Do you have enough Vercel quota?

In general, we don't really support canary builds of Next.js, or rather we don't have any guarantees for them as they are often very buggy. We have tests for Next.js canaries just so we catch any bugs that may make it into stable, but we don't recommend them to be used with the SDK.

rvanlaarhoven commented 6 months ago

Hi, it's a completely weird thing. I mostly cannot reproduce it on next build && next start either, but I did encounter it on my actual app before (It's never an issue on next dev). So this issue is mostly visible when hosted on Vercel.

I did not do anything special on Vercel. I just selected the repo and configured the three env vars that point to Sentry. I'm using a free Vercel account, and looking at my usage I'm not hitting any limits.

The canary next.js build I used was purely because I initially thought it was a Next.js issue, so I used their reproduction template. But the problem is the same on the latest stable Next.js version. I can revert to that version if that makes sense for the reproduction.

When you tested your own deployed app on Vercel, did you try multiple links on the page? Because what I'm seeing is that some links are actually clickable and they will send you to the right page. But at the same time others don't respond to clicks at all and will result in a page crash when you leave the app open for a little while.

mydea commented 6 months ago

Hey @rvanlaarhoven , @lforst is currently on vacation until Thursday, he'll look some more into it when he's back (I'd love to help out but sadly my understanding of Next.js internals is limited, and I fear this is a rather tricky thing to debug 😬 )

One additional question maybe: Does this happen on every deploy, or just on some deploys? E.g. are there sometimes deploys were all links work? Or are there always broken links?

rvanlaarhoven commented 6 months ago

No worries!

I think it happens on every deploy. Or at least that's how I've been debugging this; reducing the code of my actual app down to the core issue by stripping things, deploying again, stripping some more, etc. And I've then moved the essentials I found over to the repro I shared above and it happened again. I've been doing quite a few deploys and it always kept on happening unless I either:

Some things I did notice;

I've also been monitoring the Next.js issues page for any similar problems. I haven't seen any with the same issue, but I do see there are more issues with linking (mostly focused on intercepting routes which is not the case in my repro)

rvanlaarhoven commented 6 months ago

FWIW, I've also tried removing Sentry completely and just use the DefinePlugin() with a random var (see this branch). In that case thins work as expected and all links are working (see preview).

rvanlaarhoven commented 6 months ago

Just did some more tests.

As I mentioned earlier the prefetch does work and results in a 200 response quickly. But, on a click it triggers another fetch with a different _rsc identifier. That request then hangs until the app results in a crash and reports a Error: Connection closed..

Screenshot 2024-04-30 at 15 40 22

Whenever I disable Sentry completely and/or remove the DefinePlugin() I am not experiencing any issues. But will keep trying some more.

lforst commented 6 months ago

Honestly, this is very fishy and I struggle to understand what in the DefinePlugin may cause this (or rather how __SENTRY_DEBUG__ & __SENTRY_TRACING__ could be an issue).

Would you mind upgrading to the v8 beta of the SDK (version 8.0.0-beta.5)? I am hoping that we did some change that fixes this. The beta is already very stable and we are planning to release v8 either this week or next week.

The migration docs can be found here: https://docs.sentry.io/platforms/javascript/guides/nextjs/migration/v7-to-v8/

rvanlaarhoven commented 6 months ago

Nice, I'm not seeing any issues after migrating to v8 beta! Tested it both for the reproduction on localhost and Vercel, and tested on the actual app and seems to be fixed!

lforst commented 6 months ago

@rvanlaarhoven Very odd but I am happy that v8 fixes this. I couldn't even think of any changes that could have influenced this 🤔 Closing the issue in the meanwhile. Feel free to hit us up here if the bug happens again!