Save File on server before sending response to frontend

Question

I am using the following code to first download a file, and then send a response to the client:

async function download(uri, filename, callback) {
  request.head(uri, async function (err, res, body) {
    console.log("content-type:", res.headers["content-type"]);
    console.log("content-length:", res.headers["content-length"]);

    request(uri).pipe(fs.createWriteStream(filename)).on("close", callback);
  });
}

async function ytsSearch(name) {
  const movies = await yts.listMovies({ limit: 3, query_term: name });
  return movies;
}

async function searchAndDownload(name) {
  movies = await ytsSearch(name);
  movies.movies.forEach(async function (movie) {
    if (
      fs.existsSync(
        path.join(__dirname, "covers", movie.title_english + ".png")
      )
    ) {
      return;
    } else {
      await download(
        movie.medium_cover_image,
        `covers/${movie.title_english}.png`,
        function () {
          console.log("done");
        }
      );
    }
  });
  return movies;
}

app.use("/movie", function (req, res) {
  const { name } = req.body;
  searchAndDownload(name).then((movies) => console.log(movies));
});

However, On the client side, The response is received first, before the file is saved on the server. I know I am probably doing something wrong with so many asynchronous functions, but can't seem to find out what exactly is going wrong.

Output:

As you can see, the response is sent first, and the picture is downloaded second.
Minimal Reproducible Code:

const express = require("express");
const app = express();
const http = require("http");
const request = require("request");
const server = http.createServer(app);
const cors = require("cors");
const yts = require("yts");
const fs = require("fs");
const path = require("path");

const port =5000

async function download(uri, filename, callback) {
  request.head(uri, async function (err, res, body) {
    console.log("content-type:", res.headers["content-type"]);
    console.log("content-length:", res.headers["content-length"]);

    request(uri).pipe(fs.createWriteStream(filename)).on("close", callback);
  });
}

async function ytsSearch(name) {
  const movies = await yts.listMovies({ limit: 3, query_term: name });
  return movies;
}

async function searchAndDownload() {
  movies = await ytsSearch("minions");
  movies.movies.forEach(async function (movie) {
    if (
      fs.existsSync(
        path.join(__dirname, "covers", movie.title_english + ".png")
      )
    ) {
      return;
    } else {
      await download(
        movie.medium_cover_image,
        `covers/${movie.title_english}.png`,
        function () {
          console.log("done");
        }
      );
    }
  });
  return movies;
}

server.listen(port, () => {
  searchAndDownload().then((movies)=>console.log(movies))
});

package.json

{
  "name": "torparty",
  "version": "1.0.0",
  "description": "Group Torrent Streaming",
  "main": "index.js",
  "scripts": {
    "test": "echo \"Error: no test specified\" && exit 1",
    "start": "nodemon index.js"
  },
  "repository": {
    "type": "git",
    "url": "git+https://github.com/diivi/torparty.git"
  },
  "author": "Divyansh Singh",
  "license": "ISC",
  "bugs": {
    "url": "https://github.com/diivi/torparty/issues"
  },
  "homepage": "https://github.com/diivi/torparty#readme",
  "dependencies": {
    "express": "^4.17.1",
    "request": "^2.88.2",
    "socket.io": "^4.1.3",
    "socket.io-client": "^4.1.3",
    "yts": "^2.0.0"
  }
}

Answer 1

This code mixes callbacks and promises -- generally not much fun. Promises usually win out because they're categorically better, so fixing the code here boils down to one of two strategies:

• Convert the callbacks into promises ("promisify") by hand or using a utility (less ideally)
• Find a better library (or function within the library) that lets you use promises all the way through (ideal).

While you're at it, basically always avoid fs.fooSync methods. This blocks the Node process without much benefit.

Next, since you're using a forEach and promises, you'll need to use a for..of loop and await each item, Promise.all if you want to request everything in parallel, or some sort of task queue if requesting everything in parallel amounts to firing huge number of requests and swamping either you or the remote server. See this answer for more information about these options.

Here's a naive promisification using basically the same setup as you:

const fs = require("fs");
const http = require("http");
const path = require("path");
const request = require("request");
const yts = require("yts");

const download = (uri, filename) => {
  return new Promise((resolve, reject) => {
    request.head(uri, (err, res, body) => {
      if (err) {
        reject(err);
      }

      request(uri)
        .pipe(fs.createWriteStream(filename))
        .on("close", resolve)
      ;
    });
  });
};

const ytsSearch = name => yts.listMovies({limit: 3, query_term: name});

const searchAndDownload = async () => {
  const {movies} = await ytsSearch("minions");
  await Promise.all(movies.map(movie => {
    const filePath = path.join(__dirname, "covers", `${movie.title_english}.png`);

    if (!fs.existsSync(filePath)) {
      download(movie.medium_cover_image, filePath);
    }
  }));
  return movies;
};

searchAndDownload()
  .then(movies => console.log(movies))
;

There's still (at least) two problems. For one, the searchAndDownload function does too many things: fetches and return the movie data, but then also downloads the cover art as a side effect. Functions should do only one thing -- it's better to split this into two different functions, one that fetches the data, and another that downloads and writes cover art to disk.

Secondly, I'd use a modern requests library instead of request such as axios which cuts verbosity and lets you use promises easily.

The side effect of these suggestions is your job will be easier and you'll be less likely to find yourself in some sort of nested callback/promise nightmare.

const axios = require("axios");
const fs = require("fs");
const path = require("path");
const yts = require("yts");

const fileExists = path => fs.promises.stat(path).then(() => true, () => false);

const downloadToFile = async (url, filename) => {
  const response = await axios.get(url, {responseType: "stream"});
  return response.data.pipe(fs.createWriteStream(filename))
};

const downloadMovieCover = async movie => {
  const filePath = path.join(__dirname, "covers", `${movie.title_english}.png`);

  if (!(await fileExists(filePath))) {
    return downloadToFile(movie.medium_cover_image, filePath);
  }
};

const ytsSearch = (term, limit=3) => yts.listMovies({limit, query_term: term});

const searchMovies = async searchTerm => (await ytsSearch(searchTerm)).movies;

(async () => {
  const movies = await searchMovies("minions");
  await Promise.all(movies.map(downloadMovieCover));
  console.log(movies.length);
})()
  .catch(err => console.log(err))
;

Since you're using Express, the IIFE at the bottom with the console.log can be your route handler that responds to the client, with "minions" taken from the request query params.