fshost / node-dir

Recursive asynchronous file and directory operations for Node.js
MIT License
221 stars 44 forks source link

dir.files Error #5

Closed Roland1975 closed 11 years ago

Roland1975 commented 11 years ago

Using dir.files, I got the following error:

Error: ENOENT, stat '/path/node_modules/.bin/_mocha'

.bin is an invisible folder ; some folder and files should be excluded by default.

fshost commented 11 years ago

The behavior of methods in this module are meant to be low-level and based on the behavior of the methods in node 's fs module. If fs.stat, fs.readFile, etc pass an error in a given scenario then node-dir should also - this keeps it predictable for node devs accustomed to working with fs methods and does not introduce unexpected behavior for system tooling e.g. build systems, git utilities, etc. Something that adds in that kind of opinionated and platform-biased behavior by default probably belongs at a higher level than this module.

However, I have no problem in introducing it as optional behavior, so I have added two new options to the readFiles method, matchDir and excludeDir, to help make handling this kind of scenario as painlessly as possible if you need to. They accept regex patterns and work the same way that the match and exclude options do, except for directory names rather than file names. This makes ignoring directories pretty painless, e.g.

// to ignore directories prefixed with '.', just change this 
dir.readFiles('mydir ', fileCb, finishedCb)

// to this
dir.readFiles('mydir ', { excludeDir: /^\./ }, fileCb, finishedCb)

You should also now be able to check for things like permissions errors in the finished callback and handle that situation appropriately to whatever your use-case may be as I updated the code to ensure any errors passed in from the core node methods get passed to the finished callback.

If that doesn't cut it you can always fork and change it to your liking, wrap it in your own module, or just add a new method like so

// method to process files for visible folders (linux & os x)
dir.readVisibleFiles = function(filepath, options, cb, done) {
    if (typeof options === 'function') {
        done = cb;
        cb = options;
        options = {};
    }
    options.excludeDir = options.excludeDir || /^\./;
    dir.readFiles(filepath, options, cb, done);
};

// try it out - dot-prefixed directories will be excluded by default
dir.readVisibleFiles('myfile', fileCb, doneCb)

So far node-dir has been serving me quite well in the projects I'm using it in, but if anyone wants to add something like the above or other directory/fs related methods to this module I have no problem merging PRs so long as they do not unexpectedly modify existing methods, are documented in the readme, and appropriate mocha/chai test(s) are added to the tests.js file.

Thanks for your input.

Roland1975 commented 11 years ago

Thanks for your willingness to fix the issue :) Actually, I'm using a simpler algorithm I found on stackoverflow and the code returns no error when it parses the "faulty directory" that thrown the exception reported above; I provide it for reference. That's curious as I don't see the reason why it doesn't thrown the error.

      function fetch(dir, done) {
        var me = this;
        var walk = function(dir, done) {
            var results = [];
            fs.readdir(dir, function(err, list) {
                if (err) return done(err);
                var pending = list.length;
                if (!pending) 
                    return done(null, results);

                list.forEach(function(file) {
                    file = path.join(dir,file);
                    fs.stat(file, function(err, stat) {
                        if (stat && stat.isDirectory()) {
                            walk(file, function(err, res) {
                                results = results.concat(res);
                                if (!--pending) done(null, results);
                            });
                        } else {
                                                    results.push(file);
                            if (!--pending) 
                                done(null, results);
                        }
                    });
                });
            });
        }
        walk(dir, done);
    }  
fshost commented 11 years ago

The reason your not seeing an error with the above code is because it is using methods that don't throw errors, they pass them to the callback, but that code is not checking the err argument for fs.stat or even in the callback from its own recursion. It looks like the only possible error that could get caught and passed to the done callback is for the first readdir invocation of the root directory.

Also, its not invoking fs.readFile on each file, whereas node-dir's readFiles method does.

Try it with code added to check the err argument of the stat and walk callbacks, and with code to read each file - most likely one of those will cause the error to show up. Just make sure to check for it in your done callback as it won't be thrown directly.

If its unclear what I mean, below is the same code segment but with checks for all of the err arguments, and that invokes readFile. Do not use it in a real implementation though - the error handling is still naive and it could result in strange behavior like the done callback being invoked multiple times, besides which I haven't tested it, there may be a typo or something - its mainly to just illustrate where the missing error checks should be and how to mimic node-dir's readFiles method by invoking readFile in the code sample. I've put comments above where those lines have been added:

function fetch(dir, done) {
    var me = this;
    var walk = function(dir, done) {
        var results = [];
        fs.readdir(dir, function(err, list) {
            if (err) return done(err);
            var pending = list.length;
            if (!pending)
                return done(null, results);

            list.forEach(function(file) {
                file = path.join(dir, file);
                fs.stat(file, function(err, stat) {

                    // added err check below
                    if (err) return done(err);

                    if (stat && stat.isDirectory()) {
                        walk(file, function(err, res) {

                            // added err check below
                            if (err) return done(err);

                            results = results.concat(res);
                            if (!--pending) done(null, results);
                        });
                    } else {

                        // added code below to read the contents of each file
                        fs.readFile(file, function (err, content) {

                            // added err check below
                            if (err) return done(err);

                            // you could do something with file's content here

                        });

                        results.push(file);
                        if (!--pending)
                            done(null, results);
                    }
                });
            });
        });
    }
    walk(dir, done);
}

// invoke it with whatever dir path you want, represented by 'dirpath' below
fetch(dirpath, function (err, results) {
    // check for errors
    if (err) throw err;
});

If the above or something similar to it still doesn't throw the error and node-dir's readFile method does (assuming of course that the environment is exactly the same for both - same dir, same permissions, etc), then that would be very strange.

Btw, if you actually don't care about reading file contents as you recurse through them, you can use node-dir's files method - it provides basically the same functionality as your code sample.