Repository navigation
ci(deploy): retry the website deploy step on a transient Cloudflare failure - #1005
Merged
Merged
Conversation
…ailure Run 40103130697 failed at `Deploy the preview worker` with Cloudflare API error code 10013 after the build had already succeeded, and the next push deployed the same commit successfully. Both website deploys now run through .github/scripts/deploy-with-retry.sh, which wraps the deploy step and not the build. Three attempts, 10 s then 30 s backoff, a per-attempt timeout and a step-level timeout-minutes bound the added time to about a minute. Only code 10013, an HTTP 5xx, a connection reset and an expired attempt timeout are retried: an authentication failure, a missing environment and a wrangler validation error still fail on the first attempt, because retrying those triples the noise and delays the real message. Every retry emits a ::warning:: naming the attempt and the reason, so the flake rate stays visible instead of disappearing behind a passing job. The command is passed through from each workflow, so the preview keeps its `npx wrangler deploy --config wrangler.preview.jsonc` and production keeps `pnpm exec wrangler deploy --config wrangler.router.jsonc --var CANONICAL_HOST:$CANONICAL_HOST`. test/unit/scripts/deploy-with-retry.spec.ts runs the real script against a stub wrangler: a 10013 is retried once with a warning naming attempt 2, a 401 gets one attempt and no retry, a 5xx and a connection reset are retried, three transient failures exit non-zero with the last error on stderr, the arguments survive unchanged, and an attempt that exceeds its timeout is retried. Disabling the retry makes five of the eight fail.
🦋 Changeset detectedLatest commit: 770b9d8 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 |
Contributor
|
🧹 Site preview for this PR was deleted (PR closed). |
 [Medium impact] The PR appears safe to merge; the latest change fixes the outstanding macOS test issue.
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Two review findings on the new script, both real: - `timeout` is coreutils. Every GitHub runner has it and a stock macOS does not, so the unit suite would have failed on a laptop with exit 127 before wrangler ever ran. The script now prefers `timeout`, falls back to `gtimeout`, and otherwise runs the attempt unbounded behind a `::warning::`. The empty-array expansion is the bash 3.2 form, because that is what macOS ships. - The argument test recorded `"$*"`, which joins the arguments into one string. Unquoted `$@` in the script would still have passed it. The stub now records one field per argument and the test includes a value with spaces. Both new tests are load-bearing: reintroducing either defect fails exactly one of them, verified by editing the script and re-running the suite.
Collaborator
Coverage Report for CI Build 41227775760Warning No base build found for commit Coverage: 76.759%Details
Uncovered ChangesNo uncovered changes found. Coverage RegressionsRequires a base build to compare against. How to fix this → Coverage Stats
💛 - Coveralls |
BSD wc pads its count with leading spaces, so `case "$attempt"` matched no branch on macOS and the stub always answered success — which made the retry tests fail on a laptop and pass in CI. The arithmetic expansion strips it; demonstrated with a padded count, where the plain string misses "3" and the normalised one matches.
On a host without coreutils the script warns about the missing timeout on every run, so a blanket 'no ::warning::' assertion failed there while passing in CI, and the exhausted-retry count saw one extra line. The assertions now name the retry warning. Proved by running the suite with a PATH that has no timeout or gtimeout: 9 passing with the timeout case skipped, where before the same environment failed three tests.
This was referenced Oct 31, 2026
cameri
added a commit
that referenced
this pull request
Oct 31, 2026
## What `Dockerfile` pinned `node:24.22.1-alpine` by digest in #1005, but four other files that name a node base image still said `node:24-alpine`: | file | service / stage | |---|---| | `Dockerfile.railwayapp` | Railway build stage + runtime stage | | `Dockerfile.test` | the CI test image | | `docker-compose.yml` | `nostream-migrate` | | `test/integration/docker-compose.yml` | `migrations-test` | All five now carry the same tag and digest. ## Why it matters A floating tag means the relay image, the CI image, the Railway image and both ephemeral migration containers can each start from a different base, and a release can run different node code than the one before it without a commit saying so. ## Guard `test/unit/ci/node-base-pin.spec.ts` collects every `FROM node:…` and `image: node:…` in the repo and asserts two things: each carries a digest, and all of them are the *same* reference. That second check is what #1005 lacked — it pinned `Dockerfile` and left the rest behind, and nothing noticed. It also asserts the scan found the expected rows, so a glob that silently matches nothing cannot make the suite green. ## Verification - new spec: 3 passing - reverting `Dockerfile.test` to `node:24-alpine`: fails on both counts (digest and single reference) - `test/unit/ci/**`: 26 passing
avantikonde
pushed a commit
to avantikonde/nostream
that referenced
this pull request
Nov 2, 2026
Dockerfile pinned node:24.22.1-alpine by digest, but Dockerfile.railwayapp, Dockerfile.test and the two compose files still said node:24-alpine, so the relay image, the CI image, the Railway image and both migration containers could each start from a different base - and a floating tag moves under a release, so the same commit built twice could run different node code. All five now carry the same tag and digest, and a unit test holds them together: it fails if any reference loses its digest, and it fails if two different references appear, which is how cameri#1005 left these behind. Verified: the new spec passes; reverting Dockerfile.test to node:24-alpine fails it on both counts; the rest of test/unit/ci is 26 passing.
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
The website deploy step now goes through
.github/scripts/deploy-with-retry.shin bothdeploy-website-preview.ymlanddeploy-website.yml. The build is untouched; only thewrangler deploycall is wrapped.Run 40103130697 failed at
Deploy the preview workerwith:The build had already succeeded, the run one minute earlier on the same branch passed, and the failing commit only deleted five lines from
Peering.astro. One transient failure in roughly five runs is enough to make the job lie about the change it is testing.Behaviour
DEPLOY_ATTEMPT_TIMEOUT) plustimeout-minutes: 12on the step, so a stalled upload cannot hold the job.code: 10013, an HTTP 5xx, a connection reset, and an expired attempt timeout. An authentication failure, a missing environment or a wrangler validation error fails on the first attempt and is reported as::error::.::warning::naming the attempt and the reason, so the flake rate stays visible in the Actions UI rather than disappearing behind a green job.The command is passed in as arguments, so each workflow keeps its own invocation —
npx wranglerfor the preview,pnpm exec wrangler … --var CANONICAL_HOST:"$CANONICAL_HOST"for production — and its quoting.Tests
test/unit/scripts/deploy-with-retry.spec.tsruns the real script against a stubwrangleronPATHthat records every invocation:The specs are not vacuous: disabling the retry makes five of them fail.
.github/scripts/**is added to the preview workflow's path filter and to the change-detection list inchecks.yml, so a change to the script runs the suite that asserts on it.Closes #970.
Notes for review
release.ymland the CDN purge step are deliberately untouched — both are called out as out of scope in the issue.