Skip to content

fs: add throwIfNoEntry option to fs.lstat/fs.promises.lstat#63116

Open
Renegade334 wants to merge 2 commits intonodejs:mainfrom
Renegade334:fs-lstat-throw-if-no-entry
Open

fs: add throwIfNoEntry option to fs.lstat/fs.promises.lstat#63116
Renegade334 wants to merge 2 commits intonodejs:mainfrom
Renegade334:fs-lstat-throw-if-no-entry

Conversation

@Renegade334
Copy link
Copy Markdown
Member

This is present on lstatSync but wasn't added to the other lstat functions in #61178, leaving the API asymmetrical.

Copies a couple of deduplications/simplifications back to the equivalent stat functions.

Refs: #61178

Signed-off-by: Renegade334 <contact.9a5d6388@renegade334.me.uk>
@Renegade334 Renegade334 added the semver-minor PRs that contain new features and should be released in the next minor version. label May 4, 2026
@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. fs Issues and PRs related to the fs subsystem / file system. needs-ci PRs that need a full CI run. labels May 4, 2026
@aduh95
Copy link
Copy Markdown
Contributor

aduh95 commented May 4, 2026

Having an opt-out of synchronous throw makes perfect sense to allow a more performant path, but I don't think it applies for async API – I get the point of align the APIs, but if there are no use-cases for it, it's probably not worth adding

@Renegade334
Copy link
Copy Markdown
Member Author

Renegade334 commented May 4, 2026

Having an opt-out of synchronous throw makes perfect sense to allow a more performant path, but I don't think it applies for async API – I get the point of align the APIs, but if there are no use-cases for it, it's probably not worth adding

I agreed at the time, but now that it's in (ie. fs.stat/fs.promises.stat), it shouldn't be asymetrically half-in.

@aduh95
Copy link
Copy Markdown
Contributor

aduh95 commented May 4, 2026

There's precedence for this in #61178

@aduh95 aduh95 requested a review from juanarbol May 4, 2026 15:35
@Renegade334 Renegade334 force-pushed the fs-lstat-throw-if-no-entry branch from 200fb01 to 10413d6 Compare May 4, 2026 15:58
@codecov
Copy link
Copy Markdown

codecov Bot commented May 4, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 89.69%. Comparing base (b2f6aa3) to head (10413d6).
⚠️ Report is 20 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #63116      +/-   ##
==========================================
+ Coverage   89.65%   89.69%   +0.04%     
==========================================
  Files         712      712              
  Lines      220512   221271     +759     
  Branches    42289    42390     +101     
==========================================
+ Hits       197690   198477     +787     
+ Misses      14663    14650      -13     
+ Partials     8159     8144      -15     
Files with missing lines Coverage Δ
lib/fs.js 98.20% <100.00%> (+<0.01%) ⬆️
lib/internal/fs/promises.js 92.93% <100.00%> (-0.02%) ⬇️
src/node_file.cc 74.20% <100.00%> (-0.03%) ⬇️

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

c++ Issues and PRs that require attention from people who are familiar with C++. fs Issues and PRs related to the fs subsystem / file system. needs-ci PRs that need a full CI run. semver-minor PRs that contain new features and should be released in the next minor version.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants