Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Error callback not firing when hitting rate limit #348

Closed
divergentdave opened this issue Jun 14, 2016 · 1 comment · Fixed by #354
Closed

Error callback not firing when hitting rate limit #348

divergentdave opened this issue Jun 14, 2016 · 1 comment · Fixed by #354

Comments

@divergentdave
Copy link
Contributor

I'm using github-api in Node v4.4.5, and I've found that if I exhaust the 60 request/hour unauthenticated rate limit, then certain functions never call their callback. For example, the following script works normally until I hit the limit, then after that the callback never gets called.

#!/usr/bin/env node
"use strict";
var GitHub = require("github-api");
var gh = new GitHub({});

var username = "divergentdave";
var user = gh.getUser(username);
user.listRepos(function(error, result, request) {
    console.log("hit callback");
    if (error) {
        console.error(error);
        return;
    }
});

pi@raspberrypi ~/github-user-branches $ node check-rate-limit.js 
Limit remaining: 1
Reset date: Mon Jun 13 2016 20:45:09 GMT-0500 (CDT)
pi@raspberrypi ~/github-user-branches $ node example.js 
hit callback
pi@raspberrypi ~/github-user-branches $ node example.js 
pi@raspberrypi ~/github-user-branches $ 

Other methods, such as User.getProfile() properly call the callback with an error object, complete with details of the 403 response from the API server. I haven't arrived at the root cause yet, but it is most likely in Requestable._requestAllPages.

I'd be up for working on a PR for this, but first, testing for this issue should probably be done with fixture data for the HTTP responses, rather than hitting the GitHub servers directly. Do you have any preferences for which mocking library to use? I see that nock is a popular choice.

@clayreimann
Copy link
Member

No preference here. I think I've seen a similar error crop up in the Travis testing. I'm guessing the error actually lies with callbackErrorOrThrow receiving something other than an Axios response.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants