-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
fix(nextjs): Drop meta trace tags if rendered page is ISR #18192
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 9 commits
88d8541
9a8b486
81731ad
ddef94c
7d1bdac
0d0706a
4303908
aaf244f
f46d817
c72b29a
ed76ec8
ec73021
a49edb2
0854965
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,17 @@ | ||
| export const revalidate = 60; // ISR: revalidate every 60 seconds | ||
| export const dynamicParams = true; // Allow dynamic params beyond generateStaticParams | ||
|
|
||
| export async function generateStaticParams(): Promise<Array<{ product: string }>> { | ||
| return [{ product: 'laptop' }, { product: 'phone' }, { product: 'tablet' }]; | ||
| } | ||
|
|
||
| export default async function ISRProductPage({ params }: { params: Promise<{ product: string }> }) { | ||
| const { product } = await params; | ||
|
|
||
| return ( | ||
| <div> | ||
| <h1>ISR Product: {product}</h1> | ||
| <div id="isr-product-id">{product}</div> | ||
| </div> | ||
| ); | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,15 @@ | ||
| export const revalidate = 60; // ISR: revalidate every 60 seconds | ||
| export const dynamicParams = true; | ||
|
|
||
| export async function generateStaticParams(): Promise<never[]> { | ||
| return []; | ||
| } | ||
|
|
||
| export default function ISRStaticPage() { | ||
| return ( | ||
| <div> | ||
| <h1>ISR Static Page</h1> | ||
| <div id="isr-static-marker">static-isr</div> | ||
| </div> | ||
| ); | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,11 @@ | ||
| // No generateStaticParams - this is NOT an ISR page | ||
| export default async function NonISRPage({ params }: { params: Promise<{ item: string }> }) { | ||
| const { item } = await params; | ||
|
|
||
| return ( | ||
| <div> | ||
| <h1>Non-ISR Dynamic Page: {item}</h1> | ||
| <div id="non-isr-item-id">{item}</div> | ||
| </div> | ||
| ); | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,94 @@ | ||
| import { expect, test } from '@playwright/test'; | ||
| import { waitForTransaction } from '@sentry-internal/test-utils'; | ||
|
|
||
| test('should remove sentry-trace and baggage meta tags on ISR dynamic route page load', async ({ page }) => { | ||
| // Navigate to ISR page | ||
| await page.goto('/isr-test/laptop'); | ||
|
|
||
| // Wait for page to be fully loaded | ||
| await expect(page.locator('#isr-product-id')).toHaveText('laptop'); | ||
|
|
||
| // Check that sentry-trace and baggage meta tags are removed for ISR pages | ||
| await expect(page.locator('meta[name="sentry-trace"]')).toHaveCount(0); | ||
| await expect(page.locator('meta[name="baggage"]')).toHaveCount(0); | ||
| }); | ||
|
|
||
| test('should remove sentry-trace and baggage meta tags on ISR static route', async ({ page }) => { | ||
| // Navigate to ISR static page | ||
| await page.goto('/isr-test/static'); | ||
|
|
||
| // Wait for page to be fully loaded | ||
| await expect(page.locator('#isr-static-marker')).toHaveText('static-isr'); | ||
|
|
||
| // Check that sentry-trace and baggage meta tags are removed for ISR pages | ||
| await expect(page.locator('meta[name="sentry-trace"]')).toHaveCount(0); | ||
| await expect(page.locator('meta[name="baggage"]')).toHaveCount(0); | ||
| }); | ||
|
|
||
| test('should remove meta tags for different ISR dynamic route values', async ({ page }) => { | ||
| // Test with 'phone' (one of the pre-generated static params) | ||
| await page.goto('/isr-test/phone'); | ||
| await expect(page.locator('#isr-product-id')).toHaveText('phone'); | ||
|
|
||
| await expect(page.locator('meta[name="sentry-trace"]')).toHaveCount(0); | ||
| await expect(page.locator('meta[name="baggage"]')).toHaveCount(0); | ||
|
|
||
| // Test with 'tablet' | ||
| await page.goto('/isr-test/tablet'); | ||
| await expect(page.locator('#isr-product-id')).toHaveText('tablet'); | ||
|
|
||
| await expect(page.locator('meta[name="sentry-trace"]')).toHaveCount(0); | ||
| await expect(page.locator('meta[name="baggage"]')).toHaveCount(0); | ||
| }); | ||
|
|
||
| test('should create unique transactions for ISR pages on each visit', async ({ page }) => { | ||
| const traceIds: string[] = []; | ||
|
|
||
| // Load the same ISR page 5 times to ensure cached HTML meta tags are consistently removed | ||
| for (let i = 0; i < 5; i++) { | ||
| const transactionPromise = waitForTransaction('nextjs-15', async transactionEvent => { | ||
| return !!( | ||
| transactionEvent.transaction === '/isr-test/:product' && transactionEvent.contexts?.trace?.op === 'pageload' | ||
| ); | ||
| }); | ||
|
|
||
| if (i === 0) { | ||
| await page.goto('/isr-test/laptop'); | ||
| } else { | ||
| await page.reload(); | ||
| } | ||
|
|
||
| const transaction = await transactionPromise; | ||
| const traceId = transaction.contexts?.trace?.trace_id; | ||
|
|
||
| expect(traceId).toBeDefined(); | ||
| expect(traceId).toMatch(/[a-f0-9]{32}/); | ||
| traceIds.push(traceId!); | ||
| } | ||
|
|
||
| // Verify all 5 page loads have unique trace IDs (no reuse of cached/stale meta tags) | ||
| const uniqueTraceIds = new Set(traceIds); | ||
| expect(uniqueTraceIds.size).toBe(5); | ||
| }); | ||
|
|
||
| test('ISR route should be identified correctly in the route manifest', async ({ page }) => { | ||
| const transactionPromise = waitForTransaction('nextjs-15', async transactionEvent => { | ||
| return transactionEvent.transaction === '/isr-test/:product' && transactionEvent.contexts?.trace?.op === 'pageload'; | ||
| }); | ||
|
|
||
| await page.goto('/isr-test/laptop'); | ||
| const transaction = await transactionPromise; | ||
|
|
||
| // Verify the transaction is properly parameterized | ||
| expect(transaction).toMatchObject({ | ||
| transaction: '/isr-test/:product', | ||
| transaction_info: { source: 'route' }, | ||
| contexts: { | ||
| trace: { | ||
| data: { | ||
| 'sentry.source': 'route', | ||
| }, | ||
| }, | ||
| }, | ||
| }); | ||
| }); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,17 @@ | ||
| export const revalidate = 60; // ISR: revalidate every 60 seconds | ||
| export const dynamicParams = true; // Allow dynamic params beyond generateStaticParams | ||
|
|
||
| export async function generateStaticParams(): Promise<Array<{ product: string }>> { | ||
| return [{ product: 'laptop' }, { product: 'phone' }, { product: 'tablet' }]; | ||
| } | ||
|
|
||
| export default async function ISRProductPage({ params }: { params: Promise<{ product: string }> }) { | ||
| const { product } = await params; | ||
|
|
||
| return ( | ||
| <div> | ||
| <h1>ISR Product: {product}</h1> | ||
| <div id="isr-product-id">{product}</div> | ||
| </div> | ||
| ); | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,15 @@ | ||
| export const revalidate = 60; // ISR: revalidate every 60 seconds | ||
| export const dynamicParams = true; | ||
|
|
||
| export async function generateStaticParams(): Promise<never[]> { | ||
| return []; | ||
| } | ||
|
|
||
| export default function ISRStaticPage() { | ||
| return ( | ||
| <div> | ||
| <h1>ISR Static Page</h1> | ||
| <div id="isr-static-marker">static-isr</div> | ||
| </div> | ||
| ); | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,11 @@ | ||
| // No generateStaticParams - this is NOT an ISR page | ||
| export default async function NonISRPage({ params }: { params: Promise<{ item: string }> }) { | ||
| const { item } = await params; | ||
|
|
||
| return ( | ||
| <div> | ||
| <h1>Non-ISR Dynamic Page: {item}</h1> | ||
| <div id="non-isr-item-id">{item}</div> | ||
| </div> | ||
| ); | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,94 @@ | ||
| import { expect, test } from '@playwright/test'; | ||
| import { waitForTransaction } from '@sentry-internal/test-utils'; | ||
|
|
||
| test('should remove sentry-trace and baggage meta tags on ISR dynamic route page load', async ({ page }) => { | ||
| // Navigate to ISR page | ||
| await page.goto('/isr-test/laptop'); | ||
|
|
||
| // Wait for page to be fully loaded | ||
| await expect(page.locator('#isr-product-id')).toHaveText('laptop'); | ||
|
|
||
| // Check that sentry-trace and baggage meta tags are removed for ISR pages | ||
| await expect(page.locator('meta[name="sentry-trace"]')).toHaveCount(0); | ||
| await expect(page.locator('meta[name="baggage"]')).toHaveCount(0); | ||
| }); | ||
|
|
||
| test('should remove sentry-trace and baggage meta tags on ISR static route', async ({ page }) => { | ||
| // Navigate to ISR static page | ||
| await page.goto('/isr-test/static'); | ||
|
|
||
| // Wait for page to be fully loaded | ||
| await expect(page.locator('#isr-static-marker')).toHaveText('static-isr'); | ||
|
|
||
| // Check that sentry-trace and baggage meta tags are removed for ISR pages | ||
| await expect(page.locator('meta[name="sentry-trace"]')).toHaveCount(0); | ||
| await expect(page.locator('meta[name="baggage"]')).toHaveCount(0); | ||
| }); | ||
|
|
||
| test('should remove meta tags for different ISR dynamic route values', async ({ page }) => { | ||
| // Test with 'phone' (one of the pre-generated static params) | ||
| await page.goto('/isr-test/phone'); | ||
| await expect(page.locator('#isr-product-id')).toHaveText('phone'); | ||
|
|
||
| await expect(page.locator('meta[name="sentry-trace"]')).toHaveCount(0); | ||
| await expect(page.locator('meta[name="baggage"]')).toHaveCount(0); | ||
|
|
||
| // Test with 'tablet' | ||
| await page.goto('/isr-test/tablet'); | ||
| await expect(page.locator('#isr-product-id')).toHaveText('tablet'); | ||
|
|
||
| await expect(page.locator('meta[name="sentry-trace"]')).toHaveCount(0); | ||
| await expect(page.locator('meta[name="baggage"]')).toHaveCount(0); | ||
| }); | ||
|
|
||
| test('should create unique transactions for ISR pages on each visit', async ({ page }) => { | ||
| const traceIds: string[] = []; | ||
|
|
||
| // Load the same ISR page 5 times to ensure cached HTML meta tags are consistently removed | ||
| for (let i = 0; i < 5; i++) { | ||
| const transactionPromise = waitForTransaction('nextjs-16', async transactionEvent => { | ||
| return !!( | ||
| transactionEvent.transaction === '/isr-test/:product' && transactionEvent.contexts?.trace?.op === 'pageload' | ||
| ); | ||
| }); | ||
|
|
||
| if (i === 0) { | ||
| await page.goto('/isr-test/laptop'); | ||
| } else { | ||
| await page.reload(); | ||
| } | ||
|
|
||
| const transaction = await transactionPromise; | ||
| const traceId = transaction.contexts?.trace?.trace_id; | ||
|
|
||
| expect(traceId).toBeDefined(); | ||
| expect(traceId).toMatch(/[a-f0-9]{32}/); | ||
| traceIds.push(traceId!); | ||
| } | ||
|
|
||
| // Verify all 5 page loads have unique trace IDs (no reuse of cached/stale meta tags) | ||
| const uniqueTraceIds = new Set(traceIds); | ||
| expect(uniqueTraceIds.size).toBe(5); | ||
| }); | ||
|
|
||
| test('ISR route should be identified correctly in the route manifest', async ({ page }) => { | ||
| const transactionPromise = waitForTransaction('nextjs-16', async transactionEvent => { | ||
| return transactionEvent.transaction === '/isr-test/:product' && transactionEvent.contexts?.trace?.op === 'pageload'; | ||
| }); | ||
|
|
||
| await page.goto('/isr-test/laptop'); | ||
| const transaction = await transactionPromise; | ||
|
|
||
| // Verify the transaction is properly parameterized | ||
| expect(transaction).toMatchObject({ | ||
| transaction: '/isr-test/:product', | ||
| transaction_info: { source: 'route' }, | ||
| contexts: { | ||
| trace: { | ||
| data: { | ||
| 'sentry.source': 'route', | ||
| }, | ||
| }, | ||
| }, | ||
| }); | ||
| }); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,63 @@ | ||
| import { WINDOW } from '@sentry/react'; | ||
| import type { RouteManifest } from '../../config/manifest/types'; | ||
| import { maybeParameterizeRoute } from './parameterization'; | ||
|
|
||
| const globalWithInjectedValues = WINDOW as typeof WINDOW & { | ||
| _sentryRouteManifest: string | RouteManifest; | ||
| }; | ||
cursor[bot] marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
|
|
||
| /** | ||
| * Check if the current page is an ISR/SSG route by checking the route manifest. | ||
| */ | ||
| function isIsrSsgRoute(pathname: string): boolean { | ||
|
||
| const manifestData = globalWithInjectedValues._sentryRouteManifest; | ||
| if (!manifestData) { | ||
| return false; | ||
| } | ||
|
|
||
| let manifest: RouteManifest; | ||
| if (typeof manifestData === 'string') { | ||
| try { | ||
| manifest = JSON.parse(manifestData); | ||
| } catch { | ||
| return false; | ||
| } | ||
| } else { | ||
| manifest = manifestData; | ||
| } | ||
|
|
||
| if (!manifest.isrRoutes || !Array.isArray(manifest.isrRoutes) || manifest.isrRoutes.length === 0) { | ||
| return false; | ||
| } | ||
|
|
||
| const parameterizedPath = maybeParameterizeRoute(pathname); | ||
| const pathToCheck = parameterizedPath || pathname; | ||
|
|
||
| return manifest.isrRoutes.includes(pathToCheck); | ||
| } | ||
|
|
||
| /** | ||
| * Remove sentry-trace and baggage meta tags from the DOM if this is an ISR/SSG page. | ||
| * This prevents the browser tracing integration from using stale/cached trace IDs. | ||
| */ | ||
| export function removeIsrSsgTraceMetaTags(): void { | ||
| if (!WINDOW.document || !isIsrSsgRoute(WINDOW.location.pathname)) { | ||
| return; | ||
| } | ||
|
|
||
| // Helper function to remove a meta tag | ||
| function removeMetaTag(metaName: string): void { | ||
| try { | ||
| const meta = WINDOW.document.querySelector(`meta[name="${metaName}"]`); | ||
| if (meta) { | ||
| meta.remove(); | ||
| } | ||
| } catch { | ||
| // ignore errors when removing the meta tag | ||
| } | ||
| } | ||
|
|
||
| // Remove the meta tags so browserTracingIntegration won't pick them up | ||
| removeMetaTag('sentry-trace'); | ||
| removeMetaTag('baggage'); | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this should be gated behind the
if (typeof __SENTRY_TRACING__ === 'undefined' || __SENTRY_TRACING__) {flag like adding thebrowserTracingIntegrationis.