Skip to content

Commit f10df31

Browse files
committed
perf: drop the rate limit pre-check request
The header-based check is already done on the list response. The forge adapter is upgraded to only keep the header name for rate limit check
1 parent cdef925 commit f10df31

4 files changed

Lines changed: 8 additions & 39 deletions

File tree

‎DOCUMENTATION.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -153,7 +153,7 @@ Here is the interface for each forge:
153153
| `prNumber` | identifier of a PR (used in URLs and displayed) |
154154
| `prWebUrl` | web (non-API) URL of a PR, for the popup links |
155155
| `nextPageUrl` | `(response, data) → URL` of the next page of a paginated API response, or `null` on the last page |
156-
| `rateLimit` | optional `{ url, header, remaining(data) }`; `null` if unsupported |
156+
| `rateLimit` | optional `{ header }`: response header carrying the remaining request quota; `null` if unsupported |
157157
| `tokenStorageKey` | optional `storage.local` key holding the user's token; omit for no auth |
158158
| `authHeader` | optional `(token) → headers` sent with API requests; omit for no auth |
159159

‎background.js‎

Lines changed: 4 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -133,31 +133,9 @@ export function getFilesPRs(forge, urlInfo, requestHeaders, prs) {
133133
})
134134
}
135135

136-
// check the rate limit
137-
async function checkRateLimit(forge, requestHeaders) {
138-
// forges without a rate-limit endpoint skip this check
139-
if (!forge.rateLimit) {
140-
return
141-
}
142-
143-
console.log("Check the rate limit")
144-
145-
// fetch the rate limit endpoint
146-
const response = await fetch(forge.rateLimit.url, { headers: requestHeaders })
147-
const data = await response.json()
148-
149-
if (!response.ok) {
150-
throw new Error(`Failed to get rate limits: [${response.status}] ${response.statusText}: ${data.message}`)
151-
}
152-
153-
const remaining = forge.rateLimit.remaining(data)
154-
console.log("Rate limit remaining:", remaining)
155-
if (remaining == 0) {
156-
throw new Error("Rate limit reached!")
157-
}
158-
}
159-
160-
// check the rate limit comparing to PRs length (one files request each)
136+
// check the rate limit comparing to PRs length (one files request each); the
137+
// header rides on the list response, so the check costs no extra request (an
138+
// already-exhausted quota fails the list request itself)
161139
function checkRateLimitFromPRs(forge, prs, response) {
162140
// get the rate limit header (forges without one skip the comparison)
163141
const remaining = forge.rateLimit ? response.headers.get(forge.rateLimit.header) : null
@@ -215,9 +193,8 @@ async function main(tab) {
215193
await clearTabResult(tab.id)
216194

217195
try {
218-
// load the stored API token, then check the forge rate limit
196+
// load the stored API token
219197
const requestHeaders = await loadAuthHeaders(forge)
220-
await checkRateLimit(forge, requestHeaders)
221198

222199
// list pull requests (all pages), then keep those that touch the file
223200
const { items: prs, response } = await listAllPRs(forge, urlInfo, requestHeaders)

‎forges.js‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -21,8 +21,8 @@
2121
// prWebUrl (info, prNumber) -> web (non-API) URL of a PR, for popup links
2222
// nextPageUrl (response, data) -> URL of the next page of a paginated API
2323
// response, or null on the last page
24-
// rateLimit optional { url, header, remaining(data) }; null if the forge
25-
// exposes no rate-limit endpoint
24+
// rateLimit optional { header }: the response header carrying the
25+
// remaining request quota; null if the forge exposes none
2626
// tokenStorageKey optional storage.local key holding the user's API token;
2727
// omit if the forge supports no authentication
2828
// authHeader optional (token) -> headers object sent with API requests when
@@ -87,9 +87,7 @@ export const github = {
8787
},
8888

8989
rateLimit: {
90-
url: "https://api.github.com/rate_limit",
9190
header: "x-ratelimit-remaining",
92-
remaining: (data) => data.resources.core.remaining,
9391
},
9492

9593
// GitHub accepts classic and fine-grained personal access tokens as a

‎tests/forges.test.js‎

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -111,13 +111,7 @@ test("github.filenames returns an empty array for no files", () => {
111111
//
112112
// rate limit
113113
//
114-
test("github.rateLimit.remaining reads the core remaining count", () => {
115-
const data = { resources: { core: { remaining: 58 } } }
116-
assert.equal(github.rateLimit.remaining(data), 58)
117-
})
118-
119-
test("github.rateLimit exposes the endpoint and header", () => {
120-
assert.equal(github.rateLimit.url, "https://api.github.com/rate_limit")
114+
test("github.rateLimit exposes the remaining-quota header", () => {
121115
assert.equal(github.rateLimit.header, "x-ratelimit-remaining")
122116
})
123117

0 commit comments

Comments
 (0)