Skip to content

fs: handle early writeFile stream errors#63472

Open
cookesan wants to merge 3 commits into
nodejs:mainfrom
cookesan:fs-glob-cache-seen-siblings
Open

fs: handle early writeFile stream errors#63472
cookesan wants to merge 3 commits into
nodejs:mainfrom
cookesan:fs-glob-cache-seen-siblings

Conversation

@cookesan
Copy link
Copy Markdown
Contributor

The fs.promises.writeFile() stream path could miss a readable
error emitted before the destination file opened. The returned promise
could reject while the source stream error also surfaced as an uncaught
exception.

This attaches a temporary error listener for readable stream inputs
before opening the destination, checks recorded stream errors before and
during async iteration, and removes the listener when the write finishes
or when opening the destination fails. FileHandle.writeFile() uses the
same helper.

Tests cover early readable errors for path and FileHandle writes, plus
listener cleanup when opening the destination fails.

Tests:

ninja -C out/Release -j4
python3 tools/test.py --shell out/Release/node parallel/test-fs-promises-writefile parallel/test-fs-promises-file-handle-writeFile
make lint-js

Fixes: #58742

@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 21, 2026
Attach a temporary error listener to readable stream inputs before
opening the destination file. This lets writeFile() reject with the
stream error instead of allowing an early source error to become an
uncaught exception.

Remove the listener when the write finishes or when opening the
destination fails.

Signed-off-by: cookesan <6601329+cookesan@users.noreply.github.com>
@cookesan cookesan force-pushed the fs-glob-cache-seen-siblings branch from a3d19d5 to c1d1472 Compare May 21, 2026 16:58
@cookesan cookesan marked this pull request as ready for review May 21, 2026 17:00
@codecov
Copy link
Copy Markdown

codecov Bot commented May 21, 2026

Codecov Report

❌ Patch coverage is 98.52941% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 90.31%. Comparing base (614050b) to head (614394a).
⚠️ Report is 59 commits behind head on main.

Files with missing lines Patch % Lines
lib/internal/fs/promises.js 98.52% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #63472      +/-   ##
==========================================
- Coverage   91.67%   90.31%   -1.36%     
==========================================
  Files         361      730     +369     
  Lines      156408   234257   +77849     
  Branches    24050    43941   +19891     
==========================================
+ Hits       143384   211569   +68185     
- Misses      12751    14416    +1665     
- Partials      273     8272    +7999     
Files with missing lines Coverage Δ
lib/internal/fs/promises.js 93.05% <98.52%> (+3.29%) ⬆️

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

cookesan added 2 commits May 23, 2026 21:24
Signed-off-by: cookesan <6601329+cookesan@users.noreply.github.com>
Keep the temporary writeFile error listener through the pending next-tick error emission when a stream is already errored at entry. This lets writeFile reject from the stored stream error without letting the stream emit an unhandled error, and still removes the listener after the pending emission.

Signed-off-by: cookesan <6601329+cookesan@users.noreply.github.com>
@cookesan
Copy link
Copy Markdown
Contributor Author

Pushed a follow-up for the failing writeFile stream cases.

The already-errored stream path now keeps the temporary error listener through the pending next-tick error emission, then removes it. The tests also wait that tick before asserting listener cleanup. The FileHandle early-error test now accounts for the stream async iterator's own retained listener on that path.

Local validation:

  • make -j8
  • python3 tools/test.py --mode=release parallel/test-fs-promises-file-handle-writeFile parallel/test-fs-promises-writefile
  • make lint-js LINT_JS_TARGETS="lib/internal/fs/promises.js test/parallel/test-fs-promises-file-handle-writeFile.js test/parallel/test-fs-promises-writefile.js"
  • git diff --check

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.

fsPromise.writeFile should catch stream's error and reject the promise.

2 participants