Skip to content

Commit

Permalink
fix DNS timeout error not retried (#2300)
Browse files Browse the repository at this point in the history
  • Loading branch information
a-canya committed Oct 10, 2023
1 parent e155bb7 commit 5de4674
Show file tree
Hide file tree
Showing 3 changed files with 65 additions and 3 deletions.
8 changes: 8 additions & 0 deletions .changelog/63caeb72930c4d84bbab8d36fe10d4b5.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
{
"id": "63caeb72-930c-4d84-bbab-8d36fe10d4b5",
"type": "bugfix",
"description": "Improve recognition of retryable DNS errors.",
"modules": [
"."
]
}
16 changes: 13 additions & 3 deletions aws/retry/retryable_error.go
Original file line number Diff line number Diff line change
Expand Up @@ -97,11 +97,21 @@ func (r RetryableConnectionError) IsErrorRetryable(err error) aws.Ternary {
var netOpErr *net.OpError
var dnsError *net.DNSError

switch {
case errors.As(err, &dnsError):
if errors.As(err, &dnsError) {
// NXDOMAIN errors should not be retried
retryable = !dnsError.IsNotFound && dnsError.IsTemporary
if dnsError.IsNotFound {
return aws.BoolTernary(false)
}

// if !dnsError.Temporary(), error may or may not be temporary,
// (i.e. !Temporary() =/=> !retryable) so we should fall through to
// remaining checks
if dnsError.Temporary() {
return aws.BoolTernary(true)
}
}

switch {
case errors.As(err, &conErr) && conErr.ConnectionError():
retryable = true

Expand Down
44 changes: 44 additions & 0 deletions aws/retry/retryable_error_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -262,3 +262,47 @@ func TestCanceledError(t *testing.T) {
})
}
}

func TestDNSError(t *testing.T) {
cases := map[string]struct {
Err error
Expect aws.Ternary
}{
"IsNotFound": {
Err: &net.DNSError{
IsNotFound: true,
},
Expect: aws.FalseTernary,
},
"Temporary (IsTimeout)": {
Err: &net.DNSError{
IsTimeout: true,
},
Expect: aws.TrueTernary,
},
"Temporary (IsTemporary)": {
Err: &net.DNSError{
IsTemporary: true,
},
Expect: aws.TrueTernary,
},
"Temporary() == false but it falls through": {
Err: &net.OpError{
Op: "dial",
Err: &net.DNSError{},
},
Expect: aws.TrueTernary,
},
}

for name, c := range cases {
t.Run(name, func(t *testing.T) {
var r RetryableConnectionError

retryable := r.IsErrorRetryable(c.Err)
if e, a := c.Expect, retryable; e != a {
t.Errorf("expect %v retryable, got %v", e, a)
}
})
}
}

0 comments on commit 5de4674

Please sign in to comment.