feat: add retry with exponential backoff for network requests

Fixes #5

Author: omarkohl

internal/github/client.go

@@ -6,6 +6,8 @@ import (
 	"strings"
 
 	gogithub "github.com/google/go-github/v68/github"
+
+	"github.com/omarkohl/jip/internal/retry"
 )
 
 // Service defines the GitHub operations needed by the send pipeline.
@@ -75,12 +77,17 @@ type UpdatePROpts struct {
 
 // CreatePR creates a new pull request and returns its info.
 func (c *Client) CreatePR(head, base, title, body string, draft bool) (*PRInfo, error) {
-	pr, _, err := c.gh.PullRequests.Create(context.Background(), c.owner, c.repo, &gogithub.NewPullRequest{
-		Title: &title,
-		Head:  &head,
-		Base:  &base,
-		Body:  &body,
-		Draft: &draft,
+	var pr *gogithub.PullRequest
+	err := retry.Do(func() error {
+		var apiErr error
+		pr, _, apiErr = c.gh.PullRequests.Create(context.Background(), c.owner, c.repo, &gogithub.NewPullRequest{
+			Title: &title,
+			Head:  &head,
+			Base:  &base,
+			Body:  &body,
+			Draft: &draft,
+		})
+		return apiErr
 	})
 	if err != nil {
 		return nil, fmt.Errorf("creating PR: %w", err)
@@ -109,7 +116,10 @@ func (c *Client) UpdatePR(number int, opts UpdatePROpts) error {
 	if opts.Base != nil {
 		update.Base = &gogithub.PullRequestBranch{Ref: opts.Base}
 	}
-	_, _, err := c.gh.PullRequests.Edit(context.Background(), c.owner, c.repo, number, update)
+	err := retry.Do(func() error {
+		_, _, apiErr := c.gh.PullRequests.Edit(context.Background(), c.owner, c.repo, number, update)
+		return apiErr
+	})
 	if err != nil {
 		return fmt.Errorf("updating PR #%d: %w", number, err)
 	}
@@ -118,8 +128,11 @@ func (c *Client) UpdatePR(number int, opts UpdatePROpts) error {
 
 // CommentOnPR posts a comment on a pull request.
 func (c *Client) CommentOnPR(number int, body string) error {
-	_, _, err := c.gh.Issues.CreateComment(context.Background(), c.owner, c.repo, number, &gogithub.IssueComment{
-		Body: &body,
+	err := retry.Do(func() error {
+		_, _, apiErr := c.gh.Issues.CreateComment(context.Background(), c.owner, c.repo, number, &gogithub.IssueComment{
+			Body: &body,
+		})
+		return apiErr
 	})
 	if err != nil {
 		return fmt.Errorf("commenting on PR #%d: %w", number, err)
@@ -129,7 +142,12 @@ func (c *Client) CommentOnPR(number int, body string) error {
 
 // GetAuthenticatedUser returns the login of the authenticated user.
 func (c *Client) GetAuthenticatedUser() (string, error) {
-	user, _, err := c.gh.Users.Get(context.Background(), "")
+	var user *gogithub.User
+	err := retry.Do(func() error {
+		var apiErr error
+		user, _, apiErr = c.gh.Users.Get(context.Background(), "")
+		return apiErr
+	})
 	if err != nil {
 		return "", fmt.Errorf("getting authenticated user: %w", err)
 	}
@@ -138,8 +156,11 @@ func (c *Client) GetAuthenticatedUser() (string, error) {
 
 // RequestReviewers adds reviewers to a pull request.
 func (c *Client) RequestReviewers(number int, reviewers []string) error {
-	_, _, err := c.gh.PullRequests.RequestReviewers(context.Background(), c.owner, c.repo, number, gogithub.ReviewersRequest{
-		Reviewers: reviewers,
+	err := retry.Do(func() error {
+		_, _, apiErr := c.gh.PullRequests.RequestReviewers(context.Background(), c.owner, c.repo, number, gogithub.ReviewersRequest{
+			Reviewers: reviewers,
+		})
+		return apiErr
 	})
 	if err != nil {
 		return fmt.Errorf("requesting reviewers on PR #%d: %w", number, err)

internal/github/pr.go

@@ -7,6 +7,8 @@ import (
 	"io"
 	"net/http"
 	"strings"
+
+	"github.com/omarkohl/jip/internal/retry"
 )
 
 // PRInfo holds the essential fields of a pull request.
@@ -59,15 +61,32 @@ func (c *Client) LookupPRsByBranch(branches []string) (map[string]*PRInfo, error
 	req.Header.Set("Authorization", "bearer "+c.token)
 	req.Header.Set("Content-Type", "application/json")
 
-	resp, err := http.DefaultClient.Do(req)
-	if err != nil {
-		return nil, fmt.Errorf("sending request: %w", err)
-	}
-	defer func() { _ = resp.Body.Close() }()
+	var resp *http.Response
+	var rawBody []byte
+	err = retry.Do(func() error {
+		// Reset the request body for each attempt.
+		req.Body = io.NopCloser(bytes.NewReader(body))
+
+		var doErr error
+		resp, doErr = http.DefaultClient.Do(req)
+		if doErr != nil {
+			return doErr
+		}
+
+		rawBody, doErr = io.ReadAll(resp.Body)
+		_ = resp.Body.Close()
+		if doErr != nil {
+			return doErr
+		}
 
-	rawBody, err := io.ReadAll(resp.Body)
+		// Retry on server errors (5xx); don't retry client errors (4xx).
+		if resp.StatusCode >= 500 {
+			return fmt.Errorf("GitHub API returned %d: %s", resp.StatusCode, string(rawBody))
+		}
+		return nil
+	})
 	if err != nil {
-		return nil, fmt.Errorf("reading response: %w", err)
+		return nil, fmt.Errorf("sending request: %w", err)
 	}
 
 	if resp.StatusCode != 200 {

internal/jj/runner.go

@@ -4,6 +4,8 @@ import (
 	"fmt"
 	"os/exec"
 	"strings"
+
+	"github.com/omarkohl/jip/internal/retry"
 )
 
 // logTemplate is the jj template that outputs one JSON object per line.
@@ -127,32 +129,36 @@ func (r *realRunner) GitRemoteList() ([]byte, error) {
 }
 
 func (r *realRunner) GitFetch(remote string) error {
-	args := []string{"git", "fetch", "-R", r.repoDir, "--remote", remote}
-	cmd := exec.Command("jj", args...)
-	out, err := cmd.CombinedOutput()
-	if err != nil {
-		return fmt.Errorf("jj git fetch: %w\n%s", err, strings.TrimSpace(string(out)))
-	}
-	return nil
+	return retry.Do(func() error {
+		args := []string{"git", "fetch", "-R", r.repoDir, "--remote", remote}
+		cmd := exec.Command("jj", args...)
+		out, err := cmd.CombinedOutput()
+		if err != nil {
+			return fmt.Errorf("jj git fetch: %w\n%s", err, strings.TrimSpace(string(out)))
+		}
+		return nil
+	})
 }
 
 func (r *realRunner) GitPush(bookmarks []string, allowNew bool, remote string) error {
-	args := []string{"git", "push", "-R", r.repoDir}
-	if remote != "" {
-		args = append(args, "--remote", remote)
-	}
-	for _, b := range bookmarks {
-		args = append(args, "-b", b)
-	}
-	if allowNew {
-		args = append(args, "--allow-new")
-	}
-	cmd := exec.Command("jj", args...)
-	out, err := cmd.CombinedOutput()
-	if err != nil {
-		return fmt.Errorf("jj git push: %w\n%s", err, strings.TrimSpace(string(out)))
-	}
-	return nil
+	return retry.Do(func() error {
+		args := []string{"git", "push", "-R", r.repoDir}
+		if remote != "" {
+			args = append(args, "--remote", remote)
+		}
+		for _, b := range bookmarks {
+			args = append(args, "-b", b)
+		}
+		if allowNew {
+			args = append(args, "--allow-new")
+		}
+		cmd := exec.Command("jj", args...)
+		out, err := cmd.CombinedOutput()
+		if err != nil {
+			return fmt.Errorf("jj git push: %w\n%s", err, strings.TrimSpace(string(out)))
+		}
+		return nil
+	})
 }
 
 func (r *realRunner) Interdiff(from, to string) (string, error) {

internal/retry/retry.go

@@ -0,0 +1,70 @@
+package retry
+
+import (
+	"math"
+	"math/rand/v2"
+	"time"
+)
+
+// config holds retry parameters.
+type config struct {
+	maxAttempts    int
+	initialBackoff time.Duration
+	multiplier     float64
+	maxBackoff     time.Duration
+}
+
+// Option configures retry behavior.
+type Option func(*config)
+
+// WithMaxAttempts sets the maximum number of attempts (default 3).
+func WithMaxAttempts(n int) Option {
+	return func(c *config) { c.maxAttempts = n }
+}
+
+// WithInitialBackoff sets the initial backoff duration (default 1s).
+func WithInitialBackoff(d time.Duration) Option {
+	return func(c *config) { c.initialBackoff = d }
+}
+
+// WithMultiplier sets the backoff multiplier (default 2.0).
+func WithMultiplier(m float64) Option {
+	return func(c *config) { c.multiplier = m }
+}
+
+// WithMaxBackoff sets the maximum backoff duration (default 30s).
+func WithMaxBackoff(d time.Duration) Option {
+	return func(c *config) { c.maxBackoff = d }
+}
+
+// Do calls fn up to maxAttempts times, sleeping with exponential backoff
+// and jitter between attempts. Returns the last error if all attempts fail.
+func Do(fn func() error, opts ...Option) error {
+	cfg := config{
+		maxAttempts:    3,
+		initialBackoff: 1 * time.Second,
+		multiplier:     2.0,
+		maxBackoff:     30 * time.Second,
+	}
+	for _, o := range opts {
+		o(&cfg)
+	}
+
+	var err error
+	for attempt := range cfg.maxAttempts {
+		err = fn()
+		if err == nil {
+			return nil
+		}
+		if attempt < cfg.maxAttempts-1 {
+			backoff := float64(cfg.initialBackoff) * math.Pow(cfg.multiplier, float64(attempt))
+			if backoff > float64(cfg.maxBackoff) {
+				backoff = float64(cfg.maxBackoff)
+			}
+			// Add jitter: 50-100% of the computed backoff.
+			jittered := time.Duration(backoff * (0.5 + rand.Float64()*0.5))
+			time.Sleep(jittered)
+		}
+	}
+	return err
+}

internal/retry/retry_test.go

@@ -0,0 +1,97 @@
+package retry
+
+import (
+	"errors"
+	"testing"
+	"time"
+)
+
+func TestDoSucceedsImmediately(t *testing.T) {
+	calls := 0
+	err := Do(func() error {
+		calls++
+		return nil
+	}, WithInitialBackoff(time.Millisecond))
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+	if calls != 1 {
+		t.Fatalf("expected 1 call, got %d", calls)
+	}
+}
+
+func TestDoRetriesOnError(t *testing.T) {
+	calls := 0
+	err := Do(func() error {
+		calls++
+		if calls < 3 {
+			return errors.New("transient")
+		}
+		return nil
+	}, WithMaxAttempts(3), WithInitialBackoff(time.Millisecond))
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+	if calls != 3 {
+		t.Fatalf("expected 3 calls, got %d", calls)
+	}
+}
+
+func TestDoExhaustsAttempts(t *testing.T) {
+	calls := 0
+	sentinel := errors.New("persistent")
+	err := Do(func() error {
+		calls++
+		return sentinel
+	}, WithMaxAttempts(4), WithInitialBackoff(time.Millisecond))
+	if !errors.Is(err, sentinel) {
+		t.Fatalf("expected sentinel error, got: %v", err)
+	}
+	if calls != 4 {
+		t.Fatalf("expected 4 calls, got %d", calls)
+	}
+}
+
+func TestDoRespectsBackoff(t *testing.T) {
+	start := time.Now()
+	calls := 0
+	_ = Do(func() error {
+		calls++
+		if calls < 3 {
+			return errors.New("fail")
+		}
+		return nil
+	}, WithMaxAttempts(3), WithInitialBackoff(10*time.Millisecond), WithMultiplier(1.0))
+	elapsed := time.Since(start)
+	// With 2 sleeps of ~10ms (jittered to 5-10ms each), expect at least 8ms total.
+	if elapsed < 8*time.Millisecond {
+		t.Fatalf("expected backoff delay, but elapsed was only %v", elapsed)
+	}
+}
+
+func TestDoSucceedsAfterOneFailure(t *testing.T) {
+	calls := 0
+	err := Do(func() error {
+		calls++
+		if calls == 1 {
+			return errors.New("first fail")
+		}
+		return nil
+	}, WithMaxAttempts(2), WithInitialBackoff(time.Millisecond))
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+	if calls != 2 {
+		t.Fatalf("expected 2 calls, got %d", calls)
+	}
+}
+
+func TestDoSingleAttempt(t *testing.T) {
+	sentinel := errors.New("fail")
+	err := Do(func() error {
+		return sentinel
+	}, WithMaxAttempts(1))
+	if !errors.Is(err, sentinel) {
+		t.Fatalf("expected sentinel error, got: %v", err)
+	}
+}