Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
bump: minor
---

### Added

- `limits.api.<pool>.stop-floor`, a share of the header's `x-ratelimit-limit` (default 0.02 for every pool, in `.github/actions/log-api-budget/rate-limits.json`). The Node rate guard (`rate-guard.mjs`) now stops a pool when a response's `x-ratelimit-remaining` is at or under that share, as it does at 0: no further call to the pool goes out before the header's reset, the stop is recorded in the job's stop file for later steps, and the step holds under `API_BUDGET_HOLD`, else fails. Before, a response under the read floor only warned and calls continued until GitHub refused one. A response with no readable limit, remaining or future reset never stops on it. The check reads headers already returned, so it adds no API call.
35 changes: 24 additions & 11 deletions .github/actions/log-api-budget/rate-guard.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,9 @@
// job's stop file, so later steps, the gh wrapper's included, keep it. A response under the pool's
// read-floor share of its planned budget left (planOf) logs a warning once;
// belowFloor lets a caller skip a read it can do without. The stop reads only
// the header's remaining and GitHub's refusals, never the plan.
// the header's remaining and limit and GitHub's refusals, never the plan: a
// remaining at or under the pool's stop-floor share of the header's limit
// stops the pool until its reset, as a remaining of 0 does.
//
// The margins are limits.api in pr-pipeline.yaml, read from resolve-config's
// output (RESOLVED_CONFIG), over the defaults in rate-limits.json beside this
Expand Down Expand Up @@ -61,7 +63,7 @@ const DEFAULTS = new URL("./rate-limits.json", import.meta.url);
// The values rate-limits.json ships, which the guard plans to when that file
// cannot be read, so a checkout that misses it still keeps every floor
// (rate-guard.test.mjs holds the two equal).
const SHARES = { plan: 0.7, "writer-pause": 0.4, breaker: 0.2, "read-floor": 0.1 };
const SHARES = { plan: 0.7, "writer-pause": 0.4, breaker: 0.2, "read-floor": 0.1, "stop-floor": 0.02 };
export const BUILT_IN_LIMITS = Object.freeze({
"github-token": { ...SHARES, "limit-cap": 1000, pace: 0.2 },
"lanes-app": SHARES,
Expand All @@ -74,8 +76,9 @@ export const BUILT_IN_LIMITS = Object.freeze({
// belong to content-creation alone.
const SHARE = (value) => typeof value === "number" && value >= 0 && value <= 1;
const COUNT = (value) => Number.isInteger(value) && value >= 1;
// pace, optional, is pace.mjs's share for timer-driven lanes.
const FIELDS = { plan: (value) => SHARE(value) && value > 0, "writer-pause": SHARE, breaker: SHARE, "read-floor": SHARE, "limit-cap": COUNT, pace: SHARE };
// pace, optional, is pace.mjs's share for timer-driven lanes; stop-floor,
// optional, the share of the header's limit left at which the latch stops.
const FIELDS = { plan: (value) => SHARE(value) && value > 0, "writer-pause": SHARE, breaker: SHARE, "read-floor": SHARE, "stop-floor": SHARE, "limit-cap": COUNT, pace: SHARE };
const CONTENT_FIELDS = { ...FIELDS, "per-minute": COUNT, "per-hour": COUNT };
const REQUIRED = ["plan", "writer-pause", "breaker", "read-floor"];

Expand Down Expand Up @@ -397,10 +400,11 @@ export function planOf({ limit, remaining }, limitCap = null) {
// and where it came from, the pool to stop ("*" for every pool: a secondary
// limit is not per resource), and low (under readFloor of the planned budget
// left, planOf with limitCap). Only the header's remaining and GitHub's
// refusals latch.
// refusals latch: a remaining of 0, or, with stopFloor, a remaining at or under
// stopFloor of the header's limit whose header reset is still ahead.
// headers is a Headers or a plain object of lower-case names; body is the
// parsed body, or its text.
export function classify({ url, status, headers, body }, { now = Date.now(), readFloor = null, limitCap = null } = {}) {
export function classify({ url, status, headers, body }, { now = Date.now(), readFloor = null, limitCap = null, stopFloor = null } = {}) {
const get = headerReader(headers);
const graphqlPool = poolOf(url) === "graphql";
let parsed = body;
Expand All @@ -425,7 +429,8 @@ export function classify({ url, status, headers, body }, { now = Date.now(), rea
const secondary = flagged && messages.some((text) => SIGNALS.secondary.test(text));
const graphql = errors.some((error) => error?.type === SIGNALS.graphqlErrorType);
const tooMany = status === 429;
const latched = remaining === 0 || retryAfter !== null || message || graphql || tooMany;
const floored = stopFloor !== null && limit > 0 && remaining !== null && reset !== null && reset * 1000 > now && remaining <= stopFloor * limit;
const latched = remaining === 0 || floored || retryAfter !== null || message || graphql || tooMany;
const plan = limit > 0 && remaining !== null ? planOf({ limit, remaining }, limitCap) : null;
const low = readFloor !== null && plan !== null && plan.left < readFloor * plan.planned;
if (!latched) {
Expand All @@ -434,7 +439,7 @@ export function classify({ url, status, headers, body }, { now = Date.now(), rea
const nowSeconds = Math.floor(now / 1000);
const candidates = [
retryAfter !== null ? [nowSeconds + retryAfter, "retry-after"] : null,
remaining === 0 && reset !== null ? [reset, "x-ratelimit-reset"] : null,
(remaining === 0 || floored) && reset !== null ? [reset, "x-ratelimit-reset"] : null,
Comment thread
kyle-sexton marked this conversation as resolved.
Resolved
].filter(Boolean);
const best = candidates.length ? candidates.reduce((top, next) => (next[0] > top[0] ? next : top)) : null;
// A header time already past would write a stop that lets the next call go.
Expand Down Expand Up @@ -612,7 +617,8 @@ export function createRateLimitLatch({
const needsBody = SIGNALS.statuses.includes(response.status) || pool === "graphql";
const body = needsBody ? await response.clone().text().catch(() => "") : null;
const limitCap = margin(pool, "limit-cap", token);
const verdict = classify({ url, status: response.status, headers: response.headers, body }, { now: now(), readFloor: margin(pool, "read-floor", token), limitCap });
const stopFloor = margin(pool, "stop-floor", token);
const verdict = classify({ url, status: response.status, headers: response.headers, body }, { now: now(), readFloor: margin(pool, "read-floor", token), limitCap, stopFloor });
low.set(pool, verdict.low);
if (verdict.latched) {
stops.set(verdict.pool, Math.max(stops.get(verdict.pool) ?? 0, verdict.reset));
Expand All @@ -627,12 +633,19 @@ export function createRateLimitLatch({
unrecorded = `; the stop could not be recorded in the job's stop file (${error?.code ?? error?.message}), so later steps do not see it`;
}
}
const refused = await refusedForLimit(response);
// A stop later steps cannot see is a fault, not a limit to hold on.
const resent = !unrecorded && resend && waits(url) && (await refusedForLimit(response));
const resent = !unrecorded && resend && waits(url) && refused;
const holds = resent || (hold && !unrecorded);
const then = resent ? "it is sent again after the reset" : `this step ${holds ? "holds" : "fails"}`;
const get = headerReader(response.headers);
const [limit, remaining] = ["limit", "remaining"].map((name) => Number(get(`x-ratelimit-${name}`)));
const what =
!refused && stopFloor !== null && remaining > 0 && remaining <= stopFloor * limit
? `left GitHub's ${poolLabel(verdict.pool)} ${remaining} of ${limit} requests, at or under its stop-floor share ${stopFloor}`
: `hit GitHub's ${poolLabel(verdict.pool)} rate limit`;
log(
`${holds ? "::notice::" : "::error::"}${method} ${url} hit GitHub's ${poolLabel(verdict.pool)} rate limit (status ${response.status}):` +
`${holds ? "::notice::" : "::error::"}${method} ${url} ${what} (status ${response.status}):` +
` no call to it before ${resetLabel(verdict.reset)} (${verdict.reset_source}); ${then}${unrecorded}`,
);
if (!holds) {
Expand Down
94 changes: 94 additions & 0 deletions .github/actions/log-api-budget/rate-guard.test.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -299,6 +299,100 @@ test("the latch reads the pool's limit-cap: past the plan it warns, and a remain
assert.deepEqual(lines, ["::warning::GitHub's core API has under its read floor left (50 of the planned 1000; header remaining 4050 of 5000); reads continue"]);
});

// stop-floor 0.02 of a header limit of 5000 is 100 left. The reset,
// 2026-10-23T10:33:20Z, is an hour past NOW, so a later call is refused, not waited out.
const nearZero = (remaining, extra = {}) => [200, { "x-ratelimit-limit": "5000", "x-ratelimit-remaining": String(remaining), "x-ratelimit-reset": "1792751600", "x-ratelimit-resource": "core", ...extra }];

test("stop-floor defaults to 0.02 for every pool", () => {
for (const [name, fields] of Object.entries(apiLimits({ resolvedPath: "" }))) {
assert.equal(fields["stop-floor"], 0.02, name);
}
});

test("at or under the stop floor of the header's limit, the pool stops until its reset: recorded, no later call, red", async () => {
for (const remaining of [100, 99, 1]) {
await withDir(async (dir) => {
const stopFile = join(dir, "api-budget", "stop");
const lines = [];
let failed = 0;
const gh = fakeGitHub([nearZero(remaining), [200]]);
const latch = createRateLimitLatch({ now: () => NOW, log: (line) => lines.push(line), fail: () => (failed += 1), limits: () => apiLimits({ resolvedPath: "" }), stopFile });
const fetch = latch.guard(gh.fetch);
assert.equal((await fetch(URL_A)).status, 200, "the answer that reached the floor is returned");
await assert.rejects(fetch(URL_B), (error) => error instanceof RateLimitStop && error.message.includes("2026-10-23T10:33:20Z"));
assert.deepEqual(gh.sent, [`GET ${URL_A}`], `remaining ${remaining}: no second request`);
assert.equal(readFileSync(stopFile, "utf8"), "core 1792751600\n");
assert.ok(failed >= 1, `remaining ${remaining}: the step fails`);
const error = lines.find((line) => line.startsWith("::error::"));
assert.equal(error, `::error::GET ${URL_A} left GitHub's core API ${remaining} of 5000 requests, at or under its stop-floor share 0.02 (status 200): no call to it before 2026-10-23T10:33:20Z (x-ratelimit-reset); this step fails`);
});
}
});

test("above the stop floor nothing stops: 101 of 5000 left sends on with no error", async () => {
const gh = fakeGitHub([nearZero(101), nearZero(101)]);
const { latch, lines, failed } = latchFor();
const fetch = latch.guard(gh.fetch);
await fetch(URL_A);
await fetch(URL_B);
assert.equal(gh.sent.length, 2);
assert.equal(failed(), 0);
assert.deepEqual(lines.filter((line) => line.startsWith("::error::") || line.startsWith("::notice::")), []);
});

test("a holding latch stops at the stop floor with a notice and fails nothing", async () => {
const gh = fakeGitHub([nearZero(100), [200]]);
const { latch, lines, failed } = latchFor({ hold: true });
const fetch = latch.guard(gh.fetch);
await fetch(URL_A);
await assert.rejects(fetch(URL_B), RateLimitStop);
assert.equal(gh.sent.length, 1);
assert.equal(failed(), 0);
assert.deepEqual(lines.filter((line) => line.startsWith("::error::")), []);
assert.ok(lines.some((line) => line.startsWith("::notice::") && line.includes("at or under its stop-floor share 0.02") && line.endsWith("this step holds")));
});

test("a missing or unreadable limit, remaining or reset never stops on the stop floor", async () => {
const answers = [
[200, { "x-ratelimit-remaining": "50", "x-ratelimit-reset": "1792751600" }],
nearZero("abc"),
nearZero(50, { "x-ratelimit-limit": "lots" }),
[200, { "x-ratelimit-limit": "5000", "x-ratelimit-remaining": "50" }],
nearZero(50, { "x-ratelimit-reset": "1792747000" }),
[200],
];
const gh = fakeGitHub([...answers]);
const { latch, lines, failed } = latchFor();
const fetch = latch.guard(gh.fetch);
for (let i = 0; i < answers.length; i++) {
assert.equal((await fetch(URL_A)).status, 200, `answer ${i} is sent`);
}
assert.equal(gh.sent.length, answers.length);
assert.equal(failed(), 0);
assert.deepEqual(lines.filter((line) => line.startsWith("::error::")), []);
});

test("limits.api overrides the stop floor: at 0 only an empty pool stops", () =>
withDir(async (dir) => {
const path = join(dir, "resolved.json");
writeFileSync(path, JSON.stringify({ limits: { api: { "github-token": { "stop-floor": 0 } } } }));
const gh = fakeGitHub([nearZero(1), nearZero(1)]);
const latch = createRateLimitLatch({ now: () => NOW, log: () => {}, fail: () => assert.fail("no stop at 1 left with a stop floor of 0"), limits: () => apiLimits({ resolvedPath: path }), stopFile: null });
const fetch = latch.guard(gh.fetch);
await fetch(URL_A);
await fetch(URL_B);
assert.equal(gh.sent.length, 2);
}));

test("a refusal still reads as a rate limit hit, not a stop-floor stop", async () => {
const gh = fakeGitHub([PRIMARY]);
const { latch, lines } = latchFor();
await latch.guard(gh.fetch)(URL_A);
const error = lines.find((line) => line.startsWith("::error::"));
assert.match(error, /hit GitHub's core API rate limit \(status 403\)/);
assert.doesNotMatch(error, /stop-floor/);
});

test("a 403 Resource not accessible keeps its old behavior: returned, no latch, no failure", async () => {
const refused = [403, { "x-ratelimit-limit": "5000", "x-ratelimit-remaining": "4950" }, { message: "Resource not accessible by integration" }];
const gh = fakeGitHub([refused, [200]]);
Expand Down
10 changes: 5 additions & 5 deletions .github/actions/log-api-budget/rate-limits.json
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
{
"github-token": { "plan": 0.7, "writer-pause": 0.4, "breaker": 0.2, "read-floor": 0.1, "limit-cap": 1000, "pace": 0.2 },
"lanes-app": { "plan": 0.7, "writer-pause": 0.4, "breaker": 0.2, "read-floor": 0.1 },
"graphql": { "plan": 0.7, "writer-pause": 0.4, "breaker": 0.2, "read-floor": 0.1 },
"owner-token": { "plan": 0.7, "writer-pause": 0.4, "breaker": 0.2, "read-floor": 0.1 },
"content-creation": { "plan": 0.7, "writer-pause": 0.4, "breaker": 0.2, "read-floor": 0.1, "per-minute": 80, "per-hour": 500 }
"github-token": { "plan": 0.7, "writer-pause": 0.4, "breaker": 0.2, "read-floor": 0.1, "stop-floor": 0.02, "limit-cap": 1000, "pace": 0.2 },
"lanes-app": { "plan": 0.7, "writer-pause": 0.4, "breaker": 0.2, "read-floor": 0.1, "stop-floor": 0.02 },
"graphql": { "plan": 0.7, "writer-pause": 0.4, "breaker": 0.2, "read-floor": 0.1, "stop-floor": 0.02 },
"owner-token": { "plan": 0.7, "writer-pause": 0.4, "breaker": 0.2, "read-floor": 0.1, "stop-floor": 0.02 },
"content-creation": { "plan": 0.7, "writer-pause": 0.4, "breaker": 0.2, "read-floor": 0.1, "stop-floor": 0.02, "per-minute": 80, "per-hour": 500 }
}
7 changes: 4 additions & 3 deletions docs/conventions/pr-pipeline.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -1266,9 +1266,10 @@ limits:
# graphql, owner-token, content-creation). limit-cap sets each pool's planned
# budget, the header's x-ratelimit-limit at most the cap (github-token:
# GitHub's documented GITHUB_TOKEN limit); plan and read-floor are shares of
# it, writer-pause and breaker shares of the header's limit; content-creation
# adds per-minute and per-hour. The rate guard reads only read-floor and
# limit-cap, to warn; github-token's pace (projected share left at reset)
# it, writer-pause, breaker and stop-floor shares of the header's limit;
# content-creation adds per-minute and per-hour. The rate guard reads
# read-floor and limit-cap, to warn, and stop-floor, to stop a pool until its
# reset at or under that share left; github-token's pace (projected share left at reset)
# paces timer-driven lanes (log-api-budget/pace.mjs); the others are
# thresholds meant for the hub's limits sampler, which nothing here enforces. Their
# defaults live in .github/actions/log-api-budget/rate-limits.json, held
Expand Down
Loading
Loading