Skip to content

Commit

Permalink
avoid logging stacks for internal errors (#71575)
Browse files Browse the repository at this point in the history
`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.
  • Loading branch information
ztanner authored Oct 21, 2024
1 parent 8a905cf commit 45a328a
Show file tree
Hide file tree
Showing 5 changed files with 20 additions and 14 deletions.
21 changes: 18 additions & 3 deletions packages/next/src/export/worker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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')

Expand Down Expand Up @@ -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.
}
Expand Down Expand Up @@ -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: [] }
Expand Down
8 changes: 2 additions & 6 deletions packages/next/src/server/app-render/dynamic-rendering.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.')
})
})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.')
})
})
Expand Down Expand Up @@ -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.')
})
})
Expand Down Expand Up @@ -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.')
})
})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.')
})
})
Expand Down

0 comments on commit 45a328a

Please sign in to comment.