fix(nextjs): Drop meta trace tags if rendered page is ISR #18192
Conversation
size-limit report 📦
|
logaretm
force-pushed
the
awad/js-1152-nextjs-incremental-static-rendering-causes-cached-trace-id
branch
from
73ec1ba to
54be3bf
Compare
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
logaretm
force-pushed
the
awad/js-1152-nextjs-incremental-static-rendering-causes-cached-trace-id
branch
from
289f39c to
12badfa
Compare
logaretm
force-pushed
the
awad/js-1152-nextjs-incremental-static-rendering-causes-cached-trace-id
branch
from
12badfa to
f46d817
Compare
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
node-overhead report 🧳Note: This is a synthetic benchmark with a minimal express app and does not necessarily reflect the real-world performance impact in an application.
|
| /** | ||
| * Check if the current page is an ISR/SSG route by checking the route manifest. | ||
| */ | ||
| function isIsrSsgRoute(pathname: string): boolean { |
There was a problem hiding this comment.
Do you think we should cache the results of this function? Esp as it's calling a JSON.parse of the route manifest?
There was a problem hiding this comment.
I think that's a good idea since the manifest shouldn't change in runtime 👍 I can see we already use getManifest which caches the JSON parsing. I will create another cache as well for the route matcher rather than the actual pathname since it may get a lot of entries.
packages/nextjs/src/client/index.ts
Outdated
|
|
||
| // Remove cached trace meta tags for ISR/SSG pages before initializing | ||
| // This prevents the browser tracing integration from using stale trace IDs | ||
| removeIsrSsgTraceMetaTags(); |
There was a problem hiding this comment.
this should be gated behind the if (typeof __SENTRY_TRACING__ === 'undefined' || __SENTRY_TRACING__) { flag like adding the browserTracingIntegration is.
logaretm
force-pushed
the
awad/js-1152-nextjs-incremental-static-rendering-causes-cached-trace-id
branch
from
5dddf30 to
a49edb2
Compare
| const content = fs.readFileSync(pageFilePath, 'utf8'); | ||
| // check for generateStaticParams export | ||
| // the regex covers `export function generateStaticParams`, `export async function generateStaticParams`, `export const generateStaticParams` | ||
| return /export\s+(async\s+)?function\s+generateStaticParams|export\s+const\s+generateStaticParams/.test(content); |
There was a problem hiding this comment.
Bug: Fix Regex Over-Matching
The regex pattern matching generateStaticParams lacks word boundaries, causing false positives. It will incorrectly match identifiers like generateStaticParamsFoo or generateStaticParamsExtra because the pattern doesn't verify where the identifier ends. Add \b word boundaries after generateStaticParams in both alternations to match only the exact export name.
| * Cache for ISR/SSG route checks. Exported for testing purposes. | ||
| * @internal | ||
| */ | ||
| export const IS_ISR_SSG_ROUTE_CACHE = new Map<string, boolean>(); |
There was a problem hiding this comment.
Oh, I didn't see that 🤦♂️ I will amend in a PR.
logaretm
deleted the
awad/js-1152-nextjs-incremental-static-rendering-causes-cached-trace-id
branch
Summary
ISR pages will have a
sentry-traceandbaggagemeta tags rendered on them following the initial render or after the first invalidation causing a cached trace id to be present until the next invalidation.This happens in Next.js 15/16 and both on Turbopack and Webpack.
What I tried and didn't work
I Found no way to conditionally set/unset/change the values set by the
clientTraceMetadataoption, I found nothing useful on unit async storages, nor re-setting the propagation context works. TheclientTraceMetadatagets called way earlier at theapp-render.tsxlevel, which would call ourSentryPropagator.inject()then. We cannot intercept it either because it runs before the page wrapper is called.The main issue is timing:
sentry-traceor baggage to dummy values as some sort of "marker" for the SDK on the browser side to drop them, but again it is too late sinceclientTraceMetadatais picked up too early.Implementation
so I figured a workaround, I decided to do it on the client side by:
Sentry.initcall we remove the tags before the browser integration has had a chance to grab the meta tags.Not the cleanest way, but I verified the issue by writing tests for it and observing page loads across multiple page visits having the same trace id. The meta deletion forces them to have new id for every visit which is what we want.