internal: waitUntil should execute promises after the response is returned by MaxLeiter · Pull Request #62068 · vercel/next.js

MaxLeiter · 2024-02-14T23:11:10Z

Certain cloud runtimes enforce concurrency limits. If too many network requests are being executed concurrently via internal_runWithWaitUntil this limit can be reached and a deadlock can occur.

This change switches internal_runWithWaitUntil from immediately calling the promises provided, and instead constructs a queue to be processed sequentially after the function returns.

I recommend reviewing without whitespace: https://github.com/vercel/next.js/pull/62068/files?diff=unified&w=1

…urned Certain cloud runtimes enforce concurrency limits. If too many network requests are being executed concurrently via `internal_runWithWaitUntil` this limit can be reached and a deadlock can occur. This change switches internal_runWithWaitUntil from immediately calling the promises provided, and instead building a queue to be processed sequentially after the function returns.

X-Coauthored-by: Shu Ding <g@shud.in>

shuding · 2024-02-14T23:17:32Z

packages/next/src/server/web/internal-edge-wait-until.ts

 export function internal_getCurrentFunctionWaitUntil() {
-  return state.waitUntilPromise
+  return resolveTasksSequentially()
 }

-export function internal_runWithWaitUntil<T>(fn: () => T): T {
-  const result = fn()
-  if (
-    result &&
-    typeof result === 'object' &&
-    'then' in result &&
-    'finally' in result &&
-    typeof result.then === 'function' &&
-    typeof result.finally === 'function'
-  ) {
-    if (!state.waitUntilCounter) {
-      // Create the promise for the next batch of waitUntil calls.
-      state.waitUntilPromise = new Promise<void>((resolve) => {
-        state.waitUntilResolve = resolve
-      })
-    }
-    state.waitUntilCounter++
-    return result.finally(() => {
-      resolveOnePromise()
-    })
-  }
-
-  return result
+export function internal_runWithWaitUntil<T>(fn: () => Promise<T>) {
+  tasks.push(fn)
 }


For visibility: this ensures that those tasks are not started right away - when you call internal_runWithWaitUntil, but after the route handler's execution.

ijjk · 2024-02-14T23:28:53Z

Failing test suites

Commit: ec899dc

pnpm test-start test/e2e/app-dir/navigation/navigation.test.ts (PPR)

app dir - navigation > navigation between pages and app > should not continously initiate a mpa navigation to the same URL when router state changes

Expand output

● app dir - navigation › navigation between pages and app › should not continously initiate a mpa navigation to the same URL when router state changes

page.waitForSelector: Timeout 60000ms exceeded.
Call log:
  - waiting for locator('#link-to-slow-page')

  421 |     return this.chain(() => {
  422 |       return page
> 423 |         .waitForSelector(selector, { timeout, state: 'attached' })
      |          ^
  424 |         .then(async (el) => {
  425 |           // it seems selenium waits longer and tests rely on this behavior
  426 |           // so we wait for the load event fire before returning

  at waitForSelector (lib/browsers/playwright.ts:423:10)

Read more about building and testing Next.js in contributing.md.

ijjk · 2024-02-14T23:31:59Z

Stats from current PR

Default Build (Increase detected ⚠️)

General

	vercel/next.js canary	MaxLeiter/next.js max/edge-wait-until-changes	Change
buildDuration	20.3s	20s	N/A
buildDurationCached	8.6s	7s	N/A
nodeModulesSize	196 MB	196 MB	N/A
nextStartRea..uration (ms)	428ms	430ms	N/A

Client Bundles (main, webpack)

	vercel/next.js canary	MaxLeiter/next.js max/edge-wait-until-changes	Change
1068-HASH.js gzip	30.2 kB	30.2 kB	N/A
3f784ff6-HASH.js gzip	53.5 kB	53.5 kB	N/A
4944-HASH.js gzip	5.04 kB	5.03 kB	N/A
8423.HASH.js gzip	181 B	181 B	✓
framework-HASH.js gzip	45.2 kB	45.2 kB	✓
main-app-HASH.js gzip	241 B	241 B	✓
main-HASH.js gzip	32.1 kB	32.1 kB	N/A
webpack-HASH.js gzip	1.7 kB	1.7 kB	✓
Overall change	47.3 kB	47.3 kB	✓

Legacy Client Bundles (polyfills)

	vercel/next.js canary	MaxLeiter/next.js max/edge-wait-until-changes	Change
polyfills-HASH.js gzip	31 kB	31 kB	✓
Overall change	31 kB	31 kB	✓

Client Pages

	vercel/next.js canary	MaxLeiter/next.js max/edge-wait-until-changes	Change
_app-HASH.js gzip	196 B	196 B	✓
_error-HASH.js gzip	184 B	183 B	N/A
amp-HASH.js gzip	503 B	504 B	N/A
css-HASH.js gzip	323 B	324 B	N/A
dynamic-HASH.js gzip	2.5 kB	2.51 kB	N/A
edge-ssr-HASH.js gzip	258 B	259 B	N/A
head-HASH.js gzip	353 B	351 B	N/A
hooks-HASH.js gzip	370 B	370 B	✓
image-HASH.js gzip	4.21 kB	4.2 kB	N/A
index-HASH.js gzip	259 B	259 B	✓
link-HASH.js gzip	2.68 kB	2.67 kB	N/A
routerDirect..HASH.js gzip	313 B	314 B	N/A
script-HASH.js gzip	386 B	385 B	N/A
withRouter-HASH.js gzip	309 B	311 B	N/A
1afbb74e6ecf..834.css gzip	106 B	106 B	✓
Overall change	931 B	931 B	✓

Client Build Manifests

	vercel/next.js canary	MaxLeiter/next.js max/edge-wait-until-changes	Change
_buildManifest.js gzip	485 B	484 B	N/A
Overall change	0 B	0 B	✓

Rendered Page Sizes

	vercel/next.js canary	MaxLeiter/next.js max/edge-wait-until-changes	Change
index.html gzip	528 B	527 B	N/A
link.html gzip	540 B	540 B	✓
withRouter.html gzip	524 B	522 B	N/A
Overall change	540 B	540 B	✓

Edge SSR bundle Size

	vercel/next.js canary	MaxLeiter/next.js max/edge-wait-until-changes	Change
edge-ssr.js gzip	94.4 kB	94.4 kB	N/A
page.js gzip	151 kB	151 kB	N/A
Overall change	0 B	0 B	✓

Middleware size

	vercel/next.js canary	MaxLeiter/next.js max/edge-wait-until-changes	Change
middleware-b..fest.js gzip	623 B	627 B	N/A
middleware-r..fest.js gzip	151 B	151 B	✓
middleware.js gzip	44.4 kB	44.4 kB	N/A
edge-runtime..pack.js gzip	1.94 kB	1.94 kB	✓
Overall change	2.1 kB	2.1 kB	✓

Next Runtimes

	vercel/next.js canary	MaxLeiter/next.js max/edge-wait-until-changes	Change
app-page-exp...dev.js gzip	166 kB	166 kB	✓
app-page-exp..prod.js gzip	95.5 kB	95.5 kB	✓
app-page-tur..prod.js gzip	97.3 kB	97.3 kB	✓
app-page-tur..prod.js gzip	91.7 kB	91.7 kB	✓
app-page.run...dev.js gzip	136 kB	136 kB	✓
app-page.run..prod.js gzip	90.3 kB	90.3 kB	✓
app-route-ex...dev.js gzip	22 kB	22 kB	✓
app-route-ex..prod.js gzip	14.9 kB	14.9 kB	✓
app-route-tu..prod.js gzip	14.9 kB	14.9 kB	✓
app-route-tu..prod.js gzip	14.6 kB	14.6 kB	✓
app-route.ru...dev.js gzip	21.7 kB	21.7 kB	✓
app-route.ru..prod.js gzip	14.6 kB	14.6 kB	✓
pages-api-tu..prod.js gzip	9.43 kB	9.43 kB	✓
pages-api.ru...dev.js gzip	9.7 kB	9.7 kB	✓
pages-api.ru..prod.js gzip	9.43 kB	9.43 kB	✓
pages-turbo...prod.js gzip	22 kB	22 kB	✓
pages.runtim...dev.js gzip	22.7 kB	22.7 kB	✓
pages.runtim..prod.js gzip	22 kB	22 kB	✓
server.runti..prod.js gzip	50 kB	50 kB	✓
Overall change	925 kB	925 kB	✓

build cache Overall increase ⚠️

	vercel/next.js canary	MaxLeiter/next.js max/edge-wait-until-changes	Change
0.pack gzip	1.55 MB	1.55 MB	N/A
index.pack gzip	103 kB	103 kB	⚠️ +279 B
Overall change	103 kB	103 kB	⚠️ +279 B

Diff details

Diff for page.js

Diff too large to display

Diff for edge-ssr.js

Diff too large to display

Commit: ec899dc

MaxLeiter requested review from feedthejim, huozhi, ijjk, shuding, timneutkens, wyattjoh and ztanner as code owners February 14, 2024 23:11

ijjk added the type: next label Feb 14, 2024

MaxLeiter added 2 commits February 14, 2024 15:12

for loop

2b2cab9

handle when a new task is added to the list.

062940b

X-Coauthored-by: Shu Ding <g@shud.in>

shuding reviewed Feb 14, 2024

View reviewed changes

MaxLeiter added 2 commits February 14, 2024 15:19

add code comment

fc4b202

add code comment

ec899dc

shuding approved these changes Feb 14, 2024

View reviewed changes

shuding enabled auto-merge (squash) February 14, 2024 23:22

MaxLeiter closed this Feb 15, 2024

auto-merge was automatically disabled February 15, 2024 00:33
Pull request was closed

MaxLeiter deleted the max/edge-wait-until-changes branch February 15, 2024 00:33

github-actions bot added the locked label Feb 29, 2024

github-actions bot locked as resolved and limited conversation to collaborators Feb 29, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Comments

internal: waitUntil should execute promises after the response is returned#62068

internal: waitUntil should execute promises after the response is returned#62068
MaxLeiter wants to merge 5 commits intovercel:canaryfrom
MaxLeiter:max/edge-wait-until-changes

MaxLeiter commented Feb 14, 2024 •

edited

Loading

Uh oh!

shuding Feb 14, 2024

Uh oh!

ijjk commented Feb 14, 2024

Uh oh!

ijjk commented Feb 14, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Comments

Conversation

MaxLeiter commented Feb 14, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

shuding Feb 14, 2024

Choose a reason for hiding this comment

Uh oh!

ijjk commented Feb 14, 2024

Failing test suites

Uh oh!

ijjk commented Feb 14, 2024

Stats from current PR

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

MaxLeiter commented Feb 14, 2024 •

edited

Loading