1

I am making a POST request in one of my routes add-users. I have created an array called success. In each loop provided the API POST runs successfully I am adding a string 'user added' to the array. Once the array has completed I want to send the response to the browser with the success array.

I have noticed something strange. When I type in the url the start of add-users it runs the loop before I hit enter to navigate to the page. This seems strange? Is node listening and predicting which url I am going to hit?

Here is my current attempt but its not working for some reason.

app.get('/add-users', function (req, res) {
    var success = [];
    var count = 0;

    users.forEach(function(user, i){

        request({
            url: url,
            method: 'POST',
            json: true
        }, function(err, resp, body){
            if (!err && resp.statusCode === 200) {
               success.push('user added'); 
            }
        });

        if(count === users.length) {
          res.json(success);
        }
    });
});
  • I don't understand what you mean "it runs the loop before I hit enter". – Paul Jan 19 '17 at 10:55
  • It's most likely just your browser pre-loading the page. Chrome does it for sure, not sure if the other browsers do it too. I would recommend to show a page first and then make an AJAX call to /add-users to prevent the pre-loading – Molda Jan 19 '17 at 11:06

3 Answers3

1

The problem here is you are mixing synchronous and asynchronous code together in a wrong way. Please note that forEach is synchrounous and request is asynchronous. So, looping over users finishes faster than the first result you get from request method.

Community
  • 1
  • 1
  • I am aware that this is what is happening, but I am trying to figure out how I can resolve this. Also, should my `counter` (albeit a bad solution) not get around the async nature of this –  Jan 19 '17 at 11:16
1

Regarding browser fetching the response before hitting enter key on the url, it is very unusual behaviour. Maybe you should check your machine if it is infected by any malware!

Regarding the code used by you, count is not incremented anywhere in the forEach loop. So it remains 0 forever and never equals users.length. So the loop will end but it will never send a response.

Also, you are testing for equality between count and users.length at the wrong place in the code.

This code should work:

app.get('/add-users', function (req, res) {
    var success = [];
    var count = 0;

    users.forEach(function(user){

        request({
            url: url,
            method: 'POST',
            json: true
        }, function(err, resp, body){
            if (!err && resp.statusCode === 200) {
               success.push('user added'); 
            }

            count++;                        // <<=== increment count
                                            //
            if(count === users.length) {    // and then test if all done
               res.json(success);
            }
        });
    });
});
Santanu Biswas
  • 4,699
  • 2
  • 22
  • 21
0

@SantanuBiswas has one way you can solve your problem with the response returning before your requests are all complete, though depending on how many users you've got in your array and how slow the upstream service is, this is a potentially disastrous user experience as it will wait until all the requests are complete and only then fire back a response.

A better solution (in my opinion) would be to respond immediately with a 202 Accepted status code, and then update your DB with information about each user's status in the request handler for later reporting or debugging. Something like this (assuming you're using mongoose for your local user storage):

app.get('/add-users', function (req, res) {
   res.setStatus(202).send(); // you can put more info in the body if desired

users.forEach(function(user){

    request({
        url: url,
        method: 'POST',
        json: true
    }, function(err, resp, body){
        const status = err ? 'error' : 'complete';  // obviously you might want to put more info somewhere, eg. error log, if there is an error
        User.update({_id: user.id}, {status:status}, function(e) { if (e) console.error(err);});
    });
});

});

Still another way, though it adds complexity to your solution, is to implement websockets to have your client get updated each time a request is complete. That example is a bit longer than I have time to post here, but the docs on the site I linked are excellent.

Paul
  • 35,689
  • 11
  • 93
  • 122
  • I agree that my solution might result in bad user experience and status code 202 would be better. But the OP wants to send back the contents of success array back to the client as response and that is why there is no other choice but to wait until the entire loop is completed. Of course, the best solution for this is to implement websockets. – Santanu Biswas Jan 19 '17 at 15:32
  • Yep, understood, and you earned the green check. I just wanted to make sure he knew about the other options. – Paul Jan 19 '17 at 16:16