fix: Ensure walk() skips directories deleted before they are listed - #166
Open
nzakas wants to merge 1 commit into
Open
fix: Ensure walk() skips directories deleted before they are listed#166nzakas wants to merge 1 commit into
nzakas wants to merge 1 commit into
Conversation
`NodeHfsImpl#list()` and `DenoHfsImpl#list()` are async generators, so the directory isn't actually read until the iterator is first advanced. The ENOENT handling added in #135 only wrapped the call to `list()`, so an ENOENT thrown during iteration (e.g. a directory deleted between being discovered and being listed) escaped `walk()`. `walk()` now drives the `list()` iterator manually, catching ENOENT from `next()` and stopping the walk of that directory, while still rethrowing other errors and closing the iterator on early exit. Fixes #154 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019DHXVwqQ4iawKrV2DxPVKV
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
1 participant
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.
Summary
walk()crashed withENOENTwhen a directory was deleted between being discovered and being listed — the race hit by ESLint users running other tools concurrently (#154, eslint/eslint#18955).Root cause
NodeHfsImpl#list()andDenoHfsImpl#list()are async generators, soawait impl.list(dir)only returns an iterator;readdirdoesn't run until the firstnext(). Thetry/catchfrom #135 wrapped only thelist()call, so theENOENTsurfaced insidefor await, outside the catch. The #135 test didn't catch this because its mocklist()was a plain function that threw synchronously.Fix
walk()now drives thelist()iterator manually (Symbol.asyncIterator, falling back toSymbol.iteratorso array-returning impls still work) and wraps eachnext():ENOENT→ stop walking that directory silently (entries already yielded from it are kept)break, filter throwing) the iterator'sreturn()is called, matchingfor awaitcleanup semanticsThe existing catch around the
list()call is kept for impls that throw synchronously.Tests
list()): skip onENOENTat firstnext(); walked root missing → no entries;ENOENTpartway through iteration; non-ENOENTerrors rethrow; iterators closed on earlybreakand onentryFilterthrow. Also fixed the "ENONENT" typo in the existing test name.directoryFilterdeletes the subdirectory afterwalk()discovers it but before it's listed — the exact race from the issue.Verified the new tests reproduce the bug: against the unfixed
hfs.js, the 3 coreENOENTtests and the Node real-FS test fail; with the fix, core 191 / node 199 / deno 125 steps pass.Fixes #154
🤖 Generated with Claude Code
https://claude.ai/code/session_019DHXVwqQ4iawKrV2DxPVKV