diff --git a/.changeset/deploy-retry-transient.md b/.changeset/deploy-retry-transient.md new file mode 100644 index 000000000..0063d9ff9 --- /dev/null +++ b/.changeset/deploy-retry-transient.md @@ -0,0 +1,19 @@ +--- +"nostream": patch +--- + +ci(deploy): retry the website deploy on a transient Cloudflare failure + +Run 40103130697 failed at `Deploy the preview worker` with Cloudflare API error +`code: 10013` after a build that had already succeeded, and the next push deployed +the same commit successfully — a transient upstream failure, not a defect in the +site or the workflow. + +Both website deploys now run through `.github/scripts/deploy-with-retry.sh`, which +wraps the deploy step (not the build) in three attempts with a 10 s then 30 s +backoff, a per-attempt timeout, and a step-level `timeout-minutes`. It retries only +`code: 10013`, an HTTP 5xx, a connection reset and an expired attempt timeout; an +authentication failure, a missing environment or a wrangler validation error still +fails on the first attempt. Every retry emits a `::warning::` naming the attempt and +the reason, so the flake rate stays visible instead of disappearing behind a +passing job. diff --git a/.github/scripts/deploy-with-retry.sh b/.github/scripts/deploy-with-retry.sh new file mode 100755 index 000000000..91d9d9168 --- /dev/null +++ b/.github/scripts/deploy-with-retry.sh @@ -0,0 +1,126 @@ +#!/usr/bin/env bash +# +# Run a wrangler deploy, retrying only a transient Cloudflare failure. +# +# .github/scripts/deploy-with-retry.sh +# +# The command is taken from the caller so each workflow keeps its own +# invocation (`npx wrangler deploy --config wrangler.preview.jsonc` for a +# preview, `pnpm exec wrangler deploy --config wrangler.router.jsonc` with +# `--var` for production). +# +# Only three signals are retried, because retrying anything else triples the +# noise and delays the message that matters: +# +# - `code: 10013`, the Cloudflare API's "unknown error" seen in #970 +# - an HTTP 5xx from the Cloudflare API +# - a connection reset +# +# An expired per-attempt timeout is retried too: a hung upload is the case the +# timeout exists for, and failing on the first one would defeat it. +# +# Authentication failures, a missing environment and wrangler validation +# errors fail on the first attempt. +# +# Every attempt after the first is announced with a `::warning::` naming the +# attempt and the reason, so the flake rate stays visible in the Actions UI +# instead of disappearing behind a passing job. +# +# Environment: +# DEPLOY_ATTEMPTS total attempts (default 3) +# DEPLOY_ATTEMPT_TIMEOUT seconds allowed per attempt (default 180) +# DEPLOY_BACKOFFS seconds to wait before each retry (default "10 30") +set -uo pipefail + +attempts=${DEPLOY_ATTEMPTS:-3} +attempt_timeout=${DEPLOY_ATTEMPT_TIMEOUT:-180} +read -r -a backoffs <<< "${DEPLOY_BACKOFFS:-10 30}" + +if [ "$#" -eq 0 ]; then + echo "usage: $0 " >&2 + exit 2 +fi + +if ! [[ "$attempts" =~ ^[1-9][0-9]*$ ]]; then + echo "DEPLOY_ATTEMPTS must be a positive integer, got '$attempts'" >&2 + exit 2 +fi + +# `timeout` is coreutils: every GitHub runner has it, a stock macOS does not. +# Falling back keeps the script usable from a laptop (the unit suite runs it), +# and says so rather than silently dropping the bound. Bash 3.2 ships on macOS, +# so an empty array cannot be expanded with "${timeout_cmd[@]}" under `set -u`. +timeout_cmd=() +if command -v timeout >/dev/null 2>&1; then + timeout_cmd=(timeout --kill-after=10 "$attempt_timeout") +elif command -v gtimeout >/dev/null 2>&1; then + timeout_cmd=(gtimeout --kill-after=10 "$attempt_timeout") +else + echo "::warning::no 'timeout' or 'gtimeout' on PATH — deploy attempts are not bounded (install coreutils to restore the per-attempt limit)" +fi + +logs=$(mktemp -d) +trap 'rm -rf "$logs"' EXIT + +# Echo the transient reason found in an attempt's output, or return 1 when the +# failure is one we must not retry. +transient_reason() { + local log=$1 + local http + + if grep -qE 'code: ?10013' "$log"; then + printf 'Cloudflare API error code 10013' + return 0 + fi + http=$(grep -oE 'HTTP 5[0-9][0-9]' "$log" | head -1 || true) + if [ -n "$http" ]; then + printf '%s from the Cloudflare API' "$http" + return 0 + fi + if grep -qiE 'ECONNRESET|connection reset|socket hang up' "$log"; then + printf 'connection reset' + return 0 + fi + + return 1 +} + +for (( attempt = 1; attempt <= attempts; attempt++ )); do + log="$logs/attempt-$attempt.log" + status=0 + echo "deploy attempt $attempt/$attempts: $*" + # The whole attempt is bounded: an upload that stops making progress must not + # hold the job until the workflow's own timeout. `--kill-after` covers a + # process that ignores TERM. Without coreutils the bound is dropped and the + # warning above says so, rather than turning every deploy into exit 127. + ${timeout_cmd[@]+"${timeout_cmd[@]}"} "$@" >"$log" 2>&1 || status=$? + cat "$log" + + if [ "$status" -eq 0 ]; then + echo "deploy succeeded on attempt $attempt/$attempts" + exit 0 + fi + + reason=$(transient_reason "$log") || reason= + if [ -z "$reason" ]; then + if [ "$status" -eq 124 ] || [ "$status" -eq 137 ]; then + reason="attempt timed out after ${attempt_timeout}s" + else + echo "::error::deploy failed on attempt $attempt/$attempts with a non-transient error (exit $status) — not retrying" >&2 + tail -n 30 "$log" >&2 + exit "$status" + fi + fi + + if [ "$attempt" -ge "$attempts" ]; then + echo "::error::deploy failed after $attempts attempts — last failure: $reason" >&2 + tail -n 30 "$log" >&2 + exit "$status" + fi + + delay=${backoffs[$((attempt - 1))]:-0} + echo "::warning::wrangler deploy attempt $((attempt + 1))/$attempts after attempt $attempt/$attempts failed with: $reason — waiting ${delay}s" + if [ "$delay" -gt 0 ]; then + sleep "$delay" + fi +done diff --git a/.github/workflows/checks.yml b/.github/workflows/checks.yml index 8c1d4c523..5ef5a96c7 100644 --- a/.github/workflows/checks.yml +++ b/.github/workflows/checks.yml @@ -43,6 +43,9 @@ jobs: # must run the suite that asserts on it. #965 changed release.yml # and skipped the very tests that change broke. - '.github/workflows/**' + # test/unit/scripts/deploy-with-retry.spec.ts runs the deploy + # script, so a change to it must run the suite that asserts on it. + - '.github/scripts/**' commit-lint: name: Lint commits diff --git a/.github/workflows/deploy-website-preview.yml b/.github/workflows/deploy-website-preview.yml index 50c4b097f..5c586bd44 100644 --- a/.github/workflows/deploy-website-preview.yml +++ b/.github/workflows/deploy-website-preview.yml @@ -36,6 +36,9 @@ on: # at all: PR 885 touches none of the website paths.) - ".github/workflows/deploy-website.yml" - "website/site/worker/**" + # The deploy step above runs this script; a change to it changes how the + # preview is deployed, so the previews must be rebuilt by it. + - ".github/scripts/**" concurrency: group: website-preview-${{ github.event.pull_request.number }} @@ -131,11 +134,19 @@ jobs: - name: Deploy the preview worker if: steps.guard.outputs.skip == 'false' && github.event.action != 'closed' working-directory: website/site + # A transient Cloudflare failure must not leave the previous build being + # served while the PR reads as deployed (#970). The shared script retries + # code 10013, an HTTP 5xx and a reset connection — about a minute of + # backoff in total — and fails on the first attempt for anything else, + # including an auth failure. timeout-minutes bounds the retries plus the + # per-attempt timeout the script applies. + timeout-minutes: 12 env: CLOUDFLARE_API_TOKEN: ${{ secrets.CLOUDFLARE_API_TOKEN }} CLOUDFLARE_ACCOUNT_ID: ${{ vars.CLOUDFLARE_ACCOUNT_ID }} run: | - npx wrangler deploy \ + bash "$GITHUB_WORKSPACE/.github/scripts/deploy-with-retry.sh" \ + npx wrangler deploy \ --config wrangler.preview.jsonc \ --name nostream-web-pr-${{ github.event.pull_request.number }} diff --git a/.github/workflows/deploy-website.yml b/.github/workflows/deploy-website.yml index 573d12274..21ea84f76 100644 --- a/.github/workflows/deploy-website.yml +++ b/.github/workflows/deploy-website.yml @@ -61,6 +61,10 @@ jobs: - name: Deploy to Cloudflare Workers working-directory: website/site + # The shared script retries only a transient failure (#970) and fails on + # the first attempt for an auth or validation error; timeout-minutes + # bounds the retries plus the per-attempt timeout the script applies. + timeout-minutes: 12 env: CLOUDFLARE_API_TOKEN: ${{ secrets.CLOUDFLARE_API_TOKEN }} CLOUDFLARE_ACCOUNT_ID: ${{ vars.CLOUDFLARE_ACCOUNT_ID }} @@ -71,7 +75,9 @@ jobs: # http included, 301s to it, so www/apex cannot serve a second copy # and split search ranking. --var keeps the deployed value from # drifting away from the SITE_URL the build above baked in. - run: pnpm exec wrangler deploy --config wrangler.router.jsonc --var CANONICAL_HOST:"$CANONICAL_HOST" + run: | + bash "$GITHUB_WORKSPACE/.github/scripts/deploy-with-retry.sh" \ + pnpm exec wrangler deploy --config wrangler.router.jsonc --var CANONICAL_HOST:"$CANONICAL_HOST" - name: Purge the site's CDN cache # Same reason as the preview workflow's purge: a request that lands diff --git a/test/unit/scripts/deploy-with-retry.spec.ts b/test/unit/scripts/deploy-with-retry.spec.ts new file mode 100644 index 000000000..a106e4741 --- /dev/null +++ b/test/unit/scripts/deploy-with-retry.spec.ts @@ -0,0 +1,268 @@ +import { expect } from 'chai' +import { spawnSync } from 'child_process' +import fs from 'fs' +import os from 'os' +import path from 'path' + +const projectRoot = process.cwd() +const scriptPath = path.join(projectRoot, '.github', 'scripts', 'deploy-with-retry.sh') + +/** + * Each entry describes what the stub `wrangler` does on that attempt: print + * `stderr` and exit with `code`, or `sleep` for a number of seconds first. An + * attempt beyond the list succeeds. The stub appends its arguments to a file so + * the test can check what the caller actually invoked. + */ +type Attempt = { stderr?: string; code?: number; sleep?: number } + +const writeStub = (dir: string, attempts: Attempt[]): string => { + const binDir = path.join(dir, 'bin') + fs.mkdirSync(binDir, { recursive: true }) + const cases = attempts + .map((attempt, index) => { + const body = [ + attempt.sleep ? `sleep ${attempt.sleep}` : '', + attempt.stderr ? `printf '%s\\n' ${JSON.stringify(attempt.stderr)} >&2` : '', + // A sleep-only attempt is expected to be killed by the attempt timeout, + // so it never reaches its own exit. + `exit ${attempt.code ?? 0}`, + ] + .filter(Boolean) + .join('; ') + return ` ${index + 1}) ${body};;` + }) + .join('\n') + const stub = `#!/usr/bin/env bash +# One line per attempt, one field per argument: "$*" would join the arguments +# and hide an unquoted "$@" in the script under test. +for arg in "$@"; do printf '%s' "$arg" >> "$STUB_ARGS"; done +printf '\\n' >> "$STUB_ARGS" +# BSD wc -l pads its count with leading spaces, which would make every case +# branch miss; the arithmetic expansion normalises it to a plain integer. +attempt=$(( $(wc -l < "$STUB_ARGS") )) +case "$attempt" in +${cases} + *) echo "deployed"; exit 0;; +esac +` + + fs.writeFileSync(path.join(binDir, 'wrangler'), stub, { mode: 0o755 }) + return binDir +} + +const ARG_SEPARATOR = '' + +const parseAttempts = (recorded: string): string[][] => { + const trimmed = recorded.trim() + if (trimmed === '') { + return [] + } + return trimmed.split('\n').map((line) => { + const fields = line.split(ARG_SEPARATOR) + // Every argument is followed by the separator, so the split leaves a + // trailing empty field. + return fields[fields.length - 1] === '' ? fields.slice(0, -1) : fields + }) +} + +type Run = { + status: number | null + stdout: string + stderr: string + attempts: number + invocations: string[][] + /** Arguments of the first attempt, the one every test's expectation is about. */ + args: string[] +} + +type RunOptions = { env?: NodeJS.ProcessEnv; pathPrefixes?: string[]; systemPath?: boolean } + +const runScript = (dir: string, attempts: Attempt[], args: string[], options: RunOptions = {}): Run => { + const binDir = writeStub(dir, attempts) + const counter = path.join(dir, 'invocations.txt') + fs.writeFileSync(counter, '') + const searchPath = [ + ...(options.pathPrefixes ?? []), + binDir, + ...(options.systemPath === false ? [] : [process.env.PATH]), + ].join(':') + const result = spawnSync('bash', [scriptPath, ...args], { + cwd: projectRoot, + env: { + ...process.env, + ...options.env, + STUB_ARGS: counter, + PATH: searchPath, + // The production defaults wait 10 s then 30 s; the tests assert the + // retry happened, not how long it took. + DEPLOY_BACKOFFS: '0 0', + }, + encoding: 'utf-8', + timeout: 30_000, + }) + const invocations = parseAttempts(fs.readFileSync(counter, 'utf-8')) + + return { + status: result.status, + stdout: result.stdout ?? '', + stderr: result.stderr ?? '', + attempts: invocations.length, + invocations, + args: invocations[0] ?? [], + } +} + +const WRANGLER_ARGS = [ + 'wrangler', + 'deploy', + '--config', + 'wrangler.router.jsonc', + '--var', + 'CANONICAL_HOST:nostream.dev', +] + +const deploy = (dir: string, attempts: Attempt[], options: RunOptions = {}): Run => + runScript(dir, attempts, WRANGLER_ARGS, options) + +/** Resolve a command the way a shell would, for building a restricted PATH. */ +const resolveOnPath = (command: string): string | undefined => { + for (const entry of (process.env.PATH ?? '').split(':')) { + const candidate = path.join(entry, command) + if (fs.existsSync(candidate)) { + return candidate + } + } + return undefined +} + +// The script also warns when coreutils is absent (stock macOS), and that line is +// present on every run there. Assertions about retries must name the retry line, +// or they would fail on a laptop and pass in CI. +const RETRY_WARNING = /::warning::wrangler deploy attempt/g + +describe('deploy-with-retry.sh', () => { + let dir: string + + beforeEach(() => { + dir = fs.mkdtempSync(path.join(os.tmpdir(), 'nostream-deploy-retry-')) + }) + + afterEach(() => { + fs.rmSync(dir, { recursive: true, force: true }) + }) + + it('retries a Cloudflare 10013 failure and names the attempt and reason', () => { + const result = deploy(dir, [ + { + stderr: + 'A request to the Cloudflare API (/accounts/x/workers/scripts/y/assets-upload-session) failed.\n An unknown error has occurred. [code: 10013]', + code: 1, + }, + ]) + + expect(result.status, result.stderr).to.equal(0) + expect(result.attempts).to.equal(2) + expect(result.stdout).to.match(/::warning::wrangler deploy attempt 2\/3.*10013/) + }) + + it('does not retry an authentication failure', () => { + const result = deploy(dir, [ + { stderr: 'Authentication error [code: 10000]\n Invalid API Token (HTTP 401)', code: 1 }, + ]) + + expect(result.status).to.not.equal(0) + expect(result.attempts).to.equal(1) + expect(result.stdout).to.not.match(RETRY_WARNING) + expect(result.stderr).to.include('::error::') + }) + + it('does not retry a wrangler validation error', () => { + const result = deploy(dir, [ + { stderr: '✘ [ERROR] Missing entry-point: The entry-point file at "dist/_worker.js" was not found.', code: 1 }, + ]) + + expect(result.status).to.not.equal(0) + expect(result.attempts).to.equal(1) + expect(result.stdout).to.not.match(RETRY_WARNING) + }) + + it('retries an HTTP 5xx from the Cloudflare API', () => { + const result = deploy(dir, [{ stderr: 'A request to the Cloudflare API failed: HTTP 502', code: 1 }]) + + expect(result.status, result.stderr).to.equal(0) + expect(result.attempts).to.equal(2) + expect(result.stdout).to.match(/::warning::.*HTTP 502/) + }) + + it('retries a connection reset', () => { + const result = deploy(dir, [{ stderr: 'Error: read ECONNRESET: connection reset by peer', code: 1 }]) + + expect(result.status, result.stderr).to.equal(0) + expect(result.attempts).to.equal(2) + expect(result.stdout).to.match(/::warning::.*connection reset/) + }) + + it('gives up after the last attempt and keeps the last error on stderr', () => { + const failing: Attempt = { stderr: 'An unknown error has occurred. [code: 10013]', code: 1 } + const result = deploy(dir, [failing, failing, failing]) + + expect(result.status).to.not.equal(0) + expect(result.attempts).to.equal(3) + expect(result.stderr).to.include('::error::deploy failed after 3 attempts') + expect(result.stderr).to.include('10013') + // One warning per retry, none for the first attempt. + expect(result.stdout.match(RETRY_WARNING)).to.have.length(2) + }) + + it('passes the wrangler arguments through unchanged, one argument at a time', () => { + const result = deploy(dir, []) + + expect(result.status, result.stderr).to.equal(0) + expect(result.attempts).to.equal(1) + expect(result.args).to.deep.equal(['deploy', '--config', 'wrangler.router.jsonc', '--var', 'CANONICAL_HOST:nostream.dev']) + }) + + it('keeps an argument that contains spaces as one argument', () => { + const args = ['wrangler', 'deploy', '--var', 'SITE_TITLE:Oh My Relay'] + const result = runScript(dir, [], args) + + expect(result.status, result.stderr).to.equal(0) + // Unquoted `$@` in the script would split this into four arguments. + expect(result.args).to.deep.equal(['deploy', '--var', 'SITE_TITLE:Oh My Relay']) + }) + + it('still deploys when no timeout command is installed (stock macOS)', () => { + // A PATH with the tools the script needs and, deliberately, neither + // `timeout` nor `gtimeout`. The script must degrade to an unbounded + // attempt and say so, not exit 127 before the deploy runs. + const restricted = path.join(dir, 'restricted-bin') + fs.mkdirSync(restricted, { recursive: true }) + for (const tool of ['bash', 'cat', 'grep', 'head', 'mktemp', 'rm', 'sleep', 'tail', 'wc']) { + const resolved = resolveOnPath(tool) + if (!resolved) { + throw new Error(`${tool} not found on PATH; cannot build the restricted environment`) + } + fs.symlinkSync(resolved, path.join(restricted, tool)) + } + + const result = deploy(dir, [], { pathPrefixes: [restricted], systemPath: false }) + + expect(result.status, result.stderr).to.equal(0) + expect(result.attempts).to.equal(1) + expect(result.stdout).to.match(/::warning::no 'timeout' or 'gtimeout' on PATH/) + }) + + it('retries an attempt that exceeds the per-attempt timeout', function () { + if (!resolveOnPath('timeout') && !resolveOnPath('gtimeout')) { + // Only reached on a host without coreutils, where the script has no bound + // to exceed in the first place. + this.skip() + } + this.timeout(20_000) + const result = deploy(dir, [{ sleep: 30 }], { env: { DEPLOY_ATTEMPT_TIMEOUT: '1' } }) + + expect(result.status, result.stderr).to.equal(0) + expect(result.attempts).to.equal(2) + expect(result.stdout).to.match(/::warning::.*timed out after 1s/) + }) +})