Search code examples
javascriptreactjspromisees6-promise

Promise both resolves and rejects


It seems as though my Promise is simultaneously returning true and false. The console is returning "undefined" and then right underneath it's returning "something went wrong". The data is returned underneath these, showing that it's not actually waiting on the Promise.

Here's the function being called:

module.exports = (url) => {
  return new Promise((resolve, reject) => {
    axios({
      method: 'get',
      url: url
    })
      .then(response => {
        const html = response.data
        const $ = cheerio.load(html)
        const songtable = $('.chart-list__elements > li')
        const topsongs = []
        songtable.each(function () {
          const rank = $(this).find('.chart-element__rank__number').text()
          if (rank == 11) return false;
          const name = $(this).find('.chart-element__information__song').text()
          const artist = $(this).find('.chart-element__information__artist').text()

          topsongs.push({
            rank,
            name,
            artist
          })
        })
        resolve()
        return topsongs;
      })
      .catch(reject("something went wrong"))
    })
}

From the caller:

componentDidMount() {
    const top_songs = topsongs('https://www.billboard.com/charts/hot-100')
    .then(console.log(top_songs))
    .catch(err => console.log(err))
  }

Thanks, I'm new to Promises and have tried nearly every method of doing this. The reason that I have a Promise despite the async axios() call is that it wasn't being performed asynchronously and returned undefined data.


Solution

  • .catch(reject("something went wrong"))
    

    You need to pass a function to catch.

    You are calling reject immediately and passing its return value.


    You are also using the nested promise anti-pattern.

    axios returns a promise. There is no need to create another one.


    module.exports = (url) =>
      axios({
        method: "get",
        url: url,
      })
        .then((response) => {
          const html = response.data;
          const $ = cheerio.load(html);
          const songtable = $(".chart-list__elements > li");
          const topsongs = [];
          songtable.each(function () {
            const rank = $(this).find(".chart-element__rank__number").text();
            if (rank == 11) return false;
            const name = $(this).find(".chart-element__information__song").text();
            const artist = $(this)
              .find(".chart-element__information__artist")
              .text();
            topsongs.push({
              rank,
              name,
              artist,
            });
          });
          return topsongs;
        })
        .catch(() => {throw "something went wrong"});
    

    (Replacing the thrown error with the generic "something went wrong" doesn't seem helpful. You'd probably be better off without that catch call at all)