1

I have a method that is failing when returning an array of objects. As mentioned in the title - the array is confirmed to be populated but is empty in the response.

Here is the full flow:

The Url:

http://localhost:53000/api/v1/landmarks?lat=40.76959&lng=-73.95136&radius=160

Is routed to the corresponding index:

api.route('/api/v1/landmarks').get(Landmark.list);

The Index Calls a Service:

exports.list = (req, res) => {

    const landmark = new LandmarkService();

    landmark.getLandmarks(req)
        .then(landmarks => {

            var response = new Object();
                response.startindex = req.query.page;
                response.limit = req.query.per_page;
                response.landmarks = landmarks;

                res.json(response);
        })
        .catch(err => {
            logger.error(err);
            res.status(422).send(err.errors);
        });
};

The Service Method Uses a Data Access Class to Return the Promise

  getLandmarks(req) {

    const params = req.params || {};
    const query = req.query || {};
    const page = parseInt(query.page, 10) || 1;
    const perPage = parseInt(query.per_page, 10);
    const userLatitude = parseFloat(query.lat); 
    const userLongitude = parseFloat(query.lng); 
    const userRadius = parseFloat(query.radius) || 10;
    const utils = new Utils();
    const data = new DataService();
    const landmarkProperties = ['key','building','street','category','closing', 
            'email','name','opening','phone','postal','timestamp','type','web'];

    return data.db_GetAllByLocation(landmarksRef, landmarkLocationsRef, 
                    landmarkProperties, userLatitude, userLongitude, userRadius);

    } // getLandmarks

However, the response is always empty.

I am building an array in the called method and populating it with JSON objects. That is what is supposed to be sent back in the response. I can confirm that the attributes array is correctly populated before I hit the return statement. I can log it to the console. I can also send back a test array filled with stub values successfully.

I have a feeling it is how I am setting things up inside the Promise?

Data Access Method That Should Return Array of Objects:

db_GetAllByLocation(ref, ref_locations, properties, user_latitude, user_longitude, user_radius)
{  
        const landmarkGeoFire = new GeoFire(ref_locations);
        var geoQuery = landmarkGeoFire.query({
                center: [user_latitude, user_longitude], 
                radius: user_radius
        });
        var locations = [];
        var onKeyEnteredRegistration = geoQuery.on("key_entered", function (key, coordinates, distance) {
            var location = {}; 
                location.key = key;
                location.latitude = coordinates[0];
                location.longitude = coordinates[1];
                location.distance = distance;
                locations.push(location);                   
        });
        var attributes = [];
        var onReadyRegistration = geoQuery.on("ready", function() {
            ref.on('value', function (refsSnap) {
                   refsSnap.forEach((refSnap) => {
                        var list = refSnap;
                        locations.forEach(function(locationSnap) 
                        {
                            //console.log(refSnap.key, '==', locationSnap.key);
                            if (refSnap.key == locationSnap.key) 
                            {
                                var attribute = {};
                                for(var i=0; i<=properties.length-1; i++)
                                {
                                    if(properties[i] == 'key') {
                                        attribute[properties[i]] = refSnap.key;
                                        continue;
                                    }
                                    attribute[properties[i]] = list.child(properties[i]).val();
                                }
                                attribute['latitude'] = locationSnap.latitude;
                                attribute['longitude'] = locationSnap.longitude;
                                attribute['distance'] =  locationSnap.distance;
                                attributes.push(attribute);    
                            } // refSnap.key == locationSnap.key
                        }); // locations.forEach
                    }); // refsSnap.forEach
                    return Promise.resolve(attributes); <-- does not resolve (throws 'cannot read property .then')
                  //geoQuery.cancel();
                }); // ref.on
        }); // onreadyregistration      

        return Promise.resolve(attributes); <-- comes back empty
}
Peter
  • 5,251
  • 16
  • 63
  • 98
  • 2
    I don't see any promise in your code. – Bergi Apr 09 '18 at 20:19
  • Yes, I took the surrounding code for the call out because I didn't find it relevant but - you never know. I have added it per your suggestion. – Peter Apr 09 '18 at 20:38
  • Seems the updated code didn’t post and now I am heading home. I’ll post when I get home. But, I can confirm the calling code is wrapped in a promise. – Peter Apr 09 '18 at 21:06
  • 1
    In addition to seeing the promise, it would be good in the future to remove all the console.log statements to make it quicker to understand – Chris Phillips Apr 09 '18 at 21:22
  • It seems I may need to chain my promise - something like this: https://javascript.info/promise-chaining . – Peter Apr 10 '18 at 14:56

2 Answers2

0

It seems that data.db_GetAllByLocation is an asynchronous function, therefore the call resolve(landmarks); is getting called before the execution of the async function is finished. If the data.db_GetAllByLocation returns a promise then call the resolve(landmarks) inside the promise.

data.db_GetAllByLocation().then(function() {
  resolve();
})

Also try the following modified db_GetAllByLocation()

db_GetAllByLocation(ref, ref_locations, properties, user_latitude, user_longitude, user_radius)
    {       
   return new Promise(function(resolve, reject){
    const landmarkGeoFire = new GeoFire(ref_locations);

    var geoQuery = landmarkGeoFire.query({
      center: [user_latitude, user_longitude], 
      radius: user_radius
    });

    var locations = [{}];

    var onKeyEnteredRegistration = geoQuery.on("key_entered", function (key, coordinates, distance) {
     var location = {}; 
      location.key = key;
      location.latitude = coordinates[0];
      location.longitude = coordinates[1];
      location.distance = distance;
      locations.push(location);                   

    });

      var attributes = [{}];

    var onReadyRegistration = geoQuery.on("ready", function() {
     ref.on('value', function (refsSnap) {
         refsSnap.forEach((refSnap) => {
        var list = refSnap;
        locations.forEach(function(locationSnap) 
        {
         if (refSnap.key == locationSnap.key) 
         {
          var attribute = {};
          for(var i=0; i<=properties.length-1; i++)
          {
           if(properties[i] == 'key') {
            attribute[properties[i]] = refSnap.key;
            continue;
           }
           attribute[properties[i]] = list.child(properties[i]).val();
          }

          attribute['latitude'] = locationSnap.latitude;
          attribute['longitude'] = locationSnap.longitude;
          attribute['distance'] =  locationSnap.distance;
          attributes.push(attribute);    
         } // refSnap.key == locationSnap.key
        }); // locations.forEach
       }); // refsSnap.forEach
       // return JSON.stringify(attributes);
       return resolve(attributes);
      }); // ref.on
    

    }); // onreadyregistration

   });
            
} 
tejzpr
  • 945
  • 8
  • 19
  • Hi tejzpr. I think you are right, this is an order of execution problem. But, db_GetAllByLocation does not return a promise. It returns an array of objects. The calling method - getLandmarks - does return a promise and I have resolve() there. But I think you are right, resolve(landmarks) is getting called before the stack execution of the async function is complete. – Peter Apr 09 '18 at 23:26
  • Another reason may be that parseFloat() for userLatitude or userLongitude might be evaluating to NaN. Which in turn sends the flow to the "else" block therefore landmarks might be displaying the default value ;] – tejzpr Apr 09 '18 at 23:58
  • Thanks again. I appreciate the reply. Unfortunately that’s not it. I can confirm the parameters are getting through. – Peter Apr 10 '18 at 00:05
  • Thanks, I'll try the method modification after dinner. I had a hunch that moving all the promise logic from the service classes (where getLandmarks lives) to the persistence methods might work. I'll follow up after dinner and let you know, thanks again. – Peter Apr 10 '18 at 00:31
  • I tried your suggestion. The ref.on('end' is not recognized with the following error: Error: Query.on failed: First argument must be a valid event type = "value", "child_added", "child_removed", "child_changed", or "child_moved". I also am not so sure about chaining promises like that (having a promise call a promise). Is that normal? – Peter Apr 10 '18 at 14:08
  • 1
    Yes it is possible to chain Promises as long as the inner promises resolve properly. – tejzpr Apr 10 '18 at 17:00
  • What do you mean by "*the stack execution*"? – Bergi Apr 10 '18 at 18:40
  • @bergi https://medium.com/@thejasonfile/how-node-and-javascript-handle-asynchronous-functions-7feb9fc8a610 – tejzpr Apr 10 '18 at 19:24
  • @tejzpr That article doesn't use the term "*stack execution*". What do *you* mean by it? – Bergi Apr 10 '18 at 19:36
  • @Bergi I meant the execution context stack, I've updated my answer. – tejzpr Apr 10 '18 at 19:59
  • Then your sentence was wrong, `resolve` is called *after* the call stack execution of `db_GetAllByLocation` ends. It's called before the asynchronous stuff finishes, that's true. – Bergi Apr 10 '18 at 20:02
  • Sorry, got busy and realize I never marked an answer. Ultimately, moving the Promise flow around was the solve for this so marking your response as the answer. Thanks for your help. – Peter Apr 17 '18 at 15:23
0

OK, I sorted this by removing all my code and writing some test logic (I should have done this before I posted my question).

The below flow works for me, and, applied back to my code, gave me the results I was looking for. No need to re-post the code, but maybe the below flow will be helpful to somebody.

route

api.route('/api/v1/landmarks').get(Landmark.test);

index

exports.test = (req, res) => {

    const landmark = new LandmarkService();

    landmark.getLandmarksTest(req)
        .then(landmarks => {
            var final = {};
                final.attr1 = 'attr1';
                final.attr2 = 'attr2';
                final.landmarks = landmarks;
                res.json(final);
        })
        .catch(err => {
            logger.error(err);
            res.status(422).send(err.errors);
        });

};

service method

getLandmarksTest(req)
{

            const data = new DataService();

        data.db_PromiseTest().then(results => {
                return Promise.resolve(results);
          }).catch(err => {
                return Promise.reject(err.errors);
          });




}

data layer method

db_PromiseTest()
{
            var stub = {
                    "name": "Madame Uppercut",
                    "age": 39,
                    "secretIdentity": "Jane Wilson",
                    "powers": [
                    "Million tonne punch",
                    "Damage resistance",
                    "Superhuman reflexes"
                    ]
                };

                 return Promise.resolve(stub);


}
Peter
  • 5,251
  • 16
  • 63
  • 98
  • Avoid the [`Promise` constructor antipattern](https://stackoverflow.com/q/23803743/1048572?What-is-the-promise-construction-antipattern-and-how-to-avoid-it) in `getLandmarksTest`! Also in `db_PromiseTest` you can use just `return Promise.resolve(…);`. – Bergi Apr 10 '18 at 18:41
  • Thanks, @Bergi - working through this. I am initially getting TypeError: Cannot read property '.then' of undefined in the Index file. – Peter Apr 10 '18 at 19:37
  • Most likely you just forgot a `return` in the right place. – Bergi Apr 10 '18 at 19:42
  • Hmmm. Updated my code. Any hunch where that might be? – Peter Apr 10 '18 at 19:56
  • Yes, as I guessed, there's no `return` in the `getLandmarksTest` function. (The ones in the callbacks don't count). And you don't need those `.then(Promise.resolve)` and `.catch(Promise.reject)` things at all, they don't do anything the original promise doesn't as well - just return the `data.db_PromiseTest()` result and be done with it. – Bergi Apr 10 '18 at 20:01
  • I'd *love* to "be done with it" :). Your code works perfectly for the test code. Comes back as described. However, when I remove the 'antipattern' and reproduce the Promise.resolve() flow in my actual code, it doesn't work. I have updated my original code to show you what I have now. I think this has to do with how the GeoFire callbacks are firing. Using Promise.resolve(attributes) inside of onreadyregistration thows "Cannot read property '.then". Putting that same statement at the end solves the error but the attributes array is not populated. It comes back empty. – Peter Apr 10 '18 at 20:55
  • Yes, for your actual code you will of course have to use the `new Promise` constructor, but that's not what this answer seems to be about :-) – Bergi Apr 10 '18 at 21:02