Skip to content

[vitest-pool-workers] Add 30s timeout to waitUntil promise draining#13083

Merged
penalosa merged 5 commits intomainfrom
penalosa/vitest-7244
Mar 27, 2026
Merged

[vitest-pool-workers] Add 30s timeout to waitUntil promise draining#13083
penalosa merged 5 commits intomainfrom
penalosa/vitest-7244

Conversation

@penalosa
Copy link
Copy Markdown
Contributor

@penalosa penalosa commented Mar 27, 2026

Fixes #7244.

Previously, if a ctx.waitUntil() promise never resolved, the test suite would hang indefinitely after the test file finished. Now, any waitUntil promises that haven't settled within 30 seconds are abandoned with a warning, allowing the test suite to continue. This aligns with the production waitUntil limit.


  • Tests
    • Tests included/updated
  • Public documentation
    • Documentation not necessary because: behaviour change is self-documenting via the warning message

Open with Devin

Previously, if a ctx.waitUntil() promise never resolved, the test suite
would hang indefinitely after the test file finished. Now, any waitUntil
promises that haven't settled within 30 seconds (aligned with the
production Workers platform limit) are abandoned with a warning.
@changeset-bot
Copy link
Copy Markdown

changeset-bot bot commented Mar 27, 2026

🦋 Changeset detected

Latest commit: 69a3a41

The changes in this PR will be included in the next version bump.

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Mar 27, 2026

✅ All changesets look good

ask-bonk[bot]

This comment was marked as resolved.

@ask-bonk
Copy link
Copy Markdown
Contributor

ask-bonk bot commented Mar 27, 2026

I'm Bonk, and I've done a quick review of your PR.

This PR adds a 30-second timeout to waitUntil promise draining in vitest-pool-workers, preventing indefinite hangs when a ctx.waitUntil() promise never settles.

Issues found:

  1. Minor: Dangling setTimeout timer on the happy path (packages/vitest-pool-workers/src/worker/wait-until.ts:34-39) — When Promise.allSettled wins the race, the 30s timeout timer is never cleared via clearTimeout. It will eventually fire harmlessly (resolving an already-settled promise is a no-op), and in workerd the timer won't keep anything alive, but it's good practice to clean it up. Posted a suggestion comment with the fix.

github run

@pkg-pr-new
Copy link
Copy Markdown

pkg-pr-new bot commented Mar 27, 2026

create-cloudflare

npm i https://pkg.pr.new/create-cloudflare@13083

@cloudflare/kv-asset-handler

npm i https://pkg.pr.new/@cloudflare/kv-asset-handler@13083

miniflare

npm i https://pkg.pr.new/miniflare@13083

@cloudflare/pages-shared

npm i https://pkg.pr.new/@cloudflare/pages-shared@13083

@cloudflare/unenv-preset

npm i https://pkg.pr.new/@cloudflare/unenv-preset@13083

@cloudflare/vite-plugin

npm i https://pkg.pr.new/@cloudflare/vite-plugin@13083

@cloudflare/vitest-pool-workers

npm i https://pkg.pr.new/@cloudflare/vitest-pool-workers@13083

@cloudflare/workers-editor-shared

npm i https://pkg.pr.new/@cloudflare/workers-editor-shared@13083

wrangler

npm i https://pkg.pr.new/wrangler@13083

commit: 69a3a41

The previous test went through SELF.fetch() which returns immediately —
waitForGlobalWaitUntil() was never called in that lifecycle. The new test
calls waitForWaitUntil() directly with a never-resolving promise and
verifies it returns after the timeout instead of hanging.
@penalosa penalosa marked this pull request as ready for review March 27, 2026 00:51
@penalosa penalosa requested a review from a team as a code owner March 27, 2026 00:51
@workers-devprod
Copy link
Copy Markdown
Contributor

workers-devprod commented Mar 27, 2026

Codeowners approval required for this PR:

  • ✅ @cloudflare/wrangler
Show detailed file reviewers

Copy link
Copy Markdown
Contributor

@devin-ai-integration devin-ai-integration bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no potential bugs to report.

View in Devin Review to see 2 additional findings.

Open in Devin Review

@github-project-automation github-project-automation bot moved this from Untriaged to Approved in workers-sdk Mar 27, 2026
@penalosa penalosa merged commit cfd513f into main Mar 27, 2026
51 of 56 checks passed
@penalosa penalosa deleted the penalosa/vitest-7244 branch March 27, 2026 13:18
@github-project-automation github-project-automation bot moved this from Approved to Done in workers-sdk Mar 27, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

🚀 Feature Request: Debug hanging Vitest tests

3 participants