From 1b38e6ad002eeb8fc114f570ef0593337e877a07 Mon Sep 17 00:00:00 2001 From: Rodin Date: Mon, 11 May 2026 05:31:39 -0700 Subject: [PATCH] fix(gitea): ensure consistent lastErr return for network errors Move lastErr assignment outside the retry condition so that both network errors and HTTP 5xx paths return lastErr consistently. Previously, on the final retry attempt, a network error would return the raw err variable instead of lastErr. While they held the same value in practice, the inconsistency was confusing when reading the code. Now both paths: - Network errors: assign lastErr before checking retry, return lastErr - HTTP 5xx: assign lastErr before checking retry, return lastErr Addresses review finding #3 (MINOR) from sonnet review on PR #69. --- gitea/client.go | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/gitea/client.go b/gitea/client.go index 1cee5b4..b016349 100644 --- a/gitea/client.go +++ b/gitea/client.go @@ -374,19 +374,21 @@ func (c *Client) doGet(ctx context.Context, reqURL string) ([]byte, error) { resp, err := c.http.Do(req) if err != nil { - // Check if this is a temporary/transient network error worth retrying. - // We only retry if there are attempts remaining. + // Always capture the error for consistent return at loop end. + // This ensures both network errors and HTTP 5xx return lastErr. + lastErr = err + + // Only retry temporary network errors when attempts remain. if attempt < maxAttempts-1 && isTemporaryNetError(err) { slog.Warn("temporary network error, will retry", "attempt", attempt+1, "url", redactURL(reqURL), "error", err) - lastErr = err continue } - return nil, err + // Non-retryable network error or final attempt exhausted. + return nil, lastErr } - if resp.StatusCode >= 200 && resp.StatusCode < 300 { body, err := io.ReadAll(resp.Body) resp.Body.Close()