Resolving nested Axios requests

Question

Original Question

I'm making a service that reads a directory and loops through movie files to get their name and path, then attempts to find information about those files from a movie API (TMDB). If multiple movies are returned, another API call (using Axios) is made to get more specific information about each movie and then determines if it's likely the correct movie or not.

The code I have works for a while, but eventually stops, never actually getting to the 'Done with Analysis' part. This leads me to believe that I'm not properly resolving all my promises. I had this working before with an async/await structure, but I wanted to convert it to a promise-based structure with then and such.

Provided below is a cleaned-up version of my code:

class movieSrcAnalyzer {
  async analyze() {
    console.log("Starting Analyzer... \n");

    fs.promises
      .readdir(baseDirectory)
      .then((names) => {
        const fileCount = names.length;
        let currentFileNum = 1;
        for (const name of names) {
          this.getTMDBid(name).then((data) => {
            // This prints around 500 / 700 movies that are in the directory
            console.log(`${currentFileNum} / ${fileCount}: `, data);
            currentFileNum++;
          });
          //await this.createMovie(id, path);
        }
      })
      .then(() => "Done with Analysis.")
      .catch((error) => console.error(error));
  }

  async getTMDBid(name) {
    const originalName = name;
    let cleanName = name;

    const partRegex = /part\s([0-9]*)/g;
    const partInfo = cleanName.match(partRegex);

    cleanName = cleanName.replace(partRegex, "");
    cleanName = cleanName.replace(/\.[^/.]+$/, "");
    cleanName = cleanName.replace("_", " ");
    cleanName = cleanName.toLowerCase();

    return new Promise((resolve, reject) => {
      axios
        .get(`http://${IP_ADDRESS}:${PORT}/api/tmdb/search-name/${cleanName}`)
        .then((response) => {
          const movies = response.data.results;

          // If at least one result was found
          if (movies && movies[0] !== undefined) {
            // If there is only one result, it's likely correct
            if (!(movies.length > 1)) {
              return { id: movies[0].id, path: originalName };
            } else {
              // More than one result was found, we must narrow it down.
              for (const key in movies) {
                const result = movies[key];

                return new Promise((resolve, reject) => {
                  axios
                    .get(
                      `http://${IP_ADDRESS}:${PORT}/api/tmdb/search-id/${result.id}`
                    )
                    .then((response) => {
                      const movie = response.data;

                      getVideoDurationInSeconds(
                        `${baseDirectory}/${originalName}`
                      ).then((duration) => {
                        const durationInMinutes = duration / 60;

                        if (
                          durationInMinutes > movie.runtime - 1 &&
                          durationInMinutes < movie.runtime + 1
                        ) {
                          resolve({
                            id: result.id,
                            path: originalName,
                          });
                        }
                      });
                    })
                    .catch((error) => console.error(error));

                  if (
                    result.title.includes(cleanName) ||
                    result.original_title.includes(cleanName)
                  ) {
                    // Get more data here.
                  }
                });
              }
            }
          }
          return { id: null, path: originalName };
        })
        .then((movieData) => {
          resolve(movieData);
        })
        .catch((error) => console.error(error));
    }).then((data) => {
      return data;
    });
  }
}

I looked at several different similar questions, but none of their solutions worked for me. Most directly returned the Axios responses and then handled the data, but mine needs to handle the data differently per nested Axios requests. I'm still not great at wrapping my mind around exactly what's going on with nested promises, so if you answer this question, I'd also appreciate the 'why' as to how your solution works.

Edit 1

As per @dahn suggestions, I refactored my code to more closely match his. After the refactor, it works substantially better and is much easier to read. However, I'm still running into one last problem in the following code:

const promises = diskMovies.map((movie, index) => {
  //console.log(`${index + 1}/${diskMovies.length} - ${movie.cleanName}`);
  const tmdbMatch = this.getTMDBMatch(movie, index);
  console.log("TMDB MATCH: ", tmdbMatch);
  return tmdbMatch;
});
const matchedMovies = await Promise.all(promises);

It seems something yet again isn't being resolved, likely somewhere in getTMDBMatch. What's strange, however, is if I change my code to the below code, it works.

let matchedMovies = [];
let index = 0;
  for (const movie of diskMovies) {
    console.log(`${index + 1}/${diskMovies.length} - ${movie.cleanName}`);
    const tmdbMatch = await this.getTMDBMatch(movie, index);
    matchedMovies.push(tmdbMatch);
    index++;
}

If I await each promise from this.getTMDBMatch rather than let Promises.All take care of it, I finally manage to get through everything and complete the Analysis.

I'd very much like to know what is happening with Promises.All though. I don't get why it wouldn't succeed while this other method would.

Answer 1

A few ideas for syntax:

• It's awkward and hard to read async/await syntax mixed with .then(). Pick just one style.
• Things that return promises don't need to be (and shouldn't be) wrapped in new Promise()
• To await promises generated in a loop, collect them and await Promise.all()

A few ideas for design:

• Use a single object to keep track of each movie. It looks like all of the relevant props are:

  {
    name: '',       // the name on disk
    cleanName: '',  // the name filtered thru your regex's
    runtime: 0,     // runtime in seconds, calculated from local data
    imdbMatch: {},  // object from the api that matches
    imdbDetail: {}  // more data from the api if needed for the match
  } 
  

• Build all the movie info from disk in one place, early.

• Factor out the name cleanup logic

With all that, I suggest the following...

class movieSrcAnalyzer {
  async analyze() {
    console.log("Starting Analyzer... \n");
    // for each disk movie, find the best matching imdb movie
    const diskMovies = await this.diskMovies(baseDirectory);
    const promises = diskMovies.map(movie => {
      return this.getIMDBMatch(movie);  // will add prop(s) to movie from the best matching imdb data
    })
    const matchedMovies = await Promise.all(promises);
    console.log( "Done with Analysis.")
    return matchedMovies
  }

  // create movie objects from data on disk
  async diskMovies() {
    const names = await fs.promises.readdir(baseDirectory);   // presumes baseDirectory is in containing scope
    const promises = names.map(async (name) => {
      const cleanName = this.cleanName(name)
      const seconds = await getVideoDurationInSeconds(`${baseDirectory}/${name}`);
      const runtime = seconds / 60.0;  // runtime in minutes
      return { name, cleanName, runtime }
    });
    return Promise.all(promises)
  }

  cleanName(name) {
    let cleanName = name;
    const partRegex = /part\s([0-9]*)/g;
    const partInfo = cleanName.match(partRegex);

    cleanName = cleanName.replace(partRegex, "");
    cleanName = cleanName.replace(/\.[^/.]+$/, "");
    cleanName = cleanName.replace("_", " ");
    cleanName = cleanName.toLowerCase();
    return cleanName
  }

  // return a copy of movie with props added that represent the best match found in the api
  // movie is { name, cleanName, runtime }
  // add 'imdbMatch'
  // optionally add 'imdbDetail' if the match required more detail
  async getIMDBMatch(movie) {
    movie = Object.assign({}, movie)
    const response = await axios.get(`http://${IP_ADDRESS}:${PORT}/api/tmdb/search-name/${movie.cleanName}`)
    const results = response.data.results;
    if (results.length < 2) {
      movie.imdbMatch = results.length === 1 ? results[0] : null;
      return movie
    }
    // for more than two results, get detail on them sequentially, compare run length and other factors
    for (const result of results) {
      const response = await axios.get(`http://${IP_ADDRESS}:${PORT}/api/tmdb/search-id/${result.id}`)
      const detail = response.data;

      // imdb movie is a match if runtime matches
      if (Math.abs(movie.runtime - detail.runtime) < 1) {
        movie.imdbMatch = result;
        movie.imdbDetail = detail;
        return movie
      }
      // other checks here, like result.title vs movie.cleanName or movie.name
      // etc 
    }
    // if we get here, we didn't find a match in the imdb results.
    movie.imdbMatch = null;
    return movie
  }
}

Answer 2

I suspect the problem is that there is nothing returned from this then block

.then((names) => {
  const fileCount = names.length;
  let currentFileNum = 1;
  for (const name of names) {
    this.getTMDBid(name).then((data) => {
      // This prints around 500 / 700 movies that are in the directory
      console.log(`${currentFileNum} / ${fileCount}: `, data);
      currentFileNum++;
    });
    //await this.createMovie(id, path);
  }
})

I think you will need to collect the promises from all the this.getTMDbid calls, return them as one promise from this block(Promise.all), and then do your "Done with Analysis." things. Currently, I think there is nothing to then from on this line .then(() => "Done with Analysis.")

Here's something that might work to replace the then block above:

.then((names) => Promise.all(names.map(name => this.getTMDBid(name))))