Errors thrown inside .filter() are swallowed

[peterbe]

Sample code to reproduce:

const { fdir } = require("fdir");

const api = new fdir()
  .withFullPaths()
  .filter((filePath) => {
    if (Math.random() > 0.5) {
      throw new Error("Oh crap!");
    }
    return true;
  })
  .crawl("./client");
console.log(api.sync().length);

You can run this over and over and it will just console log out a different number each time.

Suppose it wasn't if (Math.random() > 0.5) { but something like return doubleCheck(fiePath) you wouldn't get any feedback about the accidental typo of fiePath.

[peterbe]

A nasty workaround would be:

.filter((filePath) => {
  try {
     ...whatever code you wanted to run...
  } catch (er) {
    // See bug https://github.com/thecodrr/fdir/issues/56
    console.error(er); 
    throw er;
  }
})

[thecodrr]

I am taking a look into this right now.

[thecodrr]

@peterbe have you tried this with .withErrors? fdir by default ignores all errors thrown during crawling. You can turn on errors with .withErrors to have them passed on to the caller. This will require you to handle (or ignore) IO errors as well.

[peterbe]

@peterbe have you tried this with .withErrors? fdir by default ignores all errors thrown during crawling. You can turn on errors with .withErrors to have them passed on to the caller. This will require you to handle (or ignore) IO errors as well.

I failed to notice that was an option. Perhaps it's too late now, but I wish it'd be the other way around. I.e. errors raised by default with an option to .suppressErrors(). If an expert user is perfectly aware of the options it doesn't matter, but newbies might miss it and then accidentally get super frustrated in debugging and doing their own try/catch to log the possible errors.

[peterbe]

Perhaps remove the bug label and add a "annoying refactor request" :)

[thecodrr]

errors raised by default with an option to .suppressErrors()

That is actually how it was in previous versions. My reasoning for going the opposite way was that most of the times IO errors are pretty useless (like file not found etc.) during crawling. The downside of current approach is that it captures and ignores all errors including syntax errors (like you reported). Maybe it should only handle IO errors thrown by the fs module and rethrow all other exceptions?