From c25b05aea301caf351589271fc09b4b506f6a941 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juli=C3=A1n=20Gonz=C3=A1lez?= Date: Wed, 14 Oct 2026 10:25:15 -0700 Subject: [PATCH 1/2] Retry the registry record on transient failures The registry record step made one POST and failed the job on any answer other than 200 or 409. The registry validates a record by fetching each asset's signature and certificate files from the CDN, and a transient 503 there comes back as HTTP 400 with the upstream status in its message: {"code":3, "message":"asset \"windows-amd64-msi\" detached signature files are required: fetch certificate: GET .../name_v1.2.3_windows_amd64.msi.cert returned HTTP 503"} A release that was fully published then needed a manual re-run of the failed job, and a false "release failed" notification went out. The POST now retries, re-sending the same body after 10, 20, 40 and 80 s, on a transport error, a 5xx, a 429, or a 400 whose message reports an upstream 5xx. Any other 4xx is final on the first attempt. The response handling, the flags and the 30 s per-request timeout are unchanged. Follow-up outside this repository: the registry API should answer 503 for a failed validation fetch instead of 400; once it does, the body check here can go. Co-Authored-By: Claude Fable 5.1 --- cmd/record-release/main.go | 69 ++++++++++++++++++++++++++------- cmd/record-release/main_test.go | 47 ++++++++++++++++++++++ docs/release-workflow.md | 3 ++ 3 files changed, 105 insertions(+), 14 deletions(-) diff --git a/cmd/record-release/main.go b/cmd/record-release/main.go index 34ec8dd..d7c58cd 100644 --- a/cmd/record-release/main.go +++ b/cmd/record-release/main.go @@ -313,11 +313,6 @@ func main() { os.Exit(1) } endpoint := baseURL.JoinPath("/api/v1/ingest/release") - httpReq, err := http.NewRequest(http.MethodPost, endpoint.String(), bytes.NewReader(bodyBytes)) - if err != nil { - fmt.Fprintf(os.Stderr, "record-release: error: creating HTTP request: %v\n", err) - os.Exit(1) - } client := &http.Client{ Timeout: 30 * time.Second, @@ -326,16 +321,9 @@ func main() { base: http.DefaultTransport, }, } - resp, err := client.Do(httpReq) + resp, respBody, err := postWithRetry(client, endpoint.String(), bodyBytes) if err != nil { - fmt.Fprintf(os.Stderr, "record-release: error: HTTP request failed: %v\n", err) - os.Exit(1) - } - defer resp.Body.Close() - - respBody, err := io.ReadAll(resp.Body) - if err != nil { - fmt.Fprintf(os.Stderr, "record-release: error: reading response body: %v\n", err) + fmt.Fprintf(os.Stderr, "record-release: error: %v\n", err) os.Exit(1) } @@ -363,6 +351,59 @@ func main() { } } +// retryDelays are the waits between the attempts of postWithRetry; tests +// shorten them. +var retryDelays = []time.Duration{10 * time.Second, 20 * time.Second, 40 * time.Second, 80 * time.Second} + +// retryable reports whether a response is worth another attempt: a 5xx, a +// 429, or a 400 the registry raised because the CDN answered 5xx while it was +// fetching an asset's signature files -- a transient upstream failure, not a +// bad record. Any other 4xx is final. +func retryable(status int, body []byte) bool { + return status >= 500 || status == http.StatusTooManyRequests || + (status == http.StatusBadRequest && bytes.Contains(body, []byte("returned HTTP 5"))) +} + +// postWithRetry POSTs body to endpoint, re-sending it after each retryable +// failure until the delays run out. It returns the last response and its +// body, or the transport error of the last attempt. +func postWithRetry(client *http.Client, endpoint string, body []byte) (*http.Response, []byte, error) { + for attempt := 0; ; attempt++ { + resp, err := client.Do(mustRequest(endpoint, body)) + var respBody []byte + if err == nil { + respBody, err = io.ReadAll(resp.Body) + resp.Body.Close() + if err != nil { + return nil, nil, fmt.Errorf("reading response body: %w", err) + } + if !retryable(resp.StatusCode, respBody) { + return resp, respBody, nil + } + } + if attempt == len(retryDelays) { + if err != nil { + return nil, nil, fmt.Errorf("HTTP request failed after %d attempts: %w", attempt+1, err) + } + return resp, respBody, nil + } + if err != nil { + fmt.Fprintf(os.Stderr, "record-release: attempt %d failed: %v; retrying in %s\n", attempt+1, err, retryDelays[attempt]) + } else { + fmt.Fprintf(os.Stderr, "record-release: attempt %d failed: HTTP %d; retrying in %s\n", attempt+1, resp.StatusCode, retryDelays[attempt]) + } + time.Sleep(retryDelays[attempt]) + } +} + +func mustRequest(endpoint string, body []byte) *http.Request { + req, err := http.NewRequest(http.MethodPost, endpoint, bytes.NewReader(body)) + if err != nil { + panic(err) // the endpoint was parsed by url.Parse already + } + return req +} + func transformAssets(manifest *pb.Manifest) map[string]*ReleaseAsset { assets := make(map[string]*ReleaseAsset) for platform, asset := range manifest.GetAssets() { diff --git a/cmd/record-release/main_test.go b/cmd/record-release/main_test.go index b296a2b..277a266 100644 --- a/cmd/record-release/main_test.go +++ b/cmd/record-release/main_test.go @@ -3,9 +3,13 @@ package main import ( "encoding/json" "fmt" + "io" + "net/http" + "net/http/httptest" "reflect" "strings" "testing" + "time" "unicode/utf8" pb "github.com/ConductorOne/github-workflows/pb/artifacts/v1" @@ -305,3 +309,46 @@ func attestation(predicateType, bundleHref string) *pb.AttestationDescriptor { func strPtr(s string) *string { return &s } + +// A transient failure -- including the registry's 400 for a CDN 5xx -- is +// retried with the same body; a real 400 is not. +func TestPostWithRetry(t *testing.T) { + saved := retryDelays + retryDelays = []time.Duration{0, 0, 0, 0} + t.Cleanup(func() { retryDelays = saved }) + + cases := []struct { + name string + responses []int + body string + wantCalls int + wantCode int + }{ + {"503 then 200", []int{503, 200}, "", 2, 200}, + {"400 for an upstream 503 then 200", []int{400, 200}, `{"code":3,"message":"fetch certificate: GET https://cdn.example/x.cert returned HTTP 503"}`, 2, 200}, + {"plain 400 is final", []int{400}, `{"code":3,"message":"version is required"}`, 1, 400}, + {"gives up after five", []int{503, 503, 503, 503, 503}, "", 5, 503}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + calls := 0 + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if got, _ := io.ReadAll(r.Body); string(got) != "payload" { + t.Errorf("attempt %d body = %q, want payload", calls+1, got) + } + w.WriteHeader(tc.responses[calls]) + _, _ = w.Write([]byte(tc.body)) + calls++ + })) + defer srv.Close() + + resp, _, err := postWithRetry(srv.Client(), srv.URL, []byte("payload")) + if err != nil { + t.Fatalf("postWithRetry: %v", err) + } + if calls != tc.wantCalls || resp.StatusCode != tc.wantCode { + t.Fatalf("calls = %d, status = %d; want %d, %d", calls, resp.StatusCode, tc.wantCalls, tc.wantCode) + } + }) + } +} diff --git a/docs/release-workflow.md b/docs/release-workflow.md index a0b432a..c0b7329 100644 --- a/docs/release-workflow.md +++ b/docs/release-workflow.md @@ -108,6 +108,9 @@ Records release metadata in the connector registry API: folded into one line with a count before the changelog is recorded - Includes `config_schema.json` and `baton_capabilities.json` when present - Sends release timestamp, commit SHA, and workflow run metadata +- Retries the record up to five times with doubling waits (10s to 80s) on + transport errors, 5xx responses, 429, or a 400 whose message reports an + upstream 5xx from the CDN; other 4xx responses fail immediately ### verify-release From a506ced1d6cf016469c4075e4dcdfb861871ffdb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juli=C3=A1n=20Gonz=C3=A1lez?= Date: Wed, 14 Oct 2026 10:55:07 -0700 Subject: [PATCH 2/2] Retry a response body that is cut short A connection dropped after the headers arrived surfaced as a read error and ended the record attempt at once, while a drop before the headers was retried. Both are the same transient failure; the read error now falls through to the retry path, and after the last attempt is reported with the attempt count like a failed request. Tests add the transport cases: a drop before the headers then 200, a body cut after the headers then 200, and five drops in a row ending in the "after 5 attempts" error. Restoring the early return fails only the body-cut case. Co-Authored-By: Claude Fable 5.1 --- cmd/record-release/main.go | 7 ++-- cmd/record-release/main_test.go | 63 +++++++++++++++++++++++++++++++++ 2 files changed, 66 insertions(+), 4 deletions(-) diff --git a/cmd/record-release/main.go b/cmd/record-release/main.go index d7c58cd..01776ac 100644 --- a/cmd/record-release/main.go +++ b/cmd/record-release/main.go @@ -372,12 +372,11 @@ func postWithRetry(client *http.Client, endpoint string, body []byte) (*http.Res resp, err := client.Do(mustRequest(endpoint, body)) var respBody []byte if err == nil { + // A read failure is the connection dying mid-response: retried + // like a failed request, not reported as a bad answer. respBody, err = io.ReadAll(resp.Body) resp.Body.Close() - if err != nil { - return nil, nil, fmt.Errorf("reading response body: %w", err) - } - if !retryable(resp.StatusCode, respBody) { + if err == nil && !retryable(resp.StatusCode, respBody) { return resp, respBody, nil } } diff --git a/cmd/record-release/main_test.go b/cmd/record-release/main_test.go index 277a266..99546ce 100644 --- a/cmd/record-release/main_test.go +++ b/cmd/record-release/main_test.go @@ -352,3 +352,66 @@ func TestPostWithRetry(t *testing.T) { }) } } + +// A connection dropped before or during the response is retried the same way; +// five drops in a row are the final error. +func TestPostWithRetryTransportErrors(t *testing.T) { + saved := retryDelays + retryDelays = []time.Duration{0, 0, 0, 0} + t.Cleanup(func() { retryDelays = saved }) + + drops := 0 + drop := func(w http.ResponseWriter) { + drops++ + conn, _, err := w.(http.Hijacker).Hijack() + if err != nil { + t.Fatalf("hijack: %v", err) + } + conn.Close() + } + + t.Run("dropped then 200", func(t *testing.T) { + drops = 0 + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if drops == 0 { + drop(w) + return + } + w.WriteHeader(http.StatusOK) + })) + defer srv.Close() + resp, _, err := postWithRetry(srv.Client(), srv.URL, []byte("payload")) + if err != nil || resp.StatusCode != http.StatusOK || drops != 1 { + t.Fatalf("err = %v, status = %v, drops = %d; want nil, 200, 1", err, resp, drops) + } + }) + + t.Run("body cut after the headers then 200", func(t *testing.T) { + drops = 0 + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if drops == 0 { + w.Header().Set("Content-Length", "100") + w.WriteHeader(http.StatusOK) + _, _ = w.Write([]byte("part")) + drop(w) // flushes the partial body, then closes: the client sees an unexpected EOF + return + } + w.WriteHeader(http.StatusOK) + })) + defer srv.Close() + resp, _, err := postWithRetry(srv.Client(), srv.URL, []byte("payload")) + if err != nil || resp.StatusCode != http.StatusOK || drops != 1 { + t.Fatalf("err = %v, status = %v, drops = %d; want nil, 200, 1", err, resp, drops) + } + }) + + t.Run("dropped five times", func(t *testing.T) { + drops = 0 + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { drop(w) })) + defer srv.Close() + _, _, err := postWithRetry(srv.Client(), srv.URL, []byte("payload")) + if err == nil || !strings.Contains(err.Error(), "after 5 attempts") || drops != 5 { + t.Fatalf("err = %v, drops = %d; want 'after 5 attempts', 5", err, drops) + } + }) +}