From b92d6138d33dfbd532a2e01e7531a7d09d4acfc4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dandelion=20Man=C3=A9?= Date: Mon, 3 Sep 2018 14:33:17 -0700 Subject: [PATCH] Improve GitHub rate limit error message (#755) Fixes #732; see that issue for context. Test plan: The success case still works (verified that loading sourcecred/sourcecred works). I haven't tested the error case, as getting a real RATE_LIMIT_EXCEEDED form GitHub is time-consuming, and has only happened once in practice. I'm pretty confident the code works because it's a simple adaptation of the code that catches other cases. --- src/plugins/github/fetchGithubRepo.js | 75 +++++++++++++++++---------- 1 file changed, 48 insertions(+), 27 deletions(-) diff --git a/src/plugins/github/fetchGithubRepo.js b/src/plugins/github/fetchGithubRepo.js index 5ca93eb..e4e8205 100644 --- a/src/plugins/github/fetchGithubRepo.js +++ b/src/plugins/github/fetchGithubRepo.js @@ -53,6 +53,7 @@ const GITHUB_GRAPHQL_SERVER = "https://api.github.com/graphql"; type GithubResponseError = | {|+type: "FETCH_ERROR", retry: false, error: Error|} | {|+type: "GRAPHQL_ERROR", retry: false, error: mixed|} + | {|+type: "RATE_LIMIT_EXCEEDED", retry: false, error: mixed|} | {|+type: "GITHUB_INTERNAL_EXECUTION_ERROR", retry: true, error: mixed|} | {|+type: "NO_DATA", retry: true, error: mixed|}; @@ -75,6 +76,17 @@ function tryGithubFetch(fetch, fetchOptions): Promise { error: x, }: GithubResponseError) ); + } else if ( + x.errors.length === 1 && + x.errors[0].type === "RATE_LIMITED" + ) { + return Promise.reject( + ({ + type: "RATE_LIMIT_EXCEEDED", + retry: false, + error: x, + }: GithubResponseError) + ); } else { return Promise.reject( ({ @@ -131,34 +143,43 @@ async function postQuery({body, variables}, token): Promise { Authorization: `bearer ${token}`, }, }; - return retryGithubFetch(fetch, fetchOptions).catch((error) => { - switch (error.type) { - case "GITHUB_INTERNAL_EXECUTION_ERROR": - case "NO_DATA": - console.error( - "GitHub query failed! We're tracking these issues at " + - "https://github.com/sourcecred/sourcecred/issues/350.\n" + - "If the error is a timeout or abuse rate limit, you can " + - "try loading a smaller repo, or trying again in a few minutes.\n" + - "The actual failed response can be found below:\n" + - "=================================================" - ); - console.error(error.error); - break; - case "GRAPHQL_ERROR": - console.error( - "Unexpected GraphQL error; this may be a bug in SourceCred: ", - JSON.stringify({postBody: postBody, error: error.error}) - ); - break; - case "FETCH_ERROR": - // Network error; no need for additional commentary. - break; - default: - throw new Error((error.type: empty)); + return retryGithubFetch(fetch, fetchOptions).catch( + (error: GithubResponseError) => { + const type = error.type; + switch (type) { + case "GITHUB_INTERNAL_EXECUTION_ERROR": + case "NO_DATA": + console.error( + "GitHub query failed! We're tracking these issues at " + + "https://github.com/sourcecred/sourcecred/issues/350.\n" + + "If the error is a timeout or abuse rate limit, you can " + + "try loading a smaller repo, or trying again in a few minutes.\n" + + "The actual failed response can be found below:\n" + + "=================================================" + ); + console.error(error.error); + break; + case "GRAPHQL_ERROR": + console.error( + "Unexpected GraphQL error; this may be a bug in SourceCred: ", + JSON.stringify({postBody: postBody, error: error.error}) + ); + break; + case "RATE_LIMIT_EXCEEDED": + console.error( + "You've exceeded your hourly GitHub rate limit.\n" + + "You'll need to wait until it resets." + ); + break; + case "FETCH_ERROR": + // Network error; no need for additional commentary. + break; + default: + throw new Error((type: empty)); + } + return Promise.reject(error); } - return Promise.reject(error); - }); + ); } function ensureNoMorePages(result: any, path = []) {