fs: fix rmSync error messages for non-ASCII paths

[Yeaseen]

This PR fixes incorrect and inconsistent error messages produced by fs.rmSync when deletion fails, especially for directories containing non-ASCII characters. The issue affected Linux and Windows differently but shared the same root cause: paths were embedded into custom error messages while also being passed separately to ThrowErrnoException.

1. Duplicate paths in error messages (Linux, ASCII paths)

fs.rmSync constructed messages like:

std::string message = "Permission denied: " + file_path_str;
env->ThrowErrnoException(err, "rm", message.c_str(), path_c_str);

Because ThrowErrnoException automatically appends the path, this resulted in duplicated paths when directory names were ASCII-only:

err.path: /tmp/_dir_0
err.message: EACCES, Permission denied: /tmp/_dir_0 '/tmp/_dir_0'

2. Corrupted paths for non-ASCII directory names (Linux)

When the directory name contained non-ASCII characters, embedding the path into the message caused UTF-8 bytes to be misinterpreted.

Example:

err.path: /tmp/速速速_dir
err.message: EACCES, Permission denied: '/tmp/é速速_dir'

Here, the first byte of the UTF-8 sequence was interpreted as a Latin-1 character (é), corrupting the path shown in the error message.

3. Inconsistent Windows paths (?\ prefix)

On Windows, err.path could expose raw extended-length paths prefixed with \\?\\, because the platform-specific normalization logic (StringFromPath) was not applied when constructing ErrnoException.
Example:

err.path: \\?\\C:\\temp\\速速速_dir

This is inconsistent with other fs APIs used in src/api/exceptions, which strip the prefix before exposing paths to JavaScript.

This PR fixes the issue by:

• Removing path concatenation from custom error messages
• Passing the path only via the path argument to ThrowErrnoException
• Relying on Node’s standard error formatting to attach paths safely
• Applying StringFromPath in src/api/exceptions for Windows so err.path is normalized correctly

A new test has been added: test-fs-rmSync-special-char-additional-error.js to verify that:

• fs.rmSync reports the correct error code
• err.path preserves non-ASCII directory names
• err.message includes the correct path
• No path corruption or duplication occurs

@joyeecheung @anonrig Please review this when you are available.

[codecov]

Codecov Report

❌ Patch coverage is 76.92308% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.14%. Comparing base (f96dccc) to head (de31cdd).
⚠️ Report is 4 commits behind head on main.

Files with missing lines	Patch %	Lines
src/node_file.cc	40.00%	3 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #61233      +/-   ##
==========================================
- Coverage   90.15%   90.14%   -0.02%     
==========================================
  Files         751      751              
  Lines      253443   253468      +25     
  Branches    47729    47737       +8     
==========================================
- Hits       228496   228493       -3     
- Misses      16201    16253      +52     
+ Partials     8746     8722      -24     

Files with missing lines	Coverage Δ
src/api/exceptions.cc	94.38% <100.00%> (+0.19%)	⬆️
src/node_file.cc	73.97% <40.00%> (+0.10%)	⬆️

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

[LucasmDjuiw]

8,5/10.

Ce document est clair, techniquement précis et facile à comprendre. Sa structure est fluide et explique le problème et sa solution sans détails superflus. Pour atteindre 9 ou 10, il faudrait simplifier légèrement quelques Casino Frumzi phrases ou ajouter une courte phrase de conclusion soulignant l'impact pour les utilisateurs ou les responsables de la maintenance.

[joyeecheung]

Code LGTM, though I am a bit skeptical about the tests - it may not fare well on Windows in the CI or in people's local machines to change the permission this way. Can we use other safer error paths to check it instead? Perhaps the directory not empty one?

[Yeaseen]

@joyeecheung thanks for the feedback. I agree. I updated the test case for hitting NOT A DIRECTORY error. Let's see if it passes CI.

[Yeaseen]

@joyeecheung looks good so far. The failing test on mac is unrelated to my changes. I appreciate a rerun. Thanks.

[Yeaseen]

@joyeecheung CI is green now — would appreciate your review.

[Yeaseen]

@joyeecheung @anonrig @jazelly Please review this.

[github-actions]

This pull request has been marked as stale due to 90 days of inactivity.
It will be automatically closed in 30 days if no further activity occurs. If this is still relevant, please leave a comment or update it to keep it open.

[Yeaseen]

I am just marking this as bugs in rmSync error messages. If anybody thinks it's worth fixing, I'll be happy to update this PR.

[trivikr]

This PR needs a rebase.

[trivikr]

This fails in validateRmOptionsSync() during lstatSync, so the exception has syscall: 'lstat' and never exercises the modified native rm path. Please use a fixture that reaches the removal failure, then use assert.throws() and assert err.syscall === 'rm'; checking the path occurs exactly once would also cover the duplication regression.

-try { /* ... */ } catch (err) { /* ... */ }
+assert.throws(() => { /* ... */ }, (err) => {
+  assert.strictEqual(err.syscall, 'rm');
+  // Assert the full message, or that the path occurs exactly once.
+  return true;
+});

[Yeaseen]

Thanks, and updated. I kept the permission-denied fixture because it reproduces the original rmSync failure, but added the syscall and path-occurrence assertions so the test now proves it reaches the native rm path.