From e27b7f32072b96c6894f020f343d0349125803e3 Mon Sep 17 00:00:00 2001 From: secustor Date: Mon, 15 Jan 2024 15:39:13 +0100 Subject: [PATCH] fix(Github): use `x-ratelimit-remaining` and 429 status code for rate limit detection Signed-off-by: secustor --- .changeset/sweet-ravens-glow.md | 6 +++++ .../src/reading/GithubUrlReader.ts | 15 +++++++---- .../src/github/GithubIntegration.test.ts | 25 +++++++++++++++++++ .../src/github/GithubIntegration.ts | 8 ++++++ 4 files changed, 49 insertions(+), 5 deletions(-) create mode 100644 .changeset/sweet-ravens-glow.md diff --git a/.changeset/sweet-ravens-glow.md b/.changeset/sweet-ravens-glow.md new file mode 100644 index 0000000000..045f7edbf2 --- /dev/null +++ b/.changeset/sweet-ravens-glow.md @@ -0,0 +1,6 @@ +--- +'@backstage/integration': minor +'@backstage/backend-common': patch +--- + +Fix rate limit detection by looking for HTTP status code 429 and updating the header `x-ratelimit-remaining` to look for in case of a 403 code is returned diff --git a/packages/backend-common/src/reading/GithubUrlReader.ts b/packages/backend-common/src/reading/GithubUrlReader.ts index 764431386d..83c21b2529 100644 --- a/packages/backend-common/src/reading/GithubUrlReader.ts +++ b/packages/backend-common/src/reading/GithubUrlReader.ts @@ -149,10 +149,7 @@ export class GithubUrlReader implements UrlReader { // GitHub returns a 403 response with a couple of headers indicating rate // limit status. See more in the GitHub docs: // https://docs.github.com/en/rest/overview/resources-in-the-rest-api#rate-limiting - if ( - response.status === 403 && - response.headers.get('X-RateLimit-Remaining') === '0' - ) { + if (this.integration.isRateLimited(response)) { message += ' (rate limit exceeded)'; } @@ -350,10 +347,18 @@ export class GithubUrlReader implements UrlReader { const response = await fetch(urlAsString, init); if (!response.ok) { - const message = `Request failed for ${urlAsString}, ${response.status} ${response.statusText}`; + let message = `Request failed for ${urlAsString}, ${response.status} ${response.statusText}`; if (response.status === 404) { throw new NotFoundError(message); } + + // GitHub returns a 403 response with a couple of headers indicating rate + // limit status. See more in the GitHub docs: + // https://docs.github.com/en/rest/overview/resources-in-the-rest-api#rate-limiting + if (this.integration.isRateLimited(response)) { + message += ' (rate limit exceeded)'; + } + throw new Error(message); } diff --git a/packages/integration/src/github/GithubIntegration.test.ts b/packages/integration/src/github/GithubIntegration.test.ts index d8e6020a64..36c802da52 100644 --- a/packages/integration/src/github/GithubIntegration.test.ts +++ b/packages/integration/src/github/GithubIntegration.test.ts @@ -78,6 +78,31 @@ describe('GithubIntegration', () => { ), ).toBe('https://github.com/backstage/backstage/edit/master/README.md'); }); + + describe('isRateLimited', () => { + const integration = new GithubIntegration({ host: 'h.com' }); + + it.each` + status | ratelimitRemaining | expected + ${404} | ${100} | ${false} + ${429} | ${undefined} | ${true} + ${429} | ${100} | ${true} + ${403} | ${100} | ${false} + ${403} | ${0} | ${true} + `( + '(statusCode: $status, header: $ratelimitRemaining) === $expected', + ({ status, ratelimitRemaining, expected }) => { + const headers = new Headers({ + 'x-ratelimit-remaining': ratelimitRemaining, + }); + const result = integration.isRateLimited({ + status, + headers, + } as Response); + expect(expected).toBe(result); + }, + ); + }); }); describe('replaceGithubUrlType', () => { diff --git a/packages/integration/src/github/GithubIntegration.ts b/packages/integration/src/github/GithubIntegration.ts index 50fc129ccf..e2b163dc2d 100644 --- a/packages/integration/src/github/GithubIntegration.ts +++ b/packages/integration/src/github/GithubIntegration.ts @@ -65,6 +65,14 @@ export class GithubIntegration implements ScmIntegration { resolveEditUrl(url: string): string { return replaceGithubUrlType(url, 'edit'); } + + isRateLimited(response: Response): boolean { + return ( + response.status === 429 || + (response.status === 403 && + response.headers.get('x-ratelimit-remaining') === '0') + ); + } } /**