WIP: incorrect no. of args for superagent callback

The plugin was failing to fetch the repos when
configuring a github account for the first time OR
when trying to do a "Refresh Account" from the web
page.

There were a couple of issues here:
a. JSON.parse was not necessary since superagent
   returns a parsed object if we receive json.
https://visionmedia.github.io/superagent/#parsing-response%20bodies

b. We were passing a callback with three arguments
   to superagent in .end(callback)
   for example in -
   get_oauth2(uri, {per_page: 30, page: page},
          access_token, function (error, response, body) {
                             //   ^ here, we should send
                             //   only error and response
  //In any case superagent puts the body inside response.body
  //https://visionmedia.github.io/superagent/

  This may be verified by looking at
       superagent/lib/node/index.js

       Around line 623 - (debugs added)

       Request.prototype.callback = function(err, res){
         debug('When calling the callback - err is ' + JSON.stringify(err, null, 4));
         var fn = this._callback;
         debug("And the number of arguments the callback expects is: " + fn.length );
         this.clearTimeout();
         if (this.called) return console.warn('double callback!');
         this.called = true;
         if (2 == fn.length) return fn(err, res);//<====== we want this to be true
         if (err) return this.emit('error', err);
         debug("Here, we are sending the first parameter to the callback as the response, " +
	       "but GOTCHA - our callback thinks the first param is an error and THROWS!");
         fn(res);
       };

c. Still need to understand the presence of this.group and group()
   as present in the comments in the code.
   The reason why it fails near
       get_github_repos(): Error with admin team memberships: [object Object]
   is because, the previous function in the "Step" has a call to
      get_oauth2(url, {}, token, group()) - which basically sends the last parameter
      as it is to superagent. And thus we have an object with data received from the
   API, which we think is an err.

Author: saraf

lib/api.js

@@ -73,7 +73,7 @@ function deleteHooks(reponame, url, token, callback) {
     .get(apiUrl)
     .set('Authorization', 'token ' + token)
     .set('User-Agent', 'StriderCD (http://stridercd.com)')
-    .end(function (res) {
+    .end(function (res) { //CHECK: have a callback with the first param as err and process that err
       if (res.status > 300) {
         debug('Error getting hooks', res.status, res.text);
         return callback(res.status);
@@ -87,7 +87,7 @@ function deleteHooks(reponame, url, token, callback) {
             .del(hook.url)
             .set('Authorization', 'token ' + token)
             .set('User-Agent', 'StriderCD (http://stridercd.com)')
-            .end(function (res) {
+            .end(function (res) { //CHECK: have a callback with the first param as err and process that err
               if (res.status !== 204) {
                 debug('bad status', res.status, hook.id, hook.url);
                 return next(new Error('Failed to delete a webhook: status for url ' + hook.url + ': ' + res.status));
@@ -127,7 +127,7 @@ function getFile(filename, ref, accessToken, owner, repo, done) {
   if (accessToken) {
     req = req.set('Authorization', 'token ' + accessToken);
   }
-  req.end(function (res) {
+  req.end(function (res) {  //CHECK: have a callback with the first param as err and process that err
     if (res.error) return done(res.error, null);
     if (!res.body.content) {
       return done();
@@ -152,6 +152,7 @@ var get_oauth2 = module.exports.get_oauth2 = function (url, q_params, access_tok
   // Construct the query. Allow the user to override the access_token through q_params.
   var query = _.assign({}, {access_token : access_token}, q_params);
   debug('GET OAUTH2 URL:', url);
+  debug('Inside get_oauth2: Callback type: ' + typeof callback + ' Number of arguments expected by: ' + callback.length);
   client
     .get(url)
     .query(query)
@@ -173,10 +174,11 @@ var api_call = module.exports.api_call = function (path, access_token, callback,
   // If the user provided a superagent instance, use that.
   client = client || superagent;
   var url = GITHUB_API_ENDPOINT + path;
-  debug('API CALL:', url, params, access_token);
-  get_oauth2(url, {}, access_token, function (error, res, body) {
+  debug('API CALL:', url, access_token);
+  get_oauth2(url, {}, access_token, function (error, res) {
+    debug("We get an error from the API: " + error);
     if (!error && res.statusCode == 200) {
-      var data = JSON.parse(body);
+      var data = res.body;
       callback(null, res, data);
     } else {
       callback(error, res, null);
@@ -242,11 +244,12 @@ var pageinated_api_call = module.exports.pageinated_api_call = function (path, a
 
   function loop(uri, page) {
     debug('PAGINATED API CALL URL:', uri);
-    get_oauth2(uri, {per_page: 30, page: page}, access_token, function (error, res, body) {
+    get_oauth2(uri, {per_page: 30, page: page}, access_token, function (error, res) {
+    debug("We get an error: " + error);
       if (!error && res.statusCode == 200) {
         var data;
         try {
-          data = JSON.parse(body);
+          data = res.body;
         } catch (e) {
           return callback(e, null);
         }
@@ -267,11 +270,13 @@ var pageinated_api_call = module.exports.pageinated_api_call = function (path, a
         }
       } else {
         if (!error) {
+          debug("We did not get an error, but status code was: " + res.statusCode);
           if (res.statusCode === 401 || res.statusCode === 403) {
             return callback(new Error('Github app is not authorized. Did you revoke access?'))
           }
           return callback(new Error('Status code is ' + res.statusCode + ' not 200. Body: ' + body))
         } else {
+          debug("We did get an error from the API " + error);
           return callback(error, null);
         }
       }
@@ -343,13 +348,16 @@ function getRepos(token, username, callback) {
     },
     function fetchTeamDetails(err, results) {
       if (err) {
+        debug(err.message);
+        debug(err.name);
+        debug(err.stack);
         debug('get_github_repos() - Error fetching Org Teams response - %s', err);
         return callback(err);
       }
       var teams = [];
       _.each(results, function (result) {
         try {
-          var team_data = JSON.parse(result.body);
+          var team_data = result.body;
           _.each(team_data, function (t) {
             teams.push(t);
           });
@@ -372,14 +380,17 @@ function getRepos(token, username, callback) {
       var team_details = [];
       _.each(results, function (result) {
         try {
-          var td = JSON.parse(result.body);
+          var td = result.body;
           team_details.push(td);
         } catch (e) {
           debug('get_github_repos(): Error parsing JSON in detail team response - %s', e);
         }
       });
       // For each Team with admin privs, test for membership
       var group = this.group();
+      debug("****************************************************************\nThis is: " + JSON.stringify(this, null, 4)); //CHECK: undefined
+      debug("****************************************************************\ngroup is: " + JSON.stringify(group, null, 4)); //CHECK: undefined
+
       var team_detail_requests = {};
       _.each(team_details, function (team_details) {
         if (team_details.permission != 'admin') {
@@ -388,7 +399,16 @@ function getRepos(token, username, callback) {
         team_detail_requests[team_details.id] = team_details;
         var url = GITHUB_API_ENDPOINT + '/teams/' + team_details.id + '/members/' + username;
         debug('TEAM DETAIL URL', url);
-        get_oauth2(url, {}, token, group());
+        get_oauth2(url, {}, token, group()); //<------------------- this is a problem
+                                             //get_oauth sends this callback as it is to superagent
+                                             //superagent expects a callback that takes two arguments
+                                             //in the other places it works because we are sending group() as
+                                             //as a parameter to api_call or pageinated_api_call, and they are now correctly wrapping
+                                             //get_oauth, passing in a callback that expects two arguments
+                                             //if we were to replace this get_oauth2 call by a call to api_call,
+                                             //it would probably work as a hack, but the intention behind group()
+                                             //and this.group() is yet to be understood by me
+
       });
       this.team_detail_requests = team_detail_requests;
     },

lib/index.js

@@ -23,6 +23,7 @@ var GITHUB_API_ENDPOINT = 'https://api.github.com';
  * @param {Object} client An alternative superagent instance to use.
  */
 var get_oauth2 = module.exports.get_oauth2 = function (url, q_params, access_token, callback, client) {
+  console.debug('OAUTH2 CALLBACK LENGTH:', callback.length);
   // If the user provided a superagent instance, use that.
   client = client || superagent;
   // Construct the query. Allow the user to override the access_token through q_params.
@@ -50,9 +51,9 @@ var api_call = module.exports.api_call = function (path, access_token, callback,
   client = client || superagent;
   var url = GITHUB_API_ENDPOINT + path;
   debug('github api_call(): path %s', path);
-  get_oauth2(url, {}, access_token, function (error, res, body) {
+  get_oauth2(url, {}, access_token, function (error, res) {
     if (!error && res.statusCode == 200) {
-      var data = JSON.parse(body);
+      var data = res.body;
       callback(null, res, data);
     } else {
       callback(error, res, null);
@@ -117,11 +118,11 @@ var pageinated_api_call = module.exports.pageinated_api_call = function (path, a
   var pages = [];
 
   function loop(uri, page) {
-    get_oauth2(uri, {per_page: 30, page: page}, access_token, function (error, response, body) {
+    get_oauth2(uri, {per_page: 30, page: page}, access_token, function (error, response) {
       if (!error && response.statusCode == 200) {
         var data;
         try {
-          data = JSON.parse(body);
+          data = response.body;
         } catch (e) {
           return callback(e, null);
         }
@@ -142,7 +143,7 @@ var pageinated_api_call = module.exports.pageinated_api_call = function (path, a
         }
       } else {
         if (!error) {
-          return callback('Status code is ' + response.statusCode + ' not 200. Body: ' + body)
+          return callback('Status code is ' + response.statusCode + ' not 200. Body: ' + response.body)
         } else {
           return callback(error, null);
         }
@@ -210,7 +211,7 @@ module.exports.get_github_repos = function (user, callback) {
       _.each(results, function (result) {
         try {
           debug('For Organizations: %s', result.request.uri.path.split('/')[2]);
-          var team_data = JSON.parse(result.body);
+          var team_data = result.body;
           _.each(team_data, function (t) {
             debug('Team details: %j', t);
             teams.push(t);
@@ -235,7 +236,7 @@ module.exports.get_github_repos = function (user, callback) {
       var team_details = [];
       _.each(results, function (result) {
         try {
-          var td = JSON.parse(result.body);
+          var td = result.body;
           team_details.push(td);
         } catch (e) {
           debug('get_github_repos(): Error parsing JSON in detail team response - %s', e);