From 45a328a84f112c751cc44ab585dca34e35bf2eb1 Mon Sep 17 00:00:00 2001 From: Zack Tanner <1939140+ztanner@users.noreply.github.com> Date: Mon, 21 Oct 2024 15:54:18 -0700 Subject: [PATCH] avoid logging stacks for internal errors (#71575) `NEXT_PAGE_EXPORT_ERROR` and `NEXT_STATIC_GEN_BAILOUT` shouldn't show up in failed build logs as they'll point to an internal stack not related to user code. This updates the code that throws the `ExportPageError` to instead exit the worker when `prerenderEarlyExit` is true (which is the default) so that the page export error doesn't leak into the outer error handler which is meant for user errors. For static generation bailout errors, we'll log the error message (if it exists) otherwise there's nothing to log as the stack would point to internal framework code. This also removes a redundant log that is already printed as a prerender error. --- packages/next/src/export/worker.ts | 21 ++++++++++++++++--- .../server/app-render/dynamic-rendering.ts | 8 ++----- ...dynamic-io-errors.platform-dynamic.test.ts | 1 - .../dynamic-io-errors.sync-dynamic.test.ts | 3 --- .../dynamic-io-errors.test.ts | 1 - 5 files changed, 20 insertions(+), 14 deletions(-) diff --git a/packages/next/src/export/worker.ts b/packages/next/src/export/worker.ts index 798be7662ab48..3976343b19673 100644 --- a/packages/next/src/export/worker.ts +++ b/packages/next/src/export/worker.ts @@ -48,6 +48,7 @@ import { import { needsExperimentalReact } from '../lib/needs-experimental-react' import { runWithCacheScope } from '../server/async-storage/cache-scope.external' import type { AppRouteRouteModule } from '../server/route-modules/app-route/module.compiled' +import { isStaticGenBailoutError } from '../client/components/static-generation-bailout' const envConfig = require('../shared/lib/runtime-config.external') @@ -433,16 +434,17 @@ export async function exportPages( if (attempt >= maxAttempts - 1) { // Log a message if we've reached the maximum number of attempts. // We only care to do this if maxAttempts was configured. - if (maxAttempts > 0) { + if (maxAttempts > 1) { console.info( `Failed to build ${pageKey} after ${maxAttempts} attempts.` ) } // If prerenderEarlyExit is enabled, we'll exit the build immediately. if (nextConfig.experimental.prerenderEarlyExit) { - throw new ExportPageError( + console.error( `Export encountered an error on ${pageKey}, exiting the build.` ) + process.exit(1) } else { // Otherwise, this is a no-op. The build will continue, and a summary of failed pages will be displayed at the end. } @@ -540,8 +542,21 @@ async function exportPage( `\nError occurred prerendering page "${input.path}". Read more: https://nextjs.org/docs/messages/prerender-error\n` ) + // bailoutToCSRError errors should not leak to the user as they are not actionable; they're + // a framework signal if (!isBailoutToCSRError(err)) { - console.error(isError(err) && err.stack ? err.stack : err) + // A static generation bailout error is a framework signal to fail static generation but + // and will encode a reason in the error message. If there is a message, we'll print it. + // Otherwise there's nothing to show as we don't want to leak an error internal error stack to the user. + if (isStaticGenBailoutError(err)) { + if (err.message) { + console.error(`Error: ${err.message}`) + } + } else if (isError(err) && err.stack) { + console.error(err.stack) + } else { + console.error(err) + } } return { error: true, duration: Date.now() - start, files: [] } diff --git a/packages/next/src/server/app-render/dynamic-rendering.ts b/packages/next/src/server/app-render/dynamic-rendering.ts index 02ea723a2cf36..1e01e2ba3470e 100644 --- a/packages/next/src/server/app-render/dynamic-rendering.ts +++ b/packages/next/src/server/app-render/dynamic-rendering.ts @@ -653,9 +653,7 @@ export function throwIfDisallowedDynamic( console.error(syncError) } // The actual error should have been logged when the sync access ocurred - throw new StaticGenBailoutError( - `Route "${route}" could not be prerendered.` - ) + throw new StaticGenBailoutError() } const dynamicErrors = dynamicValidation.dynamicErrors @@ -664,9 +662,7 @@ export function throwIfDisallowedDynamic( console.error(dynamicErrors[i]) } - throw new StaticGenBailoutError( - `Route "${route}" could not be prerendered.` - ) + throw new StaticGenBailoutError() } if (!dynamicValidation.hasSuspendedDynamic) { diff --git a/test/e2e/app-dir/dynamic-io-errors/dynamic-io-errors.platform-dynamic.test.ts b/test/e2e/app-dir/dynamic-io-errors/dynamic-io-errors.platform-dynamic.test.ts index b78a8f0702a40..268c63cfc768e 100644 --- a/test/e2e/app-dir/dynamic-io-errors/dynamic-io-errors.platform-dynamic.test.ts +++ b/test/e2e/app-dir/dynamic-io-errors/dynamic-io-errors.platform-dynamic.test.ts @@ -126,7 +126,6 @@ function runTests(options: { withMinification: boolean }) { 'Error: Route "/" used `Math.random()` outside of `"use cache"` and without explicitly calling `await connection()` beforehand. See more info here: https://nextjs.org/docs/messages/next-prerender-random' ) expectError('Error occurred prerendering page "/"') - expectError('Error: Route "/" could not be prerendered.') expectError('exiting the build.') }) }) diff --git a/test/e2e/app-dir/dynamic-io-errors/dynamic-io-errors.sync-dynamic.test.ts b/test/e2e/app-dir/dynamic-io-errors/dynamic-io-errors.sync-dynamic.test.ts index 953a8d60dd2ac..54d46b635fc03 100644 --- a/test/e2e/app-dir/dynamic-io-errors/dynamic-io-errors.sync-dynamic.test.ts +++ b/test/e2e/app-dir/dynamic-io-errors/dynamic-io-errors.sync-dynamic.test.ts @@ -124,7 +124,6 @@ function runTests(options: { withMinification: boolean }) { expectError('Route "/" used `searchParams.foo`') expectError('Error occurred prerendering page "/"') - expectError('Error: Route "/" could not be prerendered.') expectError('exiting the build.') }) }) @@ -212,7 +211,6 @@ function runTests(options: { withMinification: boolean }) { expectError('Route "/" used `searchParams.foo`') expectError('Error occurred prerendering page "/"') - expectError('Error: Route "/" could not be prerendered.') expectError('exiting the build.') }) }) @@ -300,7 +298,6 @@ function runTests(options: { withMinification: boolean }) { expectError('Route "/" used `cookies().get(\'token\')`') expectError('Error occurred prerendering page "/"') - expectError('Route "/" could not be prerendered.') expectError('exiting the build.') }) }) diff --git a/test/e2e/app-dir/dynamic-io-errors/dynamic-io-errors.test.ts b/test/e2e/app-dir/dynamic-io-errors/dynamic-io-errors.test.ts index 5d22d906274fb..54691ed341340 100644 --- a/test/e2e/app-dir/dynamic-io-errors/dynamic-io-errors.test.ts +++ b/test/e2e/app-dir/dynamic-io-errors/dynamic-io-errors.test.ts @@ -337,7 +337,6 @@ function runTests(options: { withMinification: boolean }) { ) } expectError('Error occurred prerendering page "/"') - expectError('Error: Route "/" could not be prerendered.') expectError('exiting the build.') }) })