Skip to content

Commit ae9f3a3

Browse files
Replace string-based network error matching with typed checks
Use errors.Is(syscall.ECONNRESET), errors.Is(syscall.ECONNREFUSED), and net.Error.Timeout() instead of substring matching on error messages. String matching is brittle: error text is not covered by Go's compatibility promise and differs across platforms. Remove duplicate "tls handshake timeout" test case (identical to "i/o timeout") and update all network error tests to use real error chains (net.OpError -> os.SyscallError -> syscall.Errno). Co-authored-by: Isaac Signed-off-by: Ubuntu <renaud.hartert@databricks.com>
1 parent c4bcd0d commit ae9f3a3

File tree

2 files changed

+43
-29
lines changed

2 files changed

+43
-29
lines changed

config/experimental/auth/retry.go

Lines changed: 17 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,10 @@ package auth
33
import (
44
"context"
55
"errors"
6+
"net"
67
"net/http"
7-
"net/url"
88
"slices"
9-
"strings"
9+
"syscall"
1010
"time"
1111

1212
"github.com/databricks/databricks-sdk-go/experimental/api"
@@ -66,16 +66,7 @@ func isRetriableTokenError(err error) bool {
6666
if code := httpStatusCode(err); code != 0 {
6767
return slices.Contains(retriableCodes, code)
6868
}
69-
var urlErr *url.Error
70-
if errors.As(err, &urlErr) {
71-
msg := urlErr.Error()
72-
for _, s := range transientNetworkErrors {
73-
if strings.Contains(msg, s) {
74-
return true
75-
}
76-
}
77-
}
78-
return false
69+
return isTransientNetworkError(err)
7970
}
8071

8172
// httpStatusCode extracts the HTTP status code from an error, if available.
@@ -93,12 +84,18 @@ func httpStatusCode(err error) int {
9384
return 0
9485
}
9586

96-
// transientNetworkErrors is the list of error substrings that indicate a
97-
// transient network failure. These mirror the checks in
98-
// httpclient/errors.go isRetriableUrlError.
99-
var transientNetworkErrors = []string{
100-
"connection reset by peer",
101-
"TLS handshake timeout",
102-
"connection refused",
103-
"i/o timeout",
87+
// isTransientNetworkError reports whether err represents a transient network
88+
// condition that is likely to resolve on retry.
89+
func isTransientNetworkError(err error) bool {
90+
if errors.Is(err, syscall.ECONNRESET) {
91+
return true
92+
}
93+
if errors.Is(err, syscall.ECONNREFUSED) {
94+
return true
95+
}
96+
var netErr net.Error
97+
if errors.As(err, &netErr) && netErr.Timeout() {
98+
return true
99+
}
100+
return false
104101
}

config/experimental/auth/retry_test.go

Lines changed: 26 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -4,13 +4,23 @@ import (
44
"context"
55
"errors"
66
"fmt"
7+
"net"
78
"net/http"
89
"net/url"
10+
"os"
11+
"syscall"
912
"testing"
1013

1114
"golang.org/x/oauth2"
1215
)
1316

17+
// timeoutError is a test error that implements net.Error with Timeout() == true.
18+
type timeoutError struct{}
19+
20+
func (e timeoutError) Error() string { return "i/o timeout" }
21+
func (e timeoutError) Timeout() bool { return true }
22+
func (e timeoutError) Temporary() bool { return false }
23+
1424
// testHTTPError is a test error that implements the httpStatusCoder interface,
1525
// mirroring httpclient.HttpError without importing it.
1626
type testHTTPError struct {
@@ -69,7 +79,10 @@ func TestRetryingTokenSource(t *testing.T) {
6979
{
7080
name: "retry on transient network error",
7181
callErrors: []error{
72-
&url.Error{Op: "Post", URL: "https://host/token", Err: fmt.Errorf("connection reset by peer")},
82+
&url.Error{Op: "Post", URL: "https://host/token", Err: &net.OpError{
83+
Op: "read", Net: "tcp",
84+
Err: &os.SyscallError{Syscall: "read", Err: syscall.ECONNRESET},
85+
}},
7386
nil,
7487
},
7588
wantToken: token,
@@ -179,22 +192,26 @@ func TestIsRetriableTokenError(t *testing.T) {
179192
},
180193
{
181194
name: "connection reset",
182-
err: &url.Error{Op: "Post", URL: "https://host/token", Err: fmt.Errorf("connection reset by peer")},
183-
want: true,
184-
},
185-
{
186-
name: "tls handshake timeout",
187-
err: &url.Error{Op: "Post", URL: "https://host/token", Err: fmt.Errorf("TLS handshake timeout")},
195+
err: &url.Error{Op: "Post", URL: "https://host/token", Err: &net.OpError{
196+
Op: "read", Net: "tcp",
197+
Err: &os.SyscallError{Syscall: "read", Err: syscall.ECONNRESET},
198+
}},
188199
want: true,
189200
},
190201
{
191202
name: "connection refused",
192-
err: &url.Error{Op: "Post", URL: "https://host/token", Err: fmt.Errorf("connection refused")},
203+
err: &url.Error{Op: "Post", URL: "https://host/token", Err: &net.OpError{
204+
Op: "dial", Net: "tcp",
205+
Err: &os.SyscallError{Syscall: "connect", Err: syscall.ECONNREFUSED},
206+
}},
193207
want: true,
194208
},
195209
{
196210
name: "i/o timeout",
197-
err: &url.Error{Op: "Post", URL: "https://host/token", Err: fmt.Errorf("i/o timeout")},
211+
err: &url.Error{Op: "Post", URL: "https://host/token", Err: &net.OpError{
212+
Op: "read", Net: "tcp",
213+
Err: timeoutError{},
214+
}},
198215
want: true,
199216
},
200217
{

0 commit comments

Comments
 (0)