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

Add tests for errors and rate limit hits of the client #40

Open
kyrcha opened this issue Oct 10, 2019 · 7 comments
Open

Add tests for errors and rate limit hits of the client #40

kyrcha opened this issue Oct 10, 2019 · 7 comments
Assignees
Labels
enhancement New feature or request

Comments

@kyrcha
Copy link
Contributor

kyrcha commented Oct 10, 2019

Derived from #29 as a todo item

@kyrcha kyrcha added the enhancement New feature or request label Oct 10, 2019
@kyrcha kyrcha self-assigned this Oct 10, 2019
@kyrcha
Copy link
Contributor Author

kyrcha commented Oct 11, 2019

It seems like I cannot make it to hit the limit.

When running a big query that returns a lot of nodes in a big organization larger than the one proposed here, I get a 502 error (I see @dpordomingo has opened a thread also here about this issue)

If I try to send continuous requests I get a 403 error for abusing the API.

I will try to record a 502 error and wait for an answer in the thread.

@smacker
Copy link
Contributor

smacker commented Oct 11, 2019

502 for a large number of nodes is understandable. I wouldn't blame github too much for that.

So there are 2 types of errors:

  • Abusing that you managed to reproduce
  • Limit. You don't need to make too big request for that. You just need to exhaust your token first as I described in PR comment. Nice request that doesn't 502 often is out repo request:
{
  "query": "query($assigneesCursor:String$assigneesPage:Int!$issueCommentsCursor:String$issueCommentsPage:Int!$issuesCursor:String$issuesPage:Int!$labelsCursor:String$labelsPage:Int!$name:String!$owner:String!$pullRequestReviewCommentsCursor:String$pullRequestReviewCommentsPage:Int!$pullRequestReviewsCursor:String$pullRequestReviewsPage:Int!$pullRequestsCursor:String$pullRequestsPage:Int!$repositoryTopicsCursor:String$repositoryTopicsPage:Int!){repository(owner: $owner, name: $name){mergeCommitAllowed,rebaseMergeAllowed,squashMergeAllowed,isArchived,url,createdAt,defaultBranchRef{name},description,isDisabled,isFork,forkCount,nameWithOwner,hasIssuesEnabled,hasWikiEnabled,homepageUrl,databaseId,primaryLanguage{name},mirrorUrl,name,id,openIssues: issues(states:[OPEN]){totalCount},owner{... on Organization{databaseId},... on User{databaseId},login,__typename},isPrivate,pushedAt,sshUrl,stargazers{totalCount},updatedAt,watchers{totalCount},repositoryTopics(first: $repositoryTopicsPage, after: $repositoryTopicsCursor){pageInfo{hasNextPage,endCursor},totalCount,nodes{topic{name}}},issues(first: $issuesPage, after: $issuesCursor){pageInfo{hasNextPage,endCursor},totalCount,nodes{body,closedAt,createdAt,url,databaseId,locked,milestone{id,title},id,number,state,title,updatedAt,author{login,__typename,... on User{databaseId,id,login}},assignees(first: $assigneesPage, after: $assigneesCursor){pageInfo{hasNextPage,endCursor},totalCount,nodes{databaseId,id,login}},labels(first: $labelsPage, after: $labelsCursor){pageInfo{hasNextPage,endCursor},totalCount,nodes{name}},comments(first: $issueCommentsPage, after: $issueCommentsCursor){pageInfo{hasNextPage,endCursor},totalCount,nodes{authorAssociation,body,createdAt,url,databaseId,id,updatedAt,author{login,__typename,... on User{databaseId,id,login}}}},timelineItems(last:1, itemTypes:CLOSED_EVENT){nodes{... on ClosedEvent{actor{login,__typename,... on User{databaseId,id,login}}}}}}},pullRequests(first: $pullRequestsPage, after: $pullRequestsCursor){pageInfo{hasNextPage,endCursor},totalCount,nodes{additions,authorAssociation,baseRef{name,repository{name,owner{login}},target{oid,... on Commit{author{user{login}}}}},body,changedFiles,closedAt,commits{totalCount},createdAt,deletions,headRef{name,repository{name,owner{login}},target{oid,... on Commit{author{user{login}}}}},url,databaseId,maintainerCanModify,mergeCommit{oid},mergeable,merged,mergedAt,mergedBy{login,__typename,... on User{databaseId,id,login}},milestone{id,title},id,number,reviewThreads{totalCount},state,title,updatedAt,author{login,__typename,... on User{databaseId,id,login}},assignees(first: $assigneesPage, after: $assigneesCursor){pageInfo{hasNextPage,endCursor},totalCount,nodes{databaseId,id,login}},labels(first: $labelsPage, after: $labelsCursor){pageInfo{hasNextPage,endCursor},totalCount,nodes{name}},comments(first: $issueCommentsPage, after: $issueCommentsCursor){pageInfo{hasNextPage,endCursor},totalCount,nodes{authorAssociation,body,createdAt,url,databaseId,id,updatedAt,author{login,__typename,... on User{databaseId,id,login}}}},reviews(first: $pullRequestReviewsPage, after: $pullRequestReviewsCursor){pageInfo{hasNextPage,endCursor},totalCount,nodes{body,commit{oid},url,databaseId,id,state,submittedAt,author{login,__typename,... on User{databaseId,id,login}},comments(first: $pullRequestReviewCommentsPage, after: $pullRequestReviewCommentsCursor){pageInfo{hasNextPage,endCursor},totalCount,nodes{authorAssociation,body,commit{oid},createdAt,diffHunk,url,databaseId,id,originalCommit{oid},originalPosition,path,position,updatedAt,author{login,__typename,... on User{databaseId,id,login}}}},comments(first: $pullRequestReviewCommentsPage, after: $pullRequestReviewCommentsCursor){pageInfo{hasNextPage,endCursor},totalCount,nodes{authorAssociation,body,commit{oid},createdAt,diffHunk,url,databaseId,id,originalCommit{oid},originalPosition,path,position,updatedAt,author{login,__typename,... on User{databaseId,id,login}}}}}}}}}}",
  "variables": {
    "assigneesCursor": null,
    "assigneesPage": 2,
    "issueCommentsCursor": null,
    "issueCommentsPage": 10,
    "issuesCursor": null,
    "issuesPage": 50,
    "labelsCursor": null,
    "labelsPage": 2,
    "name": "<repo name>",
    "owner": "<org name>",
    "pullRequestReviewCommentsCursor": null,
    "pullRequestReviewCommentsPage": 5,
    "pullRequestReviewsCursor": null,
    "pullRequestReviewsPage": 5,
    "pullRequestsCursor": null,
    "pullRequestsPage": 50,
    "repositoryTopicsCursor": null,
    "repositoryTopicsPage": 10
  }
}

@kyrcha
Copy link
Contributor Author

kyrcha commented Oct 14, 2019

After working on the above query and a quarter of the hour was able to produce the RATE_LIMITED error. An issue perhaps is that the status code is 200 OK and the error is in the body :)

Status: 200 OK
X-Github-Media-Type: github.v4; format=json
X-Oauth-Scopes: read:org, repo
X-Ratelimit-Limit: 5000
X-Ratelimit-Remaining: 0
X-Ratelimit-Reset: 1571093437

{"errors":[{"type":"RATE_LIMITED","message":"API rate limit exceeded"}]}

@kyrcha
Copy link
Contributor Author

kyrcha commented Oct 15, 2019

Sent an email to GitHub support to check their opinion since for the v3 API the response is 403.

@se7entyse7en
Copy link
Contributor

That's interesting. BTW maybe it was faster to change the code locally to build the api client without the token so that it reaches the limit much earlier. But who knows whether the response are different in case ot authenticated vs non-authenticated.

@dpordomingo did you experience the same (the non-403 status code) while working on error handling?

@kyrcha
Copy link
Contributor Author

kyrcha commented Oct 15, 2019

I don't think it is possible to send non-authenticated requests to the v4 graphql API according to docs. I've tested it also with env -u GITHUB_TOKEN and I get a 401.

@se7entyse7en
Copy link
Contributor

Ah ok, I didn't know that, I just assumed that there was an equivalent as for v3. Thanks! 👍

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

No branches or pull requests

3 participants