CODEWITHSUNDEEP
javascriptnode.jsrecursiondirectoryfs
user11857517
user11857517

Reputation:

fs.readdirSync does not always return all the contents of a folder

So I have a function that is supposed to recursively return all the files in a folder, here it is:

async function getFiles(dir) {
  const subdirs = await fs.readdirSync(dir);
  const files = await Promise.all(
    subdirs.map(async (subdir) => {
      const res = resolve(dir, subdir);
      return (await stat(res)).isDirectory() && !subdir.startsWith("__")
        ? getFiles(res)
        : res;
    })
  );
  return files.reduce((a, f) => a.concat(f), files);
}

Looks great, right? Works fine too, except, not always. I'm calling it in a pretty straightforward fashion like getFiles("./directory"), and half the time, it returns all the contents. But sometimes, it will omit contents of one subdirectory, while returning all the others.

So, let's say if the given directory has 5 subdirectories, it will only return the contents of 4. This happens infrequently and if there is some underlying pattern, I am not able to detect it. Please help!

Upvotes: 1

Views: 2601

Answers (2)

jfriend00
jfriend00

Reputation: 707308

Your code is a bit misguided for a number of reasons:

  1. You're mixing synchronous file I/O calls with promises. There's no reason to use promises if your code is entirely synchronous. That just makes things more complicated than needed.
  2. It's unclear what the call to resolve(dir, subdir) is supposed to do. If you're trying to make a full path, you should be using path.join(dir, subdir).
  3. You should be using the withFileTypes option with readdir() as that saves extra roundtrips to the file system so you can just immediately check if each file is a file or directory.
  4. You don't use await with synchronous functions.

So, if you're doing a synchronous version, you can just do this:

const fs = require('fs');
const path = require('path');

function getFilesSync(dir, files = []) {
    const listing = fs.readdirSync(dir, {withFileTypes:true});
    let dirs = [];
    for (let f of listing) {
        const fullName = path.join(dir, f.name);
        if (f.isFile()) {
            files.push(fullName);
        } else if (f.isDirectory()) {
            dirs.push(fullName);
        }
    }
    for (let d of dirs) {
        getFilesSync(d, files);
    }
    return files;
}

let files = getFilesSync(somePath);
console.log(files);

If you wanted an asynchronous version using promises, then you can do this:

const fsp = require('fs').promises;
const path = require('path');

async function getFiles(dir, files = []) {
    const listing = await fsp.readdir(dir, {withFileTypes: true});
    let dirs = [];
    for (let f of listing) {
        const fullName = path.join(dir, f.name);
        if (f.isFile()) {
            files.push(fullName);
        } else if (f.isDirectory()) {
            dirs.push(fullName);
        }
    }
    for (let d of dirs) {
        await getFiles(d, files);
    }
    return files;
}

getFiles(somePath).then(files => {
    console.log(files);
}).catch(err => {
    console.log(err);
});

Note how using the fs.promises interface along with async/await allows the asynchronous version to be very, very similar to the synchronous version.

I see your code has a subdir.startsWith("__") test in it. I don't know exactly what you were trying to do with that. You can add that into the logic I have if that's required.

Upvotes: 2

Thibault Cabanes
Thibault Cabanes

Reputation: 61

I would have put this as a comment but i do not have enough reputation :s

I'm not really clear with the async / await methods for promise so i'm not really sure about what i'm saying!

So maybe an error is occuring, but you can't see it because you don't reject or catch nothing.

I guess that with the async/await methods, an error would be rejected in your const, and then you can console.log() your const to see if, when your function omit some files, it's not because of an error that occured!

And for your last await, you put it in a return, it would be interesting to console.log() it too.

/////////////////////edited later ////////////////////////////////////////

https://javascript.info/async-await

In real situations, the promise may take some time before it rejects. In that case there will be a delay before await throws an error. We can catch that error using try..catch, the same way as a regular throw:


  try {
    let response = await fetch('http://no-such-url');
  } catch(err) {
    alert(err); // TypeError: failed to fetch
  }
}

Upvotes: 0

Related Questions