Skip to content

[deploy-helpers] respect Retry-After during asset uploads - #15632

Open
petebacondarwin wants to merge 6 commits into
mainfrom
fix/asset-upload-retry-after
Open

[deploy-helpers] respect Retry-After during asset uploads#15632
petebacondarwin wants to merge 6 commits into
mainfrom
fix/asset-upload-retry-after

Conversation

@petebacondarwin

@petebacondarwin petebacondarwin commented Sep 13, 2026

Copy link
Copy Markdown
Contributor

Partially addresses #15060.

Stacked on #15631. This PR should be reviewed as the single follow-up commit on top of that change.

The fetch layer already exposes Retry-After as APIError.retryAfterMs, but the static asset upload loop ignored it and retried using its own short exponential/gateway delays. A response such as Retry-After: 120 could therefore be retried after only a few seconds, adding more load while the asset service was asking the client to pause.

This change:

  • pauses retries and pending upload requests until the latest outstanding Retry-After deadline;
  • gives Retry-After precedence over the local retry delays instead of stacking both waits;
  • uses a request-level concurrency limiter so simultaneous gateway failures resume at concurrency one, then recover gradually as introduced by [deploy-helpers] recover asset upload concurrency after gateway errors #15631; and
  • schedules very long deadlines in safe timer slices to avoid the Node large-setTimeout clamp.

The tests simulate a 120-second directive without waiting in real time. They verify that neither the failed request nor queued assets restart early, that simultaneous 524 retries remain serialized after the pause, and that concurrency subsequently recovers.

Scope: this covers Workers static asset upload requests handled by syncAssets(). Retry handling for the initial assets-upload-session request and the separate Pages /pages/assets/upload path is unchanged and can be addressed independently.


  • Tests
    • Tests included/updated
    • Automated tests not possible - manual testing has been completed as follows:
    • Additional testing not necessary because:
  • Public documentation
    • Cloudflare docs PR(s):
    • Documentation not necessary because: This changes internal retry scheduling and adds no public configuration or API.

Note

This is a contribution from an AI agent: OpenAI Codex (GPT-5).


Devin Review

@changeset-bot

changeset-bot Bot commented Sep 13, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: eaab2b4

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 5 packages
Name Type
@cloudflare/deploy-helpers Patch
wrangler Patch
@cloudflare/remote-bindings Patch
@cloudflare/vite-plugin Patch
@cloudflare/vitest-plugin Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@ask-bonk

ask-bonk Bot commented Sep 13, 2026

Copy link
Copy Markdown
Contributor

LGTM!

github run

devin-ai-integration[bot]

This comment was marked as resolved.

@pkg-pr-new

pkg-pr-new Bot commented Sep 13, 2026

Copy link
Copy Markdown
@cloudflare/autoconfig

npm i https://pkg.pr.new/@cloudflare/autoconfig@15632

@cloudflare/build-output-utils

npm i https://pkg.pr.new/@cloudflare/build-output-utils@15632

@cloudflare/codemods

npm i https://pkg.pr.new/@cloudflare/codemods@15632

@cloudflare/config

npm i https://pkg.pr.new/@cloudflare/config@15632

@cloudflare/containers-shared

npm i https://pkg.pr.new/@cloudflare/containers-shared@15632

create-cloudflare

npm i https://pkg.pr.new/create-cloudflare@15632

@cloudflare/deploy-helpers

npm i https://pkg.pr.new/@cloudflare/deploy-helpers@15632

@cloudflare/kv-asset-handler

npm i https://pkg.pr.new/@cloudflare/kv-asset-handler@15632

miniflare

npm i https://pkg.pr.new/miniflare@15632

@cloudflare/pages-functions

npm i https://pkg.pr.new/@cloudflare/pages-functions@15632

@cloudflare/pages-shared

npm i https://pkg.pr.new/@cloudflare/pages-shared@15632

@cloudflare/unenv-preset

npm i https://pkg.pr.new/@cloudflare/unenv-preset@15632

@cloudflare/vite-plugin

npm i https://pkg.pr.new/@cloudflare/vite-plugin@15632

@cloudflare/vitest-plugin

npm i https://pkg.pr.new/@cloudflare/vitest-plugin@15632

@cloudflare/workers-auth

npm i https://pkg.pr.new/@cloudflare/workers-auth@15632

@cloudflare/workers-editor-shared

npm i https://pkg.pr.new/@cloudflare/workers-editor-shared@15632

@cloudflare/workers-utils

npm i https://pkg.pr.new/@cloudflare/workers-utils@15632

wrangler

npm i https://pkg.pr.new/wrangler@15632

commit: eaab2b4

@petebacondarwin
petebacondarwin force-pushed the fix/asset-upload-retry-after branch from e6494f9 to a3bf57a Compare September 13, 2026 12:46
devin-ai-integration[bot]

This comment was marked as resolved.

Base automatically changed from fix/asset-upload-concurrency-recovery to main September 14, 2026 12:52
@workers-devprod
workers-devprod requested review from a team and dario-piotrowicz and removed request for a team September 14, 2026 12:53
@workers-devprod

Copy link
Copy Markdown
Contributor

Codeowners approval required for this PR:

  • @cloudflare/wrangler
Show detailed file reviewers
  • .changeset/calm-assets-retry.md: [@cloudflare/wrangler]
  • .changeset/warm-assets-recover.md: [@cloudflare/wrangler]
  • packages/deploy-helpers/src/deploy/helpers/assets.ts: [@cloudflare/wrangler]
  • packages/wrangler/src/tests/deploy/assets.test.ts: [@cloudflare/wrangler]

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

Labels

None yet

Projects

Status: Untriaged

Development

Successfully merging this pull request may close these issues.

2 participants