Merge pull request #555 from CodeNow/improvements-for-github-prs

Improvements for GitHub PR's commenting

Author: tjmehta

lib/models/apis/github.js

@@ -487,7 +487,8 @@ Github.prototype.addComment = function(shortRepo, issueId, text, cb) {
     debug('back from creating issue comment', err);
     if (err) {
       err = (err.code === 404) ?
-        Boom.notFound('Issue ' + shortRepo + '/issues/' + issueId + ' not found.', { err: err }) :
+        Boom.notFound('Issue ' + shortRepo + '/issues/' + issueId + ' not found.',
+          { err: err, report: false }) :
         Boom.create(502, 'Failed to get issue comments ' + shortRepo + '/issues/' + issueId ,
           { err: err });
       cb(err);
@@ -593,7 +594,8 @@ Github.prototype.listOpenPullRequestsForBranch = function (shortRepo, branch, cb
   this.pullRequests.getAll(query, function (err, prs) {
     if (err) {
       err = (err.code === 404) ?
-        Boom.notFound('Cannot find open PRs for ' + shortRepo + '@'  + branch, { err: err }) :
+        Boom.notFound('Cannot find open PRs for ' + shortRepo + '@'  + branch,
+          { err: err, report: false  }) :
         Boom.create(502, 'Failed to get PRs for ' + shortRepo + '@'  + branch, { err: err });
       cb(err);
     }

lib/routes/actions/github.js

@@ -29,6 +29,7 @@ var pluck = require('101/pluck');
 var noop = require('101/noop');
 var error = require('error');
 var github = require('middlewares/apis').github;
+var dogstatsd = require('models/datadog');
 
 /** Receive the Github hooks
  *  @event POST rest/actions/github
@@ -43,6 +44,7 @@ var pushSessionUser = {
 };
 
 app.post('/actions/github/',
+  reportDatadogEvent,
   mw.headers('user-agent').require().matches(/^GitHub.*$/),
   mw.headers('x-github-event', 'x-github-delivery').require(),
   mw.headers('x-github-event').matches(/^ping$/).then(
@@ -141,6 +143,13 @@ app.post('/actions/github/',
   mw.res.status(202),
   mw.res.send('No action set up for that payload.'));
 
+
+function reportDatadogEvent (req, res, next) {
+  var eventName = req.get('x-github-event') || '';
+  dogstatsd.increment('api.actions.github.events', ['event:' + eventName]);
+  next();
+}
+
 function parseGitHubPRData (req, res, next) {
   var repository = keypather.get(req, 'body.repository');
   if (!repository) {

scripts/update-github-webhooks-event.js

@@ -17,10 +17,40 @@ function findAllRepos(cb) {
 }
 
 
+var allErrors = [];
+
+
+function findUser (users, cb) {
+  var user;
+  var count = 0;
+  async.whilst(
+    function () { return count < users.length; },
+    function (callback) {
+      var userId = users[count];
+      User.findByGithubId(userId, function (err, gitHubUser) {
+        count++;
+        if (gitHubUser) {
+          // force finish
+          user = gitHubUser;
+          count = users.length;
+        }
+        callback();
+      });
+    },
+    function (err) {
+      if (err) {
+        return cb(err);
+      }
+      cb(null, user);
+    }
+  );
+}
+
+
 function findUsersForRepos(repos, cb) {
   debug('findUsersForRepos', 'total repos num:', repos.length);
   async.map(repos, function (repo, callback) {
-    User.findByGithubId(repo.creators[0], function (err, user) {
+    findUser(repo.creators, function (err, user) {
       if (err) { return callback(err); }
       repo.user = user;
       callback(null, repo);
@@ -31,21 +61,39 @@ function findUsersForRepos(repos, cb) {
 
 function updateHooksEvents(repos, cb) {
   debug('updateHooksEvents', 'total repos num:', repos.length);
-  async.mapLimit(repos, function(repo, callback) {
+  async.mapLimit(repos, 50, function(repo, callback) {
+    debug('processing repo', repo);
+    if (!repo.user) {
+      debug('user not found for the repo', repo);
+      return callback();
+    }
     var github = new GitHub({token: repo.user.accounts.github.accessToken});
     // this will actually update hook (not just create if missing)
-    github.createRepoHookIfNotAlready(repo._id, function (err, result) {
+    github.createRepoHookIfNotAlready(repo._id, function (err) {
       if (err) {
-        console.log('failed to update webhook for:', repo, '; error: ', err);
+        allErrors.push(err);
+        if(err.output.statusCode === 404) {
+          debug('repos not found. just skip it', repo);
+          callback(null);
+        }
+        else if(err.output.statusCode === 502) {
+          debug('access token removed. just skip it', repo);
+          callback(null);
+        }
+        else {
+          callback(err);
+        }
+      }
+      else {
+        callback(null);
       }
-      callback(null, result);
     });
-  }, 10, cb);
+  }, cb);
 }
 
-function finish (err, results) {
+function finish (err) {
   console.log('DONE: err?', err);
-  console.log('all results', results);
+  console.log('all errors', allErrors);
   process.exit();
 }
 async.waterfall([