Skip to content

fs: treat std::errc::permission_denied as EPERM error - #64698

Open
PickBas wants to merge 2 commits into
nodejs:mainfrom
JaneaSystems:win-rmsync-eperm-retry-test
Open

fs: treat std::errc::permission_denied as EPERM error#64698
PickBas wants to merge 2 commits into
nodejs:mainfrom
JaneaSystems:win-rmsync-eperm-retry-test

Conversation

@PickBas

@PickBas PickBas commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

std::errc::permission_denied is already treated as an EPERM error when an exception is thrown, but it is not included among the omittable errors retried by the RmSync operation. This commit adds it.

This also fixes the retryDelay calculation on Windows where the retryDelay is divided by 1000 but the win32's Sleep function takes the argument as an ms unit, dividing the supplied ms unit further to a much smaller delay.

Fixes: #64016
Refs: #64017 - This PR adds a test for the referenced PR.

@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 Jul 23, 2026
Comment thread test/parallel/test-fs-rmSync-eperm-retries.js Outdated
Comment thread src/node_file.cc
@PickBas
PickBas force-pushed the win-rmsync-eperm-retry-test branch from 092d380 to b5441d9 Compare July 23, 2026 13:23
@PickBas
PickBas force-pushed the win-rmsync-eperm-retry-test branch 2 times, most recently from 977185e to 7cf7983 Compare July 23, 2026 13:39
@codecov

codecov Bot commented Jul 23, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.28%. Comparing base (ee64033) to head (abb551f).
⚠️ Report is 3 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #64698      +/-   ##
==========================================
- Coverage   90.29%   90.28%   -0.01%     
==========================================
  Files         759      759              
  Lines      247754   247754              
  Branches    46721    46718       -3     
==========================================
- Hits       223708   223694      -14     
+ Misses      15521    15518       -3     
- Partials     8525     8542      +17     
Files with missing lines Coverage Δ
src/node_file.cc 74.15% <ø> (-0.12%) ⬇️

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

@PickBas
PickBas force-pushed the win-rmsync-eperm-retry-test branch from 7cf7983 to ab0dc83 Compare July 24, 2026 08:40
@StefanStojanovic StefanStojanovic added the request-ci Add this label to start a Jenkins CI on a PR. label Jul 27, 2026
@github-actions github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jul 28, 2026
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@StefanStojanovic StefanStojanovic added the request-ci Add this label to start a Jenkins CI on a PR. label Aug 3, 2026
@github-actions github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Aug 3, 2026
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@StefanStojanovic StefanStojanovic added the request-ci Add this label to start a Jenkins CI on a PR. label Aug 4, 2026
@github-actions github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Aug 4, 2026
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

louiellan and others added 2 commits August 6, 2026 13:37
In the thrown exception, `std::errc::permission_denied` is already
treated as an `EPERM` error, but it is not included in one of the
omittable errors when retrying the `RmSync` operation. This commit
includes it.

This also fixes the `retryDelay` calculation on Windows where the
`retryDelay` is divided by `1000` but the win32's `Sleep` function
takes the argument as an `ms` unit, dividing the supplied `ms` unit
further to a much smaller delay.

Signed-off-by: louiellan <louie.lou.llaneta@gmail.com>
Fixes: nodejs#64016
Signed-off-by: PickBas <sayed.kirill@gmail.com>
@PickBas
PickBas force-pushed the win-rmsync-eperm-retry-test branch from ab0dc83 to abb551f Compare August 6, 2026 11:37
@StefanStojanovic StefanStojanovic added the request-ci Add this label to start a Jenkins CI on a PR. label Aug 6, 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++. fs Issues and PRs related to the fs subsystem / file system. needs-ci PRs that need a full CI run. request-ci Add this label to start a Jenkins CI on a PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Inconsistent retryDelay in fs.rmSync and asynchronous fs.rm (on Windows)

5 participants