retry version check on transient proxy failures

Author: jojinkb

version/version.go

@@ -8,6 +8,7 @@ import (
 	"os/exec"
 	"slices"
 	"strings"
+	"time"
 
 	"github.com/goccy/go-json"
 	"golang.org/x/mod/semver"
@@ -81,6 +82,9 @@ func getLatestVersion() (string, error) {
 		proxies = append(proxies, goproxyDefault)
 	}
 
+	client := &http.Client{Timeout: 10 * time.Second}
+
+	var lastErr error
 	for _, proxy := range proxies {
 		proxy = strings.TrimSpace(proxy)
 		proxy = strings.TrimRight(proxy, "/")
@@ -89,24 +93,61 @@ func getLatestVersion() (string, error) {
 		}
 
 		url := fmt.Sprintf("%s/github.com/%s/%s/@latest", proxy, repoOwner, repoName)
-		resp, err := http.Get(url)
-		if err != nil {
-			continue
+		version, err := fetchLatestWithRetry(client, url)
+		if err == nil {
+			return version, nil
 		}
-		defer resp.Body.Close()
+		lastErr = err
+	}
 
-		body, err := io.ReadAll(resp.Body)
-		if err != nil {
-			continue
-		}
+	if lastErr != nil {
+		return "", fmt.Errorf("failed to fetch latest version: %w", lastErr)
+	}
+	return "", fmt.Errorf("failed to fetch latest version")
+}
 
-		var version struct{ Version string }
-		if err = json.Unmarshal(body, &version); err != nil {
-			continue
+// fetchLatestWithRetry queries a single proxy, retrying a few times because
+// proxy.golang.org occasionally resets the connection or returns a transient
+// 5xx. A single drop should not fail the whole version check.
+func fetchLatestWithRetry(client *http.Client, url string) (string, error) {
+	const maxAttempts = 3
+	var lastErr error
+	for attempt := 1; attempt <= maxAttempts; attempt++ {
+		version, err := fetchLatestFromProxy(client, url)
+		if err == nil {
+			return version, nil
 		}
+		lastErr = err
+		if attempt < maxAttempts {
+			time.Sleep(time.Duration(attempt) * 200 * time.Millisecond)
+		}
+	}
+	return "", lastErr
+}
 
-		return version.Version, nil
+func fetchLatestFromProxy(client *http.Client, url string) (string, error) {
+	resp, err := client.Get(url)
+	if err != nil {
+		return "", err
 	}
+	defer resp.Body.Close()
 
-	return "", fmt.Errorf("failed to fetch latest version")
+	body, err := io.ReadAll(resp.Body)
+	if err != nil {
+		return "", err
+	}
+
+	if resp.StatusCode != http.StatusOK {
+		return "", fmt.Errorf("unexpected status %s from %s", resp.Status, url)
+	}
+
+	var version struct{ Version string }
+	if err := json.Unmarshal(body, &version); err != nil {
+		return "", fmt.Errorf("invalid response from %s: %w", url, err)
+	}
+	if version.Version == "" {
+		return "", fmt.Errorf("empty version in response from %s", url)
+	}
+
+	return version.Version, nil
 }

version/version_test.go

@@ -0,0 +1,81 @@
+package version
+
+import (
+	"net/http"
+	"net/http/httptest"
+	"testing"
+)
+
+func TestFetchLatestFromProxy(t *testing.T) {
+	tests := []struct {
+		name    string
+		status  int
+		body    string
+		want    string
+		wantErr bool
+	}{
+		{name: "ok", status: http.StatusOK, body: `{"Version":"v1.2.3"}`, want: "v1.2.3"},
+		{name: "non-200", status: http.StatusInternalServerError, body: "boom", wantErr: true},
+		{name: "invalid json", status: http.StatusOK, body: "not json", wantErr: true},
+		{name: "empty version", status: http.StatusOK, body: `{"Version":""}`, wantErr: true},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+				w.WriteHeader(tt.status)
+				_, _ = w.Write([]byte(tt.body))
+			}))
+			defer server.Close()
+
+			got, err := fetchLatestFromProxy(server.Client(), server.URL)
+			if tt.wantErr {
+				if err == nil {
+					t.Fatalf("fetchLatestFromProxy() error = nil, want error")
+				}
+				return
+			}
+			if err != nil {
+				t.Fatalf("fetchLatestFromProxy() unexpected error: %v", err)
+			}
+			if got != tt.want {
+				t.Fatalf("fetchLatestFromProxy() = %q, want %q", got, tt.want)
+			}
+		})
+	}
+}
+
+// A single transient failure should not fail the version check; the retry
+// must recover once the proxy responds successfully.
+func TestFetchLatestWithRetryRecoversFromTransientFailure(t *testing.T) {
+	var calls int
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		calls++
+		if calls < 2 {
+			// Simulate a dropped/erroring proxy response.
+			hj, ok := w.(http.Hijacker)
+			if ok {
+				conn, _, err := hj.Hijack()
+				if err == nil {
+					conn.Close()
+					return
+				}
+			}
+			w.WriteHeader(http.StatusInternalServerError)
+			return
+		}
+		_, _ = w.Write([]byte(`{"Version":"v1.2.3"}`))
+	}))
+	defer server.Close()
+
+	got, err := fetchLatestWithRetry(server.Client(), server.URL)
+	if err != nil {
+		t.Fatalf("fetchLatestWithRetry() unexpected error: %v", err)
+	}
+	if got != "v1.2.3" {
+		t.Fatalf("fetchLatestWithRetry() = %q, want %q", got, "v1.2.3")
+	}
+	if calls < 2 {
+		t.Fatalf("expected a retry, got %d call(s)", calls)
+	}
+}