Skip to content

Commit 6f3197f

Browse files
author
Ilya Radchenko
committed
Merge pull request #49 from saraf/master
incorrect no. of args for superagent callback (request for comments)
2 parents 489484d + 8a09caa commit 6f3197f

File tree

4 files changed

+50
-34
lines changed

4 files changed

+50
-34
lines changed

‎lib/api.js‎

Lines changed: 37 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ var debug = require('debug')('strider-github:api');
66
var Step = require('step');
77
var superagent = require('superagent');
88
var url = require('url');
9+
var util = require('util');
910

1011
var GITHUB_API_ENDPOINT = process.env.PLUGIN_GITHUB_API_ENDPOINT || 'https://api.github.com';
1112

@@ -47,6 +48,7 @@ function createHooks(reponame, url, secret, token, callback) {
4748
.set('User-Agent', 'StriderCD (http://stridercd.com)')
4849
.end(function (err, res) {
4950
if (err) return callback(err);
51+
5052
if (res.statusCode !== 201) {
5153
var badStatusErr = new Error('Bad status code: ' + res.statusCode);
5254
badStatusErr.statusCode = res.statusCode;
@@ -73,7 +75,8 @@ function deleteHooks(reponame, url, token, callback) {
7375
.get(apiUrl)
7476
.set('Authorization', 'token ' + token)
7577
.set('User-Agent', 'StriderCD (http://stridercd.com)')
76-
.end(function (res) {
78+
.end(function (err, res) {
79+
if(err) return callback(err);
7780
if (res.status > 300) {
7881
debug('Error getting hooks', res.status, res.text);
7982
return callback(res.status);
@@ -87,7 +90,8 @@ function deleteHooks(reponame, url, token, callback) {
8790
.del(hook.url)
8891
.set('Authorization', 'token ' + token)
8992
.set('User-Agent', 'StriderCD (http://stridercd.com)')
90-
.end(function (res) {
93+
.end(function (err, res) {
94+
if(err) return next(new Error('Failed to delete webhook ' + hook.url + 'Error: ' + err));
9195
if (res.status !== 204) {
9296
debug('bad status', res.status, hook.id, hook.url);
9397
return next(new Error('Failed to delete a webhook: status for url ' + hook.url + ': ' + res.status));
@@ -127,7 +131,8 @@ function getFile(filename, ref, accessToken, owner, repo, done) {
127131
if (accessToken) {
128132
req = req.set('Authorization', 'token ' + accessToken);
129133
}
130-
req.end(function (res) {
134+
req.end(function (err, res) {
135+
if(err) return done(err, null);
131136
if (res.error) return done(res.error, null);
132137
if (!res.body.content) {
133138
return done();
@@ -152,6 +157,7 @@ var get_oauth2 = module.exports.get_oauth2 = function (url, q_params, access_tok
152157
// Construct the query. Allow the user to override the access_token through q_params.
153158
var query = _.assign({}, {access_token : access_token}, q_params);
154159
debug('GET OAUTH2 URL:', url);
160+
debug('Inside get_oauth2: Callback type: ' + typeof callback + ' Number of arguments expected by: ' + callback.length);
155161
client
156162
.get(url)
157163
.query(query)
@@ -173,12 +179,13 @@ var api_call = module.exports.api_call = function (path, access_token, callback,
173179
// If the user provided a superagent instance, use that.
174180
client = client || superagent;
175181
var url = GITHUB_API_ENDPOINT + path;
176-
debug('API CALL:', url, params, access_token);
177-
get_oauth2(url, {}, access_token, function (error, res, body) {
182+
debug('API CALL:', url, access_token);
183+
get_oauth2(url, {}, access_token, function (error, res) {
178184
if (!error && res.statusCode == 200) {
179-
var data = JSON.parse(body);
185+
var data = res.body;
180186
callback(null, res, data);
181187
} else {
188+
debug("We get an error from the API: " + error);
182189
callback(error, res, null);
183190
}
184191
}, client);
@@ -242,11 +249,12 @@ var pageinated_api_call = module.exports.pageinated_api_call = function (path, a
242249

243250
function loop(uri, page) {
244251
debug('PAGINATED API CALL URL:', uri);
245-
get_oauth2(uri, {per_page: 30, page: page}, access_token, function (error, res, body) {
252+
get_oauth2(uri, {per_page: 30, page: page}, access_token, function (error, res) {
253+
246254
if (!error && res.statusCode == 200) {
247255
var data;
248256
try {
249-
data = JSON.parse(body);
257+
data = res.body;
250258
} catch (e) {
251259
return callback(e, null);
252260
}
@@ -267,11 +275,13 @@ var pageinated_api_call = module.exports.pageinated_api_call = function (path, a
267275
}
268276
} else {
269277
if (!error) {
278+
debug("We did not get an error, but status code was: " + res.statusCode);
270279
if (res.statusCode === 401 || res.statusCode === 403) {
271280
return callback(new Error('Github app is not authorized. Did you revoke access?'))
272281
}
273-
return callback(new Error('Status code is ' + res.statusCode + ' not 200. Body: ' + body))
282+
return callback(new Error('Status code is ' + res.statusCode + ' not 200. Body: ' + res.body))
274283
} else {
284+
debug("We did get an error from the API " + error);
275285
return callback(error, null);
276286
}
277287
}
@@ -302,6 +312,7 @@ function getRepos(token, username, callback) {
302312
// see strider-extension-loader for details
303313

304314
/* jshint -W064 */
315+
/* jshint -W040 */
305316
Step(
306317
function fetchReposAndOrgs() {
307318
// First fetch the user's repositories and organizations in parallel.
@@ -343,13 +354,16 @@ function getRepos(token, username, callback) {
343354
},
344355
function fetchTeamDetails(err, results) {
345356
if (err) {
357+
debug(err.message);
358+
debug(err.name);
359+
debug(err.stack);
346360
debug('get_github_repos() - Error fetching Org Teams response - %s', err);
347361
return callback(err);
348362
}
349363
var teams = [];
350364
_.each(results, function (result) {
351365
try {
352-
var team_data = JSON.parse(result.body);
366+
var team_data = result.body;
353367
_.each(team_data, function (t) {
354368
teams.push(t);
355369
});
@@ -372,23 +386,24 @@ function getRepos(token, username, callback) {
372386
var team_details = [];
373387
_.each(results, function (result) {
374388
try {
375-
var td = JSON.parse(result.body);
389+
var td = result.body;
376390
team_details.push(td);
377391
} catch (e) {
378392
debug('get_github_repos(): Error parsing JSON in detail team response - %s', e);
379393
}
380394
});
381395
// For each Team with admin privs, test for membership
382396
var group = this.group();
397+
383398
var team_detail_requests = {};
384399
_.each(team_details, function (team_details) {
385400
if (team_details.permission != 'admin') {
386401
return;
387402
}
388403
team_detail_requests[team_details.id] = team_details;
389-
var url = GITHUB_API_ENDPOINT + '/teams/' + team_details.id + '/members/' + username;
404+
var url = '/teams/' + team_details.id + '/members/' + username;
390405
debug('TEAM DETAIL URL', url);
391-
get_oauth2(url, {}, token, group());
406+
api_call(url, token, group());
392407
});
393408
this.team_detail_requests = team_detail_requests;
394409
},
@@ -401,18 +416,14 @@ function getRepos(token, username, callback) {
401416
}
402417
var team_detail_requests = this.team_detail_requests;
403418
var group = this.group();
404-
_.each(results, function (res) {
405-
var team_id = res.request.uri.path.split('/');
406-
if (res.request.uri.path.indexOf('/api/v3/') > -1) {
407-
// Github Enterprise url
408-
team_id = team_id[4];
409-
}
410-
else {
411-
// Github url
412-
team_id = team_id[2];
413-
}
419+
420+
_.each(results, function (result) {
421+
var team_id = result.req.path.split('/');
422+
debug("We get the following repo path: " + util.inspect(result.req.path));
423+
team_id = team_id[2];
424+
414425
var team_detail = team_detail_requests[parseInt(team_id, 10)];
415-
if (res.statusCode === 204) {
426+
if (result.statusCode === 204) {
416427
pageinated_api_call('/teams/' + team_id + '/repos', token, group());
417428
}
418429

@@ -424,6 +435,7 @@ function getRepos(token, username, callback) {
424435
debug('get_github_repos(): Error with team repos request: %s', err);
425436
return callback(err);
426437
}
438+
427439
_.each(results, function (result) {
428440
if (result && result.data) {
429441
_.each(result.data, function (team_repo) {
@@ -445,6 +457,7 @@ function getRepos(token, username, callback) {
445457
});
446458
}
447459
});
460+
448461
// Sometimes we can get multiple copies of the same team repo, so we uniq it
449462
team_repos = _.uniq(team_repos, false, function (item) {
450463
return item.id;

‎lib/index.js‎

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,8 @@ var User = require('./models').User;
1212

1313
var GITHUB_API_ENDPOINT = 'https://api.github.com';
1414

15+
/* jshint -W040 */
16+
1517
/**
1618
* get_oauth2()
1719
*
@@ -23,6 +25,7 @@ var GITHUB_API_ENDPOINT = 'https://api.github.com';
2325
* @param {Object} client An alternative superagent instance to use.
2426
*/
2527
var get_oauth2 = module.exports.get_oauth2 = function (url, q_params, access_token, callback, client) {
28+
console.debug('OAUTH2 CALLBACK LENGTH:', callback.length);
2629
// If the user provided a superagent instance, use that.
2730
client = client || superagent;
2831
// Construct the query. Allow the user to override the access_token through q_params.
@@ -50,9 +53,9 @@ var api_call = module.exports.api_call = function (path, access_token, callback,
5053
client = client || superagent;
5154
var url = GITHUB_API_ENDPOINT + path;
5255
debug('github api_call(): path %s', path);
53-
get_oauth2(url, {}, access_token, function (error, res, body) {
56+
get_oauth2(url, {}, access_token, function (error, res) {
5457
if (!error && res.statusCode == 200) {
55-
var data = JSON.parse(body);
58+
var data = res.body;
5659
callback(null, res, data);
5760
} else {
5861
callback(error, res, null);
@@ -117,11 +120,11 @@ var pageinated_api_call = module.exports.pageinated_api_call = function (path, a
117120
var pages = [];
118121

119122
function loop(uri, page) {
120-
get_oauth2(uri, {per_page: 30, page: page}, access_token, function (error, response, body) {
123+
get_oauth2(uri, {per_page: 30, page: page}, access_token, function (error, response) {
121124
if (!error && response.statusCode == 200) {
122125
var data;
123126
try {
124-
data = JSON.parse(body);
127+
data = response.body;
125128
} catch (e) {
126129
return callback(e, null);
127130
}
@@ -142,7 +145,7 @@ var pageinated_api_call = module.exports.pageinated_api_call = function (path, a
142145
}
143146
} else {
144147
if (!error) {
145-
return callback('Status code is ' + response.statusCode + ' not 200. Body: ' + body)
148+
return callback('Status code is ' + response.statusCode + ' not 200. Body: ' + response.body)
146149
} else {
147150
return callback(error, null);
148151
}
@@ -210,7 +213,7 @@ module.exports.get_github_repos = function (user, callback) {
210213
_.each(results, function (result) {
211214
try {
212215
debug('For Organizations: %s', result.request.uri.path.split('/')[2]);
213-
var team_data = JSON.parse(result.body);
216+
var team_data = result.body;
214217
_.each(team_data, function (t) {
215218
debug('Team details: %j', t);
216219
teams.push(t);
@@ -235,7 +238,7 @@ module.exports.get_github_repos = function (user, callback) {
235238
var team_details = [];
236239
_.each(results, function (result) {
237240
try {
238-
var td = JSON.parse(result.body);
241+
var td = result.body;
239242
team_details.push(td);
240243
} catch (e) {
241244
debug('get_github_repos(): Error parsing JSON in detail team response - %s', e);

‎test/test_api.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11

22
var expect = require('expect.js')
3-
, api = require('../lib/api')
3+
, api = require('../lib/api');
44

55
describe('github api', function () {
66
describe('getFile', function () {

‎test/test_webhooks.js‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,6 @@ describe('webhooks', function () {
4242
it('should work', function () {
4343
var fx = require('./sample_pull_request.json')
4444
, config = lib.pullRequestJob(fx.pull_request, fx.action)
45-
4645
expect(config).to.eql({
4746
branch: 'master',
4847
deploy: false,
@@ -51,7 +50,8 @@ describe('webhooks', function () {
5150
pull_request: {
5251
user: 'jaredly',
5352
repo: 'petulant-wookie',
54-
sha: 'f65ac3101a45bb9408c0459805b496cb73ae2d5f'
53+
sha: 'f65ac3101a45bb9408c0459805b496cb73ae2d5f',
54+
number: 1
5555
}
5656
}
5757
},

0 commit comments

Comments
 (0)