From bb0679f167385965558b4f3870c28af21b2b8d4f Mon Sep 17 00:00:00 2001 From: Beast Date: Fri, 28 Aug 2026 14:26:39 +0800 Subject: [PATCH] fix: rate limit violation --- actions/dependency-cooldown/dist/index.mjs | 44 ++++++++-- docs/dependency-cooldown.md | 5 +- .../src/registries/cargo.ts | 3 +- .../dependency-cooldown/src/registries/npm.ts | 1 + .../dependency-cooldown/src/registries/pub.ts | 1 + packages/dependency-cooldown/src/resolve.ts | 48 +++++++++- packages/dependency-cooldown/src/run.ts | 6 +- packages/dependency-cooldown/src/types.ts | 9 +- .../test/end-to-end.test.ts | 33 ++++++- .../test/registries.test.ts | 8 ++ .../dependency-cooldown/test/resolve.test.ts | 87 ++++++++++++++++++- 11 files changed, 224 insertions(+), 21 deletions(-) diff --git a/actions/dependency-cooldown/dist/index.mjs b/actions/dependency-cooldown/dist/index.mjs index 148e0d4..feebc12 100644 --- a/actions/dependency-cooldown/dist/index.mjs +++ b/actions/dependency-cooldown/dist/index.mjs @@ -9347,8 +9347,9 @@ function parseTimestamp(raw, subject) { var cargoRegistry = { id: "cargo", displayName: "crates.io", - // crates.io asks API clients to stay near one request per second. + // crates.io data-access policy: at most one API request per second. maxConcurrency: 1, + minRequestIntervalMs: 1e3, versionUrl(name, version) { return `https://crates.io/crates/${name}/${version}`; }, @@ -9376,6 +9377,7 @@ var npmRegistry = { id: "npm", displayName: "npmjs.com", maxConcurrency: 8, + minRequestIntervalMs: 0, versionUrl(name, version) { return `https://www.npmjs.com/package/${name}/v/${version}`; }, @@ -9395,6 +9397,7 @@ var pubRegistry = { id: "pub", displayName: "pub.dev", maxConcurrency: 8, + minRequestIntervalMs: 0, versionUrl(name, version) { return `https://pub.dev/packages/${name}/versions/${version}`; }, @@ -9513,6 +9516,29 @@ function renderSummary(input) { } // src/resolve.ts +var systemClock = { + now: () => Date.now(), + sleep: (ms) => new Promise((resolve4) => setTimeout(resolve4, ms)) +}; +function createStartGate(minIntervalMs, clock) { + if (!Number.isFinite(minIntervalMs) || minIntervalMs < 0) { + throw new Error( + `minRequestIntervalMs must be a non-negative number, got ${String(minIntervalMs)}.` + ); + } + let nextAllowedAt = 0; + return async () => { + if (minIntervalMs === 0) { + return; + } + const t = clock.now(); + const wait = nextAllowedAt - t; + nextAllowedAt = Math.max(nextAllowedAt, t) + minIntervalMs; + if (wait > 0) { + await clock.sleep(wait); + } + }; +} async function mapWithConcurrency(items, limit, worker) { const results = new Array(items.length); let cursor = 0; @@ -9525,7 +9551,7 @@ async function mapWithConcurrency(items, limit, worker) { await Promise.all(Array.from({ length: Math.min(limit, items.length) }, run3)); return results; } -async function resolvePublishDates(dependencies, http) { +async function resolvePublishDates(dependencies, http, clock = systemClock) { const byRegistry = /* @__PURE__ */ new Map(); for (const dependency of dependencies) { const bucket = byRegistry.get(dependency.registry); @@ -9538,10 +9564,14 @@ async function resolvePublishDates(dependencies, http) { const perRegistry = await Promise.all( [...byRegistry.entries()].map(async ([registryId, bucket]) => { const registry = REGISTRIES[registryId]; - return mapWithConcurrency(bucket, registry.maxConcurrency, async (dependency) => ({ - dependency, - publishedAt: await registry.fetchPublishedAt(dependency.name, dependency.version, http) - })); + const waitForSlot = createStartGate(registry.minRequestIntervalMs, clock); + return mapWithConcurrency(bucket, registry.maxConcurrency, async (dependency) => { + await waitForSlot(); + return { + dependency, + publishedAt: await registry.fetchPublishedAt(dependency.name, dependency.version, http) + }; + }); }) ); return perRegistry.flat(); @@ -12328,7 +12358,7 @@ async function run2(argv, overrides = {}) { console.log( mode === "check" ? `${subjects.length} newly introduced dependency version(s) to verify.` : `${subjects.length} locked dependency version(s) to verify.` ); - const dated = await resolvePublishDates(subjects, http); + const dated = await resolvePublishDates(subjects, http, overrides.clock); const { violations } = evaluatePolicy(dated, { minReleaseAgeDays: config.minReleaseAgeDays, now diff --git a/docs/dependency-cooldown.md b/docs/dependency-cooldown.md index 1e60e33..f3d560d 100644 --- a/docs/dependency-cooldown.md +++ b/docs/dependency-cooldown.md @@ -168,8 +168,9 @@ The design goal is that a new language costs one file plus one fixture. silently: put it in `uncheckable` with a reason. 2. **Add a registry client** in `src/registries/.ts` if the ecosystem uses a registry that is not already supported, exporting a `Registry` that maps a - name and version to a publish date. Set `maxConcurrency` to whatever the - registry's rate limit tolerates. Register it in `src/registries/index.ts` and + name and version to a publish date. Set `maxConcurrency` and + `minRequestIntervalMs` to whatever the registry's rate limit tolerates + (crates.io: one request per second). Register it in `src/registries/index.ts` and add the id to `RegistryId` in `src/types.ts`. 3. **Register the format** in the `LOCKFILE_FORMATS` array in `src/lockfiles/index.ts`. Lockfile discovery, diffing, reporting and the diff --git a/packages/dependency-cooldown/src/registries/cargo.ts b/packages/dependency-cooldown/src/registries/cargo.ts index 5a33349..f145506 100644 --- a/packages/dependency-cooldown/src/registries/cargo.ts +++ b/packages/dependency-cooldown/src/registries/cargo.ts @@ -4,8 +4,9 @@ import { parseTimestamp } from "./timestamp.js"; export const cargoRegistry: Registry = { id: "cargo", displayName: "crates.io", - // crates.io asks API clients to stay near one request per second. + // crates.io data-access policy: at most one API request per second. maxConcurrency: 1, + minRequestIntervalMs: 1000, versionUrl(name, version) { return `https://crates.io/crates/${name}/${version}`; diff --git a/packages/dependency-cooldown/src/registries/npm.ts b/packages/dependency-cooldown/src/registries/npm.ts index c8b2a14..3296f44 100644 --- a/packages/dependency-cooldown/src/registries/npm.ts +++ b/packages/dependency-cooldown/src/registries/npm.ts @@ -12,6 +12,7 @@ export const npmRegistry: Registry = { id: "npm", displayName: "npmjs.com", maxConcurrency: 8, + minRequestIntervalMs: 0, versionUrl(name, version) { return `https://www.npmjs.com/package/${name}/v/${version}`; diff --git a/packages/dependency-cooldown/src/registries/pub.ts b/packages/dependency-cooldown/src/registries/pub.ts index 1636b41..58b9b04 100644 --- a/packages/dependency-cooldown/src/registries/pub.ts +++ b/packages/dependency-cooldown/src/registries/pub.ts @@ -10,6 +10,7 @@ export const pubRegistry: Registry = { id: "pub", displayName: "pub.dev", maxConcurrency: 8, + minRequestIntervalMs: 0, versionUrl(name, version) { return `https://pub.dev/packages/${name}/versions/${version}`; diff --git a/packages/dependency-cooldown/src/resolve.ts b/packages/dependency-cooldown/src/resolve.ts index 9b5750a..696e632 100644 --- a/packages/dependency-cooldown/src/resolve.ts +++ b/packages/dependency-cooldown/src/resolve.ts @@ -3,6 +3,41 @@ import type { HttpClient } from "./registries/http.js"; import type { DatedDependency } from "./policy.js"; import type { LockedDependency } from "./types.js"; +export interface Clock { + now(): number; + sleep(ms: number): Promise; +} + +const systemClock: Clock = { + now: () => Date.now(), + sleep: (ms) => new Promise((resolve) => setTimeout(resolve, ms)), +}; + +/** + * Returns a gate that spaces invocations so a new one cannot start until + * `minIntervalMs` has passed since the previous invocation was admitted. + * Reservations are taken before sleeping so concurrent waiters do not share a slot. + */ +function createStartGate(minIntervalMs: number, clock: Clock): () => Promise { + if (!Number.isFinite(minIntervalMs) || minIntervalMs < 0) { + throw new Error( + `minRequestIntervalMs must be a non-negative number, got ${String(minIntervalMs)}.`, + ); + } + let nextAllowedAt = 0; + return async () => { + if (minIntervalMs === 0) { + return; + } + const t = clock.now(); + const wait = nextAllowedAt - t; + nextAllowedAt = Math.max(nextAllowedAt, t) + minIntervalMs; + if (wait > 0) { + await clock.sleep(wait); + } + }; +} + async function mapWithConcurrency( items: readonly T[], limit: number, @@ -29,6 +64,7 @@ async function mapWithConcurrency( export async function resolvePublishDates( dependencies: readonly LockedDependency[], http: HttpClient, + clock: Clock = systemClock, ): Promise { const byRegistry = new Map(); for (const dependency of dependencies) { @@ -43,10 +79,14 @@ export async function resolvePublishDates( const perRegistry = await Promise.all( [...byRegistry.entries()].map(async ([registryId, bucket]) => { const registry = REGISTRIES[registryId as keyof typeof REGISTRIES]; - return mapWithConcurrency(bucket, registry.maxConcurrency, async (dependency) => ({ - dependency, - publishedAt: await registry.fetchPublishedAt(dependency.name, dependency.version, http), - })); + const waitForSlot = createStartGate(registry.minRequestIntervalMs, clock); + return mapWithConcurrency(bucket, registry.maxConcurrency, async (dependency) => { + await waitForSlot(); + return { + dependency, + publishedAt: await registry.fetchPublishedAt(dependency.name, dependency.version, http), + }; + }); }), ); diff --git a/packages/dependency-cooldown/src/run.ts b/packages/dependency-cooldown/src/run.ts index 77d6665..36358bb 100644 --- a/packages/dependency-cooldown/src/run.ts +++ b/packages/dependency-cooldown/src/run.ts @@ -10,7 +10,7 @@ import { annotate, readPullRequestContext, setOutput, writeStepSummary } from ". import { ORG_MIN_RELEASE_AGE_DAYS, evaluatePolicy } from "./policy.js"; import { type HttpClient, createHttpClient } from "./registries/http.js"; import { renderSummary, violationMessage } from "./report.js"; -import { resolvePublishDates } from "./resolve.js"; +import { type Clock, resolvePublishDates } from "./resolve.js"; import { discoverLockfiles, listLockfilesAtRef, scanRef, scanWorkingTree } from "./scan.js"; export const DEFAULT_BYPASS_LABEL = "dependency-cooldown-bypass"; @@ -74,6 +74,8 @@ export interface RunOverrides { /** Injected by tests so no registry is contacted. */ http?: HttpClient; now?: Date; + /** Injected by tests so crates.io pacing does not sleep in wall-clock time. */ + clock?: Clock; } export async function run(argv: string[], overrides: RunOverrides = {}): Promise { @@ -120,7 +122,7 @@ export async function run(argv: string[], overrides: RunOverrides = {}): Promise : `${subjects.length} locked dependency version(s) to verify.`, ); - const dated = await resolvePublishDates(subjects, http); + const dated = await resolvePublishDates(subjects, http, overrides.clock); const { violations } = evaluatePolicy(dated, { minReleaseAgeDays: config.minReleaseAgeDays, now, diff --git a/packages/dependency-cooldown/src/types.ts b/packages/dependency-cooldown/src/types.ts index 5c3bb6e..015ddc7 100644 --- a/packages/dependency-cooldown/src/types.ts +++ b/packages/dependency-cooldown/src/types.ts @@ -44,10 +44,15 @@ export interface Registry { id: RegistryId; displayName: string; /** - * Parallel requests allowed against this registry. crates.io asks for roughly - * one request per second, so it is deliberately serialised. + * Parallel in-flight requests allowed against this registry. */ maxConcurrency: number; + /** + * Minimum milliseconds between starting requests to this registry. + * crates.io allows at most one API request per second, so cargo uses 1000. + * Registries with no documented request-rate cap use 0. + */ + minRequestIntervalMs: number; /** Human-facing page for a version, used in the report. */ versionUrl(name: string, version: string): string; fetchPublishedAt(name: string, version: string, http: HttpClient): Promise; diff --git a/packages/dependency-cooldown/test/end-to-end.test.ts b/packages/dependency-cooldown/test/end-to-end.test.ts index e14e59f..b28d8d7 100644 --- a/packages/dependency-cooldown/test/end-to-end.test.ts +++ b/packages/dependency-cooldown/test/end-to-end.test.ts @@ -6,6 +6,7 @@ import { afterEach, beforeEach, describe, expect, it } from "vitest"; import { BYPASS_REASON_MARKER } from "../src/bypass.js"; import type { HttpClient } from "../src/registries/http.js"; +import type { Clock } from "../src/resolve.js"; import { DEFAULT_BYPASS_LABEL, run } from "../src/run.js"; import { CLOUDFLARE_PAGES_BUN_LOCKB_MARKER } from "../src/scan.js"; import { readFixture } from "./helpers/fixtures.js"; @@ -14,6 +15,12 @@ const NOW = new Date("2026-08-27T12:00:00Z"); const OLD = "2020-01-01T00:00:00.000Z"; const FRESH = "2026-08-25T00:00:00.000Z"; +/** Skip wall-clock crates.io pacing; the interval itself is tested in resolve.test.ts. */ +const unpaced: Clock = { + now: () => Date.now(), + sleep: async () => {}, +}; + /** * Publish dates keyed by `name@version`; anything not listed is old enough to * pass, so each test only declares the versions it cares about. @@ -104,6 +111,7 @@ describe("check mode", () => { const code = await run(["--mode=check", `--base-ref=${baseSha}`, `--repo-dir=${repoDir}`], { http: httpStub({}), now: NOW, + clock: unpaced, }); expect(code).toBe(0); @@ -119,6 +127,7 @@ describe("check mode", () => { const code = await run(["--mode=check", `--base-ref=${baseSha}`, `--repo-dir=${repoDir}`], { http: httpStub({ "tiny-invariant@1.3.4": true }), now: NOW, + clock: unpaced, }); expect(code).toBe(1); @@ -135,6 +144,7 @@ describe("check mode", () => { const code = await run(["--mode=check", `--base-ref=${baseSha}`, `--repo-dir=${repoDir}`], { http: httpStub({ "tiny-invariant@1.3.4": true }), now: NOW, + clock: unpaced, }); expect(code).toBe(0); @@ -149,6 +159,7 @@ describe("check mode", () => { const code = await run(["--mode=check", `--base-ref=${baseSha}`, `--repo-dir=${repoDir}`], { http: httpStub({ "once_cell@1.20.2": true }), now: NOW, + clock: unpaced, }); expect(code).toBe(1); @@ -165,6 +176,7 @@ describe("check mode", () => { const code = await run(["--mode=check", `--base-ref=${baseSha}`, `--repo-dir=${repoDir}`], { http: httpStub({ "tiny-invariant@1.3.4": true }), now: NOW, + clock: unpaced, }); expect(code).toBe(0); @@ -183,6 +195,7 @@ describe("check mode", () => { const code = await run(["--mode=check", `--base-ref=${baseSha}`, `--repo-dir=${repoDir}`], { http: httpStub({ "tiny-invariant@1.3.4": true }), now: NOW, + clock: unpaced, }); expect(code).toBe(0); @@ -199,6 +212,7 @@ describe("check mode", () => { run(["--mode=check", "--base-ref=HEAD", `--repo-dir=${repoDir}`], { http: httpStub({}), now: NOW, + clock: unpaced, }), ).rejects.toThrow(new RegExp(BYPASS_REASON_MARKER)); }); @@ -212,6 +226,7 @@ describe("check mode", () => { run(["--mode=check", `--base-ref=${baseSha}`, `--repo-dir=${repoDir}`], { http: httpStub({}), now: NOW, + clock: unpaced, }), ).rejects.toThrow(/below the organisation floor/); }); @@ -226,6 +241,7 @@ describe("check mode", () => { const code = await run(["--mode=check", `--base-ref=${baseSha}`, `--repo-dir=${repoDir}`], { http: httpStub({}), now: NOW, + clock: unpaced, }); expect(code).toBe(1); @@ -241,6 +257,7 @@ describe("check mode", () => { const code = await run(["--mode=check", `--base-ref=${baseSha}`, `--repo-dir=${repoDir}`], { http: httpStub({ "serde@1.0.213": true }), now: NOW, + clock: unpaced, }); expect(code).toBe(1); @@ -258,6 +275,7 @@ describe("check mode", () => { const code = await run(["--mode=check", `--base-ref=${baseSha}`, `--repo-dir=${repoDir}`], { http: httpStub({ "tslib@2.8.1": true }), now: NOW, + clock: unpaced, }); expect(code).toBe(1); @@ -276,6 +294,7 @@ describe("check mode", () => { run(["--mode=check", `--base-ref=${baseSha}`, `--repo-dir=${repoDir}`], { http: httpStub({}), now: NOW, + clock: unpaced, }), ).rejects.toThrow(/bun\.lockb/); }); @@ -289,6 +308,7 @@ describe("check mode", () => { run(["--mode=check", `--base-ref=${baseSha}`, `--repo-dir=${repoDir}`], { http: httpStub({}), now: NOW, + clock: unpaced, }), ).rejects.toThrow(/bun\.lockb/); }); @@ -301,6 +321,7 @@ describe("check mode", () => { run(["--mode=check", `--base-ref=${baseSha}`, `--repo-dir=${repoDir}`], { http: httpStub({}), now: NOW, + clock: unpaced, }), ).rejects.toThrow(/bun\.lockb/); }); @@ -313,6 +334,7 @@ describe("check mode", () => { run(["--mode=check", `--base-ref=${baseSha}`, `--repo-dir=${repoDir}`], { http: httpStub({}), now: NOW, + clock: unpaced, }), ).rejects.toThrow(/No supported lockfile/); }); @@ -326,6 +348,7 @@ describe("check mode", () => { const code = await run(["--mode=check", `--base-ref=${baseSha}`, `--repo-dir=${repoDir}`], { http: httpStub({ "serde@1.0.213": true }), now: NOW, + clock: unpaced, }); expect(code).toBe(1); @@ -341,6 +364,7 @@ describe("check mode", () => { const code = await run(["--mode=check", `--base-ref=${baseSha}`, `--repo-dir=${repoDir}`], { http: httpStub({ "tiny-invariant@1.3.4": true }), now: NOW, + clock: unpaced, }); expect(code).toBe(0); @@ -355,6 +379,7 @@ describe("audit mode", () => { const code = await run(["--mode=audit", `--repo-dir=${repoDir}`], { http: httpStub({ "tiny-invariant@1.3.4": true }), now: NOW, + clock: unpaced, }); expect(code).toBe(0); @@ -370,6 +395,7 @@ describe("audit mode", () => { run(["--mode=audit", "--base-ref=HEAD", `--repo-dir=${repoDir}`], { http: httpStub({}), now: NOW, + clock: unpaced, }), ).rejects.toThrow(/not used in audit mode/); }); @@ -382,6 +408,7 @@ describe("audit mode", () => { const code = await run(["--mode=audit", `--repo-dir=${repoDir}`], { http: httpStub({}), now: NOW, + clock: unpaced, }); expect(code).toBe(0); @@ -410,7 +437,11 @@ describe("audit mode", () => { }, }; - const code = await run(["--mode=audit", `--repo-dir=${repoDir}`], { http, now: NOW }); + const code = await run(["--mode=audit", `--repo-dir=${repoDir}`], { + http, + now: NOW, + clock: unpaced, + }); expect(code).toBe(0); expect(summary()).toContain("human-readable-checksum"); diff --git a/packages/dependency-cooldown/test/registries.test.ts b/packages/dependency-cooldown/test/registries.test.ts index 91706d4..b4fac79 100644 --- a/packages/dependency-cooldown/test/registries.test.ts +++ b/packages/dependency-cooldown/test/registries.test.ts @@ -16,6 +16,14 @@ function stubHttp(payloads: Record): HttpClient { }; } +describe("registry request pacing", () => { + it("only crates.io asks for a one-second gap between request starts", () => { + expect(npmRegistry.minRequestIntervalMs).toBe(0); + expect(pubRegistry.minRequestIntervalMs).toBe(0); + expect(cargoRegistry.minRequestIntervalMs).toBe(1000); + }); +}); + describe("npm registry", () => { const packument = { "https://registry.npmjs.org/left-pad": { diff --git a/packages/dependency-cooldown/test/resolve.test.ts b/packages/dependency-cooldown/test/resolve.test.ts index 736f745..1ae7297 100644 --- a/packages/dependency-cooldown/test/resolve.test.ts +++ b/packages/dependency-cooldown/test/resolve.test.ts @@ -1,11 +1,17 @@ import { describe, expect, it } from "vitest"; import type { HttpClient } from "../src/registries/http.js"; -import { resolvePublishDates } from "../src/resolve.js"; +import { type Clock, resolvePublishDates } from "../src/resolve.js"; import type { LockedDependency } from "../src/types.js"; const PUBLISHED = "2026-01-15T00:00:00.000Z"; +/** Admits every request immediately so tests can assert concurrency, not wall-clock pacing. */ +const unpaced: Clock = { + now: () => Date.now(), + sleep: async () => {}, +}; + function dep(registry: LockedDependency["registry"], name: string): LockedDependency { return { registry, name, version: "1.0.0", lockfile: "lock" }; } @@ -35,7 +41,7 @@ describe("resolvePublishDates", () => { ); }); - it("serialises crates.io requests to respect its rate limit", async () => { + it("never overlaps crates.io requests", async () => { let inFlight = 0; let peak = 0; const http: HttpClient = { @@ -51,11 +57,88 @@ describe("resolvePublishDates", () => { await resolvePublishDates( ["a", "b", "c", "d"].map((name) => dep("cargo", name)), http, + unpaced, ); expect(peak).toBe(1); }); + it("starts crates.io requests at most once per second", async () => { + let now = 0; + const starts: number[] = []; + const http: HttpClient = { + async getJson() { + starts.push(now); + now += 5; + return { version: { num: "1.0.0", created_at: PUBLISHED } }; + }, + }; + + await resolvePublishDates(["a", "b"].map((name) => dep("cargo", name)), http, { + now: () => now, + sleep: async (ms) => { + now += ms; + }, + }); + + expect(starts).toEqual([0, 1000]); + }); + + it("waits in real time between crates.io request starts", async () => { + const starts: number[] = []; + const http: HttpClient = { + async getJson() { + starts.push(Date.now()); + return { version: { num: "1.0.0", created_at: PUBLISHED } }; + }, + }; + + await resolvePublishDates(["a", "b"].map((name) => dep("cargo", name)), http); + + expect(starts).toHaveLength(2); + expect(starts[1]! - starts[0]!).toBeGreaterThanOrEqual(950); + }); + + it("does not idle after a crates.io lookup that already took a second", async () => { + let now = 0; + const starts: number[] = []; + const sleeps: number[] = []; + const http: HttpClient = { + async getJson() { + starts.push(now); + now += 1500; + return { version: { num: "1.0.0", created_at: PUBLISHED } }; + }, + }; + + await resolvePublishDates(["a", "b"].map((name) => dep("cargo", name)), http, { + now: () => now, + sleep: async (ms) => { + sleeps.push(ms); + now += ms; + }, + }); + + expect(starts).toEqual([0, 1500]); + expect(sleeps).toEqual([]); + }); + + it("does not apply crates.io spacing to other registries", async () => { + const starts: number[] = []; + const http: HttpClient = { + async getJson() { + starts.push(Date.now()); + await new Promise((resolve) => setTimeout(resolve, 5)); + return { time: { "1.0.0": PUBLISHED } }; + }, + }; + + await resolvePublishDates(["a", "b"].map((name) => dep("npm", name)), http); + + expect(starts).toHaveLength(2); + expect(starts[1]! - starts[0]!).toBeLessThan(1000); + }); + it("rejects when a publish date cannot be established", async () => { const http: HttpClient = { async getJson() {