nodejs
fs: fix TypeError in glob when directory access is denied
When a directory cannot be read due to permission issues, the async version of fs.glob() returns null from readdir(), while the sync version returns an empty array. This causes a TypeError when trying to access the 'length' property of null.
Fix by making the async readdir() method return an empty array on error, consistent with the sync version behavior.
Fixes: https://github.com/nodejs/node/issues/58670 Fixes: https://github.com/nodejs/node/issues/58276
This is nice! Thanks! Would you mind adding a test case? Just make sure that it will never return null
Codecov Report
:white_check_mark: All modified and coverable lines are covered by tests.
:white_check_mark: Project coverage is 89.99%. Comparing base (5ebfb99) to head (59b82bb).
:warning: Report is 6 commits behind head on main.
Additional details and impacted files
@@ Coverage Diff @@
## main #58674 +/- ##
==========================================
+ Coverage 89.97% 89.99% +0.02%
==========================================
Files 649 649
Lines 192194 192194
Branches 37678 37683 +5
==========================================
+ Hits 172918 172958 +40
+ Misses 11873 11836 -37
+ Partials 7403 7400 -3
| Files with missing lines | Coverage Ξ | |
|---|---|---|
| lib/internal/fs/glob.js | 91.98% <100.00%> (ΓΈ) |
:rocket: New features to boost your workflow:
- :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
- :package: JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
![codecov[bot] avatar](https://avatars.githubusercontent.com/in/254?v=4 "Published by a pub.dev verified publisher"){kind=small}
Delete the test-fs-glob-readdir-error-handling.mjs file as its functionality is now covered by the test-fs-glob.mjs
I think you need to stop force-pushing so it can land. Another contributor can correct me if I'm wrong though
Sorry for the force-push. Could you please help re-approve this PR so it can land? Thank you
Failed to start CI
β Commits were pushed since the last approving review: β - fs: fix glob TypeError on restricted dirs β - Merge branch 'nodejs:main' into main β Refusing to run CI on potentially unsafe PRhttps://github.com/nodejs/node/actions/runs/16320742288
![github-actions[bot] avatar](https://avatars.githubusercontent.com/in/15368?v=4 "Published by a pub.dev verified publisher"){kind=small}
It seems like the CI did not run because new commits were pushed after the last approving review?
The last CI failures ultimately point to node-test-commit-arm-debug node-test-binary-windows-js-suites The first one had network issues when cloning the git repository, the second one had ECONNRESET error during inspector testing. This might be coincidental since there have never been problems before. PR #59091 explains the why Windows CI failure in this PR.
Another CI node-test-commit-aix failure: ENOSPC: System limit for number of file watchers reached Looks like a coincidental issue again
@Sylphy-0xd3ac It seems your first commit doesn't have the correct author. Can you squash all your 10 commits and make sure to --reset-author?
@RafaelGSS I did it, is that it?
Yes! Once CI is green (again), PR will be merged.
