fix(cloudflare): stabilize staged Worker before CDN warmup - #3038
fix(cloudflare): stabilize staged Worker before CDN warmup#3038james-elicx wants to merge 1 commit into
Conversation
|
/bigbonk review for issues |
commit: |
|
Performance benchmarksCompared 1 improved · 0 regressed · 5 within ±1.5%
View detailed results and traces 🟢 improvement · 🔴 regression · ⚫ change below 1.5% · paired base/head |
![ask-bonk[bot]](https://avatars.githubusercontent.com/in/2454580?s=60&v=4){kind=small}
|
|
||
| for (let attempt = 0; attempt < maxAttempts; attempt++) { | ||
| const url = buildWarmupUrl(options.targetUrl, probePath); | ||
| url.searchParams.set(STAGED_READINESS_QUERY_PARAM, `${probeId}-${attempt}`); |
There was a problem hiding this comment.
[P1] Keep the readiness nonce out of userland routing
This parameter is not stripped from the request: the App Router hides only _rsc before middleware/rendering, and Pages Router also receives this query unchanged. A middleware or page that rejects unknown parameters or branches on searchParams can therefore redirect/error on every probe even though the canonical warm request is healthy. In strict mode that blocks promotion; in non-strict mode it incurs the full readiness wait and fallback. Please preserve the unique edge key while removing this reserved parameter before userland, or probe through a route-insensitive readiness endpoint.
| deploymentId: options.deploymentId, | ||
| expectedBuildId: options.expectedBuildId, | ||
| expectedRscBuildId: options.expectedRscBuildId, | ||
| timeoutMs: options.warmCdnTimeout, |
There was a problem hiding this comment.
[P1] Honor the configured staged retry budget
warmCdnRetries is the existing override for staged propagation (the CLI documents a default of 60 only when it is unset), but this call never maps it to maxAttempts. As a result, even --warm-cdn-retries=0 now performs 60 serial readiness probes; with the default 10-second per-request timeout, an operator who requested no retries can wait about 11 minutes. Preserve the 60-attempt default, but derive the readiness limit from an explicitly supplied retry count or expose and wire a separate readiness budget.
| if (!readiness.ready) { | ||
| const message = `CDN warmup could not verify staged Worker readiness: ${readiness.error}.`; | ||
| if (options.warmCdnStrict) throw new Error(message); | ||
| console.warn(` ${message} Warming after promotion instead.`); |
There was a problem hiding this comment.
[P1] Do not succeed when no-promote makes fallback impossible
When readiness fails non-strictly, this leaves the full plan untouched and promises to warm it after promotion. With warmCdnPromote === false, however, the early return below runs before remainingWarmPlan is processed, so the command succeeds after probes without sending any canonical warm request. That contradicts the --warm-cdn-no-promote contract of leaving a warmed version staged. Treat readiness failure as fatal when promotion is disabled (as the no-staging branch already does), or provide a real staged fallback before returning.
|
Submitted 3 inline findings. Focused test suite passed: 24/24. |
1 participant
Summary
Root cause
The deployed proof failed twice while staging was still visibly routing a mix of the seed and uploaded Workers. In the second attempt, the loading-shell key accepted immediately during that mixed phase later missed, while the full key that reached the uploaded build only after six attempts later hit. URL, query, Vary fields, build ID, version UUID, host, and TTL matched. The existing 15-second post-fill delay occurred too late to make an early transitional fill reusable.
Scope
This changes only Cloudflare CDN warmup orchestration and its focused tests. It does not change client RSC identity, ISR eligibility/discovery, middleware, dev mode, TPR, cache adapters, or general request handling.
Validation
References