3

I'm using an HTTP-triggered Firebase cloud function to make an HTTP request. I get back an array of results (events from Meetup.com), and I push each result to the Firebase realtime database. But for each result, I also need to make another HTTP request for one additional piece of information (the category of the group hosting the event) to fold into the data I'm pushing to the database for that event. Those nested requests cause the cloud function to crash with an error that I can't make sense of.

const functions = require("firebase-functions");
const admin = require("firebase-admin");
admin.initializeApp();

const request = require('request');

exports.foo = functions.https.onRequest(
    (req, res) => {
        var ref = admin.database().ref("/foo");
        var options = {
            url: "https://api.meetup.com/2/open_events?sign=true&photo-host=public&lat=39.747988&lon=-104.994945&page=20&key=****",
            json: true
        };
        return request(
            options,
            (error, response, body) => {
                if (error) {
                    console.log(JSON.stringify(error));
                    return res.status(500).end();
                }
                if ("results" in body) {
                    for (var i = 0; i < body.results.length; i++) {
                        var result = body.results[i];
                        if ("name" in result &&
                            "description" in result &&
                            "group" in result &&
                            "urlname" in result.group
                        ) {
                            var groupOptions = {
                                url: "https://api.meetup.com/" + result.group.urlname + "?sign=true&photo-host=public&key=****",
                                json: true
                            };
                            var categoryResult = request(
                                groupOptions,
                                (groupError, groupResponse, groupBody) => {
                                    if (groupError) {
                                        console.log(JSON.stringify(error));
                                        return null;
                                    }
                                    if ("category" in groupBody &&
                                        "name" in groupBody.category
                                    ) {
                                        return groupBody.category.name;
                                    }
                                    return null;
                                }
                            );
                            if (categoryResult) {
                                var event = {
                                    name: result.name,
                                    description: result.description,
                                    category: categoryResult
                                };
                                ref.push(event);
                            }
                        }
                    }
                    return res.status(200).send("processed events");
                } else {
                    return res.status(500).end();
                }
            }
        );
    }
);

The function crashes, log says:

Error: Reference.push failed: first argument contains a function in property 'foo.category.domain._events.error' with contents = function (err) {
      if (functionExecutionFinished) {
        logDebug('Ignoring exception from a finished function');
      } else {
        functionExecutionFinished = true;
        logAndSendError(err, res);
      }
    }
    at validateFirebaseData (/user_code/node_modules/firebase-admin/node_modules/@firebase/database/dist/index.node.cjs.js:1436:15)
    at /user_code/node_modules/firebase-admin/node_modules/@firebase/database/dist/index.node.cjs.js:1479:13
    at Object.forEach (/user_code/node_modules/firebase-admin/node_modules/@firebase/util/dist/index.node.cjs.js:837:13)
    at validateFirebaseData (/user_code/node_modules/firebase-admin/node_modules/@firebase/database/dist/index.node.cjs.js:1462:14)
    at /user_code/node_modules/firebase-admin/node_modules/@firebase/database/dist/index.node.cjs.js:1479:13
    at Object.forEach (/user_code/node_modules/firebase-admin/node_modules/@firebase/util/dist/index.node.cjs.js:837:13)
    at validateFirebaseData (/user_code/node_modules/firebase-admin/node_modules/@firebase/database/dist/index.node.cjs.js:1462:14)
    at /user_code/node_modules/firebase-admin/node_modules/@firebase/database/dist/index.node.cjs.js:1479:13
    at Object.forEach (/user_code/node_modules/firebase-admin/node_modules/@firebase/util/dist/index.node.cjs.js:837:13)
    at validateFirebaseData (/user_code/node_modules/firebase-admin/node_modules/@firebase/database/dist/index.node.cjs.js:1462:14)

If I leave out the bit for getting the group category, the rest of the code works fine (just writing the name and description for each event to the database, no nested requests). So what's the right way to do this?

Renaud Tarnec
  • 79,263
  • 10
  • 95
  • 121
gdejohn
  • 7,451
  • 1
  • 33
  • 49
  • You'll clean up a lot of this code (and possibly reveal the source of the error) by using promise chains instead of deeply nesting your callbacks (which is kind of a JavaScript antipattern these days). You also have a future potential problem with ignoring the promise returned by `ref.push(event)`. – Doug Stevenson Oct 17 '18 at 18:15

3 Answers3

1

I suspect this issue is due to the callbacks. When you use firebase functions, the exported function should wait on everything to execute or return a promise that resolves once everything completes executing. In this case, the exported function will return before the rest of the execution completes.

Here's a start of something more promise based -

const functions = require("firebase-functions");
const admin = require("firebase-admin");
admin.initializeApp();

const request = require("request-promise-native");

    exports.foo = functions.https.onRequest(async (req, res) => {
    const ref = admin.database().ref("/foo");
    try {
        const reqEventOptions = {
            url:
                "https://api.meetup.com/2/open_events?sign=true&photo-host=public&lat=39.747988&lon=-104.994945&page=20&key=xxxxxx",
            json: true
        };
        const bodyEventRequest = await request(reqEventOptions);
        if (!bodyEventRequest.results) {
            return res.status(200).end();
        }
        await Promise.all(
            bodyEventRequest.results.map(async result => {
                if (
                    result.name &&
                    result.description &&
                    result.group &&
                    result.group.urlname
                ) {
                    const event = {
                        name: result.name,
                        description: result.description
                    };

                    // get group information
                    const groupOptions = {
                        url:
                            "https://api.meetup.com/" +
                            result.group.urlname +
                            "?sign=true&photo-host=public&key=xxxxxx",
                        json: true
                    };

                    const categoryResultResponse = await request(groupOptions);
                    if (
                        categoryResultResponse.category &&
                        categoryResultResponse.category.name
                    ) {
                        event.category = categoryResultResponse.category.name;
                    }

                    // save to the databse
                    return ref.push(event);
                }
            })
        );
        return res.status(200).send("processed events");
    } catch (error) {
        console.error(error.message);
    }
});

A quick overview of the changes -

  • Use await and async calls to wait for things to complete vs. being triggered in a callback (async and await are generally much easier to read than promises with .then functions as the execution order is the order of the code)
  • Used request-promise-native which supports promises / await (i.e. the await means wait until the promise returns so we need something that returns a promise)
  • Used const and let vs. var for variables; this improves the scope of variables
  • Instead of doing checks like if(is good) { do good things } use a if(isbad) { return some error} do good thin. This makes the code easier to read and prevents lots of nested ifs where you don't know where they end
  • Use a Promise.all() so retrieving the categories for each event is done in parallel
R. Wright
  • 960
  • 5
  • 9
  • Does this require telling Firebase to use Node.js 8? – gdejohn Oct 17 '18 at 18:47
  • Yes, this one is based upon Node.js 8, so you'd need to add "engines": { “node": “8” } to your package.json. – R. Wright Oct 17 '18 at 18:50
  • That's finishing with status code 500, and there's a log entry that just has the message "{}". I'm not sure where to go from there. – gdejohn Oct 17 '18 at 19:08
  • Anything in the cloud function logs with details on the message? Also, probably best to use console.error() instead of log as I had originally. – R. Wright Oct 17 '18 at 19:11
  • There's just the debug-level entries saying that it started and that it finished with status code 500, and in between there's an info-level entry that only says "{}". – gdejohn Oct 17 '18 at 19:14
  • I got it working, please check the code in my [answer](https://stackoverflow.com/a/52862973/464306). I would be grateful for any feedback, since my understanding of why it's working now is pretty shaky. – gdejohn Oct 17 '18 at 20:19
  • I think your version looks alright. (The only challenge is parallel vs. sequential execution.) I've also updated a test version above. In addition to fixing items, I updated to a more modern JS style which should make it more readable. – R. Wright Oct 17 '18 at 20:39
1

There are two main changes you should implement in your code:

  • Since request does not return a promise you need to use an interface wrapper for request, like request-promise in order to correctly chain the different asynchronous events (See Doug's comment to your question)
  • Since you will then call several times (in parallel) the different endpoints with request-promise you need to use Promise.all() in order to wait all the promises resolve before sending back the response. This is also the case for the different calls to the Firebase push() method.

Therefore, modifying your code along the following lines should work.

I let you modifying it in such a way you get the values of name and description used to construct the event object. The order of the items in the results array is exactly the same than the one of the promises one. So you should be able, knowing that, to get the values of name and description within results.forEach(groupBody => {}) e.g. by saving these values in a global array.


const functions = require('firebase-functions');
const admin = require('firebase-admin');
admin.initializeApp();

var rp = require('request-promise');

exports.foo = functions.https.onRequest((req, res) => {
  var ref = admin.database().ref('/foo');
  var options = {
    url:
      'https://api.meetup.com/2/open_events?sign=true&photo-host=public&lat=39.747988&lon=-104.994945&page=20&key=****',
    json: true
  };
  rp(options)
    .then(body => {
      if ('results' in body) {
        const promises = [];
        for (var i = 0; i < body.results.length; i++) {
          var result = body.results[i];
          if (
            'name' in result &&
            'description' in result &&
            'group' in result &&
            'urlname' in result.group
          ) {
            var groupOptions = {
              url:
                'https://api.meetup.com/' +
                result.group.urlname +
                '?sign=true&photo-host=public&key=****',
              json: true
            };

            promises.push(rp(groupOptions));
          }
        }
        return Promise.all(promises);
      } else {
        throw new Error('err xxxx');
      }
    })
    .then(results => {
      const promises = [];

      results.forEach(groupBody => {
        if ('category' in groupBody && 'name' in groupBody.category) {
          var event = {
            name: '....',
            description: '...',
            category: groupBody.category.name
          };
          promises.push(ref.push(event));
        } else {
          throw new Error('err xxxx');
        }
      });
      return Promise.all(promises);
    })
    .then(() => {
      res.send('processed events');
    })
    .catch(error => {
      res.status(500).send(error);
    });
});
Renaud Tarnec
  • 79,263
  • 10
  • 95
  • 121
  • I got it working, based on some changes to R. Wright's [answer](https://stackoverflow.com/a/52861334/464306). Please take a look at the code in my [answer](https://stackoverflow.com/a/52862973/464306). Is there anything objectionable about the approach I took compared to yours? – gdejohn Oct 17 '18 at 20:24
1

I made some changes and got it working with Node 8. I added this to my package.json:

"engines": {
    "node": "8"
}

And this is what the code looks like now, based on R. Wright's answer and some Firebase cloud function sample code.

const functions = require("firebase-functions");
const admin = require("firebase-admin");
admin.initializeApp();

const request = require("request-promise-native");

exports.foo = functions.https.onRequest(
    async (req, res) => {
        var ref = admin.database().ref("/foo");
        var options = {
            url: "https://api.meetup.com/2/open_events?sign=true&photo-host=public&lat=39.747988&lon=-104.994945&page=20&key=****",
            json: true
        };
        await request(
            options,
            async (error, response, body) => {
                if (error) {
                    console.error(JSON.stringify(error));
                    res.status(500).end();
                } else if ("results" in body) {
                    for (var i = 0; i < body.results.length; i++) {
                        var result = body.results[i];
                        if ("name" in result &&
                            "description" in result &&
                            "group" in result &&
                            "urlname" in result.group
                        ) {
                            var groupOptions = {
                                url: "https://api.meetup.com/" + result.group.urlname + "?sign=true&photo-host=public&key=****",
                                json: true
                            };
                            var groupBody = await request(groupOptions);
                            if ("category" in groupBody && "name" in groupBody.category) {
                                var event = {
                                    name: result.name,
                                    description: result.description,
                                    category: groupBody.category.name
                                };
                                await ref.push(event);
                            }
                        }
                    }
                    res.status(200).send("processed events");
                }
            }
        );
    }
);
gdejohn
  • 7,451
  • 1
  • 33
  • 49