Use provided waitUntil for pending revalidates (#74164)#6
Use provided waitUntil for pending revalidates (#74164)#6MitchLewis930 wants to merge 1 commit intopr_036_beforefrom
Conversation
Currently we are using `pendingWaitUntil` on `renderOpts` to handle pending revalidations (fetch updates and revalidate tag calls). This uses our old strategy of `waitUntil` inside of `sendResponse` which just keeps the stream open until the promise resolves. This isn't ideal if we have a proper waitUntil strategy provided so this updates to ensure we use that instead if available. Also adds some debug logs so we can track when this pending revalidates promise is resolved.
There was a problem hiding this comment.
Pull request overview
This PR updates the handling of pending revalidation promises to use a provided waitUntil function when available, rather than always assigning directly to options.waitUntil. The changes ensure that background revalidation tasks are properly tracked and awaited in serverless and edge runtime environments.
Changes:
- Modified test utilities to use
retryinstead ofwaitForfor more reliable revalidation testing - Added conditional logic to use
renderOpts.waitUntil()when available, falling back tooptions.waitUntilassignment - Added debug logging to track when pending revalidation promises complete
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| test/e2e/app-dir/app-fetch-deduping/app-fetch-deduping.test.ts | Updated test to use retry utility for polling revalidation completion instead of fixed timeout |
| packages/next/src/server/route-modules/app-route/module.ts | Added debug logging for pending revalidation promise completion |
| packages/next/src/server/base-server.ts | Added logic to use provided waitUntil function for pending revalidations in base server |
| packages/next/src/server/app-render/app-render.tsx | Refactored revalidation promise handling to conditionally use renderOpts.waitUntil() or assign to options.waitUntil |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| await retry(async () => { | ||
| await next.render('/test') | ||
| expect(invocation(next.cliOutput)).toBe(2) | ||
| }, 10_000) |
There was a problem hiding this comment.
The retry function is being used to poll for the revalidation to complete, but there's no assertion that the retry succeeded before the timeout. If the invocation count never reaches 2 within 10 seconds, the test should fail with a clear error message. Consider adding error handling or using a retry utility that throws on timeout.
| const pendingPromise = Promise.all([ | ||
| workStore.incrementalCache?.revalidateTag( | ||
| workStore.revalidatedTags || [] | ||
| ), | ||
| ...Object.values(workStore.pendingRevalidates || {}), | ||
| ...(workStore.pendingRevalidateWrites || []), | ||
| ]) | ||
| ]).finally(() => { | ||
| if (process.env.NEXT_PRIVATE_DEBUG_CACHE) { | ||
| console.log('pending revalidates promise finished for:', url) | ||
| } | ||
| }) |
There was a problem hiding this comment.
This exact code block is duplicated at lines 1468-1478. Consider extracting this logic into a shared helper function to reduce duplication and improve maintainability.
PR_036