incorrect no. of args for superagent callback (request for comments)

[saraf]

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 get a object parsed from JSON where we are expecting an error.

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/

If we DO NOT pass in a callback with TWO arguments, ===> superagent does not bother to see that we are sending it a callback with THREE arguments, and instead, it sends us the RESPONSE as the only/first argument, which we expect to be an error in the callback. <===

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.

[knownasilya]

Looks like the test fails.

[saraf]

Will look at it shortly.

On Sunday, August 16, 2015, Ilya Radchenko notifications@github.com wrote:

Looks like the test fails.

— Reply to this email directly or view it on GitHub https://github.com/Strider-CD/strider-github/pull/49#issuecomment-131473134 .

[knownasilya]

Is this finished?

[saraf]

yes ... done, except for the last bit -

(Note: I do not know if we can reliably test for the account being an enterprise git account by looking at the path for "/api/v3" (and the req object does not now have a "uri" property anymore.) Needs to be tested/looked at by someone with an enterprise git account.)

ok for review/comments/testing by another set of eyes.

thanks!

[saraf]

Done.