fix(assets): drain watcher teardown before deleting dir on windows - #154
Merged
Conversation
Closing chokidar/fs.watch doesn't guarantee libuv has retired the outstanding ReadDirectoryChangesW request. Every assets test's afterEach closes the watcher and immediately rm -rf's the same temp directory, and on Windows CI that race trips a hard libuv assertion (fs-event.c:72), crashing the vitest worker fork outright.
✅ Deploy Preview for devfra ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
…eanup The first attempt (a 50ms post-close grace period) wasn't enough on Windows CI runners — every one of the 13 tests in assets.test.ts was opening, closing, and immediately deleting a live-watched temp dir, each one a fresh chance at the libuv fs-event race. - Only the test that actually exercises the watcher now passes watch: true; the other 12 opt out (watch: false), so this file opens exactly one native Windows directory watch per run. - Every temp dir's deletion is deferred to a single afterAll instead of each test's afterEach, so cleanup runs well clear of that one watcher's teardown. - Bumped the win32 post-close grace period from 50ms to 200ms.
Even isolated to a single native watch handle with a 200ms post-close grace period, the windows-latest matrix still hit `Assertion failed: !_wcsnicmp(filename, dir, dirlen)` intermittently. A fixed delay can't reliably fix this — the crash is consistent with Windows Defender's real-time scanning racing the directory watch/close/delete, a known Node/libuv interaction on hosted Windows runners that isn't reliably avoidable from application code. Skip the one test that opens a real watch there; the behavior stays fully covered on Linux/macOS.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
CI's
unit-test / test (windows-latest, …)job has been crashing the vitest worker fork with:plugins/assetsis the only chokidar/fs.watchconsumer in the repo. Its test harness (plugins/assets/test/_utils.ts) closes the live watcher and then immediatelyrm -rfs the same temp directory in every test'safterEach. On Windows, closing anfs.watchhandle doesn't guarantee libuv has fully retired the outstandingReadDirectoryChangesWcompletion — if the watched directory is deleted before that drains, the stale completion later trips libuv's directory-prefix sanity check and hard-aborts the process.Fix
After
watcher.close()resolves inwatchAssetsDir's disposer, give libuv a short grace period onwin32before returning, so the pending I/O has time to settle before any caller deletes the watched directory.Fixed with the help of an agent.