Use provided waitUntil for pending revalidates (#74164)

[MitchLewis930]

PR_036

Summary by CodeRabbit

• Refactor

  • Enhanced the request rendering pipeline to improve handling of pending asynchronous operations and cache revalidation timing, ensuring operations complete before response transmission.
  • Refined promise coordination logic to better manage operation sequencing across application routes.

• Tests

  • Updated test utilities to use improved retry-based assertion patterns for fetch deduplication validation.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

📝 Walkthrough

Walkthrough

These changes refactor how pending promises are managed in the Next.js server-side render pipeline, introducing centralized promise aggregation with debug instrumentation, and updating a test utility to use a retry-based approach instead of fixed delays.

Changes

Cohort / File(s)	Summary
Promise handling in app rendering packages/next/src/server/app-render/app-render.tsx, packages/next/src/server/base-server.ts, packages/next/src/server/route-modules/app-route/module.ts	Refactors pending revalidation promise handling by computing pendingPromise via Promise.all().finally() with debug logging support, integrating results into render context's waitUntil. Adds conditional execution of waitUntil callbacks based on renderOpts availability before response handling.
Test utility updates test/e2e/app-dir/app-fetch-deduping/app-fetch-deduping.test.ts	Replaces sequential waitFor + render pattern with retry-based flow using 10-second timeout for improved test resilience.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

🐰 A rabbit's verse on promises kept:

Pending dreams in Promise.all we weave,
Finally logging what we perceive,
Awaiting revalidates with care,
Until the caches are debunked fair,
No more waiting—just retry, retry! 🌟

🚥 Pre-merge checks | ✅ 1 | ❌ 2

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Description check	⚠️ Warning	The pull request description is minimal and does not follow the required template structure; it only contains 'PR_036' without any explanation of what, why, or how.	Add sections explaining what changed (technical summary), why the change was needed, and how it works. Include issue/discussion links and testing details per template guidelines.
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (1 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and specifically describes the main change: using a provided waitUntil for pending revalidates, which matches the core objective of the PR.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch pr_036_after

---

📜 Recent review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between bc69d97 and 4e80990.

📒 Files selected for processing (4)

• packages/next/src/server/app-render/app-render.tsx
• packages/next/src/server/base-server.ts
• packages/next/src/server/route-modules/app-route/module.ts
• test/e2e/app-dir/app-fetch-deduping/app-fetch-deduping.test.ts

🧰 Additional context used

🧬 Code graph analysis (2)

packages/next/src/server/route-modules/app-route/module.ts (1)

packages/next/src/server/web/sandbox/context.ts (1)

• requestStore (248-250)

test/e2e/app-dir/app-fetch-deduping/app-fetch-deduping.test.ts (1)

test/lib/next-test-utils.ts (1)

• retry (792-822)

🔇 Additional comments (5)

packages/next/src/server/route-modules/app-route/module.ts (1)

595-607: Debug log on pending revalidates looks good.

The finally() hook adds useful traceability without altering behavior, and the env gate keeps it non-intrusive.

test/e2e/app-dir/app-fetch-deduping/app-fetch-deduping.test.ts (2)

1-1: Import aligns with new retry-based flow.

---

115-118: Retry-based wait makes the test more robust.

packages/next/src/server/app-render/app-render.tsx (2)

1301-1317: Good use of provided waitUntil when available.

---

1468-1484: Consistent pending revalidate handling in the dynamic path.

_{✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.}

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[MitchLewis930]

@CodeRabbit full review

[coderabbitai]

✅ Actions performed

Full review triggered.

[coderabbitai]

Actionable comments posted: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

packages/next/src/server/base-server.ts (1)

2544-2561: ⚠️ Potential issue | 🟠 Major

Use the adjusted pendingWaitUntil when sending the response.

You clear the local pendingWaitUntil when waitUntil is available, but sendResponse still receives context.renderOpts.pendingWaitUntil (Line 2556–2561). That keeps the old “wait for pending” behavior and defeats the intended change. Pass the local variable (or clear renderOpts.pendingWaitUntil) so the response can close when waitUntil is provided.

✅ Suggested fix

-            await sendResponse(
-              req,
-              res,
-              response,
-              context.renderOpts.pendingWaitUntil
-            )
+            await sendResponse(
+              req,
+              res,
+              response,
+              pendingWaitUntil
+            )

📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between bc69d97 and 4e80990.

📒 Files selected for processing (4)

• packages/next/src/server/app-render/app-render.tsx
• packages/next/src/server/base-server.ts
• packages/next/src/server/route-modules/app-route/module.ts
• test/e2e/app-dir/app-fetch-deduping/app-fetch-deduping.test.ts

🧰 Additional context used

🧬 Code graph analysis (2)

packages/next/src/server/route-modules/app-route/module.ts (1)

packages/next/src/server/web/sandbox/context.ts (1)

• requestStore (248-250)

test/e2e/app-dir/app-fetch-deduping/app-fetch-deduping.test.ts (1)

test/lib/next-test-utils.ts (1)

• retry (792-822)

🔇 Additional comments (5)

packages/next/src/server/route-modules/app-route/module.ts (1)

595-607: Debug log on pending revalidates looks good.

The finally() hook adds useful traceability without altering behavior, and the env gate keeps it non-intrusive.

test/e2e/app-dir/app-fetch-deduping/app-fetch-deduping.test.ts (2)

1-1: Import aligns with new retry-based flow.

---

115-118: Retry-based wait makes the test more robust.

packages/next/src/server/app-render/app-render.tsx (2)

1301-1317: Good use of provided waitUntil when available.

---

1468-1484: Consistent pending revalidate handling in the dynamic path.

_{✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.}