Repository navigation
feat(git): retry transient network fetch failures (PR E) - #26
Open
bschellenberger2600 wants to merge 19 commits into
Open
bschellenberger2600 wants to merge 19 commits into
bschellenberger2600 wants to merge 19 commits into
Conversation
Add fetch_retry_* config, flags, and GIT_RAIN env overrides. Classify transient vs permanent failures and backoff between attempts with cancellation-aware sleeps. Co-authored-by: Ben Schellenberger <bschellenberger2600@users.noreply.github.com>
Co-authored-by: Ben Schellenberger <bschellenberger2600@users.noreply.github.com>
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configuration
You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
…' into cursor/fetch-retry-transient-971d Co-authored-by: Ben Schellenberger <bschellenberger2600@users.noreply.github.com>
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Autofix Details
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Cancel reports no local branches
- RainRepository now returns context.Canceled on interrupt instead of an empty success result, and --sync treats cancel as skipped like the fetch-only path.
Or push these changes by commenting:
@cursor push 5dc869da6e
Preview (5dc869da6e)
diff --git a/cmd/root.go b/cmd/root.go
--- a/cmd/root.go
+++ b/cmd/root.go
@@ -394,7 +394,7 @@
if !fullSync && !rainFetchMainline {
if fetchErr := fetchOnly(repoOpts.RunCtx, repo.Path, repoOpts); fetchErr != nil {
if errors.Is(fetchErr, context.Canceled) {
- _, _ = fmt.Fprintln(out, " ~ skipped\n")
+ _, _ = fmt.Fprintln(out, " ~ skipped")
_, _ = fmt.Fprintln(out)
return delta
}
@@ -432,6 +432,11 @@
res, rainErr := git.RainRepository(repo.Path, repoOpts)
if rainErr != nil {
+ if errors.Is(rainErr, context.Canceled) {
+ _, _ = fmt.Fprintln(out, " ~ skipped")
+ _, _ = fmt.Fprintln(out)
+ return delta
+ }
_, _ = fmt.Fprintf(out, " ✗ failed: %s\n\n", safety.SanitizeText(rainErr.Error()))
delta.failed++
return delta
diff --git a/internal/git/rain.go b/internal/git/rain.go
--- a/internal/git/rain.go
+++ b/internal/git/rain.go
@@ -232,7 +232,7 @@
output, fetchErr := RunNetworkGit(runCtx, cmd, NetworkGitParamsFromRain(opts))
if fetchErr != nil {
if errors.Is(fetchErr, context.Canceled) {
- return result, nil
+ return result, fetchErr
}
// Freeze gracefully — could not reach remote (auth, network, etc.).
// This is not a hard failure; the repo is untouched. Try again later.
diff --git a/internal/git/rain_test.go b/internal/git/rain_test.go
--- a/internal/git/rain_test.go
+++ b/internal/git/rain_test.go
@@ -1,11 +1,14 @@
package git
import (
+ "context"
+ "errors"
"os"
"os/exec"
"path/filepath"
"strings"
"testing"
+ "time"
testutil "github.com/git-fire/git-testkit"
)
@@ -412,3 +415,33 @@
}
return false
}
+
+func TestRainRepository_CancelDuringFetchReturnsCanceled(t *testing.T) {
+ baseURL, cleanup := startStallTCPListener(t)
+ defer cleanup()
+
+ repo := testutil.CreateTestRepo(t, testutil.RepoOptions{
+ Name: "rain-cancel-fetch",
+ Remotes: map[string]string{
+ "origin": baseURL + "/stall.git",
+ },
+ })
+
+ ctx, cancel := context.WithCancel(context.Background())
+ go func() {
+ time.Sleep(300 * time.Millisecond)
+ cancel()
+ }()
+
+ result, err := RainRepository(repo, RainOptions{
+ RunCtx: ctx,
+ FetchTimeoutSec: 120,
+ FetchRetryMaxAttempts: 1,
+ })
+ if !errors.Is(err, context.Canceled) {
+ t.Fatalf("RainRepository() err = %v, want context.Canceled", err)
+ }
+ if len(result.Branches) != 0 {
+ t.Fatalf("canceled fetch should not report branch outcomes, got %+v", result.Branches)
+ }
+}You can send follow-ups to the cloud agent here.
Co-authored-by: Ben Schellenberger <bschellenberger2600@users.noreply.github.com>
Co-authored-by: Ben Schellenberger <bschellenberger2600@users.noreply.github.com>
Co-authored-by: Ben Schellenberger <bschellenberger2600@users.noreply.github.com>
…ient-971d Co-authored-by: Ben Schellenberger <bschellenberger2600@users.noreply.github.com>
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 4 potential issues.
Autofix Details
Bugbot Autofix prepared fixes for all 4 issues found in the latest run.
- ✅ Fixed: HTTP status retries miss git errors
- Added error: 401/403/502/503/504 patterns so git's "returned error: NNN" lines are classified correctly for retry and non-retry.
- ✅ Fixed: Max backoff skips initial delay
- Initial backoff is now capped to MaxBackoffSec before the first sleep so max always bounds retry delay.
- ✅ Fixed: Zero retry settings get replaced
- Zeros are preserved (0 attempts => one try, 0 backoff => immediate); only negative values use defaults.
- ✅ Fixed: Retry test races accept counter
- The TCP reset test now waits on an Accept-synced channel before deciding success vs synthetic reset.
Or push these changes by commenting:
@cursor push 200b8bedbf
Preview (200b8bedbf)
diff --git a/internal/git/network_retry.go b/internal/git/network_retry.go
--- a/internal/git/network_retry.go
+++ b/internal/git/network_retry.go
@@ -16,21 +16,19 @@
// NetworkGitParams configures timeout and retry behavior for RunNetworkGit.
type NetworkGitParams struct {
- TimeoutSec int
- MaxAttempts int
- InitialBackoffSec int
- MaxBackoffSec int
+ TimeoutSec int
+ MaxAttempts int
+ InitialBackoffSec int
+ MaxBackoffSec int
}
// NetworkGitParamsFromRain maps RainOptions into network git execution params.
+// Zero values are preserved so callers can disable retries (0 attempts) or use
+// immediate backoff (0 seconds). Negative values mean "unset".
func NetworkGitParamsFromRain(opts RainOptions) NetworkGitParams {
- maxAttempts := opts.FetchRetryMaxAttempts
- if maxAttempts <= 0 {
- maxAttempts = DefaultFetchRetryMaxAttempts
- }
return NetworkGitParams{
TimeoutSec: opts.FetchTimeoutSec,
- MaxAttempts: maxAttempts,
+ MaxAttempts: opts.FetchRetryMaxAttempts,
InitialBackoffSec: opts.FetchRetryInitialBackoffSec,
MaxBackoffSec: opts.FetchRetryMaxBackoffSec,
}
@@ -49,21 +47,27 @@
parent = context.Background()
}
maxAttempts := params.MaxAttempts
- if maxAttempts <= 0 {
+ if maxAttempts < 0 {
+ maxAttempts = DefaultFetchRetryMaxAttempts
+ } else if maxAttempts == 0 {
maxAttempts = 1
}
initialBackoff := params.InitialBackoffSec
- if initialBackoff <= 0 {
+ if initialBackoff < 0 {
initialBackoff = DefaultFetchRetryInitialBackoffSec
}
maxBackoff := params.MaxBackoffSec
- if maxBackoff <= 0 {
+ if maxBackoff < 0 {
maxBackoff = DefaultFetchRetryMaxBackoffSec
}
var lastOut []byte
var lastErr error
+ maxBackoffDur := time.Duration(maxBackoff) * time.Second
backoff := time.Duration(initialBackoff) * time.Second
+ if backoff > maxBackoffDur {
+ backoff = maxBackoffDur
+ }
for attemptNum := 1; attemptNum <= maxAttempts; attemptNum++ {
if parent.Err() != nil {
@@ -80,10 +84,10 @@
if !sleepWithContext(parent, backoff) {
return lastOut, parent.Err()
}
- if backoff < time.Duration(maxBackoff)*time.Second {
+ if backoff < maxBackoffDur {
backoff *= 2
- if backoff > time.Duration(maxBackoff)*time.Second {
- backoff = time.Duration(maxBackoff) * time.Second
+ if backoff > maxBackoffDur {
+ backoff = maxBackoffDur
}
}
}
@@ -130,6 +134,9 @@
" 403 ",
"http 401",
"http 403",
+ // git: "The requested URL returned error: 401\n"
+ "error: 401",
+ "error: 403",
}
for _, p := range nonRetry {
if strings.Contains(s, p) {
@@ -151,6 +158,10 @@
" 502 ",
" 503 ",
" 504 ",
+ // git: "The requested URL returned error: 503\n"
+ "error: 502",
+ "error: 503",
+ "error: 504",
"timed out after",
"operation timed out",
"network is unreachable",
diff --git a/internal/git/network_retry_test.go b/internal/git/network_retry_test.go
--- a/internal/git/network_retry_test.go
+++ b/internal/git/network_retry_test.go
@@ -14,15 +14,39 @@
func TestIsTransientNetworkFailure_Classification(t *testing.T) {
cases := []struct {
- name string
- out string
- err error
- want bool
+ name string
+ out string
+ err error
+ want bool
}{
{"timeout", "fatal: connection timed out", errors.New("exit 128"), true},
{"auth", "terminal prompts disabled", errors.New("exit 128"), false},
{"not found", "repository not found", errors.New("exit 128"), false},
{"503", "error: RPC failed; HTTP 503 curl 22", errors.New("exit 128"), true},
+ {
+ "git url error 503",
+ "fatal: unable to access 'https://example.com/repo.git': The requested URL returned error: 503\n",
+ errors.New("exit 128"),
+ true,
+ },
+ {
+ "git url error 502",
+ "fatal: unable to access 'https://example.com/repo.git': The requested URL returned error: 502\n",
+ errors.New("exit 128"),
+ true,
+ },
+ {
+ "git url error 401",
+ "fatal: unable to access 'https://example.com/repo.git': The requested URL returned error: 401\n",
+ errors.New("exit 128"),
+ false,
+ },
+ {
+ "git 401 wins over transient phrase",
+ "fatal: unable to access 'https://a/repo.git': The requested URL returned error: 401\nfatal: connection timed out\n",
+ errors.New("exit 128"),
+ false,
+ },
{"fetch timed out err", "", &FetchTimedOutError{Seconds: 2}, true},
}
for _, tc := range cases {
@@ -43,9 +67,9 @@
}
cmd := exec.Command("git", "fetch")
params := NetworkGitParams{
- MaxAttempts: 3,
- InitialBackoffSec: 1,
- MaxBackoffSec: 2,
+ MaxAttempts: 3,
+ InitialBackoffSec: 1,
+ MaxBackoffSec: 2,
}
out, err := runNetworkGitWithRetry(context.Background(), cmd, params, stub)
if err != nil {
@@ -66,7 +90,7 @@
return []byte("terminal prompts disabled"), errors.New("exit 128")
}
cmd := exec.Command("git", "fetch")
- params := NetworkGitParams{MaxAttempts: 3, InitialBackoffSec: 1}
+ params := NetworkGitParams{MaxAttempts: 3, InitialBackoffSec: 1, MaxBackoffSec: 2}
_, err := runNetworkGitWithRetry(context.Background(), cmd, params, stub)
if err == nil {
t.Fatal("expected error")
@@ -135,7 +159,7 @@
time.Sleep(50 * time.Millisecond)
cancel()
}()
- params := NetworkGitParams{MaxAttempts: 5, InitialBackoffSec: 5}
+ params := NetworkGitParams{MaxAttempts: 5, InitialBackoffSec: 5, MaxBackoffSec: 5}
_, err := runNetworkGitWithRetry(ctx, exec.Command("git", "fetch"), params, stub)
if !errors.Is(err, context.Canceled) {
t.Fatalf("err = %v, want context.Canceled", err)
@@ -145,8 +169,71 @@
}
}
+func TestRunNetworkGitWithRetry_CapsInitialBackoffToMax(t *testing.T) {
+ var calls int
+ stub := func(context.Context, *exec.Cmd, int) ([]byte, error) {
+ calls++
+ return []byte("connection timed out"), errors.New("exit")
+ }
+ start := time.Now()
+ params := NetworkGitParams{MaxAttempts: 2, InitialBackoffSec: 60, MaxBackoffSec: 1}
+ _, _ = runNetworkGitWithRetry(context.Background(), exec.Command("git", "fetch"), params, stub)
+ elapsed := time.Since(start)
+ if calls != 2 {
+ t.Fatalf("calls = %d, want 2", calls)
+ }
+ if elapsed > 5*time.Second {
+ t.Fatalf("elapsed %v; max backoff should cap initial wait", elapsed)
+ }
+}
+
+func TestRunNetworkGitWithRetry_ZeroBackoffImmediate(t *testing.T) {
+ var calls int
+ stub := func(context.Context, *exec.Cmd, int) ([]byte, error) {
+ calls++
+ if calls < 3 {
+ return []byte("connection timed out"), errors.New("exit")
+ }
+ return []byte("ok"), nil
+ }
+ start := time.Now()
+ params := NetworkGitParams{MaxAttempts: 3, InitialBackoffSec: 0, MaxBackoffSec: 0}
+ _, err := runNetworkGitWithRetry(context.Background(), exec.Command("git", "fetch"), params, stub)
+ elapsed := time.Since(start)
+ if err != nil {
+ t.Fatalf("runNetworkGitWithRetry: %v", err)
+ }
+ if calls != 3 {
+ t.Fatalf("calls = %d, want 3", calls)
+ }
+ if elapsed > time.Second {
+ t.Fatalf("elapsed %v; zero backoff should retry immediately", elapsed)
+ }
+}
+
+func TestNetworkGitParamsFromRain_PreservesZero(t *testing.T) {
+ p := NetworkGitParamsFromRain(RainOptions{
+ FetchRetryMaxAttempts: 0,
+ FetchRetryInitialBackoffSec: 0,
+ FetchRetryMaxBackoffSec: 0,
+ })
+ if p.MaxAttempts != 0 || p.InitialBackoffSec != 0 || p.MaxBackoffSec != 0 {
+ t.Fatalf("got %+v, want zeros preserved", p)
+ }
+ var calls int
+ stub := func(context.Context, *exec.Cmd, int) ([]byte, error) {
+ calls++
+ return []byte("connection timed out"), errors.New("exit")
+ }
+ _, _ = runNetworkGitWithRetry(context.Background(), exec.Command("git", "fetch"), p, stub)
+ if calls != 1 {
+ t.Fatalf("calls = %d, want 1 (zero max attempts => single try)", calls)
+ }
+}
+
func TestRunNetworkGit_RetriesTCPResetServer(t *testing.T) {
var accepts atomic.Int32
+ accepted := make(chan int, 8)
ln, err := net.Listen("tcp", "127.0.0.1:0")
if err != nil {
t.Fatalf("listen: %v", err)
@@ -158,11 +245,9 @@
if aerr != nil {
return
}
- if accepts.Add(1) < 2 {
- _ = c.Close()
- continue
- }
+ n := int(accepts.Add(1))
_ = c.Close()
+ accepted <- n
}
}()
@@ -172,12 +257,19 @@
return []byte(derr.Error()), derr
}
_ = conn.Close()
- if accepts.Load() < 2 {
- return []byte("read: connection reset by peer"), errors.New("reset")
+ select {
+ case n := <-accepted:
+ if n < 2 {
+ return []byte("read: connection reset by peer"), errors.New("reset")
+ }
+ return nil, nil
+ case <-ctx.Done():
+ return nil, ctx.Err()
+ case <-time.After(2 * time.Second):
+ return []byte("timed out waiting for accept"), errors.New("accept timeout")
}
- return nil, nil
}
- params := NetworkGitParams{MaxAttempts: 2, InitialBackoffSec: 1}
+ params := NetworkGitParams{MaxAttempts: 2, InitialBackoffSec: 1, MaxBackoffSec: 2}
_, err = runNetworkGitWithRetry(context.Background(), exec.Command("git", "fetch"), params, stub)
if err != nil {
t.Fatalf("expected success on second attempt: %v", err)You can send follow-ups to the cloud agent here.
…ient-971d Co-authored-by: Ben Schellenberger <bschellenberger2600@users.noreply.github.com>
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
There are 5 total unresolved issues (including 4 from previous reviews).
Autofix Details
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Timeouts bypass auth retry denylist
- Reordered isTransientNetworkFailure so auth/denylist phrases are checked before treating FetchTimedOutError as retryable, with regression tests.
Or push these changes by commenting:
@cursor push 458d476608
Preview (458d476608)
diff --git a/internal/git/network_retry.go b/internal/git/network_retry.go
--- a/internal/git/network_retry.go
+++ b/internal/git/network_retry.go
@@ -16,10 +16,10 @@
// NetworkGitParams configures timeout and retry behavior for RunNetworkGit.
type NetworkGitParams struct {
- TimeoutSec int
- MaxAttempts int
- InitialBackoffSec int
- MaxBackoffSec int
+ TimeoutSec int
+ MaxAttempts int
+ InitialBackoffSec int
+ MaxBackoffSec int
}
// NetworkGitParamsFromRain maps RainOptions into network git execution params.
@@ -108,16 +108,15 @@
if err == nil {
return false
}
- var timedOut *FetchTimedOutError
- if errors.As(err, &timedOut) {
- return true
- }
if errors.Is(err, context.Canceled) || errors.Is(err, context.DeadlineExceeded) {
return false
}
s := strings.ToLower(string(output) + " " + err.Error())
+ // Auth / permanent failures must win over FetchTimedOutError: a multi-remote
+ // fetch can log authentication failed (or similar) and still hit the attempt
+ // deadline if another remote hangs.
nonRetry := []string{
"authentication failed",
"could not read username",
@@ -137,6 +136,11 @@
}
}
+ var timedOut *FetchTimedOutError
+ if errors.As(err, &timedOut) {
+ return true
+ }
+
transient := []string{
"could not resolve host",
"connection timed out",
diff --git a/internal/git/network_retry_test.go b/internal/git/network_retry_test.go
--- a/internal/git/network_retry_test.go
+++ b/internal/git/network_retry_test.go
@@ -14,16 +14,19 @@
func TestIsTransientNetworkFailure_Classification(t *testing.T) {
cases := []struct {
- name string
- out string
- err error
- want bool
+ name string
+ out string
+ err error
+ want bool
}{
{"timeout", "fatal: connection timed out", errors.New("exit 128"), true},
{"auth", "terminal prompts disabled", errors.New("exit 128"), false},
{"not found", "repository not found", errors.New("exit 128"), false},
{"503", "error: RPC failed; HTTP 503 curl 22", errors.New("exit 128"), true},
{"fetch timed out err", "", &FetchTimedOutError{Seconds: 2}, true},
+ {"fetch timed out with auth", "fatal: Authentication failed for 'https://example.com/repo.git'", &FetchTimedOutError{Seconds: 2}, false},
+ {"fetch timed out with prompts disabled", "fatal: could not read Username for 'https://example.com': terminal prompts disabled", &FetchTimedOutError{Seconds: 2}, false},
+ {"fetch timed out with 403", "remote: HTTP 403\n fatal: unable to access", &FetchTimedOutError{Seconds: 2}, false},
}
for _, tc := range cases {
if got := isTransientNetworkFailure([]byte(tc.out), tc.err); got != tc.want {
@@ -43,9 +46,9 @@
}
cmd := exec.Command("git", "fetch")
params := NetworkGitParams{
- MaxAttempts: 3,
- InitialBackoffSec: 1,
- MaxBackoffSec: 2,
+ MaxAttempts: 3,
+ InitialBackoffSec: 1,
+ MaxBackoffSec: 2,
}
out, err := runNetworkGitWithRetry(context.Background(), cmd, params, stub)
if err != nil {
@@ -76,6 +79,27 @@
}
}
+func TestRunNetworkGitWithRetry_DoesNotRetryTimeoutAfterAuthOutput(t *testing.T) {
+ var calls int
+ stub := func(context.Context, *exec.Cmd, int) ([]byte, error) {
+ calls++
+ return []byte("fatal: Authentication failed for 'https://example.com/a.git'\n"), &FetchTimedOutError{Seconds: 2}
+ }
+ cmd := exec.Command("git", "fetch", "--all")
+ params := NetworkGitParams{MaxAttempts: 3, InitialBackoffSec: 1}
+ _, err := runNetworkGitWithRetry(context.Background(), cmd, params, stub)
+ if err == nil {
+ t.Fatal("expected error")
+ }
+ var timedOut *FetchTimedOutError
+ if !errors.As(err, &timedOut) {
+ t.Fatalf("err = %v, want FetchTimedOutError", err)
+ }
+ if calls != 1 {
+ t.Fatalf("calls = %d, want 1 (no retry when timeout follows auth failure)", calls)
+ }
+}
+
func TestRunNetworkGit_RetriesFlakyHTTPServer(t *testing.T) {
var hits atomic.Int32
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {You can send follow-ups to the cloud agent here.
…ient-971d Co-authored-by: Ben Schellenberger <bschellenberger2600@users.noreply.github.com>
…nRepos Co-authored-by: Ben Schellenberger <bschellenberger2600@users.noreply.github.com>
…ient-971d Co-authored-by: Ben Schellenberger <bschellenberger2600@users.noreply.github.com>
…ient-971d Co-authored-by: Ben Schellenberger <bschellenberger2600@users.noreply.github.com>
Returning success with an empty RainResult on interrupt made --sync print "no local branches". Propagate context.Canceled and treat it like the fetch-only path (skipped), so cancelled hydrates are not misreported. Co-authored-by: Ben Schellenberger <bschellenberger2600@users.noreply.github.com>
Match git's "returned error: NNN" HTTP status lines for retry/non-retry, cap initial backoff by max, preserve explicit zero retry settings, and synchronize the TCP reset retry test with Accept.
isTransientNetworkFailure treated FetchTimedOutError as retryable before scanning command output for authentication / denylist phrases, so a multi-remote fetch that failed auth then hit the deadline was retried. Check the denylist first so auth failures stay non-retryable.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Autofix Details
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Permanent fetch failures get retried
- Expanded non-retry matching for repository 'URL' not found, HTTP 404, and invalid username/password so those permanent failures are not retried when mixed with timeouts or transient network phrases.
Or push these changes by commenting:
@cursor push d832c50a17
Preview (d832c50a17)
diff --git a/internal/git/network_retry.go b/internal/git/network_retry.go
--- a/internal/git/network_retry.go
+++ b/internal/git/network_retry.go
@@ -128,20 +128,28 @@
"permission denied",
"repository not found",
"terminal prompts disabled",
+ "invalid username or password",
"invalid credentials",
" 401 ",
" 403 ",
+ " 404 ",
"http 401",
"http 403",
+ "http 404",
// git: "The requested URL returned error: 401\n"
"error: 401",
"error: 403",
+ "error: 404",
}
for _, p := range nonRetry {
if strings.Contains(s, p) {
return false
}
}
+ // git: "fatal: repository 'https://…' not found" (URL breaks the contiguous phrase)
+ if strings.Contains(s, "repository") && strings.Contains(s, "not found") {
+ return false
+ }
var timedOut *FetchTimedOutError
if errors.As(err, &timedOut) {
diff --git a/internal/git/network_retry_test.go b/internal/git/network_retry_test.go
--- a/internal/git/network_retry_test.go
+++ b/internal/git/network_retry_test.go
@@ -51,6 +51,24 @@
{"fetch timed out with auth", "fatal: Authentication failed for 'https://example.com/repo.git'", &FetchTimedOutError{Seconds: 2}, false},
{"fetch timed out with prompts disabled", "fatal: could not read Username for 'https://example.com': terminal prompts disabled", &FetchTimedOutError{Seconds: 2}, false},
{"fetch timed out with 403", "remote: HTTP 403\n fatal: unable to access", &FetchTimedOutError{Seconds: 2}, false},
+ {
+ "fetch timed out with repository url not found",
+ "fatal: repository 'https://example.com/missing.git' not found\n",
+ &FetchTimedOutError{Seconds: 2},
+ false,
+ },
+ {
+ "git url error 404 wins over timeout",
+ "fatal: unable to access 'https://example.com/repo.git': The requested URL returned error: 404\n",
+ &FetchTimedOutError{Seconds: 2},
+ false,
+ },
+ {
+ "invalid username or password wins over transient phrase",
+ "fatal: Invalid username or password\nfatal: could not resolve host: other.example\n",
+ errors.New("exit 128"),
+ false,
+ },
}
for _, tc := range cases {
if got := isTransientNetworkFailure([]byte(tc.out), tc.err); got != tc.want {You can send follow-ups to the cloud agent here.
Expand isTransientNetworkFailure permanent-failure matching so repository 'URL' not found, HTTP 404, and invalid username/password win over timeouts or unrelated transient phrases in multi-remote fetch output.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Pack count 404 skips retries
- Removed the bare " 404 " non-retry token that matched pack-objects sideband stats, so timeouts/resets after a Total/reused count of 404 are retried again while real HTTP 404s still match http/error patterns.
Or push these changes by commenting:
@cursor push f7bd005f37
Preview (f7bd005f37)
diff --git a/internal/git/network_retry.go b/internal/git/network_retry.go
--- a/internal/git/network_retry.go
+++ b/internal/git/network_retry.go
@@ -132,7 +132,7 @@
"invalid credentials",
" 401 ",
" 403 ",
- " 404 ",
+ // Omit " 404 ": pack-objects stats ("Total 404 (delta …)") contain it.
"http 401",
"http 403",
"http 404",
diff --git a/internal/git/network_retry_test.go b/internal/git/network_retry_test.go
--- a/internal/git/network_retry_test.go
+++ b/internal/git/network_retry_test.go
@@ -64,6 +64,18 @@
false,
},
{
+ "pack object count 404 does not block timeout retry",
+ "Total 404 (delta 12), reused 404 (delta 0), pack-reused 0\n",
+ &FetchTimedOutError{Seconds: 2},
+ true,
+ },
+ {
+ "pack object count 404 does not block reset retry",
+ "Total 404 (delta 12), reused 404 (delta 0), pack-reused 0\nfatal: connection reset by peer\n",
+ errors.New("exit 128"),
+ true,
+ },
+ {
"invalid username or password wins over transient phrase",
"fatal: Invalid username or password\nfatal: could not resolve host: other.example\n",
errors.New("exit 128"),You can send follow-ups to the cloud agent here.
Reviewed by Cursor Bugbot for commit 9e98567. Configure here.
The bare " 404 " non-retry token matched upload-pack sideband stats such as "Total 404 (delta …)", which blocked retries for later timeouts and connection resets. Rely on "http 404" / "error: 404" instead and add regression coverage.
…heck S1030) Co-authored-by: Ben Schellenberger <bschellenberger2600@users.noreply.github.com>
…ient-971d Co-authored-by: Ben Schellenberger <bschellenberger2600@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stacked stall-review fix 5/6. Base:
cursor/parallel-rain-workers-971d(PR D). PR F stacks on this branch.Summary
fetch_retry_max_attempts(default 2),fetch_retry_initial_backoff_sec(2),fetch_retry_max_backoff_sec(30)--fetch-retry-max-attempts,--fetch-retry-initial-backoff,--fetch-retry-max-backoffGIT_RAIN_GLOBAL_FETCH_RETRY_*Tests
TestIsTransientNetworkFailure_ClassificationTestRunNetworkGitWithRetry_*(stub + flaky HTTP 503 harness)Verification
go vet ./...— passgo test -race -count=1 ./...— pass except knownTestNetworkFetch_UnauthenticatedHTTPSFailsWithoutPrompt(PR test: make network auth fetch regression test deterministic #21)