abort and report on sync IO errors in dev

Author: lubieowoce

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

@@ -15,6 +15,8 @@ import {
   type WorkStore,
 } from '../app-render/work-async-storage.external'
 import type {
+  CommonDevStoreModern,
+  DevRequestStoreModern,
   PrerenderStoreModernRuntime,
   RequestStore,
 } from '../app-render/work-unit-async-storage.external'
@@ -2668,16 +2670,19 @@ async function renderWithRestartOnCacheMissInDev(
   getPayload: (requestStore: RequestStore) => Promise<RSCPayload>,
   onError: (error: unknown) => void
 ) {
-  const { renderOpts } = ctx
+  const { renderOpts, workStore } = ctx
   const { clientReferenceManifest, ComponentMod, setReactDebugChannel } =
     renderOpts
+  const captureOwnerStack = ComponentMod.captureOwnerStack
+
   assertClientReferenceManifest(clientReferenceManifest)
 
   // If the render is restarted, we'll recreate a fresh request store
   let requestStore: RequestStore = initialRequestStore
 
   const environmentName = () => {
-    const currentStage = requestStore.stagedRendering!.currentStage
+    const { stagedRendering } = requestStore as DevRequestStoreModern
+    const currentStage = stagedRendering.currentStage
     switch (currentStage) {
       case RenderStage.Static:
         return 'Prerender'
@@ -2692,6 +2697,25 @@ async function renderWithRestartOnCacheMissInDev(
     }
   }
 
+  const throwIfInvalidDynamic = (expectedStage: RenderStage) => {
+    const { stagedRendering, dynamicTracking } =
+      requestStore as DevRequestStoreModern
+    if (
+      expectedStage !== RenderStage.Dynamic &&
+      // Sync IO errors advance us to the dynamic stage.
+      stagedRendering.currentStage === RenderStage.Dynamic
+    ) {
+      // We should always have an error set, but be defensive
+      if (dynamicTracking.syncDynamicErrorWithStack) {
+        throw dynamicTracking.syncDynamicErrorWithStack
+      }
+    }
+
+    if (workStore.invalidDynamicUsageError) {
+      throw workStore.invalidDynamicUsageError
+    }
+  }
+
   //===============================================
   // Initial render
   //===============================================
@@ -2716,12 +2740,18 @@ async function renderWithRestartOnCacheMissInDev(
     initialDataController.signal
   )
 
-  requestStore.prerenderResumeDataCache = prerenderResumeDataCache
   // `getRenderResumeDataCache` will fall back to using `prerenderResumeDataCache` as `renderResumeDataCache`,
   // so not having a resume data cache won't break any expectations in case we don't need to restart.
   requestStore.renderResumeDataCache = null
-  requestStore.stagedRendering = initialStageController
-  requestStore.cacheSignal = cacheSignal
+  Object.assign(requestStore, {
+    stagedRendering: initialStageController,
+    prerenderResumeDataCache,
+    cacheSignal,
+    dynamicTracking: createDynamicTrackingState(
+      false // isDebugDynamicAccesses
+    ),
+    captureOwnerStack,
+  } satisfies CommonDevStoreModern)
 
   let debugChannel = setReactDebugChannel && createDebugChannel()
 
@@ -2752,6 +2782,8 @@ async function renderWithRestartOnCacheMissInDev(
           return stream
         },
         (stream) => {
+          throwIfInvalidDynamic(/* expected stage */ RenderStage.Static)
+
           // Runtime stage
           initialStageController.advanceStage(RenderStage.Runtime)
 
@@ -2766,6 +2798,8 @@ async function renderWithRestartOnCacheMissInDev(
           return stream
         },
         async (maybeStream) => {
+          throwIfInvalidDynamic(/* expected stage */ RenderStage.Runtime)
+
           // Dynamic stage
 
           // If we had cache misses in either of the previous stages,
@@ -2803,6 +2837,10 @@ async function renderWithRestartOnCacheMissInDev(
 
   await cacheSignal.cacheReady()
   initialReactController.abort()
+  throwIfInvalidDynamic(
+    // If we're warming caches, we shouldn't have advanced past the runtime stage.
+    RenderStage.Runtime
+  )
 
   //===============================================
   // Final render (restarted)
@@ -2815,12 +2853,18 @@ async function renderWithRestartOnCacheMissInDev(
 
   // We've filled the caches, so now we can render as usual,
   // without any cache-filling mechanics.
-  requestStore.prerenderResumeDataCache = null
   requestStore.renderResumeDataCache = createRenderResumeDataCache(
     prerenderResumeDataCache
   )
-  requestStore.stagedRendering = finalStageController
-  requestStore.cacheSignal = null
+  Object.assign(requestStore, {
+    stagedRendering: finalStageController,
+    prerenderResumeDataCache: null,
+    cacheSignal: null,
+    dynamicTracking: createDynamicTrackingState(
+      false // isDebugDynamicAccesses
+    ),
+    captureOwnerStack: ComponentMod.captureOwnerStack,
+  } satisfies CommonDevStoreModern)
 
   // The initial render already wrote to its debug channel.
   // We're not using it, so we need to create a new one.
@@ -2843,11 +2887,15 @@ async function renderWithRestartOnCacheMissInDev(
         )
       },
       (stream) => {
+        throwIfInvalidDynamic(/* expected stage */ RenderStage.Static)
+
         // Runtime stage
         finalStageController.advanceStage(RenderStage.Runtime)
         return stream
       },
       (stream) => {
+        throwIfInvalidDynamic(/* expected stage */ RenderStage.Runtime)
+
         // Dynamic stage
         finalStageController.advanceStage(RenderStage.Dynamic)
         return stream

packages/next/src/server/app-render/dynamic-rendering.ts

@@ -23,10 +23,10 @@
 import type { WorkStore } from '../app-render/work-async-storage.external'
 import type {
   WorkUnitStore,
-  RequestStore,
   PrerenderStoreLegacy,
   PrerenderStoreModern,
   PrerenderStoreModernRuntime,
+  DevRequestStoreModern,
 } from '../app-render/work-unit-async-storage.external'
 
 // Once postpone is in stable we should switch to importing the postpone export directly
@@ -296,14 +296,18 @@ export function abortOnSynchronousPlatformIOAccess(
 }
 
 export function trackSynchronousPlatformIOAccessInDev(
-  requestStore: RequestStore
+  requestStore: DevRequestStoreModern,
+  errorWithStack: Error
 ): void {
+  const { stagedRendering, dynamicTracking } = requestStore
   // We don't actually have a controller to abort but we do the semantic equivalent by
   // advancing the request store out of the prerender stage
-  if (requestStore.stagedRendering) {
-    // TODO: error for sync IO in the runtime stage
-    // (which is not currently covered by the validation render in `spawnDynamicValidationInDev`)
-    requestStore.stagedRendering.advanceStage(RenderStage.Dynamic)
+
+  // TODO: error for sync IO in the runtime stage
+  // (which is not currently covered by the validation render in `spawnDynamicValidationInDev`)
+  stagedRendering.advanceStage(RenderStage.Dynamic)
+  if (dynamicTracking.syncDynamicErrorWithStack === null) {
+    dynamicTracking.syncDynamicErrorWithStack = errorWithStack
   }
 }
 

packages/next/src/server/app-render/work-unit-async-storage.external.ts

@@ -29,7 +29,7 @@ export interface CommonWorkUnitStore {
   readonly implicitTags: ImplicitTags
 }
 
-export interface RequestStore extends CommonWorkUnitStore {
+interface BaseRequestStore extends CommonWorkUnitStore {
   readonly type: 'request'
 
   /**
@@ -65,13 +65,41 @@ export interface RequestStore extends CommonWorkUnitStore {
    * The resume data cache for this request. This will be a immutable cache.
    */
   renderResumeDataCache: RenderResumeDataCache | null
+}
+
+export type RequestStore = ProdRequestStore | DevRequestStore
+export type ProdRequestStore = BaseRequestStore & NoneOf<DevStore>
+export type DevRequestStore = BaseRequestStore & DevStore
+export type DevRequestStoreModern = BaseRequestStore & DevStoreModern
 
-  // DEV-only
+type DevStore = DevStoreLegacy | DevStoreModern
+type DevStoreLegacy = CommonDevStore & NoneOf<CommonDevStoreModern>
+type DevStoreModern = CommonDevStore & CommonDevStoreModern
+
+type CommonDevStore = {
   usedDynamic?: boolean
-  devFallbackParams?: OpaqueFallbackRouteParams | null
-  stagedRendering?: StagedRenderingController | null
-  cacheSignal?: CacheSignal | null
-  prerenderResumeDataCache?: PrerenderResumeDataCache | null
+  devFallbackParams: OpaqueFallbackRouteParams | null
+}
+
+export type CommonDevStoreModern = {
+  readonly stagedRendering: StagedRenderingController
+  readonly captureOwnerStack: () => string | null
+  readonly dynamicTracking: DynamicTrackingState
+} & (
+  | {
+      // In the initial render, we track and fill caches
+      readonly cacheSignal: CacheSignal
+      readonly prerenderResumeDataCache: PrerenderResumeDataCache
+    }
+  | {
+      // In the final (restarted) render, we do not track or fill caches
+      readonly cacheSignal: null
+      readonly prerenderResumeDataCache: null
+    }
+)
+
+type NoneOf<TObj extends Record<string, any>> = {
+  [key in keyof TObj]?: undefined
 }
 
 /**

packages/next/src/server/async-storage/request-store.ts

@@ -263,7 +263,7 @@ function createRequestStoreImpl(
     serverComponentsHmrCache:
       serverComponentsHmrCache ||
       (globalThis as any).__serverComponentsHmrCache,
-    devFallbackParams,
+    devFallbackParams: devFallbackParams ?? null,
   }
 }
 

packages/next/src/server/node-environment-extensions/utils.tsx

@@ -1,6 +1,7 @@
 import { workAsyncStorage } from '../app-render/work-async-storage.external'
 import {
   workUnitAsyncStorage,
+  type DevRequestStoreModern,
   type PrerenderStoreModern,
 } from '../app-render/work-unit-async-storage.external'
 import {
@@ -87,7 +88,31 @@ export function io(expression: string, type: ApiType) {
     }
     case 'request':
       if (process.env.NODE_ENV === 'development') {
-        trackSynchronousPlatformIOAccessInDev(workUnitStore)
+        if (workUnitStore.stagedRendering) {
+          // If the prerender signal is already aborted we don't need to construct
+          // any stacks because something else actually terminated the prerender.
+          let message: string
+          switch (type) {
+            case 'time':
+              message = `Route "${workStore.route}" used ${expression} before accessing either uncached data (e.g. \`fetch()\`) or \`connection()\`. Accessing the current time in a Server Component requires reading one of these data sources first. Alternatively, consider moving this expression into a Client Component or Cache Component. See more info here: https://nextjs.org/docs/messages/next-prerender-current-time`
+              break
+            case 'random':
+              message = `Route "${workStore.route}" used ${expression} before accessing either uncached data (e.g. \`fetch()\`) or \`connection()\`. Accessing random values synchronously in a Server Component requires reading one of these data sources first. Alternatively, consider moving this expression into a Client Component or Cache Component. See more info here: https://nextjs.org/docs/messages/next-prerender-random`
+              break
+            case 'crypto':
+              message = `Route "${workStore.route}" used ${expression} before accessing either uncached data (e.g. \`fetch()\`) or \`connection()\`. Accessing random cryptographic values synchronously in a Server Component requires reading one of these data sources first. Alternatively, consider moving this expression into a Client Component or Cache Component. See more info here: https://nextjs.org/docs/messages/next-prerender-crypto`
+              break
+            default:
+              throw new InvariantError(
+                'Unknown expression type in abortOnSynchronousPlatformIOAccess.'
+              )
+          }
+
+          trackSynchronousPlatformIOAccessInDev(
+            workUnitStore,
+            applyOwnerStack(new Error(message), workUnitStore)
+          )
+        }
       }
       break
     case 'prerender-ppr':
@@ -101,7 +126,10 @@ export function io(expression: string, type: ApiType) {
   }
 }
 
-function applyOwnerStack(error: Error, workUnitStore: PrerenderStoreModern) {
+function applyOwnerStack(
+  error: Error,
+  workUnitStore: PrerenderStoreModern | DevRequestStoreModern
+) {
   // TODO: Instead of stitching the stacks here, we should log the original
   // error as-is when it occurs, and let `patchErrorInspect` handle adding the
   // owner stack, instead of logging it deferred in the `LogSafely` component