diff --git a/internal/gensupport/retry.go b/internal/gensupport/retry.go index 20b57d925f1..089ee3189ba 100644 --- a/internal/gensupport/retry.go +++ b/internal/gensupport/retry.go @@ -8,6 +8,7 @@ import ( "errors" "io" "net" + "net/url" "strings" "time" @@ -29,8 +30,6 @@ var ( backoff = func() Backoff { return &gax.Backoff{Initial: 100 * time.Millisecond} } - // syscallRetryable is a platform-specific hook, specified in retryable_linux.go - syscallRetryable func(error) bool = func(err error) bool { return false } ) const ( @@ -56,30 +55,33 @@ func shouldRetry(status int, err error) bool { if status == statusTooManyRequests || status == statusRequestTimeout { return true } - if err == io.ErrUnexpectedEOF { + if errors.Is(err, io.ErrUnexpectedEOF) { return true } - // Transient network errors should be retried. - if syscallRetryable(err) { + if errors.Is(err, net.ErrClosed) { return true } - if err, ok := err.(interface{ Temporary() bool }); ok { - if err.Temporary() { - return true + switch e := err.(type) { + case *net.OpError, *url.Error: + // Retry socket-level errors ECONNREFUSED and ECONNRESET (from syscall). + // Unfortunately the error type is unexported, so we resort to string + // matching. + retriable := []string{"connection refused", "connection reset", "broken pipe"} + for _, s := range retriable { + if strings.Contains(e.Error(), s) { + return true + } } - } - var opErr *net.OpError - if errors.As(err, &opErr) { - if strings.Contains(opErr.Error(), "use of closed network connection") { - // TODO: check against net.ErrClosed (go 1.16+) instead of string + case interface{ Temporary() bool }: + if e.Temporary() { return true } } - // If Go 1.13 error unwrapping is available, use this to examine wrapped + // If error unwrapping is available, use this to examine wrapped // errors. - if err, ok := err.(interface{ Unwrap() error }); ok { - return shouldRetry(status, err.Unwrap()) + if e, ok := err.(interface{ Unwrap() error }); ok { + return shouldRetry(status, e.Unwrap()) } return false } diff --git a/internal/gensupport/retry_test.go b/internal/gensupport/retry_test.go new file mode 100644 index 00000000000..f17c4206a17 --- /dev/null +++ b/internal/gensupport/retry_test.go @@ -0,0 +1,97 @@ +// Copyright 2024 Google LLC. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package gensupport + +import ( + "errors" + "fmt" + "io" + "net" + "net/url" + "testing" +) + +func TestShouldRetry(t *testing.T) { + t.Parallel() + + for _, test := range []struct { + desc string + code int + inputErr error + shouldRetry bool + }{ + { + desc: "nil error", + inputErr: nil, + shouldRetry: false, + }, + { + desc: "429 error", + inputErr: fmt.Errorf("too many requests"), + code: 429, + shouldRetry: true, + }, + { + desc: "408 error", + inputErr: fmt.Errorf("request timed out"), + code: 408, + shouldRetry: true, + }, + { + desc: "unknown error", + inputErr: errors.New("foo"), + shouldRetry: false, + }, + { + desc: "503 error", + inputErr: fmt.Errorf("service unavailable"), + code: 503, + shouldRetry: true, + }, + { + desc: "403 error", + inputErr: fmt.Errorf("forbidden"), + code: 403, + shouldRetry: false, + }, + { + desc: "connection refused", + inputErr: &url.Error{Op: "blah", URL: "blah", Err: errors.New("connection refused")}, + shouldRetry: true, + }, + { + desc: "connection reset", + inputErr: &net.OpError{Op: "blah", Net: "tcp", Err: errors.New("connection reset by peer")}, + shouldRetry: true, + }, + { + desc: "io.ErrUnexpectedEOF", + inputErr: io.ErrUnexpectedEOF, + shouldRetry: true, + }, + { + desc: "wrapped retryable error", + inputErr: fmt.Errorf("Test unwrapping of a temporary error: %w", io.ErrUnexpectedEOF), + shouldRetry: true, + }, + { + desc: "wrapped non-retryable error", + inputErr: fmt.Errorf("Test unwrapping of a non-retriable error: %w", io.EOF), + shouldRetry: false, + }, + { + desc: "wrapped net.ErrClosed", + inputErr: &net.OpError{Err: net.ErrClosed}, + shouldRetry: true, + }, + } { + t.Run(test.desc, func(s *testing.T) { + got := shouldRetry(test.code, test.inputErr) + if got != test.shouldRetry { + s.Errorf("got %v, want %v", got, test.shouldRetry) + } + }) + } +} diff --git a/internal/gensupport/retryable_linux.go b/internal/gensupport/retryable_linux.go deleted file mode 100644 index a916c3da29b..00000000000 --- a/internal/gensupport/retryable_linux.go +++ /dev/null @@ -1,16 +0,0 @@ -// Copyright 2020 Google LLC. -// Use of this source code is governed by a BSD-style -// license that can be found in the LICENSE file. - -//go:build linux -// +build linux - -package gensupport - -import "syscall" - -func init() { - // Initialize syscallRetryable to return true on transient socket-level - // errors. These errors are specific to Linux. - syscallRetryable = func(err error) bool { return err == syscall.ECONNRESET || err == syscall.ECONNREFUSED } -}