Bound and classify GitHub API retries (#207) · github/gh-actions-lock@2173763 · GitHub
Skip to content

Commit 2173763

Browse files
Bound and classify GitHub API retries (#207)
* Bound and classify GitHub API retries The retry transport is now the only retry layer, with a bounded cost: - Real timeouts: 10s dial, 10s TLS handshake, 30s response headers, 60s per request. A stalled connection no longer hangs the run. - At most 3 attempts per request, full-jitter backoff (1s base, 8s cap), and a 60s retry-wait budget shared by the whole run. - Retry-After (seconds or HTTP date) and X-RateLimit-Reset are honored only when the wait fits the remaining budget; otherwise the caller gets the rate-limit answer at once. - A per-host circuit breaker: after 3 requests in a row exhaust their retries, later requests to that host fail at once. - Untrusted TLS certificates and 401/403/404/409/422 are answers, not outages, and are never retried. - Batches are split only on query-level GraphQL errors. An outage or rate limit rejected every half alike, so a 50-pin batch used to fan out into 99 retried requests. - The anonymous SSO-fallback client shares the same transport and budget. Requests that end without an answer return UnknownOutcomeError, and IsTransient follows the transport's rules. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> * Retry GitHub API failures patiently, as a CLI can A CLI run can wait out a short outage or rate limit, so retry like one: - Up to 8 attempts per request, with exponential full-jitter backoff capped at 30s per wait. - Keep retrying until GitHub has given no answer for 5 minutes; any answer resets that window. Retry-After and rate-limit resets are waited out when they fall inside it, with a "waiting until HH:MM UTC" line. - Drop the per-host circuit breaker and the 60s run budget. - Show each wait on stderr, throttled to one line per cause, e.g. "GitHub API unavailable (503 Service Unavailable); retrying in 20s, attempt 3/8", so a long wait doesn't look hung. Timeouts, the single retry layer, no batch splitting on outages, and the answer/outage taxonomy are unchanged. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> * Retry patiently in CI, fail fast in a terminal In CI (CI=true or GITHUB_ACTIONS=true) keep the patient policy: 8 attempts, backoff up to 30s, a 5-minute no-answer window, and rate-limit waits inside it. Elsewhere, retry a blip for a few seconds (3 attempts, backoff up to 2s, a 10s window) and report a longer rate limit's reset time instead of waiting for it. runPolicy is the one place that decides. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> * Honor Retry-After on server errors Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> * Retry GraphQL rate-limit rejections GitHub rejects a GraphQL query over its rate limit with HTTP 200 and a RATE_LIMITED error. Treat that as a rate limit in the transport, under the same CI and terminal policies, rather than as an answer. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
1 parent 1370c18 commit 2173763

14 files changed

Lines changed: 981 additions & 342 deletions

‎cmd/gh-actions-lock/command_test.go‎

Lines changed: 9 additions & 1 deletion

‎cmd/gh-actions-lock/parity_test.go‎

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,11 +6,13 @@ import (
66
"encoding/json"
77
"fmt"
88
"io"
9+
"net"
910
"net/http"
1011
"os"
1112
"path/filepath"
1213
"strings"
1314
"sync/atomic"
15+
"syscall"
1416
"testing"
1517

1618
parserlock "github.com/github/actions-lockfile/go/pkg/lockfile"
@@ -818,6 +820,42 @@ jobs:
818820
assert.Contains(t, stderr, "could not verify locked actions/checkout")
819821
}
820822

823+
// TestParity_UnreachableAPIBlocks is #199's repro: with the API unreachable
824+
// (connection refused on every request), --verify must not report the
825+
// lockfile valid.
826+
func TestParity_UnreachableAPIBlocks(t *testing.T) {
827+
path := writeTempWorkflow(t, `
828+
name: ci
829+
on: push
830+
jobs:
831+
test:
832+
runs-on: ubuntu-latest
833+
steps:
834+
- uses: actions/checkout@v4
835+
`, "actions/checkout@v4=sha1-"+parityTestSHA)
836+
837+
stdout, stderr, err := runCommandWithHTTP(t, unreachable{}, "--verify", "--json=valid,findings", path)
838+
require.ErrorIs(t, err, errSilent, stderr)
839+
var p parityPayload
840+
require.NoError(t, json.Unmarshal([]byte(stdout), &p), stdout+stderr)
841+
assert.False(t, p.Valid)
842+
require.NotEmpty(t, p.Findings)
843+
for _, f := range p.Findings {
844+
assert.Equal(t, "reachability-unknown", f.Category)
845+
assert.Equal(t, "error", f.Severity)
846+
assert.Contains(t, f.Detail, "connection refused")
847+
}
848+
}
849+
850+
// unreachable fails every request the way a refused connection does.
851+
type unreachable struct{}
852+
853+
func (unreachable) raw() {}
854+
855+
func (unreachable) RoundTrip(*http.Request) (*http.Response, error) {
856+
return nil, &net.OpError{Op: "dial", Net: "tcp", Err: syscall.ECONNREFUSED}
857+
}
858+
821859
// TestParity_DependabotProxyKeepsRenamedName: Dependabot reads back only
822860
// actions.lock, so under the proxy a same-ID rename must not rewrite
823861
// `uses:` or re-key the pin; the warning still reaches --json.

‎cmd/gh-actions-lock/run.go‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ import (
1919
parserlock "github.com/github/actions-lockfile/go/pkg/lockfile"
2020
"github.com/github/gh-actions-lock/cmd/gh-actions-lock/format"
2121
"github.com/github/gh-actions-lock/internal/dep"
22+
"github.com/github/gh-actions-lock/internal/ghapi"
2223
"github.com/github/gh-actions-lock/internal/lockfile"
2324
"github.com/github/gh-actions-lock/internal/pin"
2425
"github.com/github/gh-actions-lock/internal/pinpool"
@@ -125,6 +126,8 @@ func runCheck(cmd *cobra.Command, opts *checkOptions, newResolver resolverFunc)
125126
errOut := cmd.ErrOrStderr()
126127
console := ui.NewWithWriter(errOut)
127128
defer console.StopProgress()
129+
ghapi.SetRetryNotifier(func(msg string) { console.TermNotice("%s", msg) })
130+
defer ghapi.SetRetryNotifier(nil)
128131

129132
// Expand verify mode flags (see verify.go for both modes).
130133
applyVerifyFlags(opts)

‎internal/ghapi/client.go‎

Lines changed: 10 additions & 146 deletions

0 commit comments

Comments
 (0)