Spend a rate-limit token on every request, not every operation

RO
4d ago
+298-1

What

Every HTTP request the CLI's SDK client sends now takes a token from the shared, cross-process token bucket in internal/resilience (50 burst, 10/s refill, unchanged). Until now the bucket was charged once per SDK operation, in OnOperationGate. The operation's token now pays for its first request, so a one-request command costs one token, the same as before.

A RoundTripper (internal/resilience/transport.go) wraps the transport the SDK client already uses. When the bucket is empty it waits for a refill and does not fail. It stops if the caller cancels or the request's own timeout expires. The operation gate, the bulkhead, the circuit breaker and the Retry-After block are all unchanged.

Why

The bucket's own docs say it limits requests (TokensPerRequest, "client limit 10/s"). In practice, these requests never reached it:

  • Pages 2..N of every --all walk. The SDK follows the Link header with raw doRequestURL calls that never pass through the operation gate (client.go:640 @ SDK 35f3d72d).
  • basecamp api get (api.go:55) and the /projects.json walk that --in <id> makes on every invocation (resolver.go:432). The SDK's raw Get/GetAll aren't gated operations (client.go:355).
  • Retries.

So a page walk ran as fast as the server answered, and parallel api get loops had no limit at all. That matches the traffic shape behind the 15 October recordings.json load (an old CLI build, OAuth client 19503): about 12.8 requests/s with about 6 in flight on average, walking projects × types × pages.

Measured against a local mock server

All runs used real CLI binaries against a synthetic HTTP server with a fresh state directory each time.

Scenariomainthis branch
1-page command, 80 ms server2 requests, 0.17 s2 requests, 0.16 s
200-page --all, 80 ms server16.4 s, 12.2/s16.3 s, 12.4/s (fits in burst + refill)
200-page --all, 0 ms server0.23 s, ~877/s15.1 s, 13.3/s mean (burst then 10/s)
8 parallel api get loops × 40 pages, 470 ms server15.8/s, 160 per 10 s10.8/s, 149 per 10 s
8 parallel api get loops × 60 pages, 0 ms server145/s11.2/s

Normal commands don't change. Bulk walks and agent loops now hold to the bucket's 10/s across every basecamp process sharing a cache directory. That means one OS user's processes on one machine, across all of that user's profiles.

Testing

  • bin/ci is green locally, including e2e.
  • New tests in internal/resilience/transport_test.go run a real SDK client against httptest and count tokens spent:
    • a 5-page walk spends 5
    • 3 raw Gets spend 3
    • a 503-then-200 retry spends 2
    • a single-page operation spends 1
  • Red/green: with Transport returning the bare base transport, the walk, raw and retry tests fail (spent 1, 0 and 0). If the prepaid marker is removed, the walk and single-page tests fail (spent 6 and 2), which shows nothing is double-charged.
  • Pace waits for a refill on a held clock (2 waits, 1 s for 3 tokens at 2/s) and returns context.Canceled when the caller cancels mid-wait.

Trade-offs and what this does not do

  • Numbers unchanged on purpose. At the incident's ~470 ms per request, 10/s is still about 47 s of server time per 10 s, against the 20 s budget in basecamp/bc3#13873. Matching the documented 50 per 10 s (5/s), or capping in-flight requests (by Little's law, server-seconds per second = requests in flight), would need a person's call. This PR is not that call.
  • Under extreme contention (hundreds of waiters), a paced request can reach the SDK's 30 s client timeout. It then fails as a network error, not as the gate's named "client limit" error.
  • The transport does not apply the shared Retry-After block. The operation gate still does, and the SDK's retry loop still honours each response's Retry-After. Raw api get during a block is now paced, but not blocked.
  • api get and name lookups still don't take a bulkhead slot. The bulkhead counts processes, not requests.
  • --in <numeric id> still walks /projects.json on every invocation. That's a separate, cheap follow-up.
  • Already-installed old CLIs are unaffected. Only server-side enforcement covers them.

Summary by cubic

Spends one rate-limit token on every HTTP request the CLI's SDK client sends, instead of once per SDK operation. Requests that previously bypassed the shared cross-process token bucket — pages 2..N of --all walks, raw basecamp api get calls, and retries — are now paced at the documented 10/s across every process sharing a cache directory.

Behavior notes

  • A new pacedTransport wraps the SDK client's existing transport and waits for a refill when the bucket is empty instead of failing; it stops on caller cancellation or the request timeout, closing the request body and sending nothing when canceled mid-wait.
  • The operation gate's token now pays for that operation's first request, so nothing is charged twice; retries of a typed operation spend a token per attempt. The gate, bulkhead, circuit breaker, and Retry-After block are unchanged.
  • api get is paced but still not subject to the shared Retry-After block or the bulkhead; under heavy contention a paced request can reach the SDK's 30 s timeout and fail as a network error.
  • Already-installed CLI versions are unaffected.