Skip to content

feat(nextjs): Wait for the resumed part of a document before starting the pageload - #25142

Open
chargome wants to merge 6 commits into
chargome/feat/nextjs-prerender-trace-guardfrom
chargome/feat/nextjs-deferred-pageload-start
Open

chargome wants to merge 6 commits into
chargome/feat/nextjs-prerender-trace-guardfrom
chargome/feat/nextjs-deferred-pageload-start

Conversation

@chargome

@chargome chargome commented Oct 7, 2026 •

Copy link
Copy Markdown
Member

Builds on #25141.

When a document resumes a prerendered shell, its sentry-trace tag sits at the start of the resumed part, which arrives after the SDK has started. In a local demo six of seven visits found no tag at init, so the pageload could only link the server trace instead of joining it.

The app router pageload now continues whatever tag is in the document, last one wins. In apps with cacheComponents enabled, if there is no tag, no Server-Timing trace and the document is still streaming, it waits for the tag before creating the pageload span. Other apps keep the immediate start: without cacheComponents a tag is either in <head> from the start or never comes, so waiting only held the pageload back. The wait ends at DOMContentLoaded or after 10 seconds. The span still starts at the time origin and gets its children from the performance timeline, so it looks the same as before, only inside the server's trace. No tag by the end of parsing means a fresh trace, which is the fully static case.

A client-side navigation during the wait starts the pageload with a fresh trace first, so the navigation ends it like any pageload it interrupts, as today. Started later, the pageload would end the navigation span instead. The pageload keeps the URL the document was loaded with, even if the navigation already changed it.

Waiting was chosen over moving a started span to another trace. That needs core changes and does not work with span streaming once children are flushed. The cost: fetches, errors and clicks between SDK start and the resumed part are not attached to the pageload. The Server-Timing follow-ups (JS-3864, JS-3866) remove that window where the header can be set.

figure-ppr

Fixes #24944

🤖 Generated with Claude Code

@chargome
chargome added this pull request to stack #25143 October 7, 2026 14:59
@chargome chargome self-assigned this Oct 7, 2026
@chargome

chargome commented Oct 7, 2026

Copy link
Copy Markdown
Member Author

bugbot run

@github-actions

github-actions Bot commented Oct 7, 2026 •

Copy link
Copy Markdown
Contributor

size-limit report 📦

Path Size % Change Change
@sentry/browser 29.83 kB - -
@sentry/browser - with treeshaking flags 27.96 kB - -
@sentry/browser - with treeshaking flags tracing without tracing 27.87 kB - -
@sentry/browser (incl. Tracing) 52.02 kB - -
@sentry/browser (incl. Tracing + Span Streaming) 52.02 kB - -
@sentry/browser (incl. Tracing, Profiling) 54.96 kB - -
@sentry/browser (incl. Tracing, Replay) 91.75 kB - -
@sentry/browser (incl. Tracing, Replay) - with treeshaking flags 80.62 kB - -
@sentry/browser (incl. Tracing, Replay with Canvas) 96.47 kB - -
@sentry/browser (incl. Tracing, Replay, Feedback) 109.43 kB - -
@sentry/browser (incl. Feedback) 47.35 kB - -
@sentry/browser (incl. sendFeedback) 34.89 kB - -
@sentry/browser (incl. FeedbackAsync) 40 kB - -
@sentry/browser (incl. Metrics) 30.85 kB - -
@sentry/browser (incl. Logs) 31.13 kB - -
@sentry/browser (incl. Metrics & Logs) 31.78 kB - -
@sentry/react 31.64 kB - -
@sentry/react (incl. Tracing) 54.34 kB - -
@sentry/vue 37.98 kB - -
@sentry/vue (incl. Tracing) 54.95 kB - -
@sentry/svelte 29.86 kB - -
@sentry/remix (Remix 3 client bundle) 57.02 kB - -
CDN Bundle 31.56 kB - -
CDN Bundle (incl. Tracing) 52.52 kB - -
CDN Bundle (incl. Logs, Metrics) 33.77 kB - -
CDN Bundle (incl. Tracing, Logs, Metrics) 54.46 kB - -
CDN Bundle (incl. Replay, Logs, Metrics) 74.7 kB - -
CDN Bundle (incl. Tracing, Replay) 90.19 kB - -
CDN Bundle (incl. Tracing, Replay, Logs, Metrics) 92.11 kB - -
CDN Bundle (incl. Tracing, Replay, Feedback) 96.34 kB - -
CDN Bundle (incl. Tracing, Replay, Feedback, Logs, Metrics) 98.28 kB - -
CDN Bundle - uncompressed 92.98 kB - -
CDN Bundle (incl. Tracing) - uncompressed 155.83 kB - -
CDN Bundle (incl. Logs, Metrics) - uncompressed 99.52 kB - -
CDN Bundle (incl. Tracing, Logs, Metrics) - uncompressed 161.79 kB - -
CDN Bundle (incl. Replay, Logs, Metrics) - uncompressed 229.56 kB - -
CDN Bundle (incl. Tracing, Replay) - uncompressed 276 kB - -
CDN Bundle (incl. Tracing, Replay, Logs, Metrics) - uncompressed 281.94 kB - -
CDN Bundle (incl. Tracing, Replay, Feedback) - uncompressed 289.7 kB - -
CDN Bundle (incl. Tracing, Replay, Feedback, Logs, Metrics) - uncompressed 295.63 kB - -
@sentry/nextjs (client) 57.13 kB +0.68% +383 B 🔺
@sentry/sveltekit (client) 52.41 kB - -
@sentry/core/server 40.99 kB - -
@sentry/core/browser 13.81 kB - -
@sentry/node 151.96 kB +0.01% +4 B 🔺
@sentry/node/import (ESM hook with diagnostics-channel injection) 83.78 kB - -
@sentry/node - without tracing 94.54 kB +0.01% +3 B 🔺
@sentry/node - without channel injection 130.14 kB +0.01% +6 B 🔺
@sentry/aws-serverless 102.69 kB +0.01% +7 B 🔺
@sentry/cloudflare (withSentry) - minified 210.38 kB - -
@sentry/cloudflare (withSentry) 521.61 kB - -
@sentry/nextjs/cloudflare (withSentry) - minified 229.35 kB +0.57% +1.3 kB 🔺

View base workflow run

@chargome
chargome force-pushed the chargome/feat/nextjs-deferred-pageload-start branch from 217546f to fa2f4e1 Compare October 7, 2026 15:19
@chargome

chargome commented Oct 7, 2026

Copy link
Copy Markdown
Member Author

bugbot run

@cursor cursor Bot left a comment •

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stale Bugbot comment from a previous run.

Comment thread packages/nextjs/src/client/routing/appRouterRoutingInstrumentation.ts Outdated
@chargome
chargome force-pushed the chargome/feat/nextjs-deferred-pageload-start branch from fa2f4e1 to c617c75 Compare October 8, 2026 07:55
@chargome

chargome commented Oct 8, 2026

Copy link
Copy Markdown
Member Author

bugbot run

@chargome
chargome force-pushed the chargome/feat/nextjs-deferred-pageload-start branch from c617c75 to ecb46df Compare October 8, 2026 07:59
@chargome

chargome commented Oct 8, 2026

Copy link
Copy Markdown
Member Author

bugbot run

@chargome
chargome force-pushed the chargome/feat/nextjs-deferred-pageload-start branch from ecb46df to 8ead4b1 Compare October 8, 2026 08:02
@chargome

chargome commented Oct 8, 2026

Copy link
Copy Markdown
Member Author

bugbot run

@cursor cursor Bot left a comment •

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stale Bugbot comment from a previous run.

Comment thread packages/nextjs/src/client/routing/appRouterRoutingInstrumentation.ts Outdated
Comment thread packages/nextjs/test/client/appRouterRoutingInstrumentation.test.ts
@chargome

chargome commented Oct 8, 2026

Copy link
Copy Markdown
Member Author

bugbot run

@cursor cursor Bot left a comment •

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stale Bugbot comment from a previous run.

@chargome
chargome force-pushed the chargome/feat/nextjs-deferred-pageload-start branch from d3fd241 to 1ae9f0e Compare October 9, 2026 14:25
@chargome

chargome commented Oct 9, 2026

Copy link
Copy Markdown
Member Author

bugbot run

@cursor cursor Bot left a comment •

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stale Bugbot comment from a previous run.

Comment thread packages/nextjs/src/client/browserTracingIntegration.ts
@chargome

chargome commented Oct 9, 2026

Copy link
Copy Markdown
Member Author

bugbot run

@cursor cursor Bot left a comment •

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stale Bugbot comment from a previous run.

@chargome chargome closed this Oct 12, 2026
@chargome chargome reopened this Oct 12, 2026
@chargome
chargome force-pushed the chargome/feat/nextjs-deferred-pageload-start branch from 7c3abfc to 2358cdb Compare October 12, 2026 09:23
@chargome

Copy link
Copy Markdown
Member Author

bugbot run

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ 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 2358cdb. Configure here.

@chargome
chargome force-pushed the chargome/feat/nextjs-deferred-pageload-start branch from 2358cdb to 85c7756 Compare October 12, 2026 09:55
@chargome
chargome force-pushed the chargome/feat/nextjs-deferred-pageload-start branch 4 times, most recently from 426b7cb to d62ddd3 Compare October 12, 2026 11:59
@chargome
chargome force-pushed the chargome/feat/nextjs-deferred-pageload-start branch 5 times, most recently from 669a79a to e0b0cae Compare October 12, 2026 14:27
chargome and others added 6 commits October 13, 2026 10:49
… the pageload

A document that resumes a prerendered shell gets its sentry-trace tag with
the resumed chunk, which lands in <body> after the SDK has usually already
run. A pageload created at that point can only link the server trace.

Continue whatever tag is in the document, last one wins, and when there is
no tag, no Server-Timing trace and the document is still streaming, wait
for the tag before creating the pageload span, bounded by DOMContentLoaded
and a 10 second cap. The span's start is backdated to the time origin and
its children come from the performance timeline, so it looks the same as
before, only inside the server's trace.

A client-side navigation during the wait starts the pageload with a fresh
trace first, so the navigation is filed as its redirect instead of being
the only record of the visit.

Fixes #24944
Refs JS-3863
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…a navigation

A pageload that waits for its trace meta tag can be settled by a navigation,
and a `popstate` has already changed the location by then. The browser SDK
reads the location when the span starts, so the pageload got the URL of the
navigation. Capture the document URL when the instrumentation sets up and
put it on the span and the request data instead.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…ntions constants in the test

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The Next.js routing instrumentation never flags a navigation as a redirect,
so a navigation during the wait ends the pageload like any pageload it
interrupts and starts its own trace, as it does today.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…s on

The wait exists for a document that resumes a prerendered shell, whose tag
arrives with the resumed part. Without `cacheComponents` a tag is either in
`<head>` from the start or never comes, so waiting only held the pageload
back: fetches before the tag were not attached and the span ended earlier.
The build now injects `_sentryCacheComponents`, and the client only waits
when it is set.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@chargome
chargome force-pushed the chargome/feat/nextjs-deferred-pageload-start branch from 9f0f16b to c248cc5 Compare October 13, 2026 08:49
* How long the pageload start may wait for the resumed part of a Cache Components document. The wait
* normally ends with `DOMContentLoaded`; the cap only matters for a response the server keeps open.
*/
const PAGELOAD_TRACE_META_TAG_TIMEOUT_MS = 10_000;

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this one is arbitrarily chosen and should likely never be met in real world conditions

@chargome
chargome marked this pull request as ready for review October 13, 2026 09:55
@chargome
chargome requested a review from a team as a code owner October 13, 2026 09:55
@chargome
chargome requested review from nicohrubec and s1gr1d and removed request for a team October 13, 2026 09:55
Comment on lines +49 to +51
if (instrumentPageLoad) {
client.on('startNavigationSpan', settlePendingPageloadWait);
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

M: this hook is registered in afterAllSetup which runs later than setup (this is where the other browserTracingInstrumentation calls this hook).

Can we add an E2E test that clicks a link during a slow dynamic hole? I think that would catch it.
Because if we now have a navigation that starts during the waiting time (slow dynamic hole), we would get a navigation span before the pageload span. maybeEndActiveSpan() would probably cancel that. Definitely worth testing at least.

const scope = getCurrentScope();
const { normalizedRequest } = scope.getScopeData().sdkProcessingMetadata;
if (normalizedRequest) {
scope.setSDKProcessingMetadata({ normalizedRequest: { ...normalizedRequest, url: documentUrl } });

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do we get in the documentUrl? Does this need a check on dataCollection.urlQueryParams?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll check again and guard it if needed

Comment on lines +141 to +147
span.addLink({
context: {
traceId: traceparent.traceId,
spanId: traceparent.parentSpanId,
traceFlags: traceparent.parentSampled ? TRACE_FLAG_SAMPLED : TRACE_FLAG_NONE,
},
});

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should probably add a new link type for that. Maybe server_request?

@chargome chargome Oct 13, 2026 •

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding the link is a very rare case do you think this is actually needed? What would we gain?

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Client decision rule for trace meta tags under cacheComponents

2 participants