From 32f81958b2c356470a248d5aead940949fb7a5c6 Mon Sep 17 00:00:00 2001 From: Gondermann Date: Tue, 15 Sep 2026 15:40:00 +0200 Subject: [PATCH] Make network transport errors retryable errors.IsRetryable only checks the HTTP status codes to determine if a retry is appropriate. We should consider transport errors, such as timeouts and connection refused issues to also be retryable, as they might disappear without configuration changes. As per Go documentation [1], any net/http method (which gophercloud uses) should return url.Error when a transport error occurs. This provides a Timeout() and Temporary() method [2] which we can use to determine, if the operation can be reasonably retried. Additionally, we check for some syscall errors such as ECONNREFUSED and ECONNRESET, and some io errors explicitly and retry on them aswell. [1] https://pkg.go.dev/net/http#Client.Do [2] https://pkg.go.dev/net/url#Error On-behalf-of: SAP nils.gondermann@sap.com Fixes #879 --- internal/util/errors/errors.go | 44 +++++++++++++++++++++++++++++ internal/util/errors/errors_test.go | 41 +++++++++++++++++++++++++++ 2 files changed, 85 insertions(+) diff --git a/internal/util/errors/errors.go b/internal/util/errors/errors.go index 710dba16a..33399de54 100644 --- a/internal/util/errors/errors.go +++ b/internal/util/errors/errors.go @@ -20,7 +20,11 @@ import ( "bytes" "errors" "fmt" + "io" + "net" "net/http" + "net/url" + "syscall" "github.com/gophercloud/gophercloud/v2" ) @@ -64,6 +68,12 @@ func (e noMatchesError) Is(err error) bool { // returned as 409 but are retryable because quota can free up without spec // changes. func IsRetryable(err error) bool { + if IsTransportError(err) { + // We assume transport errors (such as "connection refused") are retryable, + // because they may vanish without spec changes. + return true + } + if IsConflict(err) { // Neutron returns 409 for quota-exceeded errors, but these are // retryable because quota can free up without spec changes. @@ -123,3 +133,37 @@ func IsConflict(err error) bool { func IsNotImplementedError(err error) bool { return gophercloud.ResponseCodeIs(err, http.StatusNotImplemented) } + +// IsTransportError returns true if err is a retryable transport error +func IsTransportError(err error) bool { + // As per documentation, Golang net/http methods which are used by gophercloud + // will always wrap transport errors in url.Error + var errURL *url.Error + if !errors.As(err, &errURL) { + return false + } + + if errURL.Timeout() || errURL.Temporary() { + // Every network error Golang considers temporary, we also accept as temporary + return true + } + + if errors.Is(err, syscall.ECONNREFUSED) || + errors.Is(err, syscall.ECONNRESET) || + errors.Is(err, syscall.ECONNABORTED) || + errors.Is(err, syscall.EHOSTUNREACH) || + errors.Is(err, syscall.ENETUNREACH) || + errors.Is(err, syscall.EPIPE) { + // These syscalls are not considered temporary by Golang, but we want to retry on these + return true + } + + if errors.Is(err, io.EOF) || + errors.Is(err, io.ErrUnexpectedEOF) || + errors.Is(err, net.ErrClosed) { + // Socket closed unexpectedly + return true + } + + return false +} diff --git a/internal/util/errors/errors_test.go b/internal/util/errors/errors_test.go index da38c5fca..964693b05 100644 --- a/internal/util/errors/errors_test.go +++ b/internal/util/errors/errors_test.go @@ -17,8 +17,13 @@ limitations under the License. package errors import ( + "context" "fmt" + "io" + "net" "net/http" + "net/url" + "syscall" "testing" "github.com/gophercloud/gophercloud/v2" @@ -31,6 +36,14 @@ func newHTTPError(statusCode int, body string) error { } } +func newURLError(wrappedError error) error { + return &url.Error{ + Op: "Op", + URL: "URL", + Err: wrappedError, + } +} + func TestIsRetryable(t *testing.T) { tests := []struct { name string @@ -97,6 +110,34 @@ func TestIsRetryable(t *testing.T) { err: fmt.Errorf("wrapping: %w", newHTTPError(http.StatusConflict, `{"NeutronError": {"type": "OverQuota"}}`)), want: true, }, + { + name: "generic url.Error is not retryable", + err: newURLError(fmt.Errorf("non retryable error")), + want: false, + }, + { + name: "wrapped context.DeadlineExceeded is retryable", + err: newURLError(context.DeadlineExceeded), + want: true, + }, + { + name: "wrapped ECONNRESET syscall error is retryable", + err: newURLError(&net.OpError{ + Op: "write", + Err: syscall.ECONNRESET, + }), + want: true, + }, + { + name: "ECONNREFUSED syscall error is retryable", + err: newURLError(syscall.ECONNREFUSED), + want: true, + }, + { + name: "io error is retryable", + err: newURLError(io.ErrUnexpectedEOF), + want: true, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) {