revert ppr logging changes (#57486)

need to investigate this better

---------

Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>

errors/ppr-postpone-error.mdx

@@ -1,11 +0,0 @@
----
-title: Understanding the postpone error triggered during static generation
----
-
-## Why This Error Occurred
-
-When Partial Prerendering (PPR) is enabled, using APIs that opt into Dynamic Rendering like `cookies`, `headers`, or `fetch` (such as with `cache: 'no-store'` or `revalidate: 0`) will cause Next.js to throw a special error to know which part of the page cannot be statically generated. If you catch this error, Next.js will not be able to recognize that dynamic content was used and will only be able to statically render the page, opting you out of partial prerendering and lead to potentially undefined behavior.
-
-## Possible Ways to Fix It
-
-To resolve this issue, ensure that you are not wrapping Next.js APIs that opt into dynamic rendering in a `try/catch` block.

packages/next/src/client/components/maybe-postpone.ts

@@ -18,8 +18,5 @@ export function maybePostpone(
   const React = require('react') as typeof import('react')
   if (typeof React.unstable_postpone !== 'function') return
 
-  // Keep track of if the postpone API has been called.
-  staticGenerationStore.postponeWasTriggered = true
-
   React.unstable_postpone(reason)
 }

packages/next/src/client/components/static-generation-async-storage.external.ts

@@ -27,7 +27,6 @@ export interface StaticGenerationStore {
   forceStatic?: boolean
   dynamicShouldError?: boolean
   pendingRevalidates?: Promise<any>[]
-  postponeWasTriggered?: boolean
 
   dynamicUsageDescription?: string
   dynamicUsageStack?: string

packages/next/src/export/helpers/is-dynamic-usage-error.ts

@@ -7,4 +7,6 @@ export const isDynamicUsageError = (err: any) =>
   err.digest === DYNAMIC_ERROR_CODE ||
   isNotFoundError(err) ||
   err.digest === NEXT_DYNAMIC_NO_SSR_CODE ||
-  isRedirectError(err)
+  isRedirectError(err) ||
+  // TODO: (wyattjoh) remove once we bump react
+  err.$$typeof === Symbol.for('react.postpone')

packages/next/src/export/routes/app-page.ts

@@ -13,6 +13,7 @@ import {
   NEXT_URL,
   NEXT_ROUTER_PREFETCH,
 } from '../../client/components/app-router-headers'
+import { isDynamicUsageError } from '../helpers/is-dynamic-usage-error'
 import { NEXT_CACHE_TAGS_HEADER } from '../../lib/constants'
 import { hasNextSupport } from '../../telemetry/ci-info'
 import { lazyRenderAppPage } from '../../server/future/route-modules/app-page/module.render'
@@ -79,35 +80,8 @@ export async function exportAppPage(
     pathname = '/404'
   }
 
-  if (isAppPrefetch) {
-    await generatePrefetchRsc(
-      req,
-      path,
-      res,
-      pathname,
-      htmlFilepath,
-      renderOpts,
-      fileWriter
-    )
-
-    return { revalidate: 0 }
-  }
-
-  const result = await lazyRenderAppPage(req, res, pathname, query, renderOpts)
-  const html = result.toUnchunkedString()
-  const { metadata } = result
-  const flightData = metadata.pageData
-  const revalidate = metadata.revalidate ?? false
-  const postponed = metadata.postponed
-
-  if (revalidate === 0) {
-    if (isDynamicError) {
-      throw new Error(
-        `Page with dynamic = "error" encountered dynamic data method on ${path}.`
-      )
-    }
-
-    if (!(renderOpts as any).store.staticPrefetchBailout) {
+  try {
+    if (isAppPrefetch) {
       await generatePrefetchRsc(
         req,
         path,
@@ -117,60 +91,106 @@ export async function exportAppPage(
         renderOpts,
         fileWriter
       )
+
+      return { revalidate: 0 }
     }
 
-    const { staticBailoutInfo = {} } = metadata
+    const result = await lazyRenderAppPage(
+      req,
+      res,
+      pathname,
+      query,
+      renderOpts
+    )
+    const html = result.toUnchunkedString()
+    const { metadata } = result
+    const flightData = metadata.pageData
+    const revalidate = metadata.revalidate ?? false
+    const postponed = metadata.postponed
 
-    if (revalidate === 0 && debugOutput && staticBailoutInfo?.description) {
-      const err = new Error(
-        `Static generation failed due to dynamic usage on ${path}, reason: ${staticBailoutInfo.description}`
-      )
-
-      // Update the stack if it was provided via the bailout info.
-      const { stack } = staticBailoutInfo
-      if (stack) {
-        err.stack = err.message + stack.substring(stack.indexOf('\n'))
+    if (revalidate === 0) {
+      if (isDynamicError) {
+        throw new Error(
+          `Page with dynamic = "error" encountered dynamic data method on ${path}.`
+        )
       }
 
-      console.warn(err)
+      if (!(renderOpts as any).store.staticPrefetchBailout) {
+        await generatePrefetchRsc(
+          req,
+          path,
+          res,
+          pathname,
+          htmlFilepath,
+          renderOpts,
+          fileWriter
+        )
+      }
+
+      const { staticBailoutInfo = {} } = metadata
+
+      if (revalidate === 0 && debugOutput && staticBailoutInfo?.description) {
+        const err = new Error(
+          `Static generation failed due to dynamic usage on ${path}, reason: ${staticBailoutInfo.description}`
+        )
+
+        // Update the stack if it was provided via the bailout info.
+        const { stack } = staticBailoutInfo
+        if (stack) {
+          err.stack = err.message + stack.substring(stack.indexOf('\n'))
+        }
+
+        console.warn(err)
+      }
+
+      return { revalidate: 0 }
+    }
+
+    let headers: OutgoingHttpHeaders | undefined
+    if (metadata.fetchTags) {
+      headers = { [NEXT_CACHE_TAGS_HEADER]: metadata.fetchTags }
+    }
+
+    // Writing static HTML to a file.
+    await fileWriter(
+      ExportedAppPageFiles.HTML,
+      htmlFilepath,
+      html ?? '',
+      'utf8'
+    )
+
+    // Writing the request metadata to a file.
+    const meta: RouteMetadata = {
+      status: undefined,
+      headers,
+      postponed,
+    }
+
+    await fileWriter(
+      ExportedAppPageFiles.META,
+      htmlFilepath.replace(/\.html$/, '.meta'),
+      JSON.stringify(meta, null, 2)
+    )
+
+    // Writing the RSC payload to a file.
+    await fileWriter(
+      ExportedAppPageFiles.FLIGHT,
+      htmlFilepath.replace(/\.html$/, '.rsc'),
+      flightData
+    )
+
+    return {
+      // Only include the metadata if the environment has next support.
+      metadata: hasNextSupport ? meta : undefined,
+      hasEmptyPrelude: Boolean(postponed) && html === '',
+      hasPostponed: Boolean(postponed),
+      revalidate,
+    }
+  } catch (err: any) {
+    if (!isDynamicUsageError(err)) {
+      throw err
     }
 
     return { revalidate: 0 }
   }
-
-  let headers: OutgoingHttpHeaders | undefined
-  if (metadata.fetchTags) {
-    headers = { [NEXT_CACHE_TAGS_HEADER]: metadata.fetchTags }
-  }
-
-  // Writing static HTML to a file.
-  await fileWriter(ExportedAppPageFiles.HTML, htmlFilepath, html ?? '', 'utf8')
-
-  // Writing the request metadata to a file.
-  const meta: RouteMetadata = {
-    status: undefined,
-    headers,
-    postponed,
-  }
-
-  await fileWriter(
-    ExportedAppPageFiles.META,
-    htmlFilepath.replace(/\.html$/, '.meta'),
-    JSON.stringify(meta, null, 2)
-  )
-
-  // Writing the RSC payload to a file.
-  await fileWriter(
-    ExportedAppPageFiles.FLIGHT,
-    htmlFilepath.replace(/\.html$/, '.rsc'),
-    flightData
-  )
-
-  return {
-    // Only include the metadata if the environment has next support.
-    metadata: hasNextSupport ? meta : undefined,
-    hasEmptyPrelude: Boolean(postponed) && html === '',
-    hasPostponed: Boolean(postponed),
-    revalidate,
-  }
 }

packages/next/src/export/routes/app-route.ts

@@ -16,7 +16,7 @@ import type {
   MockedRequest,
   MockedResponse,
 } from '../../server/lib/mock-request'
-import { isDynamicUsageError } from '../../server/app-render/is-dynamic-usage-error'
+import { isDynamicUsageError } from '../helpers/is-dynamic-usage-error'
 import { SERVER_DIRECTORY } from '../../shared/lib/constants'
 import { hasNextSupport } from '../../telemetry/ci-info'
 

packages/next/src/server/app-render/app-render.tsx

@@ -72,8 +72,6 @@ import { createComponentTree } from './create-component-tree'
 import { getAssetQueryString } from './get-asset-query-string'
 import { setReferenceManifestsSingleton } from './action-encryption-utils'
 import { createStaticRenderer } from './static/static-renderer'
-import { isPostpone } from '../lib/router-utils/is-postpone'
-import { isDynamicUsageError } from './is-dynamic-usage-error'
 
 export type GetDynamicParamFromSegment = (
   // [slug] / [[slug]] / [...slug]
@@ -465,16 +463,13 @@ async function renderToHTMLOrFlightImpl(
 
   const capturedErrors: Error[] = []
   const allCapturedErrors: Error[] = []
-  const postponeErrors: Error[] = []
   const isNextExport = !!renderOpts.nextExport
   const serverComponentsErrorHandler = createErrorHandler({
     _source: 'serverComponentsRenderer',
     dev,
     isNextExport,
-    postponeErrors: renderOpts.ppr ? postponeErrors : undefined,
     errorLogger: appDirDevErrorLogger,
     capturedErrors,
-    skipLogging: renderOpts.ppr,
   })
   const flightDataRendererErrorHandler = createErrorHandler({
     _source: 'flightDataRenderer',
@@ -482,7 +477,6 @@ async function renderToHTMLOrFlightImpl(
     isNextExport,
     errorLogger: appDirDevErrorLogger,
     capturedErrors,
-    skipLogging: renderOpts.ppr,
   })
   const htmlRendererErrorHandler = createErrorHandler({
     _source: 'htmlRenderer',
@@ -491,7 +485,6 @@ async function renderToHTMLOrFlightImpl(
     errorLogger: appDirDevErrorLogger,
     capturedErrors,
     allCapturedErrors,
-    skipLogging: renderOpts.ppr,
   })
 
   patchFetch(ComponentMod)
@@ -785,6 +778,15 @@ async function renderToHTMLOrFlightImpl(
           throw err
         }
 
+        // If there was a postponed error that escaped, it means that there was
+        // a postpone called without a wrapped suspense component.
+        if (err.$$typeof === Symbol.for('react.postpone')) {
+          // Ensure that we force the revalidation time to zero.
+          staticGenerationStore.revalidate = 0
+
+          throw err
+        }
+
         if (err.digest === NEXT_DYNAMIC_NO_SSR_CODE) {
           warn(
             `Entire page ${pagePath} deopted into client-side rendering. https://nextjs.org/docs/messages/deopted-into-client-rendering`,
@@ -1001,24 +1003,10 @@ async function renderToHTMLOrFlightImpl(
   if (staticGenerationStore.isStaticGeneration) {
     const htmlResult = await renderResult.toUnchunkedString(true)
 
-    if (
-      renderOpts.ppr &&
-      staticGenerationStore.postponeWasTriggered &&
-      !extraRenderResultMeta.postponed
-    ) {
-      throw new Error(
-        `Postpone signal was caught while rendering ${urlPathname}. These errors should not be caught during static generation. Learn more: https://nextjs.org/docs/messages/ppr-postpone-errors`
-      )
-    }
-
-    for (const err of capturedErrors) {
-      if (!isDynamicUsageError(err) && !isPostpone(err)) {
-        throw err
-      }
-
-      if (isDynamicUsageError(err)) {
-        staticGenerationStore.revalidate = 0
-      }
+    // if we encountered any unexpected errors during build
+    // we fail the prerendering phase and the build
+    if (capturedErrors.length > 0) {
+      throw capturedErrors[0]
     }
 
     if (staticGenerationStore.forceStatic === false) {

packages/next/src/server/app-render/create-error-handler.tsx

@@ -24,8 +24,6 @@ export function createErrorHandler({
   errorLogger,
   capturedErrors,
   allCapturedErrors,
-  postponeErrors,
-  skipLogging,
 }: {
   _source: string
   dev?: boolean
@@ -33,8 +31,6 @@ export function createErrorHandler({
   errorLogger?: (err: any) => Promise<void>
   capturedErrors: Error[]
   allCapturedErrors?: Error[]
-  postponeErrors?: Error[]
-  skipLogging?: boolean
 }): ErrorHandler {
   return (err) => {
     if (allCapturedErrors) allCapturedErrors.push(err)
@@ -77,25 +73,19 @@ export function createErrorHandler({
         })
       }
 
-      if (postponeErrors) {
-        postponeErrors.push(err)
-      }
-
-      if (!skipLogging) {
-        if (errorLogger) {
-          errorLogger(err).catch(() => {})
-        } else {
-          // The error logger is currently not provided in the edge runtime.
-          // Use `log-app-dir-error` instead.
-          // It won't log the source code, but the error will be more useful.
-          if (process.env.NODE_ENV !== 'production') {
-            const { logAppDirError } =
-              require('../dev/log-app-dir-error') as typeof import('../dev/log-app-dir-error')
-            logAppDirError(err)
-          }
-          if (process.env.NODE_ENV === 'production') {
-            console.error(err)
-          }
+      if (errorLogger) {
+        errorLogger(err).catch(() => {})
+      } else {
+        // The error logger is currently not provided in the edge runtime.
+        // Use `log-app-dir-error` instead.
+        // It won't log the source code, but the error will be more useful.
+        if (process.env.NODE_ENV !== 'production') {
+          const { logAppDirError } =
+            require('../dev/log-app-dir-error') as typeof import('../dev/log-app-dir-error')
+          logAppDirError(err)
+        }
+        if (process.env.NODE_ENV === 'production') {
+          console.error(err)
         }
       }
     }

packages/next/src/server/base-server.ts

@@ -2664,7 +2664,7 @@ export default abstract class Server<ServerOptions extends Options = Options> {
       }
 
       if (isDataReq) {
-        // If this isn't a prefetch and this isn't a resume request, we want to
+        // If this isn't a prefetch and this isn't a resume request, we  want to
         // respond with the dynamic flight data. In the case that this is a
         // resume request the page data will already be dynamic.
         if (!isAppPrefetch && !resumed) {

test/e2e/app-dir/ppr-errors/app/layout.jsx

@@ -1,9 +0,0 @@
-import React from 'react'
-
-export default function Root({ children }) {
-  return (
-    <html>
-      <body>{children}</body>
-    </html>
-  )
-}

test/e2e/app-dir/ppr-errors/app/page.jsx

@@ -1,26 +0,0 @@
-import React, { Suspense } from 'react'
-import { cookies } from 'next/headers'
-
-export default async function Page() {
-  return (
-    <>
-      <h2>Dynamic Component Catching Errors</h2>
-      <p>
-        This shows the dynamic component that reads cookies but wraps the read
-        in a try/catch.
-      </p>
-      <div id="container">
-        <Suspense fallback={<div>Loading...</div>}>
-          <Foobar />
-        </Suspense>
-      </div>
-    </>
-  )
-}
-
-async function Foobar() {
-  try {
-    cookies()
-  } catch (err) {}
-  return null
-}

test/e2e/app-dir/ppr-errors/next.config.js

@@ -1,10 +0,0 @@
-/**
- * @type {import('next').NextConfig}
- */
-const nextConfig = {
-  experimental: {
-    ppr: true,
-  },
-}
-
-module.exports = nextConfig

test/e2e/app-dir/ppr-errors/ppr.test.ts

@@ -1,10 +0,0 @@
-import { nextBuild } from 'next-test-utils'
-
-describe('ppr build errors', () => {
-  it('should fail the build', async () => {
-    const out = await nextBuild(__dirname, [], { stderr: true })
-    expect(out.stderr).toContain(
-      'Postpone signal was caught while rendering /. These errors should not be caught during static generation.'
-    )
-  })
-})

test/e2e/app-dir/ppr/components/dynamic.jsx

@@ -2,22 +2,14 @@ import React, { use } from 'react'
 import { cookies } from 'next/headers'
 import { Delay, Login } from './state'
 
-export function Dynamic({ fallback, catchErrors }) {
+export function Dynamic({ fallback }) {
   const dynamic = fallback !== true
 
   let signedIn
   let active
   if (dynamic) {
-    if (catchErrors) {
-      try {
-        signedIn = cookies().has('session') ? true : false
-        active = cookies().has('delay') ? true : false
-      } catch (err) {}
-    } else {
-      signedIn = cookies().has('session') ? true : false
-      active = cookies().has('delay') ? true : false
-    }
-
+    signedIn = cookies().has('session') ? true : false
+    active = cookies().has('delay') ? true : false
     if (active) {
       use(new Promise((resolve) => setTimeout(resolve, 1000)))
     }

test/e2e/app-dir/ppr/ppr.test.ts

@@ -6,7 +6,7 @@ createNextDescribe(
     files: __dirname,
     skipDeployment: true,
   },
-  ({ next, isNextDev, isNextStart }) => {
+  ({ next, isNextDev }) => {
     describe.each([
       { pathname: '/suspense/node' },
       { pathname: '/suspense/node/nested/1' },