Promise both resolves and rejects

Question

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.

The age old [antipattern](https://stackoverflow.com/questions/23803743/what-is-the-explicit-promise-construction-antipattern-and-how-do-i-avoid-it) of using a promise constructor when you already have one — Adam Jenkins, May 25 '20 at 23:09
"The reason that I have a Promise despite the async axios() call is that it wasn't being performed asynchronously and returned undefined data." — If you are returning `undefined` then wrapping everything in another Promise is unlikely to solve it and likely to be one of the worst solutions if it does solve it. — Quentin, May 25 '20 at 23:15
Plus this `.catch(reject("something went wrong"))` is wrong. You pass a function reference to `.catch()`. You are calling `reject()` immediately (which you do not want) and then passing it's return value which is `undefined` to `.catch()`. So, your code is equivalent to `reject("something went wrong"); xxx.catch(undefined)`. So, you're always calling the `reject()`. And, as others have said, get rid of this anti-pattern and you won't even have this line of code. — jfriend00, May 25 '20 at 23:40

score 3 · Accepted Answer · answered May 25 '20 at 23:09

.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)

I see what you mean with the promise inside a promise, but is there anything to modify on the calling end? I'm still getting an error of the data being undefined in the ComponentDidMount function ```Uncaught TypeError: Cannot read property 'then' of undefined```, and then the data is printed later — llamasquared, May 27 '20 at 15:48
Never mind, I had to change the state from within the callee to let it know to re-render. Thank you! — llamasquared, May 27 '20 at 17:47

Adam Jenkins · Answer 2 · 2020-05-25T23:47:55.140

You already have a promise, just return it.

  return 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;
      })

And just for "syntactic sugar", async/await makes everything a little easier to read:

module.exports = async (url) => {
   const { data } = await axios({method:'get',url});
   const $ = cheerio.load(data);

   ...

   return topsongs;
}

Promise both resolves and rejects

2 Answers2