Skip to content

fix: don't race the typechecker spawn grace period on windows#10814

Merged
sheremet-va merged 1 commit into
vitest-dev:mainfrom
xevrion:fix/typecheck-win-spawn-race
Jul 22, 2026
Merged

fix: don't race the typechecker spawn grace period on windows#10814
sheremet-va merged 1 commit into
vitest-dev:mainfrom
xevrion:fix/typecheck-win-spawn-race

Conversation

@xevrion

@xevrion xevrion commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

Description

On Windows a nonexistent typechecker command still spawns (the shell starts and then fails), so Typechecker.spawn can't rely on the error event. It currently waits 200ms for the close event and otherwise declares the spawn successful. On a slow runner the shell takes longer than 200ms to fail, the timer wins, and instead of Spawning typechecker failed - is typescript installed? the user gets an unhandled Typecheck Error with the shell's raw output. The same race makes the handles non-existing typechecker command gracefully test in test/typescript flaky on Windows CI, most recently in this run.

In run mode there is no need for a grace period at all: start awaits the process right after spawning, so this change just waits for close and resolves or rejects based on the exit. That makes the error deterministic regardless of machine speed. Watch mode keeps the 200ms timer since the watch process never exits.

The win32 code paths can't be exercised from my machine, so Windows CI on this PR is the verification for the changed behavior; the test/typescript suite passes unchanged on Linux, which doesn't touch these branches.

Refs #10618

Please don't delete this checklist! Before submitting the PR, please make sure you do the following:

  • It's really useful if your PR references an issue where it is discussed ahead of time. If the feature is substantial or introduces breaking changes without a discussion, PR might be closed.
  • Ideally, include a test that fails without this PR but passes with it. (the existing handles non-existing typechecker command gracefully test is the regression test - it fails intermittently on Windows without this change)
  • Please, don't make changes to pnpm-lock.yaml unless you introduce a new test example.
  • Please check Allow edits by maintainers to make review process faster. Note that this option is not available for repositories that are owned by Github organizations.

Tests

  • Run the tests with pnpm test:ci.

Documentation

  • If you introduce new functionality, document it. You can run documentation with pnpm run docs command.

Changesets

  • Changes in changelog are generated from PR name. Please, make sure that it explains your changes in an understandable manner. Please, prefix changeset messages with feat:, fix:, perf:, docs:, or chore:.

@github-actions github-actions Bot added the maybe automated User is likely an AI agent, or the content was generated by an AI assistant without user control label Jul 22, 2026
@xevrion xevrion mentioned this pull request Jul 22, 2026
Open
@github-actions

github-actions Bot commented Jul 22, 2026

Copy link
Copy Markdown

Hello @xevrion. Your PR has been labeled maybe automated because it appears to have been fully generated by AI with no human involvement.

To keep your PR open, please follow these steps:

  • Confirm that you are a real human. If you are an automated agent, disclose that
  • Confirm you've read, reviewed and stand behind its content
  • Confirm you've read the full issue along with all of its comments, as well as any linked issues and their comments
  • Make sure it follows our contribution guidelines and uses the correct GitHub template
  • Disclose any AI tools you used (e.g. Claude, Copilot, Codex)

Please, do not generate or format the response with AI. If you do not speak English, reply in your native language or use translation software like Google Translate or Deepl. If the response is generated, the PR will be closed automatically.

These measures help us reduce maintenance burden and keep the team's work efficient. See our AI contributions policy for more context.

@netlify

netlify Bot commented Jul 22, 2026

Copy link
Copy Markdown

✅ Deploy Preview for vitest-dev ready!

Built without sensitive environment variables

Name Link
🔨 Latest commit 2cea55a
🔍 Latest deploy log https://app.netlify.com/projects/vitest-dev/deploys/6a6099e9936b310008d28cd6
😎 Deploy Preview https://deploy-preview-10814--vitest-dev.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.
🤖 Make changes Run an agent on this branch

To edit notification comments on pull requests, go to your Netlify project configuration.

@xevrion

xevrion commented Jul 22, 2026

Copy link
Copy Markdown
Contributor Author

hi, i'm a human, ive used claude code to help with this one, i've reviewed the changes and i stand with it.
and the background is: the windows unit job on my other PR failed on the typechecker test, so I dug into why. The root cause is the 200ms grace timer in Typechecker.spawn is racing against the close event of a command that failed to start, and on a slow runner sometimes, the timer wins and you get the raw cmd error instead of the "Spawning typechecker failed" message.
This pr removes the race in run mode by waiting for close and keeps the timer only for watch mode.

@sheremet-va
sheremet-va merged commit b19e5ec into vitest-dev:main Jul 22, 2026
28 of 29 checks passed
@sheremet-va sheremet-va removed the maybe automated User is likely an AI agent, or the content was generated by an AI assistant without user control label Jul 22, 2026
@sheremet-va

sheremet-va commented Jul 22, 2026

Copy link
Copy Markdown
Member

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@sheremet-va sheremet-va sheremet-va approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@xevrion @sheremet-va