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

Handle rate limiting #87

Open
laymonage opened this issue May 28, 2021 · 1 comment
Open

Handle rate limiting #87

laymonage opened this issue May 28, 2021 · 1 comment
Labels
enhancement Nice to have

Comments

@laymonage
Copy link
Member

laymonage commented May 28, 2021

Following #86.

GitHub's GraphQL API has a limit of 5000 query points/hour.

giscus authenticates as the app (bot) for the following operations:

  • Fetching repository information (repositoryId and categoryIds)
  • Fetching discussions anonymously (not signed in)
  • Posting new discussions (if no matching discussion exists)

For all other operations (posting comments, reactions, etc.) giscus authenticates as the user, so the 5000 points/hour limit is imposed on the user. In this case, I think the 5000 points/hour limit is generous. Going beyond that limit is highly unlikely, but possible (e.g. if the user also uses GitHub's API by other means).

The bot hitting the 5000 points/hour is more likely, especially if we have a lot of users (or maybe a few but with high traffic). At the very least, I think we should handle this case. A possible solution is to show an error message indicating the rate limit and a hint to sign in. Though at this point I think it wouldn't be much effort to also handle the case where the user already hits the rate limit.

For reference, utterances only sends a warning to the console when it hits the rate limit. It's also much easier to detect it on the REST API since you can just check the header. With the GraphQL API, you need to explicitly include rateLimit in the query.

@laymonage laymonage added the enhancement Nice to have label May 28, 2021
@laymonage
Copy link
Member Author

Okay, so after #188 I just learned that:

The app's token limit is imposed on its installation, not on the app itself. Because we request the token to be scoped within the specified repository, the 5000 points/hour limit is shared per installation, not to all users.

export async function getAppAccessToken(repoWithOwner: string): Promise<string> {
const installationId = await getInstallationId(repoWithOwner);
if (!installationId)
throw {
message: 'giscus is not installed on this repository',
documentation_url:
'https://docs.github.com/en/rest/reference/apps#get-a-repository-installation-for-the-authenticated-app',
};
const response: GResponse = await fetch(GITHUB_ACCESS_TOKEN_URL(installationId), {
method: 'POST',
headers: getHeaders(),
body: JSON.stringify({ repositories: [parseRepoWithOwner(repoWithOwner).name] }),
}).then((value) => value.json());
return response.token;
}

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

No branches or pull requests

1 participant