Skip to content

vfs: fix promises.watch() with an already-aborted signal - #66254

Open
trivikr wants to merge 1 commit into
nodejs:mainfrom
trivikr:vfs-watch-reject-preaborted-signal
Open

trivikr wants to merge 1 commit into
nodejs:mainfrom
trivikr:vfs-watch-reject-preaborted-signal

Conversation

@trivikr

@trivikr trivikr commented Sep 24, 2026

Copy link
Copy Markdown
Member

Fixes: #66253

fs.promises.watch() over a virtual mount resolved the first next() with { done: true } when the signal was already aborted, while native fs.promises.watch() rejects with ABORT_ERR.

Buffer the AbortError in VFSWatchAsyncIterable so the first next() rejects once and later calls behave like an exhausted iterator, matching the native async generator. Concurrently pending next() calls now resolve after the rejecting one instead of all rejecting.


Assisted-by: opencode

fs.promises.watch() over a virtual mount resolved the first next()
with { done: true } when the signal was already aborted, while native
fs.promises.watch() rejects with ABORT_ERR.

Buffer the AbortError in VFSWatchAsyncIterable so the first next()
rejects once and later calls behave like an exhausted iterator,
matching the native async generator. Concurrently pending next()
calls now resolve after the rejecting one instead of all rejecting.

Signed-off-by: Kamat, Trivikram <16024985+trivikr@users.noreply.github.com>
Assisted-by: opencode
@nodejs-github-bot nodejs-github-bot added needs-ci PRs that need a full CI run. vfs Issues and PRs related to the virtual filesystem subsystem. labels Sep 24, 2026
@trivikr trivikr added the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label Sep 24, 2026
@codecov

codecov Bot commented Sep 24, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 76.47059% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.28%. Comparing base (a2c8da5) to head (ebc1f19).

Files with missing lines Patch % Lines
lib/internal/vfs/watcher.js 76.47% 4 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main   #66254   +/-   ##
=======================================
  Coverage   90.28%   90.28%           
=======================================
  Files         789      789           
  Lines      272878   272893   +15     
  Branches    52097    52102    +5     
=======================================
+ Hits       246360   246380   +20     
+ Misses      16975    16973    -2     
+ Partials     9543     9540    -3     
Files with missing lines Coverage Δ
lib/internal/vfs/watcher.js 94.74% <76.47%> (-0.47%) ⬇️

... and 25 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

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

Labels

needs-ci PRs that need a full CI run. request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. vfs Issues and PRs related to the virtual filesystem subsystem.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

vfs: promises.watch() resolves { done: true } instead of rejecting with a pre-aborted signal

2 participants