chaining promises in functions

Question

I have a small problem, how to create a promise chain in a sensible way so that the makeZip function will first add all the necessary files, then create the zip, and finally delete the previously added files? (The makeZip function also has to return a promise). In the example below I don't call deleteFile anywhere because I don't know exactly where to call it. when I tried to call it inside the add file function to delete the file immediately after adding it, for some unknown reason the console displayed the zip maked! log first and then file deleted.

const deleteFile = (file, result) => {
  new Promise((resolve, reject) => {
    fs.unlink(`./screenshots/${file}`, (err) => {
      if (err) return reject(err);
      console.log(`${file} deleted!`);
      return resolve();
    });
  });
};

const addFile = (file) => {
  new Promise((resolve, reject) => {
    try {
      zip.addLocalFile(`./screenshots/${file}`);
      console.log(`${file} added`);
      return resolve();
    } catch {
      return reject(new Error("failed to add file"));
    }
  });
};

const makeZip = () => {
  Promise.all(fs.readdirSync("./screenshots").map((file) => addFile(file)))
    .then(() => {
      return new Promise((resolve, reject) => {
        try {
          zip.writeZip(`./zip_files/supername.zip`);
          console.log("zip maked!");
          resolve();
        } catch {
          return reject(new Error("failed making zip"));
        }
      });
    })
    .catch((err) => console.log(err));
};

I think your program is invalid, Why `addFile()` return promise object ? Is `zip.addLocalFile()` and `zip.writeZip()` asynchronous ? If so, Did you forget to `await` before `zip.addLocalFile()` and `zip.writeZip()` ? — Nur, May 03 '21 at 09:52
Are `zip.addLocalFile` and `zip.writeZip` actually asynchronous? If yes, you [promisified them wrong](https://stackoverflow.com/q/22519784/1048572) (resolving immediately after the call, not when they finished - they should take a callback?), if no, you shouldn't need to wrap them in `new Promise` at all. — Bergi, May 03 '21 at 09:56
Even he (@Vasper) forget to return promise from every function — Nur, May 03 '21 at 09:57

score 2 · Answer 1 · answered May 03 '21 at 10:12

the main cause of this is that you are not returning the promises you are instantiating in your function calls. Also I have some cool suggestion to make that can improve you code cleanliness.

[TIP]: Ever checked the promisify function in NodeJS util package, it comes with node and it is very convenient for converting functions that require callbacks as arguments into promise returning functions., I will demonstrate below anyhow.

// so I will work with one function because the problem resonates with the rest, so
// let us look at the add file function.

// so let us get the promisify function first
const promisify = require('util').promisify;

const addFile = (file) => {
  // if addLocalFile is async then you can just return it
  return zip.addLocalFile(`./screenshots/${file}`);
};

// okay so here is the promisify example, realized it wasn't applicable int the function
// above
const deleteFile = (file, result) => {
  // so we will return here a. So because the function fs.unlink, takes a second arg that
  // is a callback we can use promisify to convert the function into a promise
  // returning function.
  return promisify(fs.unlink)(`./screenshots/${file}`);
  // so from there you can do your error handling.
};

So now let us put it all together in your last function, that is, makeZip

const makeZip = () => {
  // good call on this, very interesting.
  Promise.all(fs.readdirSync("./screenshots").map((file) => addFile(file)))
    .then(() => {
      return zip.writeZip(`./zip_files/supername.zip`);
    })
    .then(() => {
      //... in here you can then unlink your files.
    });
    .catch((err) => console.log(err));
};

Everything should be good with these suggestions, hope it works out...

I would just promisify `unlink` only once on top - `const unlink = promisify(fs.unlink);` — moonwave99, May 03 '21 at 10:43

score -1 · Answer 2 · answered May 03 '21 at 10:35

Thank you all for the hints, the solution turned out to be much simpler, just use the fs.unlinkSync method instead of the asynchronous fs.unlink.

const deleteFile = (file) => {
  try {
    fs.unlinkSync(`./screenshots/${file}`);
    console.log(`${file} removed`);
  } catch (err) {
    console.error(err);
  }
};

const addFile = (file) => {
  try {
    zip.addLocalFile(`./screenshots/${file}`);
    console.log(`${file} added`);
    deleteFile(file);
  } catch (err) {
    console.error(err);
  }
};

const makeZip = () => {
  fs.readdirSync("./screenshots").map((file) => addFile(file));
  zip.writeZip(`./zip_files/supername.zip`);
  console.log("zip maked!");
};

chaining promises in functions

2 Answers2