Repository navigation
feat(nextjs): Mark pageloads of cached pages - #25318
Merged
Merged
Conversation
Member
Author
|
bugbot run |
Contributor
size-limit report 📦Compared: 0e9ac4f (PR test merge using develop at 8592e7e) Against: 8592e7e (develop) · base run #64393 ✅ Baseline matches: this diff contains only this PR's changes.
|
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Member
Author
|
bugbot run |
Member
Author
|
bugbot run |
Base automatically changed from
chargome/feat/nextjs-deferred-pageload-start
to
develop
October 20, 2026 15:37
s1gr1d
reviewed
Oct 21, 2026
Comment on lines
+56
to
+57
| const prerenderMarker = readPrerenderMarker(); | ||
| const prerenderOptions = prerenderMarker ? cachedDocumentSpanOptions(prerenderMarker) : undefined; |
Member
There was a problem hiding this comment.
This is where a mixed case can happen...say a shell was regenerated at runtime and the page has a dynamic hole. The document then has the sentry-prerender marker and a sentry-trace tag.
The pageload continues the server trace, and it also gets document.cached: true and a cache_origin link. Both are kind of true, but the document.cached attribute could differentiate between being a full cache or if it's just the shell (but see first comment regarding the attribute changes).
And we should also test this.
Member
There was a problem hiding this comment.
Added a unit test and an E2E test (/runtime-shell). After a runtime prerender, the document has both tags.
| export interface TraceMetaTagValues { | ||
| sentryTrace: string; | ||
| baggage: string | undefined; | ||
| } | ||
|
|
||
| /** Set on a pageload whose document was served from a cache, so no server render ran for the request. */ | ||
| export const DOCUMENT_CACHED_ATTRIBUTE = 'document.cached'; |
Member
There was a problem hiding this comment.
document.cached reads like browser HTTP cache. Also, with the boolean we can't know if it's a fully or partially (with shells) cached page.
Maybe document.static_html: 'full' | 'partial'?
This would also fit other frameworks and we can differentiate between fully static pages and partial (Astro islands, Next.js holes).
And we should also add this to sentry conventions. Added a PR: getsentry/sentry-conventions#740
Member
There was a problem hiding this comment.
After discussing this with the team we decided on page.server_rendering.type: static | partially_static | dynamic. dynamic or others maybe added later on.
We could also add *.age or *.revalidate for showing how old the ISR page is and the revalidate interval.
Member
There was a problem hiding this comment.
Could we assert the marker here too? (also in the other tests)
await expect(page.locator('meta[name="sentry-prerender"]')).toHaveCount(1);| * to later visitors, so the client never continues it, it only marks the pageload as served from a | ||
| * cached document and links the request that generated it. | ||
| */ | ||
| export const PRERENDER_META_TAG_NAME = 'sentry-prerender'; |
Member
There was a problem hiding this comment.
"prerender" already has a specified meaning in the browser API (document.prerendering - see docs).
What about sentry-cache-origin, to match the link type? The client will turn this it into a cache_origin link.
| // Both link the one request that prerendered the shell, which is neither visitor's own trace. | ||
| const linkOf = (links: unknown): { trace_id: string; attributes?: Record<string, unknown> } => { | ||
| const link = (links as Array<{ trace_id: string; attributes?: Record<string, unknown> }>)[0]; | ||
| expect(link?.attributes?.['sentry.link.type']).toBe('cache_origin'); |
Member
There was a problem hiding this comment.
nit: We can use SENTRY_LINK_TYPE from @sentry/conventions/attributes (also in the streaming app copy)
…n file Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
A visitor of a cached page gets a pageload in its own trace with no server span, which looks like missing server instrumentation. While Next.js prerenders a document, the server now writes a `sentry-shell` meta tag with the trace of the prerender instead of nothing. The client never continues it: it sets `sentry.nextjs.cached_shell` on the pageload and adds a `cache_origin` span link to the request that generated the shell. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The attribute and the link are part of the start options now, so `tracesSampler` and `ignoreSpans` see that a pageload came from a cached shell. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…ork neutral `document.cached` and `sentry-prerender` instead of a Next.js specific attribute and `sentry-shell`, so other frameworks with prerendering can set the same attribute. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
s1gr1d
changed the title
s1gr1d
force-pushed
the
chargome/feat/nextjs-cached-shell-marker
branch
from
October 27, 2026 07:52
26177fa to
86f1ac5
Compare
Member
|
bugbot run |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 86f1ac5. Configure here.
…js-cached-shell-marker # Conflicts: # CHANGELOG.md
Contributor
Optional E2E testsAll optional checks were successful. 10 successful. See the run. Successful checks
|
…js-cached-shell-marker # Conflicts: # CHANGELOG.md
Member
|
btw, this is merged: getsentry/sentry-conventions#740 |
…js-cached-shell-marker Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
chargome
marked this pull request as ready for review
October 30, 2026 12:48
chargome
requested review from
andreiborza,
msonnb,
mydea,
nicohrubec and
s1gr1d
and removed request for
a team
October 30, 2026 12:48
s1gr1d
approved these changes
Oct 30, 2026
51 tasks done
This was referenced Nov 9, 2026
s1gr1d
added a commit
that referenced
this pull request
Nov 11, 2026
**Why:** Pageloads of pages prerendered during `vite build` look like they have no server instrumentation. Now these pages get an empty `sentry-cache-origin` meta tag, and the pageload gets `page.server_rendering.type: static`. Unlike Next.js (#25318), the pageload gets no link, because the SDK does not reliably send spans from the build. **Unsure about:** To leave out `ssr = false` shells, the server checks for `node_ids:` in Kit's boot script, which Kit writes only for server-rendered pages ([source](https://github.com/sveltejs/kit/blob/%40sveltejs/kit%403.2.0/packages/kit/src/runtime/server/page/render.js#L452-L467)). Example (with UI changes from getsentry/sentry#130771 applied) or pre-rendered, static page: https://sentry-4x3dkn58u.sentry.dev/explore/traces/trace/97190404f66048bd89da29cc0a21304f/ <img width="1382" height="406" alt="image" src="https://github.com/user-attachments/assets/8317ca3a-42c7-4e92-973a-7bf22376b1d6" /> Linear: [JSSDK-37](https://linear.app/getsentry/issue/JSSDK-37/sveltekit-page-options)
This was referenced Nov 11, 2026
s1gr1d
added a commit
that referenced
this pull request
Nov 13, 2026
**Why:** Visitors of a Nuxt page cached by a route rule (`cache`, `swr`, `isr` on Vercel/Netlify) continued the server trace of the request that rendered it. Like Next.js (#25318) and SvelteKit (#26301), these pages now get a `sentry-cache-origin` meta tag in place of the trace tags. The pageload is `static` and links the render if it was sampled. `isr` only counts as cached on presets with a platform cache: on `node-server`, Nitro renders `isr` pages per request. Tested on a Vercel deploy. **How to review:** Start with `runtime/utils/pageMetaTags.ts` (server decision) and `runtime/utils/cacheOriginPageload.ts` (pageload). **Unsure about:** `withCacheOrigin` now exists in Next.js, SvelteKit and Nuxt. I plan to share it from `@sentry/browser` in a follow-up. Example with the UI changes from getsentry/sentry#130771 <img width="1374" height="390" alt="image" src="https://github.com/user-attachments/assets/d066316e-63ec-44ba-8105-3acdbeb46fe7" /> https://sentry-4x3dkn58u.sentry.dev/explore/traces/trace/e7492219ab72481989d6a2cd670fe8d9/ Linear: [JSSDK-36 ](https://linear.app/getsentry/issue/JSSDK-36/nuxt-rendering-modes) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
s1gr1d
added a commit
that referenced
this pull request
Nov 13, 2026
Stacked on #26301. **Why:** Vercel ISR (`config.isr`) caches the HTML of one request and serves it to every visitor until it expires. That HTML holds the live `sentry-trace` and `baggage` tags of the render, so every visitor continued the same old server trace. Next.js had the same bug, fixed in #25141. Now a page that Vercel renders for its ISR cache gets a `sentry-cache-origin` tag with the render's `sentry-trace` in place of the trace tags. The pageload starts its own trace, gets `page.server_rendering.type: static`, and links the render with a `cache_origin` link, as in Next.js (#25318). To detect an ISR render, `sentryHandle` reads the `x-vercel-isr` request header. Vercel sends it to every invocation of an ISR function, also for background revalidations and for routes that get `config.isr` from a layout. The adapter's own `__pathname` marker is gone before Kit sees the request, and `handle` cannot see the route config (tested both on a Vercel deployment). Like a `use cache` hit, the pageload links only a sampled render. An unsampled render still marks the pageload as `static`. A page with `ssr = false` gets no tags, as in the build. **How to review:** Start with `getCacheOrigin` in `handle.ts`. It returns the render's trace (ISR), `''` (build) or `undefined` (rendered for this request only). Then `withCacheOrigin` in `client/traceMetaTags.ts`. The two browser-tracing variants only wrap their existing span options. **Unsure about:** A client can send `x-vercel-isr` to a normal SSR route, and the function sees it. That only changes the pageload of the request that sends it: it links its own server request instead of continuing it. I think that is acceptable. I did not check draft mode (`bypassToken`), where Vercel does not cache the render. Example with the UI changes from getsentry/sentry#130771 (a visitor that got an already happened ISR render): <img width="1378" height="418" alt="image" src="https://github.com/user-attachments/assets/842594fd-38cc-418f-b981-369da8305375" /> https://sentry-4x3dkn58u.sentry.dev/explore/traces/trace/63b1dbe5fe3849f2b13df81530857c03/ Linear: [JSSDK-37](https://linear.app/getsentry/issue/JSSDK-37/sveltekit-page-options)
s1gr1d
added a commit
that referenced
this pull request
Nov 13, 2026
…26363) ## What Next.js, Nuxt and SvelteKit now use one shared implementation of the `sentry-cache-origin` tag from #25318, #26305 and #26307. There is no change for users. - `@sentry/core` holds the tag name, the server code that writes the tag, and the client code that marks and links the pageload. All of these are `_INTERNAL_` exports. The client part is in `@sentry/core/browser`, and the server part is in `@sentry/core/server`. - Nuxt now links the request's root span explicitly, like SvelteKit. The Nuxt E2E tests check that the tag holds the `http.server` root span. ## Why Each SDK had its own copy of the tag name, the tag writer and the pageload logic. With one module, the server writer and the client reader cannot drift apart. A round-trip test now checks that the client reads the tag the server writes. Each framework keeps its own decision about which page is reused (build, `x-vercel-isr`, Nitro route rules, Next.js prerender context). Those rules differ per framework, so a shared helper would only pass them through. The Nuxt change already behaves the same today: no child span is active while Nuxt renders the page. The explicit root span protects the link from a future child span. That is the bug SvelteKit had with `sveltekit.resolve`. Two edge cases are now handled the same way in all three SDKs, and the SDKs never write either one: - An empty tag marks the pageload without a link. Before this change, only Next.js ignored an empty tag, and Next.js never writes one. - Core reads the first tag. Before this change, Next.js read the last one, but a Next.js document never has more than one tag.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
When Next.js serves a page from its cache (an ISR page, or a
cacheComponentsshell that was prerendered at runtime), the pageload starts its own trace and has no server span. That is correct, but in Sentry it looks like missing server instrumentation.While Next.js prerenders a document, the server now writes a
sentry-cache-originmeta tag with thesentry-tracevalue of the prerender (instead of nothing). The client never continues it: it setspage.server_rendering.typeon the pageload (staticwhen the whole page came from the cache,partially_staticwhen parts of it rendered for this request) and links the prerender with acache_originlink, both at span start, sotracesSamplerandignoreSpanssee them. Like ause cachehit, the pageload links only a sampled prerender.So the fixes of #25141 still hold: a cached page does not attach its visitors to a request that was not theirs, and a
sampled=0value in the tag cannot drop their pageloads. Pages fromnext buildget no tag, because the SDK does not run during the build.The attribute and the tag are named without Next.js in them on purpose: other frameworks with prerendering (Nuxt, SvelteKit, Astro, React Router) can set the same attribute.
page.server_rendering.typeis being added to@sentry/conventionsin getsentry/sentry-conventions#740;cache_originis already a documentedsentry.link.typevalue and is what the cache lifecycle view in Sentry reads.Refs JS-3886
🤖 Generated with Claude Code