feat: add specific exceptions on provider errors and add retry#103
feat: add specific exceptions on provider errors and add retry#103levivannoort wants to merge 4 commits into
Conversation
Greptile SummaryThis PR hardens
Confidence Score: 5/5Safe to merge; the retry/timeout logic is well-structured, idempotency guards are correct, and the getLatestCommit() field relaxation handles the null-author case without dropping any core commit data. The rewritten call() method correctly sequences all checks (curl error → rate-limit → 5xx → decode → return), idempotency logic matches RFC 7231, and the Retry-After capping prevents indefinite build blocks. The getLatestCommit() change is safe because the removed fields were always null-coalesced to '' in the return statement. No files require special attention, though Gitea::getRepositoryName() and GitHub::getRepositoryName() are missing the >= 400 guard present in the sibling getRepository() methods. Important Files Changed
Reviews (4): Last reviewed commit: "feat: enhance rate limit handling for Gi..." | Re-trigger Greptile |
Summary
Hardens
Adapter::call()against the transient network and provider conditions that were surfacing as build timeouts in CLO-4353, and relaxes an overly strict response validator inGitHub::getLatestCommit()that was causing legitimate commits to fail.Motivation
Build workers were occasionally hanging for minutes per request. Two root causes:
CURLOPT_CONNECTTIMEOUTandCURLOPT_TIMEOUTwere unset, so a stalled TCP handshake against the provider could pin a worker until the kernel TCP timeout (~2 min) elapsed. Multiplied across retries, this trivially produced multi-minute hangs.Retry-Afterwithout bound — a single GitHub secondary rate-limit response (Retry-After: 3600) could block a build for an hour.A secondary issue:
GitHub::getLatestCommit()requiredauthor.avatar_urlandauthor.html_urlto be present. GitHub returnsauthor: nullwhenever the commit's author email isn't linked to a GitHub user account (deleted users, bots, outside contributors with unlinked emails), causing the entire commit fetch — and the build — to fail.Approach
src/VCS/Adapter.php— request lifecycleCURLOPT_CONNECTTIMEOUT=5,CURLOPT_TIMEOUT=15. Fail fast instead of pinning workers.GET/HEAD/PUT/DELETE/OPTIONS) via a newisIdempotent()helper.POSTandPATCHmay have already been processed server-side, so they fail fast rather than risk duplicate side effects.getRetryDelay()now multiplies the base delay (1s/2s/4s) by a random factor in[0.5, 1.5], so concurrent callers don't re-synchronize on the same retry schedule.Retry-After. New$maxRetryAfterSeconds = 300caps how long we'll honor a server-provided header. Typical values (e.g.Retry-After: 60) pass through unchanged; pathological values (e.g.Retry-After: 3600from GitHub secondary rate limits) are clamped so a build can't be blocked indefinitely.Retry-Afterparsing. NewparseRetryAfter()handles both forms allowed by RFC 7231 — delta-seconds and HTTP-date — instead of the previous bare(int)cast which silently produced0for date-formatted values.maxRetries→maxAttempts(the count is total attempts, not extra retries — the old name was misleading). Removed deadlastResponseStatus/lastResponseBody/lastResponseHeadersstate and the now-unreachable post-loop fallback.src/VCS/Adapter/Git/GitHub.php—getLatestCommit()author.avatar_urlandauthor.html_urlfrom the required-fields check. They're still returned in the response payload, defaulting to''via the existing null-coalescing in the return statement. Core commit data (sha,message, commit-level author name,html_urlon the commit) remains required.Test plan
composer test/ phpunit)GETagainst a deliberately failing endpoint retries 3× with jittered delay and surfacesProviderServerErrorPOSTagainst a 5xx response fails fast (no retry) and throwsProviderServerErrorRetry-After: 60waits ~60s;Retry-After: 3600is clamped to 300sRetry-AfterHTTP-date format (e.g.Wed, 21 Oct 2026 07:28:00 GMT) is parsed correctlygetLatestCommit()succeeds against a commit whose author email is not linked to a GitHub account (e.g. a commit authored by a deleted user or unlinked email) —commitAuthorAvatarreturns''instead of throwing🤖 Generated with Claude Code