fix: retry transient GitHub API errors instead of failing the dispatch

.github/workflows/test.yaml

@@ -0,0 +1,37 @@
+name: Test
+
+on:
+  pull_request:
+  push:
+    branches:
+      - main
+
+jobs:
+  shell-tests:
+    name: entrypoint.sh tests
+    runs-on: ubuntu-latest
+    permissions:
+      contents: read
+    steps:
+      - name: Checkout Code
+        uses: actions/checkout@v4
+
+      - name: Run api() retry tests (bash)
+        run: |
+          set -e
+          for t in tests/*.test.sh; do
+            echo "== $t (bash) =="
+            bash "$t"
+          done
+
+      # Exercise the real runtime shell. The Docker entrypoint runs the script
+      # with busybox `sh` in alpine:3.15.0 (see Dockerfile), which relies on ash
+      # extensions the ubuntu runner's /bin/sh (dash) does not support, so run
+      # the suite inside that image rather than under the host shell.
+      - name: Run api() retry tests (busybox sh, alpine:3.15.0)
+        run: |
+          docker run --rm -v "$PWD":/work -w /work \
+            public.ecr.aws/docker/library/alpine:3.15.0 \
+            sh -c 'apk add --no-cache bash curl jq coreutils >/dev/null && \
+                   set -e && \
+                   for t in tests/*.test.sh; do echo "== $t (busybox) =="; sh "$t"; done'

entrypoint.sh

@@ -82,31 +82,73 @@ validate_args() {
   fi
 }
 
-lets_wait() {
-  echo "Sleeping for ${wait_interval} seconds"
-  sleep "$wait_interval"
-}
-
+# Perform a GitHub REST API call, retrying transient failures.
+#
+# GitHub occasionally returns transient errors on these endpoints - most notably
+# the workflow-dispatch trigger, which intermittently 500s with the body
+# {"message":"Failed to run workflow dispatch","status":"500"}. Network errors
+# and HTTP 429/5xx are retried with capped exponential backoff. Other 4xx
+# responses are client errors and fail fast, and once retries are exhausted the
+# failure is still surfaced to the caller so a genuine outage is never masked.
+#
+# Tunable via environment (defaults in parentheses):
+#   API_MAX_ATTEMPTS        total attempts before giving up   (5)
+#   API_RETRY_BASE_SECONDS  first backoff delay; doubles each retry (3)
+#   API_RETRY_MAX_SECONDS   cap on any single backoff delay   (30)
 api() {
-  path=$1; shift
-  if response=$(curl --fail-with-body -sSL \
-      "${GITHUB_API_URL}/repos/${INPUT_OWNER}/${INPUT_REPO}/actions/$path" \
-      -H "Authorization: Bearer ${INPUT_GITHUB_TOKEN}" \
-      -H 'Accept: application/vnd.github.v3+json' \
-      -H 'Content-Type: application/json' \
-      "$@")
-  then
-    echo "$response"
-  else
+  local path=$1; shift
+  local max_attempts=${API_MAX_ATTEMPTS:-5}
+  local delay=${API_RETRY_BASE_SECONDS:-3}
+  local max_delay=${API_RETRY_MAX_SECONDS:-30}
+  local attempt=1
+  local body_file http_code response
+
+  while true; do
+    body_file=$(mktemp)
+    # No --fail-with-body: capture the status code via -w and branch on it,
+    # so we can tell a retryable 5xx apart from a fatal 4xx. A transport-level
+    # failure (curl exits non-zero, prints nothing) is mapped to code 000.
+    http_code=$(curl -sSL \
+        -o "$body_file" \
+        -w '%{http_code}' \
+        "${GITHUB_API_URL}/repos/${INPUT_OWNER}/${INPUT_REPO}/actions/$path" \
+        -H "Authorization: Bearer ${INPUT_GITHUB_TOKEN}" \
+        -H 'Accept: application/vnd.github.v3+json' \
+        -H 'Content-Type: application/json' \
+        "$@") || http_code=000
+    response=$(cat "$body_file")
+    rm -f "$body_file"
+
+    # Success (2xx): return the response body.
+    if [ "$http_code" -ge 200 ] && [ "$http_code" -lt 300 ]; then
+      echo "$response"
+      return 0
+    fi
+
+    # Retry transport failures (000), rate limiting (429) and server errors (5xx).
+    # Retrying the non-idempotent dispatch POST assumes the failure was
+    # pre-creation - true for the observed "Failed to run workflow dispatch" 500.
+    # Even if a run were somehow created, trigger_workflow keys off newly-appeared
+    # runs and an ArgoCD sync is idempotent, so a duplicate is harmless.
+    if [ "$http_code" = "000" ] || [ "$http_code" = "429" ] || [ "$http_code" -ge 500 ]; then
+      if [ "$attempt" -lt "$max_attempts" ]; then
+        echo >&2 "api transient failure (HTTP ${http_code}) on ${path}; attempt ${attempt}/${max_attempts}, retrying in ${delay}s"
+        [ -n "$response" ] && echo >&2 "response: $response"
+        sleep "$delay"
+        attempt=$((attempt + 1))
+        delay=$((delay * 2))
+        [ "$delay" -gt "$max_delay" ] && delay=$max_delay
+        continue
+      fi
+    fi
+
+    # Non-retryable error, or retries exhausted: surface the failure.
     echo >&2 "api failed:"
     echo >&2 "path: $path"
+    echo >&2 "http_code: $http_code"
     echo >&2 "response: $response"
-    if [[ "$response" == *'"Server Error"'* ]]; then
-      echo >&2 "Server error - trying again"
-    else
-      exit 1
-    fi
-  fi
+    exit 1
+  done
 }
 
 lets_wait() {
@@ -242,4 +284,9 @@ main() {
   fi
 }
 
-main
+# Allow the script to be sourced by the test harness without executing main.
+# TWAW_SOURCE_ONLY is never set in production (the Docker entrypoint runs the
+# script directly), so the default behaviour is unchanged.
+if [ "${TWAW_SOURCE_ONLY:-}" != "1" ]; then
+  main
+fi

tests/api_retry.test.sh

@@ -0,0 +1,107 @@
+#!/usr/bin/env bash
+# Tests for the transient-error retry behaviour of entrypoint.sh's api() helper.
+# Runs the real api() against a scripted `curl` stub (tests/stub/curl).
+set -u
+
+HERE=$(cd "$(dirname "$0")" && pwd)
+ROOT=$(cd "$HERE/.." && pwd)
+
+# Put the curl stub first on PATH; every other tool (jq, sed, mktemp) stays real.
+export PATH="$HERE/stub:$PATH"
+
+export CURL_STUB_COUNTER
+CURL_STUB_COUNTER=$(mktemp)
+export CURL_STUB_SCENARIO
+CURL_STUB_SCENARIO=$(mktemp)
+trap 'rm -f "$CURL_STUB_COUNTER" "$CURL_STUB_SCENARIO"' EXIT
+
+# Values api() reads to build the request URL (contents irrelevant to the stub).
+export INPUT_OWNER=cloudbeds INPUT_REPO=argocd-mfd INPUT_GITHUB_TOKEN=x
+# Retry knobs: keep attempts small and eliminate real sleeping so tests are fast.
+export API_MAX_ATTEMPTS=4
+export API_RETRY_BASE_SECONDS=0
+
+# Source the script for its functions only; do not run main.
+# shellcheck disable=SC1090
+TWAW_SOURCE_ONLY=1 . "$ROOT/entrypoint.sh"
+set +e  # entrypoint.sh enables `set -e`; disable it so the harness controls flow.
+
+fail=0
+pass() { printf 'ok   - %s\n' "$1"; }
+die()  { printf 'FAIL - %s\n' "$1"; fail=1; }
+
+# scenario "<printf-escaped lines>" resets the counter and writes the response script.
+scenario() { : > "$CURL_STUB_COUNTER"; printf '%b' "$1" > "$CURL_STUB_SCENARIO"; }
+attempts() { cat "$CURL_STUB_COUNTER"; }
+
+S500='500\t{"message":"Failed to run workflow dispatch","status":"500"}'
+
+# --- Test 1: transient 500s then success --------------------------------------
+# GitHub's dispatch endpoint intermittently returns a 500 with this exact body
+# (no "Server Error" string). api() must retry and ultimately succeed.
+scenario "$S500\n$S500\n204\t\n"
+# api() calls `exit` on hard failure, so always invoke it in a subshell.
+out=$(api "workflows/update-application.yaml/dispatches" --data '{}' 2>/dev/null); rc=$?
+if [ "$rc" -eq 0 ] && [ "$(attempts)" -eq 3 ]; then
+  pass "retries transient 500 then succeeds (3 attempts)"
+else
+  die "retries transient 500 then succeeds: rc=$rc attempts=$(attempts) (want rc=0 attempts=3)"
+fi
+
+# --- Test 2: genuine 4xx fails fast, no retry ---------------------------------
+scenario '404\t{"message":"Not Found"}\n'
+out=$(api "workflows/missing.yaml/dispatches" --data '{}' 2>/dev/null); rc=$?
+if [ "$rc" -ne 0 ] && [ "$(attempts)" -eq 1 ]; then
+  pass "does not retry a 4xx (fails fast in 1 attempt)"
+else
+  die "does not retry a 4xx: rc=$rc attempts=$(attempts) (want rc!=0 attempts=1)"
+fi
+
+# --- Test 3: persistent 500 exhausts retries then fails ------------------------
+scenario "$S500\n$S500\n$S500\n$S500\n$S500\n"
+out=$(api "workflows/update-application.yaml/dispatches" --data '{}' 2>/dev/null); rc=$?
+if [ "$rc" -ne 0 ] && [ "$(attempts)" -eq "$API_MAX_ATTEMPTS" ]; then
+  pass "gives up after API_MAX_ATTEMPTS on persistent 500 (surfaces failure)"
+else
+  die "persistent 500 exhausts retries: rc=$rc attempts=$(attempts) (want rc!=0 attempts=$API_MAX_ATTEMPTS)"
+fi
+
+# --- Test 4: transport error (no HTTP response) is retried --------------------
+scenario 'NETERR\n200\t{"ok":true}\n'
+if out=$(api "runs/123" 2>/dev/null); then rc=0; else rc=$?; fi
+if [ "$rc" -eq 0 ] && [ "$(attempts)" -eq 2 ] && printf '%s' "$out" | grep -q '"ok":true'; then
+  pass "retries a transport failure then succeeds (2 attempts)"
+else
+  die "retries a transport failure: rc=$rc attempts=$(attempts) out=$out"
+fi
+
+# --- Test 5: a 429 (rate limit) is retried like a 5xx -------------------------
+scenario '429\t{"message":"API rate limit exceeded"}\n204\t\n'
+out=$(api "workflows/update-application.yaml/dispatches" --data '{}' 2>/dev/null); rc=$?
+if [ "$rc" -eq 0 ] && [ "$(attempts)" -eq 2 ]; then
+  pass "retries a 429 then succeeds (2 attempts)"
+else
+  die "retries a 429: rc=$rc attempts=$(attempts) (want rc=0 attempts=2)"
+fi
+
+# --- Test 6: backoff doubles each retry and caps at API_RETRY_MAX_SECONDS ------
+# The sleep stub records each requested delay (via SLEEP_LOG) instead of really
+# sleeping, so the backoff sequence can be asserted without waiting. With base 3
+# and cap 10 over 5 persistent failures, the delays are 3, 6, 10 (12 capped), 10.
+SLEEP_LOG=$(mktemp)
+scenario "$S500\n$S500\n$S500\n$S500\n$S500\n"
+out=$(
+  export SLEEP_LOG API_RETRY_BASE_SECONDS=3 API_RETRY_MAX_SECONDS=10 API_MAX_ATTEMPTS=5
+  api "workflows/update-application.yaml/dispatches" --data '{}' 2>/dev/null
+); rc=$?
+seq=$(tr '\n' ' ' < "$SLEEP_LOG" | sed 's/ *$//')
+rm -f "$SLEEP_LOG"
+if [ "$rc" -ne 0 ] && [ "$(attempts)" -eq 5 ] && [ "$seq" = "3 6 10 10" ]; then
+  pass "backoff doubles then caps at API_RETRY_MAX_SECONDS (delays: 3 6 10 10)"
+else
+  die "backoff sequence: rc=$rc attempts=$(attempts) delays=[$seq] (want attempts=5 delays='3 6 10 10')"
+fi
+
+echo "----"
+if [ "$fail" -eq 0 ]; then echo "ALL PASS"; else echo "SOME FAILED"; fi
+exit "$fail"

tests/api_retry_seterr.test.sh

@@ -0,0 +1,39 @@
+#!/bin/sh
+# Production runs entrypoint.sh with `set -e` (see the top of the script). The
+# retry loop uses `A && B` lists and arithmetic that can trip `set -e` if written
+# carelessly, so this test exercises the multi-retry path with `set -e` left ON
+# and asserts the loop runs to completion instead of aborting early.
+set -eu
+
+HERE=$(cd "$(dirname "$0")" && pwd)
+ROOT=$(cd "$HERE/.." && pwd)
+export PATH="$HERE/stub:$PATH"
+
+CURL_STUB_COUNTER=$(mktemp); export CURL_STUB_COUNTER
+CURL_STUB_SCENARIO=$(mktemp); export CURL_STUB_SCENARIO
+trap 'rm -f "$CURL_STUB_COUNTER" "$CURL_STUB_SCENARIO"' EXIT
+export INPUT_OWNER=cloudbeds INPUT_REPO=argocd-mfd INPUT_GITHUB_TOKEN=x
+export API_MAX_ATTEMPTS=4 API_RETRY_BASE_SECONDS=0
+
+# Sourcing re-enables `set -e`; deliberately do NOT disable it afterwards.
+TWAW_SOURCE_ONLY=1 . "$ROOT/entrypoint.sh"
+
+S500='500\t{"message":"Failed to run workflow dispatch","status":"500"}'
+: > "$CURL_STUB_COUNTER"
+printf '%b' "$S500\n$S500\n204\t\n" > "$CURL_STUB_SCENARIO"
+
+# api() calls exit on hard failure and inherits `set -e`; run it in a subshell
+# inside an `if` so neither its exit nor a non-zero status aborts this harness.
+if ( api "workflows/update-application.yaml/dispatches" --data '{}' >/dev/null 2>&1 ); then
+  rc=0
+else
+  rc=$?
+fi
+n=$(cat "$CURL_STUB_COUNTER")
+
+if [ "$rc" -eq 0 ] && [ "$n" -eq 3 ]; then
+  echo "ok   - retry loop completes cleanly under set -e (3 attempts)"
+  exit 0
+fi
+echo "FAIL - set -e retry path: rc=$rc attempts=$n (want rc=0 attempts=3)"
+exit 1

tests/stub/curl

@@ -0,0 +1,58 @@
+#!/usr/bin/env bash
+# Faithful-enough `curl` stub for exercising entrypoint.sh's api() function.
+#
+# It understands only the flags api() actually uses (--fail-with-body, -o <file>,
+# -w <fmt>) and ignores everything else (-H, -sSL, --data, the URL). Behaviour is
+# driven by two files so a single test can script a sequence of responses:
+#
+#   $CURL_STUB_SCENARIO : one line per attempt. Either "<CODE><TAB><BODY>" or the
+#                         literal token NETERR to simulate a transport failure.
+#   $CURL_STUB_COUNTER  : running attempt counter (created on first call).
+#
+# It emulates real curl closely enough that both the old (--fail-with-body, body
+# on stdout) and the new (-o file + -w '%{http_code}') implementations work.
+set -u
+
+fail_with_body=0
+out_file=""
+w_fmt=""
+
+while [ $# -gt 0 ]; do
+  case "$1" in
+    --fail-with-body) fail_with_body=1 ;;
+    -o) shift; out_file="$1" ;;
+    -w) shift; w_fmt="$1" ;;
+  esac
+  shift
+done
+
+n=$(cat "$CURL_STUB_COUNTER" 2>/dev/null || echo 0)
+n=$((n + 1))
+echo "$n" > "$CURL_STUB_COUNTER"
+
+line=$(sed -n "${n}p" "$CURL_STUB_SCENARIO")
+if [ "$line" = "NETERR" ] || [ -z "$line" ]; then
+  # Transport-level failure (connection reset, DNS, timeout): no HTTP response.
+  exit 7
+fi
+
+code=$(printf '%s' "$line" | cut -f1)
+body=$(printf '%s' "$line" | cut -f2-)
+
+# Body goes to the -o file if requested, otherwise to stdout (like real curl).
+if [ -n "$out_file" ]; then
+  printf '%s' "$body" > "$out_file"
+else
+  printf '%s' "$body"
+fi
+
+# The -w template is written to stdout with %{http_code} substituted.
+if [ -n "$w_fmt" ]; then
+  printf '%s' "${w_fmt//%\{http_code\}/$code}"
+fi
+
+# --fail-with-body makes real curl exit 22 on HTTP >= 400 (still emitting body).
+if [ "$fail_with_body" -eq 1 ] && [ "$code" -ge 400 ]; then
+  exit 22
+fi
+exit 0

tests/stub/sleep

@@ -0,0 +1,6 @@
+#!/bin/sh
+# Test stub for `sleep`: records the requested duration (when SLEEP_LOG is set)
+# and returns immediately instead of actually sleeping, so backoff timing can be
+# asserted without real delays. When SLEEP_LOG is unset it is a silent no-op.
+[ -n "${SLEEP_LOG:-}" ] && printf '%s\n' "$1" >> "$SLEEP_LOG"
+exit 0