Skip to content

Commit e264b01

Browse files
divergentdaveclayreimann
divergentdave
authored and
clayreimann
committed
refactor(requestable): fix error handling to allow for either an axios response or an Error
* Add request/response fixture data for rate limit * Add tests for exhausted rate limit * Add status code and text to request error messages * Check created milestone ID before other tests * Fix ResponseError handling * Improve successCallback error handling * Add fixture data to replace live search API
1 parent 30f9e76 commit e264b01

File tree

10 files changed

+53787
-5
lines changed

10 files changed

+53787
-5
lines changed

‎lib/Requestable.js

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -276,10 +276,16 @@ function getNextPage(linksHeader = '') {
276276
}
277277

278278
function callbackErrorOrThrow(cb, path) {
279-
return function handler(response) {
280-
let message = `error making request ${response.config.method} ${response.config.url}`;
281-
let error = new ResponseError(message, path, response);
282-
log(`${message} ${JSON.stringify(response.data)}`);
279+
return function handler(object) {
280+
let error;
281+
if (object.hasOwnProperty('config')) {
282+
const {status, statusText, config: {method, url}} = object;
283+
let message = (`${status} error making request ${method} ${url}: "${statusText}"`);
284+
error = new ResponseError(message, path, object);
285+
log(`${message} ${JSON.stringify(object.data)}`);
286+
} else {
287+
error = object;
288+
}
283289
if (cb) {
284290
log('going to error callback');
285291
cb(error);

‎package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,7 @@
7474
"minami": "^1.1.1",
7575
"mocha": "^2.3.4",
7676
"must": "^0.13.1",
77+
"nock": "^8.0.0",
7778
"vinyl-buffer": "^1.0.0",
7879
"vinyl-source-stream": "^1.1.0"
7980
},

‎test/error.spec.js

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,83 @@
1+
import assert from 'assert';
2+
import expect from 'must';
3+
import nock from 'nock';
4+
5+
import Github from '../lib/GitHub';
6+
import testUser from './fixtures/user.json';
7+
import {assertSuccessful, assertFailure} from './helpers/callbacks';
8+
import fixtureExhausted from './fixtures/repos-ratelimit-exhausted.js';
9+
import fixtureOk from './fixtures/repos-ratelimit-ok.js';
10+
11+
describe('Rate limit error', function() {
12+
let github;
13+
let user;
14+
let scope;
15+
16+
before(function() {
17+
github = new Github();
18+
user = github.getUser(testUser.USERNAME);
19+
});
20+
21+
beforeEach(function() {
22+
scope = fixtureExhausted();
23+
});
24+
25+
it('should reject promise with 403 error', function() {
26+
return user.listRepos().then(function() {
27+
assert.fail(undefined, undefined, 'Promise was resolved instead of rejected');
28+
}, function(error) {
29+
expect(error).to.be.an.error();
30+
expect(error).to.have.own('response');
31+
expect(error.response).to.have.own('status');
32+
expect(error.response.status).to.be(403);
33+
});
34+
});
35+
36+
it('should call callback', function(done) {
37+
user.listRepos(assertFailure(done, function(error) {
38+
expect(error).to.be.an.error();
39+
expect(error).to.have.own('response');
40+
expect(error.response).to.have.own('status');
41+
expect(error.response.status).to.be(403);
42+
done();
43+
}));
44+
});
45+
46+
afterEach(function() {
47+
scope.done();
48+
nock.cleanAll();
49+
});
50+
});
51+
52+
describe('Rate limit OK', function() {
53+
let github;
54+
let user;
55+
let scope;
56+
57+
before(function() {
58+
github = new Github();
59+
user = github.getUser(testUser.USERNAME);
60+
});
61+
62+
beforeEach(function() {
63+
scope = fixtureOk();
64+
});
65+
66+
it('should resolve promise', function() {
67+
return expect(user.listRepos()).to.resolve.to.object();
68+
});
69+
70+
it('should call callback with array of results', function(done) {
71+
user.listRepos(assertSuccessful(done, function(error, result) {
72+
expect(error).is.not.an.error();
73+
expect(error).is.not.truthy();
74+
expect(result).is.array();
75+
done();
76+
}));
77+
});
78+
79+
afterEach(function() {
80+
scope.done();
81+
nock.cleanAll();
82+
});
83+
});

‎test/fixtures/record.js

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
import fs from 'fs';
2+
import nock from 'nock';
3+
import path from 'path';
4+
import GitHub from '../../lib/GitHub';
5+
import testUser from './user.json';
6+
7+
const gh = new GitHub();
8+
9+
let fileName;
10+
gh.getRateLimit().getRateLimit()
11+
.then((resp) => {
12+
if (resp.data.rate.remaining === 0) {
13+
fileName = 'repos-ratelimit-exhausted.js';
14+
} else {
15+
fileName = 'repos-ratelimit-ok.js';
16+
}
17+
nock.recorder.rec({
18+
dont_print: true
19+
});
20+
gh.getUser(testUser.USERNAME).listRepos();
21+
setTimeout(() => {
22+
const fixtures = nock.recorder.play();
23+
const filePath = path.join(__dirname, fileName);
24+
const text = ('/* eslint-disable */\n' +
25+
'import nock from \'nock\';\n' +
26+
'export default function fixture() {\n' +
27+
' let scope;\n' +
28+
' scope = ' + fixtures.join('\nscope = ').trim().replace(/\n/g, '\n ') + '\n' +
29+
' return scope;\n' +
30+
'}\n');
31+
fs.writeFileSync(filePath, text);
32+
console.log('Wrote fixture data to', fileName);
33+
}, 10000);
34+
}).catch(console.error);

‎test/fixtures/repos-ratelimit-exhausted.js

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
/* eslint-disable */
2+
import nock from 'nock';
3+
export default function fixture() {
4+
let scope;
5+
scope = nock('https://api.github.com:443', {"encodedQueryParams":true})
6+
.get('/users/mikedeboertest/repos')
7+
.query({"type":"all","sort":"updated","per_page":"100"})
8+
.reply(403, {"message":"API rate limit exceeded for 174.20.8.171. (But here's the good news: Authenticated requests get a higher rate limit. Check out the documentation for more details.)","documentation_url":"https://developer.github.com/v3/#rate-limiting"}, { server: 'GitHub.com',
9+
date: 'Sat, 18 Jun 2016 11:50:00 GMT',
10+
'content-type': 'application/json; charset=utf-8',
11+
'content-length': '246',
12+
connection: 'close',
13+
status: '403 Forbidden',
14+
'x-ratelimit-limit': '60',
15+
'x-ratelimit-remaining': '0',
16+
'x-ratelimit-reset': '1466253529',
17+
'x-github-media-type': 'github.v3; format=json',
18+
'access-control-expose-headers': 'ETag, Link, X-GitHub-OTP, X-RateLimit-Limit, X-RateLimit-Remaining, X-RateLimit-Reset, X-OAuth-Scopes, X-Accepted-OAuth-Scopes, X-Poll-Interval',
19+
'access-control-allow-origin': '*',
20+
'content-security-policy': 'default-src \'none\'',
21+
'strict-transport-security': 'max-age=31536000; includeSubdomains; preload',
22+
'x-content-type-options': 'nosniff',
23+
'x-frame-options': 'deny',
24+
'x-xss-protection': '1; mode=block',
25+
'x-github-request-id': 'AE1408AB:EA59:14F2183:57653568' });
26+
return scope;
27+
}

‎test/fixtures/repos-ratelimit-ok.js

Lines changed: 31 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)