Skip to content

fs: do not swallow exceptions thrown by completion callbacks - #66308

Merged
nodejs-github-bot merged 2 commits into
nodejs:mainfrom
marcopiraccini:fs-trycatch-scope
Oct 29, 2026
Merged

nodejs-github-bot merged 2 commits into
nodejs:mainfrom
marcopiraccini:fs-trycatch-scope

Conversation

@marcopiraccini

@marcopiraccini marcopiraccini commented Sep 26, 2026 •

Copy link
Copy Markdown
Contributor

Since #57706 the completion callbacks of fs.mkdtemp, fs.realpath.native, fs.readlink, fs.mkdir({ recursive }), fs.readdir({ recursive }) and dir.read() call into JS while the TryCatch meant for StringBytes::Encode is still in scope.
Any exception thrown by the user callback, or by the nextTick queue drained right after it (e.g. after await fsp.mkdtemp()), is swallowed: the process prints nothing and exits 0, and uncaughtException never fires. v22 is not affected.

This moves the TryCatch into a small helper that produces the value first and calls Resolve()/Reject() only after leaving it.

Fixes: #65667

@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. lib / src Issues and PRs involving general changes in the lib/ or src/ directories. needs-ci PRs that need a full CI run. labels Sep 26, 2026
@marcopiraccini
marcopiraccini marked this pull request as ready for review September 26, 2026 03:53
@codecov

codecov Bot commented Sep 26, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 90.90909% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.67%. Comparing base (d630b5d) to head (4e3d6b8).
⚠️ Report is 52 commits behind head on main.

Files with missing lines Patch % Lines
src/node_file-inl.h 82.35% 0 Missing and 3 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #66308      +/-   ##
==========================================
+ Coverage   90.59%   90.67%   +0.07%     
==========================================
  Files         797      797              
  Lines      278230   278235       +5     
  Branches    53482    53499      +17     
==========================================
+ Hits       252066   252284     +218     
+ Misses      16558    16334     -224     
- Partials     9606     9617      +11     
Files with missing lines Coverage Δ
src/node_dir.cc 71.61% <100.00%> (+0.82%) ⬆️
src/node_file.cc 76.13% <100.00%> (+0.24%) ⬆️
src/node_file-inl.h 86.15% <82.35%> (+0.24%) ⬆️

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

Since 93644d5 the TryCatch guarding StringBytes::Encode in the
completion callbacks of mkdtemp, realpath.native, readlink, recursive
mkdir, recursive readdir and dir.read is still active when the JS
callback runs. Exceptions thrown by the callback, or by the nextTick
queue drained after it, are caught by it and never reported.

Leave the TryCatch before calling into JS.

Fixes: nodejs#65667
Refs: nodejs#57706
Signed-off-by: marcopiraccini <[email protected]>
Signed-off-by: marcopiraccini <[email protected]>
@ShogunPanda ShogunPanda 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 Oct 26, 2026
@github-actions github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label Oct 26, 2026
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@ShogunPanda ShogunPanda added commit-queue PRs queued for automated landing through the Commit Queue. commit-queue-squash PRs the Commit Queue should land as one squashed commit. labels Oct 29, 2026
@nodejs-github-bot
nodejs-github-bot merged commit c9f1d2d into nodejs:main Oct 29, 2026
69 checks passed
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Landed in c9f1d2d

@nodejs-github-bot nodejs-github-bot removed the commit-queue PRs queued for automated landing through the Commit Queue. label Oct 29, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

c++ Issues and PRs that require attention from people who are familiar with C++. commit-queue-squash PRs the Commit Queue should land as one squashed commit. lib / src Issues and PRs involving general changes in the lib/ or src/ directories. needs-ci PRs that need a full CI run.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

node:test aborts in InternalCallbackScope::Close after listen EPERM

3 participants