soldair / node-walkdir

Walk a directory tree emitting events based on the contents. API compatable with node-findit. Walk a tree of any depth. Fast! Handles permission errors. Stoppable. windows support. Pull requests are awesome. watchers are appreciated.
MIT License
130 stars 22 forks source link

Walkdir doesn't emit 'error' event in case fs.lstat fail #10

Closed IL55 closed 9 years ago

IL55 commented 11 years ago

Hi, I wrote small test which (I expect) should pass OK. The idea is to fail fs.lstat function, and 'error' callback should be called But I see next:

mocha ./specs Error: Uncaught, unspecified 'error' event.

var sinon = require('sinon'); var chai = require("chai"); var should = chai.Should(); var chaiAsPromised = require("chai-as-promised"); chai.use(chaiAsPromised); var walk = require('walkdir'); var fs = require('fs'); .....

it.only("Check walkdir", function (done) {

sinon.stub(fs, "lstat", function (filename, callback) {
  var errENOENT = {
    errno: 34,
    code: "ENOENT",
    description: "no such file or directory"
  };
  callback(errENOENT, null);
});

var emitter = walk("/", { "no_recurse": true });

emitter.on('error', function (path, error) {
  error.should.exist;
});

emitter.on('directory', function (dir, stat) {
});

emitter.on('end', function (path, stat) {
  done();
});

});

soldair commented 11 years ago

hey thanks so much for the test. I don't think that this is a realistic case, though an error is an error and i say we think about this together.

The whole point is to walk a directory potentially recursively and emit events for paths that exist. E_NOENT is something that doesn't exist. This error should never be emitted by something that is finding things. I'm not not finding things ;)

To create this case you would have to delete a file in between the readdir and the lstat. this can happen but it should not break the walk. it doesn't exist it shouldn't be found and is not an error.

What if i did not break and retry on errors related to the file system being too busy? this would be an error that i should at certainly fail for. My library cannot ensure its contract of finding all of the paths below target directory. Only after the contract is ensured (to make this library better) I would need to implement some polling for example while i wait for the fs to stop being busy and fail after configurable timeout or attempts.

if you want to pull request failing tests for such things i use "tap" as my test harness