From dab4d00ea1880e650be7792d9294cabe70d49235 Mon Sep 17 00:00:00 2001 From: Josh Story Date: Sat, 19 Oct 2024 16:11:33 -0700 Subject: [PATCH] sync access warnings for Request APIs should include a stack (#71519) This updates our logging to use Error objects to provide stacks to better locate where sync access of Request APIs is happening --- ...e-deduped-by-callsite-server-error-loger.ts | 4 ++-- packages/next/src/server/request/cookies.ts | 12 ++++++------ packages/next/src/server/request/draft-mode.ts | 6 +++--- packages/next/src/server/request/headers.ts | 12 ++++++------ packages/next/src/server/request/params.ts | 18 +++++++++--------- .../next/src/server/request/search-params.ts | 18 +++++++++--------- .../async-request-warnings.test.ts | 4 ++-- .../[slug]/page.tsx | 9 +++++++-- .../blog/[slug]/page.tsx | 9 +++++++-- .../app/revalidate-1/[slug]/data.json/route.ts | 8 ++++++-- .../app/static/[slug]/data.json/route.ts | 8 ++++++-- test/e2e/app-dir/app-routes/handlers/hello.ts | 6 ++++-- 12 files changed, 67 insertions(+), 47 deletions(-) diff --git a/packages/next/src/server/create-deduped-by-callsite-server-error-loger.ts b/packages/next/src/server/create-deduped-by-callsite-server-error-loger.ts index 4bd1f0a1e3b57..9d88dc854beab 100644 --- a/packages/next/src/server/create-deduped-by-callsite-server-error-loger.ts +++ b/packages/next/src/server/create-deduped-by-callsite-server-error-loger.ts @@ -1,6 +1,6 @@ import * as React from 'react' -const errorRef: { current: null | string } = { current: null } +const errorRef: { current: null | Error } = { current: null } // React.cache is currently only available in canary/experimental React channels. const cache = @@ -33,7 +33,7 @@ const flushCurrentErrorIfNew = cache( * @returns */ export function createDedupedByCallsiteServerErrorLoggerDev( - getMessage: (...args: Args) => string + getMessage: (...args: Args) => Error ) { return function logDedupedError(...args: Args) { const message = getMessage(...args) diff --git a/packages/next/src/server/request/cookies.ts b/packages/next/src/server/request/cookies.ts index 1d367b53a466f..a503e124617ce 100644 --- a/packages/next/src/server/request/cookies.ts +++ b/packages/next/src/server/request/cookies.ts @@ -538,10 +538,10 @@ const warnForSyncIteration = process.env : createDedupedByCallsiteServerErrorLoggerDev( function getSyncIterationMessage(route?: string) { const prefix = route ? ` In route ${route} ` : '' - return ( + return new Error( `${prefix}cookies were iterated over. ` + - `\`cookies()\` should be awaited before using its value. ` + - `Learn more: https://nextjs.org/docs/messages/sync-dynamic-apis` + `\`cookies()\` should be awaited before using its value. ` + + `Learn more: https://nextjs.org/docs/messages/sync-dynamic-apis` ) } ) @@ -553,10 +553,10 @@ const warnForSyncAccess = process.env.__NEXT_DISABLE_SYNC_DYNAMIC_API_WARNINGS expression: string ) { const prefix = route ? ` In route ${route} a ` : 'A ' - return ( + return new Error( `${prefix}cookie property was accessed directly with \`${expression}\`. ` + - `\`cookies()\` should be awaited before using its value. ` + - `Learn more: https://nextjs.org/docs/messages/sync-dynamic-apis` + `\`cookies()\` should be awaited before using its value. ` + + `Learn more: https://nextjs.org/docs/messages/sync-dynamic-apis` ) }) diff --git a/packages/next/src/server/request/draft-mode.ts b/packages/next/src/server/request/draft-mode.ts index d8f4d607c0b69..c455db9503d4b 100644 --- a/packages/next/src/server/request/draft-mode.ts +++ b/packages/next/src/server/request/draft-mode.ts @@ -199,10 +199,10 @@ const warnForSyncAccess = process.env.__NEXT_DISABLE_SYNC_DYNAMIC_API_WARNINGS expression: string ) { const prefix = route ? ` In route ${route} a ` : 'A ' - return ( + return new Error( `${prefix}\`draftMode()\` property was accessed directly with \`${expression}\`. ` + - `\`draftMode()\` should be awaited before using its value. ` + - `Learn more: https://nextjs.org/docs/messages/draft-mode-sync-access` + `\`draftMode()\` should be awaited before using its value. ` + + `Learn more: https://nextjs.org/docs/messages/draft-mode-sync-access` ) }) diff --git a/packages/next/src/server/request/headers.ts b/packages/next/src/server/request/headers.ts index 34228e4d83c86..332fea791242b 100644 --- a/packages/next/src/server/request/headers.ts +++ b/packages/next/src/server/request/headers.ts @@ -460,10 +460,10 @@ const warnForSyncIteration = process.env : createDedupedByCallsiteServerErrorLoggerDev( function getSyncIterationMessage(route?: string) { const prefix = route ? ` In route ${route} ` : '' - return ( + return new Error( `${prefix}headers were iterated over. ` + - `\`headers()\` should be awaited before using its value. ` + - `Learn more: https://nextjs.org/docs/messages/sync-dynamic-apis` + `\`headers()\` should be awaited before using its value. ` + + `Learn more: https://nextjs.org/docs/messages/sync-dynamic-apis` ) } ) @@ -475,10 +475,10 @@ const warnForSyncAccess = process.env.__NEXT_DISABLE_SYNC_DYNAMIC_API_WARNINGS expression: string ) { const prefix = route ? ` In route ${route} a ` : 'A ' - return ( + return new Error( `${prefix}header property was accessed directly with \`${expression}\`. ` + - `\`headers()\` should be awaited before using its value. ` + - `Learn more: https://nextjs.org/docs/messages/sync-dynamic-apis` + `\`headers()\` should be awaited before using its value. ` + + `Learn more: https://nextjs.org/docs/messages/sync-dynamic-apis` ) }) diff --git a/packages/next/src/server/request/params.ts b/packages/next/src/server/request/params.ts index 101b55ebb0652..9eb87f0ca761f 100644 --- a/packages/next/src/server/request/params.ts +++ b/packages/next/src/server/request/params.ts @@ -420,10 +420,10 @@ const warnForSyncAccess = process.env.__NEXT_DISABLE_SYNC_DYNAMIC_API_WARNINGS expression: string ) { const prefix = route ? ` In route ${route} a ` : 'A ' - return ( + return new Error( `${prefix}param property was accessed directly with ${expression}. ` + - `\`params\` should be awaited before accessing its properties. ` + - `Learn more: https://nextjs.org/docs/messages/sync-dynamic-apis` + `\`params\` should be awaited before accessing its properties. ` + + `Learn more: https://nextjs.org/docs/messages/sync-dynamic-apis` ) }) @@ -437,16 +437,16 @@ const warnForEnumeration = process.env.__NEXT_DISABLE_SYNC_DYNAMIC_API_WARNINGS if (missingProperties.length) { const describedMissingProperties = describeListOfPropertyNames(missingProperties) - return ( + return new Error( `${prefix}params are being enumerated incompletely missing these properties: ${describedMissingProperties}. ` + - `\`params\` should be awaited before accessing its properties. ` + - `Learn more: https://nextjs.org/docs/messages/sync-dynamic-apis` + `\`params\` should be awaited before accessing its properties. ` + + `Learn more: https://nextjs.org/docs/messages/sync-dynamic-apis` ) } else { - return ( + return new Error( `${prefix}params are being enumerated. ` + - `\`params\` should be awaited before accessing its properties. ` + - `Learn more: https://nextjs.org/docs/messages/sync-dynamic-apis` + `\`params\` should be awaited before accessing its properties. ` + + `Learn more: https://nextjs.org/docs/messages/sync-dynamic-apis` ) } }) diff --git a/packages/next/src/server/request/search-params.ts b/packages/next/src/server/request/search-params.ts index cb90773db0c64..4b1a458cfb89f 100644 --- a/packages/next/src/server/request/search-params.ts +++ b/packages/next/src/server/request/search-params.ts @@ -680,10 +680,10 @@ const warnForSyncAccess = process.env.__NEXT_DISABLE_SYNC_DYNAMIC_API_WARNINGS expression: string ) { const prefix = route ? ` In route ${route} a ` : 'A ' - return ( + return new Error( `${prefix}searchParam property was accessed directly with ${expression}. ` + - `\`searchParams\` should be awaited before accessing properties. ` + - `Learn more: https://nextjs.org/docs/messages/sync-dynamic-apis` + `\`searchParams\` should be awaited before accessing properties. ` + + `Learn more: https://nextjs.org/docs/messages/sync-dynamic-apis` ) }) @@ -697,16 +697,16 @@ const warnForEnumeration = process.env.__NEXT_DISABLE_SYNC_DYNAMIC_API_WARNINGS if (missingProperties.length) { const describedMissingProperties = describeListOfPropertyNames(missingProperties) - return ( + return new Error( `${prefix}searchParams are being enumerated incompletely missing these properties: ${describedMissingProperties}. ` + - `\`searchParams\` should be awaited before accessing its properties. ` + - `Learn more: https://nextjs.org/docs/messages/sync-dynamic-apis` + `\`searchParams\` should be awaited before accessing its properties. ` + + `Learn more: https://nextjs.org/docs/messages/sync-dynamic-apis` ) } else { - return ( + return new Error( `${prefix}searchParams are being enumerated. ` + - `\`searchParams\` should be awaited before accessing its properties. ` + - `Learn more: https://nextjs.org/docs/messages/sync-dynamic-apis` + `\`searchParams\` should be awaited before accessing its properties. ` + + `Learn more: https://nextjs.org/docs/messages/sync-dynamic-apis` ) } }) diff --git a/test/development/app-dir/async-request-warnings/async-request-warnings.test.ts b/test/development/app-dir/async-request-warnings/async-request-warnings.test.ts index 31c16aa67bd59..9732d07b1e7a9 100644 --- a/test/development/app-dir/async-request-warnings/async-request-warnings.test.ts +++ b/test/development/app-dir/async-request-warnings/async-request-warnings.test.ts @@ -10,8 +10,8 @@ describe('dynamic-requests warnings', () => { const browser = await next.browser('/request/cookies') - const browserLogsserLogs = await browser.log() - const browserConsoleErrors = browserLogsserLogs + const browserLogs = await browser.log() + const browserConsoleErrors = browserLogs .filter((log) => log.source === 'error') .map((log) => log.message) const terminalOutput = next.cliOutput.slice(nextDevBootstrapOutputIndex) diff --git a/test/e2e/app-dir/app-routes/app/conflicting-dynamic-static-segments/[slug]/page.tsx b/test/e2e/app-dir/app-routes/app/conflicting-dynamic-static-segments/[slug]/page.tsx index 816bbea9cae28..f0eb53d022028 100644 --- a/test/e2e/app-dir/app-routes/app/conflicting-dynamic-static-segments/[slug]/page.tsx +++ b/test/e2e/app-dir/app-routes/app/conflicting-dynamic-static-segments/[slug]/page.tsx @@ -1,3 +1,8 @@ -export default function Page({ params }) { - return

Page {params.slug}

+export default async function Page({ + params, +}: { + params: Promise<{ slug: string }> +}) { + const { slug } = await params + return

Page {slug}

} diff --git a/test/e2e/app-dir/app-routes/app/conflicting-dynamic-static-segments/blog/[slug]/page.tsx b/test/e2e/app-dir/app-routes/app/conflicting-dynamic-static-segments/blog/[slug]/page.tsx index 72b7591a8ad4a..ced219f0c6928 100644 --- a/test/e2e/app-dir/app-routes/app/conflicting-dynamic-static-segments/blog/[slug]/page.tsx +++ b/test/e2e/app-dir/app-routes/app/conflicting-dynamic-static-segments/blog/[slug]/page.tsx @@ -1,3 +1,8 @@ -export default function Page({ params }) { - return

Blog Sub Page {params.slug}

+export default async function Page({ + params, +}: { + params: Promise<{ slug: string }> +}) { + const { slug } = await params + return

Blog Sub Page {slug}

} diff --git a/test/e2e/app-dir/app-routes/app/revalidate-1/[slug]/data.json/route.ts b/test/e2e/app-dir/app-routes/app/revalidate-1/[slug]/data.json/route.ts index 020c2d83adb4e..975ef0f51e050 100644 --- a/test/e2e/app-dir/app-routes/app/revalidate-1/[slug]/data.json/route.ts +++ b/test/e2e/app-dir/app-routes/app/revalidate-1/[slug]/data.json/route.ts @@ -7,6 +7,10 @@ export function generateStaticParams() { return [{ slug: 'first' }, { slug: 'second' }] } -export const GET = (req: NextRequest, { params }) => { - return NextResponse.json({ params, now: Date.now() }) +export const GET = async ( + req: NextRequest, + { params }: { params: Promise<{ slug: string }> } +) => { + const resolvedParams = await params + return NextResponse.json({ params: resolvedParams, now: Date.now() }) } diff --git a/test/e2e/app-dir/app-routes/app/static/[slug]/data.json/route.ts b/test/e2e/app-dir/app-routes/app/static/[slug]/data.json/route.ts index 5733ecc643a40..570a309dcf13a 100644 --- a/test/e2e/app-dir/app-routes/app/static/[slug]/data.json/route.ts +++ b/test/e2e/app-dir/app-routes/app/static/[slug]/data.json/route.ts @@ -5,6 +5,10 @@ export function generateStaticParams() { return [{ slug: 'first' }, { slug: 'second' }] } -export const GET = (req: NextRequest, { params }) => { - return NextResponse.json({ params, now: Date.now() }) +export const GET = async ( + req: NextRequest, + { params }: { params: Promise<{ slug: string }> } +) => { + const resolvedParams = await params + return NextResponse.json({ params: resolvedParams, now: Date.now() }) } diff --git a/test/e2e/app-dir/app-routes/handlers/hello.ts b/test/e2e/app-dir/app-routes/handlers/hello.ts index d51b65d11c432..6084f14eaf8ad 100644 --- a/test/e2e/app-dir/app-routes/handlers/hello.ts +++ b/test/e2e/app-dir/app-routes/handlers/hello.ts @@ -3,7 +3,7 @@ import { withRequestMeta } from '../helpers' const helloHandler = async ( request: NextRequest, - { params }: { params?: Record } + { params }: { params?: Promise> } ): Promise => { const { pathname } = request.nextUrl @@ -11,10 +11,12 @@ const helloHandler = async ( throw new Error('missing WebSocket constructor!!') } + const resolvedParams = params ? await params : null + return new Response('hello, world', { headers: withRequestMeta({ method: request.method, - params: params ?? null, + params: resolvedParams, pathname, }), })