Repository navigation
[wrangler] Fix spawn EBADF when watching large assets directories on macOS - #16459
alfaroukhesham wants to merge 1 commit into
Conversation
…macOS On macOS, chokidar 4 opens one fs.watch handle per asset file. Past OPEN_MAX, child-process spawns fail with EBADF and wrangler dev exits. Watch the assets directory with a single recursive fs.watch on darwin and win32, and keep chokidar on Linux. Co-authored-by: alfaroukhesham <alfaroukhesham@users.noreply.github.com>
🦋 Changeset detectedLatest commit: 8a080d1 The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
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 |
|
Hi @alfaroukhesham. Thanks for taking the time to contribute to Workers SDK. This PR has been automatically closed because you are not currently a registered (vouched) collaborator for this repository. AI tools now let our team generate proposed code changes quickly. Reviewing changes, checking that they solve the right problem, and maintaining code quality over time still take substantial care and effort. We therefore limit PRs to registered collaborators so we can focus that effort where it is most useful. For the majority of external contributors, the most effective way to help is through issues: describing a problem or use case, providing a minimal reproduction, investigating its cause, or helping us verify a fix. Those contributions help us understand what needs to change and are very welcome. We will take an initial look at this PR and decide whether it is something we want to invest time in reviewing and maintaining. We may reopen it if we decide to proceed, but we cannot guarantee that it will be reopened or receive a full review. |
|
Codeowners approval required for this PR:
Show detailed file reviewers
|
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| const filePath = | ||
| filename == null || filename === "" | ||
| ? assetsDir | ||
| : path.resolve(assetsDir, filename.toString()); | ||
| if (ignoredDirs.some((dir) => isPathInsideDir(filePath, dir))) { |
There was a problem hiding this comment.
🟡 Unnamed events reload ignored directories
When fs.watch omits a filename, writes under ignored directories trigger an assets refresh if assets include the project root. The fallback assetsDir never matches those ignored paths, so dev-session writes reload the Worker.
Learn more
Native filesystem watch events can omit the filename. When the assets directory is the project root, the project also contains write directories such as .wrangler; the ignored directory list normally prevents writes there from refreshing the Worker. For an unnamed event, replacing the unknown path with the assets root makes the ignore check ineffective, and the event calls the refresh listener. This can reload the Worker in response to its own development-session writes.
Example: With assets set to /app, a watch event from /app/.wrangler/tmp/output.js with filename === null becomes /app. The ignore check accepts it and emits another bundle refresh, unlike the same event with its filename present.
Recommended fix: Define a deliberate fallback for unnamed native events that avoids refreshing for writes in excluded directories while retaining real asset changes where possible. Consider whether a rescan or another watcher backend is necessary when the event cannot be attributed.
Was this helpful? React with 👍 or 👎 to provide feedback.
| const prefix = resolvedDir.endsWith(path.sep) | ||
| ? resolvedDir | ||
| : resolvedDir + path.sep; | ||
| return resolvedTarget === resolvedDir || resolvedTarget.startsWith(prefix); |
There was a problem hiding this comment.
🟡 Windows persist writes trigger asset reloads
When a Windows persist folder differs in capitalization from dev.persist, isPathInsideDir does not ignore its events. Case-sensitive prefix checks treat the same Windows directory as a different path, so persist writes reload the Worker.
Learn more
The native watcher constructs a path from each filename and compares it against the ignored directory list. On Windows, paths differing only by capitalization refer to the same directory, but startsWith and strict equality compare them as distinct strings. The new native watcher therefore refreshes the bundle when Windows returns the actual directory casing instead of the spelling used in dev.persist.
Example: Set assets to C:\app and dev.persist to C:\app\persist-state; a real directory C:\app\Persist-State containing kv.sqlite produces an event for Persist-State\kv.sqlite. The event is not ignored even though it belongs to the configured persist directory.
Recommended fix: Normalize comparison paths using Windows-appropriate case-insensitive semantics for the win32 backend; preserve separator-boundary checks so sibling paths do not match.
Was this helpful? React with 👍 or 👎 to provide feedback.
|
|
||
| Fix `wrangler dev` crashing with `spawn EBADF` on macOS when the assets directory contains many files | ||
|
|
||
| On macOS, chokidar 4 opens one `fs.watch` handle per asset file. Past the process file-descriptor limit this makes child-process spawns fail with `EBADF`, so `wrangler dev` exits. `wrangler dev` now watches the assets directory with a single recursive `fs.watch` on macOS and Windows, which uses one native handle for the whole tree. |
There was a problem hiding this comment.
🟡 Changeset details obscure user benefit
The changeset explains watcher internals instead of focusing on wrangler dev users. Repository review rules require user-facing impact and discourage implementation details in changesets.
| On macOS, chokidar 4 opens one `fs.watch` handle per asset file. Past the process file-descriptor limit this makes child-process spawns fail with `EBADF`, so `wrangler dev` exits. `wrangler dev` now watches the assets directory with a single recursive `fs.watch` on macOS and Windows, which uses one native handle for the whole tree. | |
| Large assets directories no longer cause `wrangler dev` to exit unexpectedly on macOS. Asset changes still trigger hot reloads on macOS and Windows. |
Was this helpful? React with 👍 or 👎 to provide feedback.
| for (const listener of allListeners) { | ||
| void listener("change", filePath); | ||
| } |
There was a problem hiding this comment.
📝 Info: Asset debug messages lose event detail
The native adapter reports every event as change. getAssetChangeMessage consequently labels additions and removals as generic changes, while the refresh path still handles them.
Was this helpful? React with 👍 or 👎 to provide feedback.
Fixes #16264
wrangler devon macOS exits withspawn EBADFonceassets.directoryholds more than about 10,000 files. Chokidar 4 opens onefs.watch(kqueue) handle per file; past macOSOPEN_MAX(10240)posix_spawnfails withEBADF(libuv/libuv#5204), so the existingEMFILEhandling never runs.On darwin and win32, watch the assets directory with a single recursive
fs.watch(one FSEvents / ReadDirectoryChangesW handle for the tree). Linux keeps chokidar. Existing debounce, ignored write dirs (.wrangler, persist), error-to-warning handling, and teardown behaviour are unchanged.Verified manually on macOS (Apple Silicon, Node 26) with a project whose assets directory holds 11,000 files: released wrangler 4.152.0 crashes with spawn EBADF on wrangler dev; with this fix wrangler dev starts, serves assets, hot reloads on asset edits, ignores changes under .wrangler/, and the wrangler process holds 43 open file descriptors with none per asset file.
wrangler devhot-reload behaviour; there is no new flag, config field, or user-facing API.Note
This contribution was written with an AI coding agent and reviewed by me.