Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 42 additions & 0 deletions internal/forge/forge.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"context"
"errors"
"fmt"
"io"
"strings"
)

Expand Down Expand Up @@ -89,6 +90,47 @@ func IsNotFork(err error) bool {
return errors.Is(err, ErrNotFork)
}

// IsTransient reports whether err represents a transient failure that
// may succeed on retry. It checks for:
// - non-fast-forward race conditions (ErrNonFastForward)
// - forge-specific API errors that self-report transient-ness via the
// transientReporter interface (e.g., HTTP 429, 500–504)
// - HTTP client/network timeouts
// - unexpected connection closures (io.EOF, io.ErrUnexpectedEOF)
//
// Callers can use this to decide whether retrying an operation is
// worthwhile before falling back to a log-and-continue strategy.
func IsTransient(err error) bool {
if err == nil {
return false
}
if IsNonFastForward(err) {
return true
}
// Forge-specific error types (github.APIError, gitlab.APIError,
// jira.APIError) implement this interface to self-report whether
// the status code indicates a transient server-side failure.
type transientReporter interface {
IsTransient() bool
}
var te transientReporter
if errors.As(err, &te) {
return te.IsTransient()
}
// HTTP client timeout (distinct from context cancellation).
var timeout interface{ Timeout() bool }
if errors.As(err, &timeout) && timeout.Timeout() {
return true
}
// Unexpected connection closure — the server dropped the connection
// before a full response was read. Common under load or during
// transient GCP/GitHub infrastructure issues.
if errors.Is(err, io.EOF) || errors.Is(err, io.ErrUnexpectedEOF) {
return true
}
return false
}

// ErrNotSupported indicates that the forge implementation does not
// support the requested operation.
var ErrNotSupported = errors.New("operation not supported by this forge")
Expand Down
9 changes: 9 additions & 0 deletions internal/forge/github/github.go
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,15 @@ func (e *APIError) Error() string {
return s
}

// IsTransient reports whether the API error represents a transient
// failure that may succeed on retry: server errors (500–504) and
// rate limits (429). This method satisfies the transientReporter
// interface used by forge.IsTransient.
func (e *APIError) IsTransient() bool {
return e.StatusCode == http.StatusTooManyRequests ||
(e.StatusCode >= 500 && e.StatusCode <= 504)
}

// Unwrap returns sentinel errors for well-known API responses.
//
// ErrBranchProtected is intentionally NOT mapped here. Branch protection
Expand Down
28 changes: 28 additions & 0 deletions internal/forge/github/github_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2665,6 +2665,34 @@ func TestIsTransientStatus(t *testing.T) {
}
}

func TestAPIError_IsTransient(t *testing.T) {
t.Parallel()

tests := []struct {
name string
code int
want bool
}{
{name: "429 rate limit", code: 429, want: true},
{name: "500 internal server error", code: 500, want: true},
{name: "502 bad gateway", code: 502, want: true},
{name: "503 service unavailable", code: 503, want: true},
{name: "504 gateway timeout", code: 504, want: true},
{name: "200 OK", code: 200, want: false},
{name: "401 unauthorized", code: 401, want: false},
{name: "403 forbidden", code: 403, want: false},
{name: "404 not found", code: 404, want: false},
{name: "422 unprocessable entity", code: 422, want: false},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
t.Parallel()
err := &APIError{StatusCode: tt.code, Message: http.StatusText(tt.code)}
assert.Equal(t, tt.want, err.IsTransient())
})
}
}

func TestIsRetryable_PrimaryRateLimitAs403(t *testing.T) {
// GitHub sometimes returns primary rate limits as 403 with body
// containing "API rate limit exceeded" instead of 429. This must
Expand Down
9 changes: 9 additions & 0 deletions internal/forge/gitlab/gitlab.go
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,15 @@ func (e *APIError) Error() string {
return fmt.Sprintf("gitlab api: %d %s", e.StatusCode, e.Message)
}

// IsTransient reports whether the API error represents a transient
// failure that may succeed on retry: server errors (500–504) and
// rate limits (429). This method satisfies the transientReporter
// interface used by forge.IsTransient.
func (e *APIError) IsTransient() bool {
return e.StatusCode == http.StatusTooManyRequests ||
(e.StatusCode >= 500 && e.StatusCode <= 504)
}

func (e *APIError) Unwrap() error {
if e.StatusCode == http.StatusNotFound {
return forge.ErrNotFound
Expand Down
9 changes: 9 additions & 0 deletions internal/forge/jira/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,15 @@ func (e *APIError) Error() string {
return fmt.Sprintf("jira api: %d %s", e.StatusCode, e.Message)
}

// IsTransient reports whether the API error represents a transient
// failure that may succeed on retry: server errors (500–504) and
// rate limits (429). This method satisfies the transientReporter
// interface used by forge.IsTransient.
func (e *APIError) IsTransient() bool {
return e.StatusCode == http.StatusTooManyRequests ||
(e.StatusCode >= 500 && e.StatusCode <= 504)
}

func (e *APIError) Unwrap() error {
if e.StatusCode == http.StatusNotFound {
return forge.ErrNotFound
Expand Down
124 changes: 124 additions & 0 deletions internal/forge/transient_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,124 @@
package forge

import (
"errors"
"fmt"
"io"
"testing"

"github.com/stretchr/testify/assert"
)

// fakeTransientErr implements the transientReporter interface used by
// forge.IsTransient to let forge-specific API errors self-report
// transient-ness.
type fakeTransientErr struct {
transient bool
}

func (e *fakeTransientErr) Error() string { return "fake error" }
func (e *fakeTransientErr) IsTransient() bool {
return e.transient
}

// fakeTimeoutErr implements the Timeout() interface to simulate HTTP
// client timeout errors.
type fakeTimeoutErr struct {
timeout bool
}

func (e *fakeTimeoutErr) Error() string { return "timeout error" }
func (e *fakeTimeoutErr) Timeout() bool { return e.timeout }
func (e *fakeTimeoutErr) Temporary() bool { return e.timeout }

func TestIsTransient(t *testing.T) {
t.Parallel()

tests := []struct {
name string
err error
want bool
}{
{
name: "nil error",
err: nil,
want: false,
},
{
name: "ErrNonFastForward",
err: ErrNonFastForward,
want: true,
},
{
name: "wrapped ErrNonFastForward",
err: fmt.Errorf("commit failed: %w", ErrNonFastForward),
want: true,
},
{
name: "transient reporter true",
err: &fakeTransientErr{transient: true},
want: true,
},
{
name: "transient reporter false",
err: &fakeTransientErr{transient: false},
want: false,
},
{
name: "wrapped transient reporter",
err: fmt.Errorf("api call: %w", &fakeTransientErr{transient: true}),
want: true,
},
{
name: "timeout error",
err: &fakeTimeoutErr{timeout: true},
want: true,
},
{
name: "non-timeout error with Timeout method",
err: &fakeTimeoutErr{timeout: false},
want: false,
},
{
name: "io.EOF",
err: io.EOF,
want: true,
},
{
name: "wrapped io.EOF",
err: fmt.Errorf("read body: %w", io.EOF),
want: true,
},
{
name: "io.ErrUnexpectedEOF",
err: io.ErrUnexpectedEOF,
want: true,
},
{
name: "ErrNotFound is not transient",
err: ErrNotFound,
want: false,
},
{
name: "ErrForbidden is not transient",
err: ErrForbidden,
want: false,
},
{
name: "ErrBranchProtected is not transient",
err: ErrBranchProtected,
want: false,
},
{
name: "generic error is not transient",
err: errors.New("something broke"),
want: false,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
t.Parallel()
assert.Equal(t, tt.want, IsTransient(tt.err))
})
}
}
Loading
Loading