Skip to content

fix(cloudflare/workers): cut Worker read API fan-out that trips 429/971 - #932

Open
john-royal wants to merge 1 commit into
mainfrom
fix/cloudflare-worker-read-api-fanout
Open

fix(cloudflare/workers): cut Worker read API fan-out that trips 429/971#932
john-royal wants to merge 1 commit into
mainfrom
fix/cloudflare-worker-read-api-fanout

Conversation

@john-royal

Copy link
Copy Markdown
Contributor

Cut Cloudflare Worker read/reconcile API fan-out that was tripping account-level 429 / code 971 on repeated plan/deploy for plain workers.dev Workers (#926).

  • Memoize GET /accounts/{id}/workers/subdomain for the provider lifetime via cachedFunction
  • Allow CLOUDFLARE_WORKERS_SUBDOMAIN to bypass that lookup when the account subdomain is already known
  • Skip listDomains, account-wide route discovery, and getScriptSchedule when those surfaces are unmanaged (empty-array props still observe so deletions/drift converge)
  • Bound Worker read Effect.all concurrency to 1
  • Same domain/cron gating on reconcile so deploys don't pay for unused calls either
// plain workers.dev Worker — read no longer fans out domains/routes/crons
yield* Cloudflare.Worker("Api", { main: "./src/api.ts" });

// optional: skip the account-subdomain GET entirely
process.env.CLOUDFLARE_WORKERS_SUBDOMAIN = "my-team";

Memoize account subdomain lookups, skip unmanaged domain/route/cron
observation, and serialize the remaining Worker read calls so plain
workers.dev Workers stop stampeding Cloudflare on every plan/deploy.

Closes #926

Co-authored-by: Cursor <cursoragent@cursor.com>
@alchemy-version-bot

Copy link
Copy Markdown
Contributor

Install the packages built from this commit:

alchemy

bun add alchemy@https://pkg.ing/alchemy/96225a2

@alchemy.run/better-auth

bun add @alchemy.run/better-auth@https://pkg.ing/@alchemy.run/better-auth/96225a2

@alchemy.run/pr-package

bun add @alchemy.run/pr-package@https://pkg.ing/@alchemy.run/pr-package/96225a2

@benpsnyder

Copy link
Copy Markdown
Contributor

Thanks for turning this around so quickly. Before this lands I want to correct the root cause I gave you in #926, because I got it wrong and this PR inherits the error.

The account-subdomain lookup was the symptom, not the cause

I filed #926 pointing at GET /accounts/{id}/workers/subdomain because that is the endpoint the 429 / code 971 surfaced on. Reading the beta.64 provider more carefully, that endpoint is called twice per plan+deploy — once in read and once in reconcile. Two calls cannot exhaust an account-level throttle.

The dominant cost is readWorkerRoutes:

read
  └─ readWorkerRoutes(scriptName)
       ├─ listAllZones(accountId)                 // paginated over every zone in the account
       └─ listWorkerRoutesInZones(...)
            └─ Effect.all(zones.map(listRoutes), { concurrency: "unbounded" })

Routes are zone-scoped with no account-level enumeration API, so every Worker read enumerates all account zones and then issues one GET /zones/{id}/workers/routes per zone, all at once. On the account in #926 that is 328 zones, so a single Worker read is on the order of 330+ requests fired concurrently. A couple of plan/deploy cycles clears Cloudflare's global per-user API budget, and whichever call lands next takes the 971 — in my case the subdomain GET.

So the memoization and CLOUDFLARE_WORKERS_SUBDOMAIN bypass in this PR are worth having, but they are not what was causing the throttle.

The consequence for this PR

shouldObserveWorkerRoutes gates the fan-out; it never bounds it. For a Worker with olds.routes !== undefined or any persisted routes — i.e. exactly the Workers that use zone routes — the account-wide sweep at concurrency: "unbounded" is unchanged. Several of my stacks declare routes, and they will still trip this after the PR merges.

The fix is already in the file. reconcileRoutes doesn't sweep the account — it derives its zone set from desired ∪ previous:

const zoneIds = Array.from(new Set([
  ...desired.map((route) => route.zoneId),
  ...previous.map((route) => route.zoneId),
]));
const liveAll = yield* listWorkerRoutesInZones(scriptName, zoneIds);

read can do the same thing with output.routes[].zoneId plus whatever zones olds.routes resolves to. That preserves drift detection for routes Alchemy manages, and it drops the account-wide sweep to zero calls for a Worker with no routes and a handful for a Worker with them. Bounding the concurrency: "unbounded" in listWorkerRoutesInZones would be worth doing independently — the same helper is on the reconcile path.

Discovering routes attached entirely out of band in a zone Alchemy has never touched is the only thing lost, and that is an adoption concern that probably shouldn't cost a full account sweep on every read anyway.

Second point: the domain gating re-opens #942

Separately I hit #942 — an omitted domain deleting a custom hostname that was attached out of band. This PR closes the clean case, which I appreciate. But:

const reconcileCustomDomains =
  news.domain !== undefined || previousCustomDomains.length > 0;

The second disjunct re-arms it. If output.domains holds any non-workers.dev URL — true for any state written by a build whose read observed domains unconditionally, i.e. everything before this PR — then reconcile runs with desiredDomains === [] and reconcileDomains deletes every live domain, out-of-band ones included. That is the adopted / partially-managed case #942 is actually about, and it is the state most existing users' state files are in right now.

On my own workaround

The downstream patch I described in #926 had the same misattribution, and one part of it is worse than what you have written here: I gated the domain read on olds?.domain === undefined. On adoption olds is undefined, so it hides an adopted Worker's live domains from read while reconcile still deletes them. shouldObserveWorkerDomains consulting output.domains is the right call. I am dropping my version.

Evidence quality

To be clear about what this is: static analysis of the beta.64 provider plus the account inventory I posted in #926. I do not have captured request-count telemetry from the original failure, so I am not claiming an exact number for the run that broke. You asked earlier for a preferred test shape — if it would help, I will put together a reproduction that stubs the Cloudflare operations and asserts request counts for (a) a plain workers.dev Worker and (b) a Worker declaring one route on a many-zone account, which would make the before/after concrete for both halves of this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants