diff --git a/.github/workflows/split.yml b/.github/workflows/split.yml index ffbcdef9..54cd12ee 100644 --- a/.github/workflows/split.yml +++ b/.github/workflows/split.yml @@ -90,6 +90,57 @@ jobs: PACKAGIST_TOKEN: ${{ secrets.PACKAGIST_TOKEN }} run: | PKG_URL="https://github.com/${SPLIT_ORG}/marko-${{ matrix.package }}" + RESP_FILE=/tmp/packagist_resp + + # Every package in the matrix reaches this step within the same second, + # so a release fires ~92 update-package calls at Packagist at once and + # reliably trips its limits. On 0.8.5 that returned HTTP 500 for eight + # packages; the tags had already pushed, so the job failed loudly while + # the only real damage was a stale Packagist index — including + # marko/core, which everything else resolves against. + # + # Retry the transient classes and let the definitive ones fall through + # to the caller: 404 means "not registered" and drives the create-package + # self-heal below, and 401/403 mean a bad token, which no retry fixes. + # HTTP is global rather than echoed so the log lines below can't be + # captured as part of the status code. + HTTP="" + update_with_retry() { + local attempt delay + for attempt in 1 2 3 4 5; do + # curl leaves the response file untouched when it cannot connect, + # so clear it first — otherwise a transport failure reports the + # previous attempt's body as if it were this one's. + : > "$RESP_FILE" + + # `|| true` keeps a transport failure from aborting the step under + # `bash -e` so it can be retried; the status is validated below. + HTTP=$(curl -s -o "$RESP_FILE" -w "%{http_code}" \ + --connect-timeout 10 --max-time 60 \ + -X POST https://packagist.org/api/update-package \ + -H "Content-Type: application/json" \ + -H "Authorization: Bearer markshust:${PACKAGIST_TOKEN}" \ + -d "{\"repository\":\"${PKG_URL}\"}") || true + + case "$HTTP" in + 000|429|5??) ;; # transient — worth another attempt + *) return 0 ;; # definitive — caller decides what it means + esac + + echo "Packagist update returned HTTP ${HTTP} (attempt ${attempt}/5)" + head -c 500 "$RESP_FILE" 2>/dev/null || true + echo + + # Jittered backoff. Without the jitter every job would have failed + # on the same second and would retry on the same second too. + if [[ "$attempt" -lt 5 ]]; then + delay=$(( 2 ** attempt + RANDOM % 5 )) + echo "Retrying in ${delay}s..." + sleep "$delay" + fi + done + return 0 + } # Try to update first. If Packagist returns 404 the package isn't # registered yet — self-heal by calling create-package, then retry @@ -97,11 +148,7 @@ jobs: # the manual `register-packagist.sh` step from the new-package # workflow: contributors can add `packages/foo/` via PR and the # first release tag will register + publish it automatically. - HTTP=$(curl -s -o /tmp/packagist_resp -w "%{http_code}" \ - -X POST https://packagist.org/api/update-package \ - -H "Content-Type: application/json" \ - -H "Authorization: Bearer markshust:${PACKAGIST_TOKEN}" \ - -d "{\"repository\":\"${PKG_URL}\"}") + update_with_retry if [[ "$HTTP" == "404" ]]; then echo "marko/${{ matrix.package }} not registered on Packagist — registering now..." @@ -110,12 +157,22 @@ jobs: -H "Content-Type: application/json" \ -d "{\"repository\":{\"url\":\"${PKG_URL}\"}}" echo "Registered. Re-running update so the tag gets indexed..." - curl -sf -X POST https://packagist.org/api/update-package \ - -H "Content-Type: application/json" \ - -H "Authorization: Bearer markshust:${PACKAGIST_TOKEN}" \ - -d "{\"repository\":\"${PKG_URL}\"}" - elif [[ "$HTTP" -ge 400 ]]; then - echo "Packagist update failed with HTTP ${HTTP}:" - cat /tmp/packagist_resp - exit 1 + update_with_retry fi + + # Match on the success class rather than testing `-ge 400`. The curl + # above no longer aborts the step on a transport failure, so that case + # now reaches here as 000 — numerically zero, and therefore something + # a `-ge 400` test would wave through as success, leaving the package + # stale behind a green job. + case "$HTTP" in + 2??) + echo "marko/${{ matrix.package }} updated on Packagist (HTTP ${HTTP})" + ;; + *) + echo "Packagist update failed with HTTP ${HTTP} after 5 attempts:" + cat "$RESP_FILE" 2>/dev/null || true + echo + exit 1 + ;; + esac diff --git a/tests/SplitWorkflowTest.php b/tests/SplitWorkflowTest.php index 74746116..cc2b75a7 100644 --- a/tests/SplitWorkflowTest.php +++ b/tests/SplitWorkflowTest.php @@ -43,3 +43,40 @@ it('configures the target organization as an environment variable for easy changes', function () use ($workflowContent): void { expect($workflowContent)->toContain('SPLIT_ORG: marko-php'); }); + +it('retries the Packagist update on transient upstream failures', function () use ($workflowContent): void { + // All 92 split jobs POST to Packagist within the same second, so a release + // reliably trips its rate limits. 0.8.5 lost 8 packages to a burst of 500s + // that the single un-retried curl surfaced as a hard job failure. + expect($workflowContent)->toContain('update_with_retry'); +}); + +it('treats 5xx, 429, and curl transport errors as retryable', function () use ($workflowContent): void { + expect($workflowContent)->toContain('000|429|5??)'); +}); + +it('backs off between Packagist retries with jitter so they do not re-collide', function () use ($workflowContent): void { + // Every job fails at the same instant, so a fixed backoff would just line + // the retries up again on the same second. + expect($workflowContent)->toContain('RANDOM') + ->toContain('sleep "$delay"'); +}); + +it('still self-heals an unregistered package via create-package on 404', function () use ($workflowContent): void { + expect($workflowContent)->toContain('api/create-package') + ->toContain('"$HTTP" == "404"'); +}); + +it('fails the job on any non-2xx Packagist response once retries are exhausted', function () use ($workflowContent): void { + // Retrying means the curl can no longer abort the step itself, so a + // transport failure arrives here as 000 — numerically zero, and waved + // through by a `-ge 400` test as if it had succeeded. + expect($workflowContent)->toContain('2??)') + ->toContain('exit 1'); +}); + +it('clears the response file between attempts', function () use ($workflowContent): void { + // curl leaves it untouched when it cannot connect, which otherwise reports + // the previous attempt's body as though it belonged to this one. + expect($workflowContent)->toContain(': > "$RESP_FILE"'); +});