2

I am trying to learn Angular/node & express. I am currently attempting to write an endpoint for my backed that

  1. Needs to use axios to run a get request to an external API that will return a list of IDs (numbers)
  2. For each of those IDs supplied run a request that gets the details based off that id.

But my question is in regards to number 2. I have a list of IDs as input, and I'd like to run a request to an external API (using axios) based on each of these IDs. It may be helpful to note - The request to an external API based on the ID returns an object with details of that ID so my overall goal with this endpoint of my API is to return an array of objects, where each object contains the details of the IDs.

There have been a couple questions similar to mine...

  1. Pushing responses of axios request into array (This one is very similiar to my question)
  2. Axios random number of requests

However, they are using React.js, and I am having difficulties adapting their solution to node/express.

I am trying to model my approach based off of the code snippet provided in the top asnwer of the first question. But, my solution is returning an empty object as a response.

My question: What can I do differently to make multiple axios GET requests to an external API where each request is created dynamically

app.route('/test').get((req, res) => {
    axios
        .get('https://www.thecocktaildb.com/api/json/v1/1/filter.php?i=vodka')//This will not be hardcoded, but grabbed as a parameter from the endpoint
        .then(function(response) {
            //Purpose of this initial .then() clause is to make a call to the cocktaildb API to get the IDs of all the cocktails with a given ingredient eg: vodka.

            var data = response.data;//response.data contains the JSON object containing the contents received by the cocktaildb API request.
            var cocktailIds = [];

            //collect all cocktail ids so we can later make more requests to obtain the details associated with that ID.
            data.drinks.forEach(drink => {
                cocktailIds.push(drink['idDrink']);
            });

            //this is passed on to the next then clause. It is a list of cocktail ids.
            return cocktailIds;
        })
        .then((drinks) => {
            //the promises variable contains a list of all the requests we will have to make in order to get the details of all cocktail ids. I have tested that they are valid requests.
            const promises = drinks.map(id => {
                //console.log(getCocktailDetailsUrl + id);
                return axios.get(getCocktailDetailsUrl + id)
                .then(({data}) => {
                    return data;
                })
            })
            
            //I was hoping Promise.All to execute all of the requests in the promise and response to be stored in the cocktailDetails variable
            const cocktailDetails = Promise.all(promises)
            .then(values => {
                return values;
            })
            .catch(error => {
                console.log("There was an error when sending requests for details of all cocktails");
                console.log(error);
            })

            //Sending response only formatted this way for testing purposes
            if(cocktailDetails) {
                //this block is executed, and an empty object is returned as response
                console.log("cocktails was sent as response");
                res.send(cocktailDetails);
            } else {
                console.log("cocktails was not sent as response");
                res.send("cocktailDetails was not poppulated at the time of sending response");
            }
        })
        .catch(function (error) {
            res.send("There was an iswsue with your request to the cocktaildb API.");
            console.log('The following is the error from the request to the cocktaildb API: ' + error);
        })
});

As I mentioned before, my response contains an empty object. I know I must use promise.all somehow, but I am not sure how to implement it properly.

Improve this question
3
  • is bluebird installed and assigned to global.Promise or just required as const Promise = require('bluebird')? AFAIK Promise.all is not available in node.js Commented Jun 2, 2019 at 18:20
  • 1
    Note sure what bluebird is. But, promise.all is working fine, I'm just not using it correctly. For instance, I noticed that when I modify the above code to.. Promise.all(promises) .then(values => { console.log(values[0]); return values; }) I actually get the desired response I wanted from the first request in the list of requests I plan to make. But, I thought I would be getting the response data to be stored in const cocktailDetails. I need to figre out how to gain access to the response outside of that .then() Commented Jun 2, 2019 at 18:25
  • see my answer for why, but you can use the other guy's answer as well as his is right too Commented Jun 2, 2019 at 18:27

2 Answers 2

Reset to default
1

Your issue is that you aren't waiting for all the promises to resolve before sending the response. Additional critique, your code can be dramatically simplified to the following:

app.route('/test').get((req, res) => {
    axios.get('https://www.thecocktaildb.com/api/json/v1/1/filter.php?i=vodka')//This will not be hardcoded, but grabbed as a parameter from the endpoint
    .then(function(response) {

        const promises = response.data.drinks.map(drink =>axios.get(getCocktailDetailsUrl + drink.idDrink).then(({data})=>data));

        //I was hoping Promise.All to execute all of the requests in the promise and response to be stored in the cocktailDetails variable
        return Promise.all(promises)
        .then(values => {
            res.send(values);// shouldn't you be using res.json() here?
        });//if an error is thrown for any reason here, it will be caught by your outer catch 
    })
    .catch(function (error) {
        res.send("There was an iswsue with your request to the cocktaildb API.");
        console.log('The following is the error from the request to the cocktaildb API: ' + error);
    });
});
Improve this answer
Sign up to request clarification or add additional context in comments.

7 Comments

Thanks for the additional critique, I knew my solution was not elegant, and was looking for tips. In regards to waiting for the promises to resolve, I cannot simply add await before Promise.all as await can only be used inside an async Function, would I have to reimplement my code so that this functionality exists in an async function. Edit: added more to comment as I accidentally submitted to early
Promises should qualify as async functions. i see a ton of people doing this. I haven't personally adopted async/await yet so i don't know. I just stick to good old promises. did you try my code? it should work. also read my comment on the res.send() line. shouldn't you be using res.json() there?
Your solution worked. I really appreciate the help thank you. And yes, it should be res.json(). I only asked because I figured I could simply add await in front of my Promise.all, but I am getting a 'await is only valid in async function'. I don't need to solve the issue as your solution works and is what I will be using, I was just trying to figure out for learning purposes as I am having a hard time grasping promises and related concepts.
@Ryan promises are simple. if you give them out but forget about them, you won't be able to tell if the promise was kept, so always return the promise if you need the values elsewhere. if you are doing an action depending on the result of the promise, then you need to wait for it to resolve before doing your action. hope that helps you in your understanding. i'm not great at explaining sometimes
That was helpful, thank you. I had a question about this part of your solution const promises = response.data.drinks.map(drink =>axios.get(getCocktailDetailsUrl + drink.idDrink).then(({data})=>data));. Could you explain the logic of what the .then(({data})=>data) is doing?
|
0

You should wait for the promise before trying to send a response.

Add await before Promise.all and you don't need the then block

const cocktailDetails = await Promise.all(promises)
Improve this answer

1 Comment

this is right, but doesn't answer the question. you should point out to the programmer why he gets an empty response (he doesn't wait for the Promise.all() to resolve before sending his response.)

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.