Skip to content

fs: abort in-flight stat operations#63142

Open
mertcanaltin wants to merge 1 commit into
nodejs:mainfrom
mertcanaltin:mert/fs-stat-inflight-fix
Open

fs: abort in-flight stat operations#63142
mertcanaltin wants to merge 1 commit into
nodejs:mainfrom
mertcanaltin:mert/fs-stat-inflight-fix

Conversation

@mertcanaltin

Copy link
Copy Markdown
Member

Follow-up to #57775. The signal option only honored the pre-aborted case (#57775 (comment) by @Renegade334). This wires the signal as an abort listener so fs.stat callbacks reject with AbortError when the signal fires mid-request.

Refs: #57775

@nodejs-github-bot nodejs-github-bot added fs Issues and PRs related to the fs subsystem / file system. needs-ci PRs that need a full CI run. labels May 6, 2026
@mertcanaltin
mertcanaltin force-pushed the mert/fs-stat-inflight-fix branch from 4425502 to e4d317e Compare May 6, 2026 05:18
@bakkot

bakkot commented May 6, 2026

Copy link
Copy Markdown
Contributor

Drive-by: this looks like it's still not actually canceling the underlying call, which would involve invoking uv_cancel somewhere.

@mertcanaltin
mertcanaltin force-pushed the mert/fs-stat-inflight-fix branch from e4d317e to 8149c79 Compare May 7, 2026 19:42
@codecov

codecov Bot commented May 7, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 96.66667% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 90.03%. Comparing base (1fd4949) to head (6d3d629).
⚠️ Report is 39 commits behind head on main.

Files with missing lines Patch % Lines
src/node_file.cc 83.33% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@           Coverage Diff            @@
##             main   #63142    +/-   ##
========================================
  Coverage   90.03%   90.03%            
========================================
  Files         713      713            
  Lines      224699   224978   +279     
  Branches    42473    42541    +68     
========================================
+ Hits       202310   202563   +253     
- Misses      14179    14183     +4     
- Partials     8210     8232    +22     
Files with missing lines Coverage Δ
lib/fs.js 98.21% <100.00%> (+0.01%) ⬆️
src/node_file.cc 74.25% <83.33%> (+0.05%) ⬆️

... and 49 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.

@mertcanaltin
mertcanaltin force-pushed the mert/fs-stat-inflight-fix branch from 8149c79 to fc73958 Compare May 8, 2026 20:28
Signed-off-by: Mert Can Altin <mertgold60@gmail.com>
@mertcanaltin
mertcanaltin force-pushed the mert/fs-stat-inflight-fix branch from fc73958 to 6d3d629 Compare May 8, 2026 20:30
@mertcanaltin

Copy link
Copy Markdown
Member Author

Thanks for review @bakkot I added a new binding function for cancel

@trivikr

trivikr commented Jul 16, 2026

Copy link
Copy Markdown
Member

cc @nodejs/fs

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

Labels

fs Issues and PRs related to the fs subsystem / file system. needs-ci PRs that need a full CI run.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants