fs: prevent multiple invocations of callback in Dir class

[0hmX]

Refs: #58419

I am only implementing this for the Dir class because I can create a case where it produces an error.

const fs = require('fs');

(async () => {
  const dir = await fs.promises.opendir(__dirname);
  let callCount = 0;
  // Monkey-patch the callback to throw on the first call
  function testCallback(err, dirent) {
    callCount++;
    if (callCount === 1) {
      console.log('First callback, throwing error!');
      throw new Error('Callback throws!');
    } else {
      console.log('Second callback:', err);
    }
  }
  // Call the internal read method directly (for demonstration)
  dir.read(testCallback);
})();

[codecov]

Codecov Report

Attention: Patch coverage is 72.72727% with 3 lines in your changes missing coverage. Please review.

Project coverage is 90.20%. Comparing base (264cad7) to head (4a99cc2).
Report is 126 commits behind head on main.

Files with missing lines	Patch %	Lines
lib/internal/fs/dir.js	72.72%	3 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #58430      +/-   ##
==========================================
+ Coverage   90.18%   90.20%   +0.02%     
==========================================
  Files         629      635       +6     
  Lines      186657   187225     +568     
  Branches    36658    36761     +103     
==========================================
+ Hits       168328   168891     +563     
+ Misses      11126    11095      -31     
- Partials     7203     7239      +36     

Files with missing lines	Coverage Δ
lib/internal/fs/dir.js	94.47% <72.72%> (-0.59%)	⬇️

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

[LiviaMedeiros]

I am unsure if we need to add a test for this, as this bug is not a feature. I have added a test that I was using for development. Will remove it if we do not need it.

We definitely do need a test for this: to ensure that the bug is fixed, and to ensure that it doesn't appear again after changes in the future.

[0hmX]

@LiviaMedeiros have added the test

[LiviaMedeiros]

Thanks for working on this! Unfortunately, it might be a bit harder to fix this issue.

[LiviaMedeiros]

This is not correct: fs.Dir should not catch and suppress errors from user-provided callback, this error must bubble up.

The test shows that the simulated error disappears on its way, probably due to how FSReqCallback's oncomplete is handled on C++ side.

We might need to either investigate the cause and make errors inside oncomplete be thrown, or detach the callback call from it (e.g. using queueMicrotask).