Refactoring nested callbacks

index.md

The code for sending a PM to multiple users for review is looking like this:

controller.hears(['pr-review (\\w+/\\w+/\\d+) with ((<@\\w+>\\s?){1,})'], botMessageTypes.ALL,
                 function(bot, message) {
  var repoDataGroup = message.match[1];
  var userIdsGroup = message.match[2];

  var accountName = parseRepoDetails(repoDataGroup).account;
  var repoName = parseRepoDetails(repoDataGroup).repository;
  var prId = parseRepoDetails(repoDataGroup).prId;
  var githubRepoUrl = format("https://github.com/{account}/{repository}/pull/{prId}",
                             { account: accountName, repository: repoName, prId: prId });

  bot.reply(message, "Gotcha, sending a PM to each reviewer. :robot_face:");

  bot.api.users.list({}, function(err, teamData) {
    if(err) {
      bot.botkit.log("Couldn't fetch team data from API", err);
      return;
    }

    var members = teamData.members;
    var msgSender = _.find(members, function(member) { return member.id == message.user });

    parsedUserIds(userIdsGroup).forEach(function(id) {
      var msgReceiver = _.find(members, function(member) { return member.id == id });

      bot.startPrivateConversation({ user: id }, function(err, conversation) {
        if(err) {
          bot.botkit.log("Couldn't initiate private conversation", err);
          return;
        }

        var message = format("Hey {to}, {from} just requested a Pull Request Review, here's the link: {githubLink}, please go and check it out!", 
                             { to: msgReceiver.name,
                               from: msgSender.name,
                               githubLink: githubRepoUrl });

                               conversation.say(message);
      });
    });
  });
});

One of the problems with this is that due to the async calls to the Slack WEBAPI and the bot api, we need to use callbacks to perform an action after the first was executed.

I've been looking in to multiple ways to solve the problem, there are libraries like asyncawait, async, asyncwaterfall, and another couple for promises.

My first attempt was to use the async library, it has a waterfall method useful to chain function execution, that got me to this code:

  async.waterfall([
    function getUsersData(callback) {
      bot.api.users.list({}, function(err, data) {
        if(err) {
          callback(err);
          return;
        }

        callback(null, data);
      });
    },
    function sendPMToReceivers(data, cb) {
      var members = data.members;
      var msgSender = findMember(members, message.user);

      async.each(parsedUserIds(userIdsGroup), function(id, eachCb) {
        var msgReceiver = findMember(members, id);

        bot.startPrivateConversation({ user: id }, function(err, conversation) {
          if(err) {
            eachCb(err);
            return;
          }

          var message = format("Hey {to}, {from} just requested a Pull Request Review, here's the link: {githubLink}, please go and check it out!", 
                               { to: msgReceiver.name,
                               from: msgSender.name,
                               githubLink: githubRepoUrl });

          conversation.say(message);
          eachCb();
        });
      }, function(err) {
        if(err) {
          bot.botkit.log(err);
          return;
        }

        cb();
      });
    }
  ], function (err, result) {
    if(err) { 
      bot.botkit.log(err);
      return;
    }
  });

It helps to avoid nesting, but the code now looks much uglier, it's hard to follow what's going on. I'm surely missing something, because that doesn't feel right to me.

I started to reimplement using promises, the code looks like this:

controller.hears(['pr-review (\\w+/\\w+/\\d+) with ((<@\\w+>\\s?){1,})'], botMessageTypes.ALL,
                 function(bot, message) {
  var repoDataGroup = message.match[1];
  var userIdsGroup = message.match[2];

  var accountName = parseRepoDetails(repoDataGroup).account;
  var repoName = parseRepoDetails(repoDataGroup).repository;
  var prId = parseRepoDetails(repoDataGroup).prId;
  var githubRepoUrl = format("https://github.com/{account}/{repository}/pull/{prId}",
                             { account: accountName, repository: repoName, prId: prId });

  bot.reply(message, "Gotcha, sending a PM to each reviewer. :robot_face:");

  bot.api.users.list({}).then(function(teamData) {
    var members = teamData.members;
    var msgSender = findMember(members, message.user);

    parsedUserIds(userIdsGroup).forEach(function(id) {
      var msgReceiver = findMember(members, id);

      bot.startPrivateConversationAsync({ user: id }).then(function(conversation) {
        var message = format("Hey {to}, {from} just requested a Pull Request Review, here's the link: {githubLink}, please go and check it out!", 
                             { to: msgReceiver.name,
                               from: msgSender.name,
                               githubLink: githubRepoUrl });

        conversation.say(message);
      });
    });
  });
});

I haven't been able to make this work yet, as the bot api does not return promises, and using Promise.promisifyAll from bluebird breaks the api.

Also, I haven't found any working examples for botkit using anything other than callbacks, so no luck that way.