-
Notifications
You must be signed in to change notification settings - Fork 29.2k
[Cache Components] Error for Sync IO in Server Components during Static Prerender #82500
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -341,21 +341,6 @@ export function abortAndThrowOnSynchronousRequestDataAccess( | |
) | ||
} | ||
|
||
/** | ||
* Use this function when dynamically prerendering with dynamicIO. | ||
* We don't want to error, because it's better to return something | ||
* (and we've already aborted the render at the point where the sync dynamic error occured), | ||
* but we should log an error server-side. | ||
* @internal | ||
*/ | ||
export function warnOnSyncDynamicError(dynamicTracking: DynamicTrackingState) { | ||
if (dynamicTracking.syncDynamicErrorWithStack) { | ||
// the server did something sync dynamic, likely | ||
// leading to an early termination of the prerender. | ||
console.error(dynamicTracking.syncDynamicErrorWithStack) | ||
} | ||
} | ||
|
||
// For now these implementations are the same so we just reexport | ||
export const trackSynchronousRequestDataAccessInDev = | ||
trackSynchronousPlatformIOAccessInDev | ||
|
@@ -778,6 +763,14 @@ export function throwIfDisallowedDynamic( | |
dynamicValidation: DynamicValidationState, | ||
serverDynamic: DynamicTrackingState | ||
): void { | ||
if (serverDynamic.syncDynamicErrorWithStack) { | ||
logDisallowedDynamicError( | ||
workStore, | ||
serverDynamic.syncDynamicErrorWithStack | ||
) | ||
throw new StaticGenBailoutError() | ||
} | ||
Comment on lines
+766
to
+772
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Critical behavioral change: sync dynamic errors now fail immediately even when properly wrapped in Suspense boundaries, breaking previously valid patterns. View Details📝 Patch Detailsdiff --git a/packages/next/src/server/app-render/dynamic-rendering.ts b/packages/next/src/server/app-render/dynamic-rendering.ts
index 2c88cd8e0b..492512254d 100644
--- a/packages/next/src/server/app-render/dynamic-rendering.ts
+++ b/packages/next/src/server/app-render/dynamic-rendering.ts
@@ -763,14 +763,6 @@ export function throwIfDisallowedDynamic(
dynamicValidation: DynamicValidationState,
serverDynamic: DynamicTrackingState
): void {
- if (serverDynamic.syncDynamicErrorWithStack) {
- logDisallowedDynamicError(
- workStore,
- serverDynamic.syncDynamicErrorWithStack
- )
- throw new StaticGenBailoutError()
- }
-
if (prelude !== PreludeState.Full) {
if (dynamicValidation.hasSuspenseAboveBody) {
// This route has opted into allowing fully dynamic rendering
@@ -779,6 +771,14 @@ export function throwIfDisallowedDynamic(
return
}
+ if (serverDynamic.syncDynamicErrorWithStack) {
+ logDisallowedDynamicError(
+ workStore,
+ serverDynamic.syncDynamicErrorWithStack
+ )
+ throw new StaticGenBailoutError()
+ }
+
// We didn't have any sync bailouts but there may be user code which
// blocked the root. We would have captured these during the prerender
// and can log them here and then terminate the build/validating render
AnalysisThe logic in Previous behavior (correct): Sync dynamic errors (like New behavior (incorrect): Sync dynamic errors now always cause immediate failures regardless of Suspense boundary coverage, breaking pages that were previously working correctly. The test case This affects real applications where developers have correctly wrapped dynamic operations in Suspense boundaries as recommended by Next.js documentation, but those patterns will now start failing after this change. RecommendationMove the export function throwIfDisallowedDynamic(
workStore: WorkStore,
prelude: PreludeState,
dynamicValidation: DynamicValidationState,
serverDynamic: DynamicTrackingState
): void {
if (prelude !== PreludeState.Full) {
if (dynamicValidation.hasSuspenseAboveBody) {
// This route has opted into allowing fully dynamic rendering
// by including a Suspense boundary above the body. In this case
// a lack of a shell is not considered disallowed so we simply return
return
}
if (serverDynamic.syncDynamicErrorWithStack) {
logDisallowedDynamicError(
workStore,
serverDynamic.syncDynamicErrorWithStack
)
throw new StaticGenBailoutError()
}
// ... rest of the logic
}
} Also revert the corresponding test expectations that were changed to accommodate this incorrect behavior. |
||
|
||
if (prelude !== PreludeState.Full) { | ||
if (dynamicValidation.hasSuspenseAboveBody) { | ||
// This route has opted into allowing fully dynamic rendering | ||
|
@@ -786,17 +779,6 @@ export function throwIfDisallowedDynamic( | |
return | ||
} | ||
|
||
if (serverDynamic.syncDynamicErrorWithStack) { | ||
// There is no shell and the server did something sync dynamic likely | ||
// leading to an early termination of the prerender before the shell | ||
// could be completed. We terminate the build/validating render. | ||
logDisallowedDynamicError( | ||
workStore, | ||
serverDynamic.syncDynamicErrorWithStack | ||
) | ||
throw new StaticGenBailoutError() | ||
} | ||
|
||
// We didn't have any sync bailouts but there may be user code which | ||
// blocked the root. We would have captured these during the prerender | ||
// and can log them here and then terminate the build/validating render | ||
|
Uh oh!
There was an error while loading. Please reload this page.