-1

I am working on a student project. My goal is to collect data and store it. I use nodeJs with express, then I run queries with superagent on ThemoviesDb to collect movies and then store them on neo4j.

Here is my example :

app.get('/', function(req,res){

var hostname = "api.themoviedb.org/3";
var path = "/movie/";



// random id, to store random film  
for(i=0;i<39;i++){
    randomId = Math.floor(Math.random() * 500) + 80;
    console.log(' --------------------- Random id is     ---------------------- ' + randomId);
    superagent.get(hostname + path + randomId)
        .query({ api_key: api_key, append_to_response : 'credits' })
        .end((err, res) => {
            if (err) { 
                if(err.status_code = '34') {
                    return console.log('id : ' + randomId + 'ne correspond pas à un film')
                }
                else{
                    return console.log(err);
                }
            }
            cpt ++;

            title = res.body.original_title;

            if(title){  // Test if title isn't nul, undefined, Nan, empty or 0 to add it
                console.log('randomId --> '  + res.body.id + '--> ' + title );
                if(!Filmadded.includes(film)){
                Filmadded.push(film);
            }       
            }               
        });

    }
                console.log('cpt : ' + cpt + '/39');
res.send('it works' + i );
});

I I'm just executing a loop ( 39 because the limit of the api is 40 ) and for each I make a request to get a movie.

The result : enter image description here

As you can see i first have all the id's displayed then the titles that match the id. --> I would like to wait until the request is over to move on. I looked promised but I do not have everything.

I then think that my problem on id / film is due to this.

Thank you for your help and sorry for my english

Jamesp
  • 50
  • 10
  • The variables film and Filmadded come from nowhere, so does cpt. Can you post the full code or point to a git repo? – HMR Dec 03 '17 at 06:32

2 Answers2

1

Your superagent() call is asynchronous. As such, it does not block and your for loop just runs to completion starting all 39 superagent() calls in parallel. You could probably code to run all these in parallel (if the target host allows it), but since you asked for the ability to run them one after another, here's an implementation of your function using the promise capabilities of superagent() and await to serialize the async call inside your for loop so it runs one, waits for it to finish, then runs the next one:

app.get('/', async function(req, res){

    let hostname = "api.themoviedb.org/3";
    let path = "/movie/";

    // random id, to store random film  
    for (let i = 0; i < 39; i++) {
        let randomId = Math.floor(Math.random() * 500) + 80;
        console.log(' --------------------- Random id is     ---------------------- ' + randomId);
        try {
            let result = await superagent.get(hostname + path + randomId)
                .query({ api_key: api_key, append_to_response : 'credits' });
            cpt ++;

            let title = result.body.original_title;

            if (title) {  // Test if title isn't nul, undefined, Nan, empty or 0 to add it
                console.log('randomId --> '  + res.body.id + '--> ' + title );
                if (!Filmadded.includes(film)) {
                    Filmadded.push(film);
                }       
            }               
        } catch(err) {
            if(err.status_code = '34') {
                console.log('id : ' + randomId + 'ne correspond pas à un film')
            }
            else{
                console.log(err);
            }   
        }
        console.log('cpt : ' + cpt + '/39');
    }
    res.send('it works' + i );
});

In addition, you need to make sure you're declaring all variables you are using so they are accidental globals that can conflict when there are other requests also running.

Other things that don't look correct about this code, but I don't know what you intend:

  1. The variable cpt looks like it needs to be initialized and declared somewhere.
  2. The variable Filmadded probably needs to be scoped locally (not some higher scoped variable that can conflict when multiple requests are running on your server).
  3. It isn't clear what you actually intend to do for error handling here when a superagent() call fails. Here it just logs errors, but you probably need to be able to return an error status if you're getting errors.
jfriend00
  • 683,504
  • 96
  • 985
  • 979
0

As you said, superagent's get function is async, meaning that the event loop doesn't wait for the function to finish before executing the next command. So the loop initiates 40 executions of your loop, which includes creating a random id and then using superagent with that id. So we're talking about two actions - one is synchronous and the second is asynchronous.

Let's look at it in another way. Say we had the following loop:

for(i=0; i<39; i++) {
    const randomId = Math.floor(Math.random() * 500) + 80;
    console.log("RANDOM IS: ", randomId);
    setTimeout(function(){
        console.log("PRINT AGAIN: ", randomId);
    }, 10000);
}

What you'll have here, is 40 rows of "RANDOM IS: [random_number]" in a consecutive manner, and only after 10 seconds you'll have 40 rows of "PRINT AGAIN: [random_number]", and that's because you set a timeout of 10 seconds for the second logging.

You can compare that setTimeout with 10 seconds to an async function - only in an async function you can't really tell when the function will finish. So basically what you have is similar to the above example - 40 loggings of the random number, and some random-timed promises executions.

So what you might want to consider is promise chaining using the reduce function of js Array, or use es6 async await function notation.

You can use superagent functions as promises, and use then and catch instead. Then, chaining the promises means you wait for one promise to finish and only then executing the following one.

Gilad Bar
  • 1,302
  • 8
  • 17