fix: retry the packagist update on transient upstream failures - #148
Open
markshust wants to merge 1 commit into
Open
fix: retry the packagist update on transient upstream failures#148markshust wants to merge 1 commit into
markshust wants to merge 1 commit into
Conversation
Every package in the split matrix reaches the notify 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 eight of them came back HTTP 500 at the same instant. The single un-retried curl turned that into eight failed jobs, and because the failure landed after "Split and push", the tags were already correct — the only real damage was a stale Packagist index, marko/core among it, which every other package resolves against. Re-running the failed jobs was enough to fix it, which is the definition of a retry we should not be doing by hand. Wrap the update call in five attempts with jittered exponential backoff. Only the transient classes retry (5xx, 429, and curl's 000 transport failure); 404 still falls through to the create-package self-heal, and 401/403 fail immediately since no retry fixes a bad token. The jitter matters as much as the backoff: all 92 jobs fail on the same second, so a fixed delay would just line their retries up on the same second too. Retrying means the curl can no longer be allowed to abort the step, so the final status is now matched against the 2xx class rather than tested with `-ge 400` — a transport failure arrives as 000, which is numerically zero and would otherwise be waved through as success, leaving the package stale behind a green job. Verified behaviourally against a stub Packagist rather than only by reading: recovery after two 500s (the 0.8.5 case), failure once five attempts are exhausted, 429 retry, the 404 registration path, fail-fast on 401, and connection-refused exiting non-zero. That harness also turned up a smaller wart now fixed here — curl leaves the response file untouched when it cannot connect, so a failed attempt printed the previous attempt's body as its own. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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.
Problem
Every package in the split matrix reaches the
Notify Packagiststep within the same second, so a release fires ~92update-packagecalls at Packagist at once and reliably trips its limits.On 0.8.5 eight of them came back
HTTP 500at the same instant (run 30208431432). The single un-retriedcurlturned that into eight failed jobs —core,health,lsp,hashing,http,notification,page-cache,mail-smtp.Because the failure landed after
Split and push, the tags were already correct in every split repo. The only real damage was a stale Packagist index — includingmarko/core, which every other package resolves against. Re-running the failed jobs was enough to fix it, which is the definition of a retry we should not be doing by hand.Change
Wrap the update call in five attempts with jittered exponential backoff:
5xx,429, and curl's000transport failure.404still falls through to the existingcreate-packageself-heal;401/403fail immediately, since no retry fixes a bad token.The jitter matters as much as the backoff — all 92 jobs fail on the same second, so a fixed delay would just line their retries up on the same second too.
Retrying means the
curlcan no longer be allowed to abort the step, so the final status is now matched against the2xxclass rather than tested with-ge 400. A transport failure arrives as000, which is numerically zero and would otherwise be waved through as success, leaving the package stale behind a green job.Verification
Tested behaviourally against a stub Packagist rather than only by reading the diff:
update:200500 500 200500 ×5429 200404 → create → 200401503 401000after 5 attemptsThat harness also turned up a smaller wart, fixed here: curl leaves the response file untouched when it cannot connect, so a failed attempt printed the previous attempt's body as its own.
tests/SplitWorkflowTest.phpgains six cases covering the retry loop, the retryable classes, the jittered backoff, the preserved 404 path, the non-2xx failure check, and the response-file reset. Fullcomposer ciis green.Note
The workflow tests assert on the YAML's structure, matching the existing convention in that file — they pin the shape of the fix but do not execute it. The behavioural table above is what actually verifies the logic, and it was run locally rather than in CI.
🤖 Generated with Claude Code