colio303
colio303

Reputation: 81

NodeJS Express: How can I get my object to build completely before sending it in the response?

I am trying to build a result_arr of location objects to send as a response, but I am not sure how to send the response only when the entire array has been built. The response contains an empty array, but result_arr array is filled after the response has already been sent.

function handle_getLocations(req, res, done){
var con_id = req.body["contractor_id"];
console.log("Contractor ID :" + con_id.toString());
var result_arr = new Array();

employee.getActiveByContractor(con_id, function(err, employees){
    if (err) {
        console.log("Logging error in json:\n");
        res.json({"code" : 100, "status" : "Error in connection database"});
        return;
    };

    if(employees.length === 0) done(null);

    for(var i=0;i<employees.length;i++){
        assignment.getLocationsByEmployeeID(employees[i].employee_id, function(err, locations){
            if (err) {
                console.log("Logging error in json:\n");
                res.json({"code" : 100, "status" : "Error in connection database"});
                return;
            };
            console.log("Number of locations: " + locations.length.toString());
            for(var j=0;j<locations.length;j++){
                console.log("Assignment is: " + locations[j].assignment_id.toString());
                location.getAllByID(locations[j].location_id, function(err, loc){
                    if (err) {
                        console.log("Logging error in json:\n");
                        res.json({"code" : 100, "status" : "Error in connection database"});
                        return;
                    };

                    var loc_obj = {};
                    loc_obj.display_name = loc[0].display_name;
                    loc_obj.location_id = loc[0].location_id;
                    console.log("Location is: " + loc_obj.display_name);
                    console.log("Location ID is: " + loc_obj.location_id.toString());

                    result_arr.push(loc_obj);
                    console.log(result_arr);
                    done(result_arr);
                });
            };
        });
    };
});

};

I know that in nodejs the idea is to not make blocking calls, but I am not sure how to make sure all of the information is sent in the response.

Upvotes: 0

Views: 77

Answers (3)

Aruna
Aruna

Reputation: 12022

You are calling many asynchronous functions in the loop and do not have any logic to check when all they are completed to send the response back to the client.

I modified your code a bit to add the logic in VannilaJS way which is very messy below but working code.

Anyways I would suggest you to use promise based/asynchronous modules like async, bluebird etc to handle this nicely. Using them, you can improve readability and easy maintainability in your code to get rid of callback hells and other disadvantages.

async http://caolan.github.io/async/

bluebird https://github.com/petkaantonov/bluebird

You can read more about this on the below link,

https://strongloop.com/strongblog/node-js-callback-hell-promises-generators/

function handle_getLocations(req, res, done){
   var con_id = req.body["contractor_id"];
   console.log("Contractor ID :" + con_id.toString());
   var result_arr = new Array();

   employee.getActiveByContractor(con_id, function(err, employees){
    if (err) {
        console.log("Logging error in json:\n");
        res.json({"code" : 100, "status" : "Error in connection database"});
        return;
    };

    if(employees.length === 0) done(null);

    var employeesChecked = 0;
    var errors = [];

    function sendResponse(){
       if(employeesChecked === employees.length) {
       res.json(result_arr);
       //done(result_arr);  // If required, uncomment this line and comment the above line
      }
    }

    for(var i=0;i<employees.length;i++){
        assignment.getLocationsByEmployeeID(employees[i].employee_id, function(err, locations){
            var locationsChecked = 0;
            if (err) {
                console.log(err);
                errors.push(err);
                ++employeesChecked;

                sendResponse();
            } else {
            console.log("Number of locations: " + locations.length.toString());
            for(var j=0;j<locations.length;j++){
                console.log("Assignment is: " + locations[j].assignment_id.toString());
                location.getAllByID(locations[j].location_id, function(err, loc){
                    ++locationsChecked;
                    if (err) {
                        console.log(err);
                        errors.push(err);
                    } else {
                    var loc_obj = {};
                    loc_obj.display_name = loc[0].display_name;
                    loc_obj.location_id = loc[0].location_id;
                    console.log("Location is: " + loc_obj.display_name);
                    console.log("Location ID is: " + loc_obj.location_id.toString());

                    result_arr.push(loc_obj);
                    console.log(result_arr);
                   }

                   if(locationsChecked === locations.length) {
                      ++employeesChecked;
                   }

                   sendResponse();
                });
            }
          }
        });
    }
});

}

Upvotes: 1

Bird Law
Bird Law

Reputation: 77

As Basim says, this is a good time to use Promises.

getLocationsByEmployeeID and getAllByID are async so they won't be done by the time the loop is finished and you send your response.

Promises are built into the latest Node.js version. Learn here: https://www.udacity.com/course/javascript-promises--ud898

Suggestion:

  1. Create promise wrappers for getLocationsByEmployeeID and getAllByID
  2. Use Promise.all to make sure every getLocationsByEmployeeID and getAllByID are complete
  3. return your http response within Promise.all's "success" callback

Upvotes: 0

Basim Hennawi
Basim Hennawi

Reputation: 2721

In order not to consume much time during the request-response life time, you need to separate each logic in a single endpoint, but sometimes as your case, you may need to hit the database more than a time to fetch data that depends on another, so assuming that employee.getActiveByContractor returning promise and as it's an async method so you need to to chain it with .then like this:

 employee.getActiveByContractor(con_id)
        .then(function(employees) {

Also, you my need to read about Promise.

Upvotes: 0

Related Questions