fs: fix parentPath returned from getDirent() when type is UV_DIRENT_UNKNOWN

[LiviaMedeiros]

The parentPath must be a path to the parent directory rather than path to the file itself.

/cc @Ethan-Arrowood @nodejs/fs

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 88.42%. Comparing base (b02cd41) to head (9613c19).
Report is 8 commits behind head on main.

Additional details and impacted files

@@ Coverage Diff @@## main #55553 +/- ##
==========================================
- Coverage 88.42% 88.42% -0.01% 
==========================================
Files 654 654 Lines 187852 187852 Branches 36134 36135 +1 ==========================================
- Hits 166102 166101 -1 - Misses 14989 14991 +2 + Partials 6761 6760 -1 

Files with missing lines	Coverage Δ
lib/internal/fs/utils.js	99.69% <100.00%> (ø)

... and 35 files with indirect coverage changes

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/63296/

[aduh95]

Let's wait for #55548 to land first: since there's no other test failure, it seems plausible that this change does not have any impact on the public API, it might also be due to a lack of coverage, so better safe than sorry.

[LiviaMedeiros]

This is not directly connected to the deprecated .path property. This PR fixes .parentPath, while .path just happens to return same value.
AFAICT this issue is facing public API, but it's hard to reproduce: it must be withFileTypes: true, it must be async, and most importantly, libuv must report UV_DIRENT_UNKNOWN. I have no idea how to robustly make it return UV_DIRENT_UNKNOWN type (assigning invalid type using debugfs didn't work for me). It also explains why this bug was never caught: if someone encountered corrupted files, they probably had more important and urgent problems to investigate than wrong .parentPath value.

If anyone knows how to test this on CI without exposing internals, I'd be happy to increase coverage here.

Landing this after #55548 (which is semver-major and will change tests for .path property) is likely to make backporting this PR to previous release lines necessary. But given that the bug is hard to encounter, and that test in #55547 is rewritten using sync call and can be landed without this, I'm fine with it.

[aduh95]

#55548 […] will change tests for .path property

It should remove them yes

Landing this after #55548 […] is likely to make backporting this PR to previous release lines necessary

Hum no, I would say we should not backport it, or at least not unless we find a way to not interfere with the .path property

[aduh95]

#55548 […] will change tests for .path property

It should remove them yes

Landing this after #55548 […] is likely to make backporting this PR to previous release lines necessary

Hum no, I would say we should not backport it, or at least not unless we find a way to not interfere with the .path property

[LiviaMedeiros]

Rebased on top of #55548 ( semver-majorPRs that contain breaking changes and should be released in the next major version. ), hence the dont-land labels.

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/63553/

[nodejs-github-bot]

Landed in c91ce21