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
vercel · Oct 19, 2024 · dab4d00 · dab4d00
1 parent d62627c
commit dab4d00
Show file tree

Hide file tree

Showing 12 changed files with 67 additions and 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
@@ -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<Args extends any[]>(
-  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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -1,3 +1,8 @@
-export default function Page({ params }) {
-  return <h1>Page {params.slug}</h1>
+export default async function Page({
+  params,
+}: {
+  params: Promise<{ slug: string }>
+}) {
+  const { slug } = await params
+  return <h1>Page {slug}</h1>
 }
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
@@ -1,3 +1,8 @@
-export default function Page({ params }) {
-  return <h1>Blog Sub Page {params.slug}</h1>
+export default async function Page({
+  params,
+}: {
+  params: Promise<{ slug: string }>
+}) {
+  const { slug } = await params
+  return <h1>Blog Sub Page {slug}</h1>
 }
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
@@ -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
@@ -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
@@ -3,18 +3,20 @@ import { withRequestMeta } from '../helpers'
 
 const helloHandler = async (
   request: NextRequest,
-  { params }: { params?: Record<string, string | string[]> }
+  { params }: { params?: Promise<Record<string, string | string[]>> }
 ): Promise<Response> => {
   const { pathname } = request.nextUrl
 
   if (typeof WebSocket === 'undefined') {
     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,
     }),
   })