javascript - Why node.js `fs.existsSync` doesn't work well when wrapped in promise? - Stack Overflow

admin管理员组

文章数量:1352150

I'm writing a function createFile to create a file in a directory unless it already exists. I'm using Node.js native fs package to do all file operations. I wanted to make my function asynchronous so I wrapped all fs functions in promises:

function writeFilePromise(writePath, textContent) {
    return new Promise((resolve, reject) => {
      fs.writeFile(writePath, textContent, (err) => {
        reject();
      });
      resolve();
    });
  }

  function mkDirPromise(dir) {
    return new Promise(((resolve, reject) => {
      fs.mkdir(path.join(constants.FILES_STORAGE_DIR, dir), (err) => {
        reject(err);
      });
      resolve();
    }));
  }

Then I wanted to also wrap fs.existsSync in promise in order to plete my function but wrapping it resulted in occasional incorrect behavior, namely, if a directory for the file didn't exist and I wanted to create one the directory would be created empty without the file. Through debugging I found that only synchronous fs.existsSync would work always. This the function code:

function createFile(dir, fileName, httpMethod, textContent) {
    return new Promise(((resolve, reject) => {
      const searchPath = path.join(ROOT_DIR, dir, fileName);
      if (httpMethod === POST && fs.existsSync(searchPath)) {
        reject();
      } else {
        const fileExistsStatus = fs.existsSync(path.join(ROOT_DIR, dir));
        (async function fsOperations() {
          try {
            if (!fileExistsStatus) {
              await mkDirPromise(dir);
            }
            await writeFilePromise(searchPath, textContent);
            resolve();
          } catch (err) {
            reject(err);
          }
        }());
      }
    }));
  }

What am I missing and how can I turn my function into truly asynchronous?

I'm writing a function createFile to create a file in a directory unless it already exists. I'm using Node.js native fs package to do all file operations. I wanted to make my function asynchronous so I wrapped all fs functions in promises:

function writeFilePromise(writePath, textContent) {
    return new Promise((resolve, reject) => {
      fs.writeFile(writePath, textContent, (err) => {
        reject();
      });
      resolve();
    });
  }

  function mkDirPromise(dir) {
    return new Promise(((resolve, reject) => {
      fs.mkdir(path.join(constants.FILES_STORAGE_DIR, dir), (err) => {
        reject(err);
      });
      resolve();
    }));
  }

Then I wanted to also wrap fs.existsSync in promise in order to plete my function but wrapping it resulted in occasional incorrect behavior, namely, if a directory for the file didn't exist and I wanted to create one the directory would be created empty without the file. Through debugging I found that only synchronous fs.existsSync would work always. This the function code:

function createFile(dir, fileName, httpMethod, textContent) {
    return new Promise(((resolve, reject) => {
      const searchPath = path.join(ROOT_DIR, dir, fileName);
      if (httpMethod === POST && fs.existsSync(searchPath)) {
        reject();
      } else {
        const fileExistsStatus = fs.existsSync(path.join(ROOT_DIR, dir));
        (async function fsOperations() {
          try {
            if (!fileExistsStatus) {
              await mkDirPromise(dir);
            }
            await writeFilePromise(searchPath, textContent);
            resolve();
          } catch (err) {
            reject(err);
          }
        }());
      }
    }));
  }

What am I missing and how can I turn my function into truly asynchronous?

• javascript
• node.js
• asynchronous
• fs

Share Improve this question Follow asked Jun 8, 2018 at 20:14 YosYos 30155 silver badges1616 bronze badges 4

• Your question is confusing. I think you could make this clearer. I think that you are wanting to use fs.exists instead of fs.existsSync, but you've used the existsSync throughout your entire post making no reference to the async version. Can you update your post to include the code sample that isn't working? – Gary Commented Jun 8, 2018 at 20:42
• @Gary fs.exists is deprecated. – Estus Flask Commented Jun 8, 2018 at 20:43
• 2 You really shouldn't use either one. Just trying to read/write from the file and handle the errors. The problem with exists is that the value you get back might be wrong by the time that you try to use it (things can edit the files other than your app). (edited to remove incorrect claim about existsSync being deprecated) – Gary Commented Jun 8, 2018 at 20:44
• @Gary Checking that a file exists seems ok to me, because there can be numerous reason why read/write fails. Yes, that's a good point about exists, existsSync has less chance to fail. Any way, the problem here is incorrect promisification. – Estus Flask Commented Jun 8, 2018 at 20:51

Add a ment  | 

3 Answers 3

Sorted by: Reset to default 5

occasional incorrect behavior, namely, if a directory for the file didn't exist and I wanted to create one the directory would be created empty without the file

This may be caused by incorrect implementations of writeFilePromise and especially mkDirPromise. fs.writeFile and fs.mkdir are asynchronous but a promise is resolved synchronously. It should be:

  function writeFilePromise(writePath, textContent) {
    return new Promise((resolve, reject) => {
      fs.writeFile(writePath, textContent, (err) => {
        if (err)
          reject(err);
        else
          resolve();
      });
    });
  }

  function mkDirPromise(dir) {
    return new Promise(((resolve, reject) => {
      fs.mkdir(path.join(constants.FILES_STORAGE_DIR, dir), (err) => {
        if (err)
          reject(err);
        else
          resolve();
      });
    }));
  }

This is what util.promisify is for:

const writeFilePromise = util.promisify(fs.writeFile);

Even then this is a wheel reinvention, because there are already third-party packages that do this and even more, namely fs-extra.

createFile has poor control flow and makes use of promise construction antipattern. Since it uses async..await, it should be:

async function createFile(dir, fileName, httpMethod, textContent) {
  const searchPath = path.join(ROOT_DIR, dir, fileName);
  if (httpMethod === POST && fs.existsSync(searchPath)) {
    throw new Error();
  } else {
    const fileExistsStatus = fs.existsSync(path.join(ROOT_DIR, dir));
    if (!fileExistsStatus) {
      await mkDirPromise(dir);
    }
    await writeFilePromise(searchPath, textContent);
  }
}

It should be mentioned that existsSync is stable API method, it's acceptable to use it to check if a file exists. As the documentation states,

Note that fs.exists() is deprecated, but fs.existsSync() is not. (The callback parameter to fs.exists() accepts parameters that are inconsistent with other Node.js callbacks. fs.existsSync() does not use a callback.)

First of all, you've structured writeFilePromise and mkDirPromise so that they will always resolve, and never reject. Since fs.writeFile and fs.mkdir are asynchronous, the thread immediately moves on to resolve() once they've kicked off. I think what you meant was...

function writeFilePromise(writePath, textContent) {
    return new Promise((resolve, reject) => {
        fs.writeFile(writePath, textContent, (err) => {
            if (err) reject();
            else resolve();
        });
    });
}

function mkDirPromise(dir) {
    return new Promise((resolve, reject) => {
        fs.mkdir(path.join(constants.FILES_STORAGE_DIR, dir), (err) => {
            if (err) reject();
            else resolve();
        });
    });
}

In terms of fs.exists, that's been depricated, so I wouldn't remend using it. Instead, try fs.access:

function accessPromise(dir) {
    return new Promise((resolve, reject) => {
        fs.access(dir, (err) => {
            if (err) reject();
            else resolve();
        });
    });
}

Finally, try adjusting where you use the async function declaration, to ensure you're synchronizing your code correctly:

async function createFile(dir, fileName, httpMethod, textContent) {
    const searchPath = path.join(ROOT_DIR, dir, fileName);
    if (httpMethod === POST && await accessPromise(searchPath)) {
        return false;
    } else {
        const fileExistsStatus = await accessPromise(path.join(ROOT_DIR, dir));
        try {
            if (!fileExistsStatus) {
                await mkDirPromise(dir);
            }
            await writeFilePromise(searchPath, textContent);
            return true;
        } catch (err) {
            return false;
        }
    }
}

Remember to use await createFile(dir, fileName, httpMethod, textContent) when calling that function.

First, consider rejecting with something meaningful instead of just reject()

Since you are thinking with async and promise, I don't remend using fs.xxxSync() functions. Also, fs.exists is deprecated, try using fs.stat().

I guess you will only create the file if the HTTP method is POST, but the file will always be created when HTTP method is not POST in the current if-else logic.

It is unnecessary to create an immediately-invoked async function.

Try this:

function createFile(dir, fileName, httpMethod, textContent) {
    return new Promise((resolve, reject) => {
        const searchPath = path.join(ROOT_DIR, dir, fileName);
        if (httpMethod !== POST) {
            return reject(new Error('Invalid HTTP method'));
        }
        fs.exists(searchPath, (exists) => {
            if (exists) {
                return reject(new Error('Already exists'));
            }
            fs.exists(path.join(ROOT_DIR, dir), async (exists) => {
                try {
                    if (!exists) {
                        await mkDirPromise(dir);
                    }
                    await writeFilePromise(searchPath, textContent);
                    resolve();
                } catch (err) {
                    reject(err);
                }
            });
        });
    });
}

本文标签： javascriptWhy nodejs fsexistsSync doesn39t work well when wrapped in promiseStack Overflow