Mongo returns undefined but data is there

Question

I have the following code. Using $geoNear, i find the closest transit stop to a given gps coordinate. As some of you might know, $geoNear only returns the location (loc) of the closest point. To get the details of the closest stop, I query the STOPS collection with using the location.

The issue is, it randomly returns undefined. When I query the stops collection from mongo shell, using the agency_key and and loc parameters I confirm that data is there. I suspect this issue might be caused by asynchrony but I can't find out why. I tried to simplify my question to 'calling async function in forEach loop' but I have nothing so far.

Why does it return undefined?

 ...
    TmStop.aggregate([{
              $geoNear: {
                near: [lon, lat],
                maxDistance: radiusInDegrees,
                includeLocs: "distance.location",
                distanceField: "distance.calculated",
                query: {
                  agency_key: {
                    $in: agencyKeys
                  }
                }
              }
            }, {
              $project: {
                route_id: 1,
                route_type: 1,
                direction_id: 1,
                "distance.calculated": 1,
                "distance.location": 1
              }
            }])
            .exec(function(e, results) {
          if (e) {
            console.log('e->', e.errmsg);
            var res = "Something went wrong with the database: " + e;
            cb(e, res);
          } else if (!e) {
            if (results.length) {
              console.log('results->', results.length);
              var i = 0;
              results.forEach(function(result, index) {
                console.log(index, result);
                Stop.find({
                    agency_key: {
                      $in: agencyKeys
                    },
                    loc: result.distance.location
                  })
                  .exec(function(e, stop) {
                    if (e) {
                      throw new Error('Error getting stop due to:' + e);
                    }

                    var obj = {};
                    obj.route_id = result.route_id;
                    obj.route_type = result.route_type;
                    obj.direction_id = result.direction_id;
                    obj.distance = result.distance.calculated;
                    obj.locationUsed = result.distance.location;

                    // console.log('### ', index, ' ####');
                    // console.log('@Stop.find agencyKeys-> ', agencyKeys);
                    // console.log('@Stop.find loc->', result.distance.location);
                    // console.log(stop[0]);

                    obj.stop = stop[0];
                    objArr.push(obj);
                    i++;
                    if (i === results.length) {
                      cb(e, objArr);
                    }
                  });
              }); //end of forEach
            } else {
              cb(e, []);
            }
          }
        });

Answer 1

Of course. You have a .forEach() loop that calls an async function without waiting for any ( or more importantly "all" calls ) to complete. So your loop iteration needs to respect the completion calls of the inner function being invoked.

A good tool for this is the async library, and for safety's sake we'll make the call async.mapLimit ( since you are emitting an array ) which will allow a specified number of operations to run similtaneously, rather than right up to th stack limit ( or possibly beyond with an uncontrolled .forEach() or for loop ).

So inside the callback from the aggregation operation, change the code listing to this instead:

        // If aggregation query returned anything
        if (results.length) {
          console.log('results->', results.length);

          async.mapLimit(results,10,function(result,callback) {
              Stop.findOne({ 
                  "agency_key": { "$in": agency_keys },
                  "loc": result.distance.location
              },function(e,stop) {
                  result.stop = stop;      // remember that result is already a plain object
                  callback(e,result);
              });
          },cb)               // cb is automatically passed e and results
        } else {
            cb(e,results);    // then the array was empty
        }

There are a couple of improvements there, most notably in that while you have been dealing with mongoose documents returned from regular .find() operations you may have noticed you just cannot add new properties to them, since they are mongoosse documents ( a more complex object ) and not plain objects. They just pretend to be one in serialization and helper accessor methods.

But the results of .aggregate() are in fact just a "plain object". So there is no need to copy over object properties to a new object just to assign a new property. For future reference, you don't need to do that for "Mongoose documents" as explicitly as you have in your listing, but just call .toObject() on the document instead. The result returns a "plain object". But in the code needed here, it's just a simple assignment.

Also there is usage of .findOne() rather than .find(). It's noted in your code that you are only looking for the singular match anyway and reference the first element of the results. Therefore just ask for one result. The query itself is also likely to return a single result anyway due to the singular exact location from "near" and without the other query paramters, but I really don't have that information at hand.

Finally of course there is the .map() or rather .mapLimit() for which after all your intent is to simply add a new property to each element of the results array from the Stop data and then return that element with the additional property. Each iteration is going to issue the supplied callback with the modified data, then on completion of all iteration a final callback is called which will contain the emitted array of all elements returned. This is basically the built in arguments of your cb, so it's just supplied in that form rather than wrapping in another function.