Repository navigation
Conversation
Member
Author
|
bugbot run |
Contributor
size-limit report 📦Compared: d6a96a2 (PR test merge using develop at 53daa00) Against: 632cbfe (develop) · base run #64190
|
![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 testsSome optional checks were not successful. They do not block the merge. 1 not successful, 56 successful. See the run.
Successful checks
|
…js-cached-shell-marker # Conflicts: # CHANGELOG.md
This branch has not been deployed
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