Skip to content

Commit c857e52

Browse files
sheurichsheurich
authored
Remove deprecated Temporary() usage in DNS retry logic (#8441)
Replace deprecated `Temporary()` with `Timeout()` in DNS retry logic. See golang/go#45729 for context.
1 parent ea43ed6 commit c857e52

File tree

2 files changed

+48
-49
lines changed

2 files changed

+48
-49
lines changed

‎bdns/dns.go‎

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@ import (
99
"net"
1010
"net/http"
1111
"net/netip"
12-
"net/url"
1312
"slices"
1413
"strconv"
1514
"strings"
@@ -260,10 +259,10 @@ func (dnsClient *impl) exchangeOne(ctx context.Context, hostname string, qtype u
260259
case r := <-ch:
261260
if r.err != nil {
262261
var isRetryable bool
263-
// According to the http package documentation, retryable
264-
// errors emitted by the http package are of type *url.Error.
265-
var urlErr *url.Error
266-
isRetryable = errors.As(r.err, &urlErr) && urlErr.Temporary()
262+
// Check if the error is a timeout error. Network errors
263+
// that can timeout implement the net.Error interface.
264+
var netErr net.Error
265+
isRetryable = errors.As(r.err, &netErr) && netErr.Timeout()
267266
hasRetriesLeft := tries < dnsClient.maxTries
268267
if isRetryable && hasRetriesLeft {
269268
tries++

‎bdns/dns_test.go‎

Lines changed: 44 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -545,9 +545,10 @@ func (te *testExchanger) Exchange(m *dns.Msg, a string) (*dns.Msg, time.Duration
545545
}
546546

547547
func TestRetry(t *testing.T) {
548-
isTempErr := &url.Error{Op: "read", Err: tempError(true)}
549-
nonTempErr := &url.Error{Op: "read", Err: tempError(false)}
548+
isTimeoutErr := &url.Error{Op: "read", Err: testTimeoutError(true)}
549+
nonTimeoutErr := &url.Error{Op: "read", Err: testTimeoutError(false)}
550550
servFailError := errors.New("DNS problem: server failure at resolver looking up TXT for example.com")
551+
timeoutFailError := errors.New("DNS problem: query timed out looking up TXT for example.com")
551552
type testCase struct {
552553
name string
553554
maxTries int
@@ -577,28 +578,28 @@ func TestRetry(t *testing.T) {
577578
expected: servFailError,
578579
expectedCount: 1,
579580
},
580-
// Temporary err, then non-OpError stops at two tries
581+
// Timeout err, then non-OpError stops at two tries
581582
{
582583
name: "err-then-non-operror",
583584
maxTries: 3,
584585
te: &testExchanger{
585-
errs: []error{isTempErr, errors.New("nope")},
586+
errs: []error{isTimeoutErr, errors.New("nope")},
586587
},
587588
expected: servFailError,
588589
expectedCount: 2,
589590
},
590-
// Temporary error given always
591+
// Timeout error given always
591592
{
592-
name: "persistent-temp-error",
593+
name: "persistent-timeout-error",
593594
maxTries: 3,
594595
te: &testExchanger{
595596
errs: []error{
596-
isTempErr,
597-
isTempErr,
598-
isTempErr,
597+
isTimeoutErr,
598+
isTimeoutErr,
599+
isTimeoutErr,
599600
},
600601
},
601-
expected: servFailError,
602+
expected: timeoutFailError,
602603
expectedCount: 3,
603604
metricsAllRetries: 1,
604605
},
@@ -613,56 +614,56 @@ func TestRetry(t *testing.T) {
613614
expected: nil,
614615
expectedCount: 1,
615616
},
616-
// Temporary error given just once causes two tries
617+
// Timeout error given just once causes two tries
617618
{
618-
name: "single-temp-error",
619+
name: "single-timeout-error",
619620
maxTries: 3,
620621
te: &testExchanger{
621622
errs: []error{
622-
isTempErr,
623+
isTimeoutErr,
623624
nil,
624625
},
625626
},
626627
expected: nil,
627628
expectedCount: 2,
628629
},
629-
// Temporary error given twice causes three tries
630+
// Timeout error given twice causes three tries
630631
{
631-
name: "double-temp-error",
632+
name: "double-timeout-error",
632633
maxTries: 3,
633634
te: &testExchanger{
634635
errs: []error{
635-
isTempErr,
636-
isTempErr,
636+
isTimeoutErr,
637+
isTimeoutErr,
637638
nil,
638639
},
639640
},
640641
expected: nil,
641642
expectedCount: 3,
642643
},
643-
// Temporary error given thrice causes three tries and fails
644+
// Timeout error given thrice causes three tries and fails
644645
{
645-
name: "triple-temp-error",
646+
name: "triple-timeout-error",
646647
maxTries: 3,
647648
te: &testExchanger{
648649
errs: []error{
649-
isTempErr,
650-
isTempErr,
651-
isTempErr,
650+
isTimeoutErr,
651+
isTimeoutErr,
652+
isTimeoutErr,
652653
},
653654
},
654-
expected: servFailError,
655+
expected: timeoutFailError,
655656
expectedCount: 3,
656657
metricsAllRetries: 1,
657658
},
658-
// temporary then non-Temporary error causes two retries
659+
// timeout then non-timeout error causes two retries
659660
{
660-
name: "temp-nontemp-error",
661+
name: "timeout-nontimeout-error",
661662
maxTries: 3,
662663
te: &testExchanger{
663664
errs: []error{
664-
isTempErr,
665-
nonTempErr,
665+
isTimeoutErr,
666+
nonTimeoutErr,
666667
},
667668
},
668669
expected: servFailError,
@@ -708,7 +709,7 @@ func TestRetry(t *testing.T) {
708709

709710
testClient := New(time.Second*10, staticProvider, metrics.NoopRegisterer, clock.NewFake(), 3, "", blog.UseMock(), tlsConfig)
710711
dr := testClient.(*impl)
711-
dr.dnsClient = &testExchanger{errs: []error{isTempErr, isTempErr, nil}}
712+
dr.dnsClient = &testExchanger{errs: []error{isTimeoutErr, isTimeoutErr, nil}}
712713
ctx, cancel := context.WithCancel(context.Background())
713714
cancel()
714715
_, _, err = dr.LookupTXT(ctx, "example.com")
@@ -717,7 +718,7 @@ func TestRetry(t *testing.T) {
717718
t.Errorf("expected %s, got %s", context.Canceled, err)
718719
}
719720

720-
dr.dnsClient = &testExchanger{errs: []error{isTempErr, isTempErr, nil}}
721+
dr.dnsClient = &testExchanger{errs: []error{isTimeoutErr, isTimeoutErr, nil}}
721722
ctx, cancel = context.WithTimeout(context.Background(), -10*time.Hour)
722723
defer cancel()
723724
_, _, err = dr.LookupTXT(ctx, "example.com")
@@ -726,7 +727,7 @@ func TestRetry(t *testing.T) {
726727
t.Errorf("expected %s, got %s", context.DeadlineExceeded, err)
727728
}
728729

729-
dr.dnsClient = &testExchanger{errs: []error{isTempErr, isTempErr, nil}}
730+
dr.dnsClient = &testExchanger{errs: []error{isTimeoutErr, isTimeoutErr, nil}}
730731
ctx, deadlineCancel := context.WithTimeout(context.Background(), -10*time.Hour)
731732
deadlineCancel()
732733
_, _, err = dr.LookupTXT(ctx, "example.com")
@@ -759,10 +760,10 @@ func TestIsTLD(t *testing.T) {
759760
}
760761
}
761762

762-
type tempError bool
763+
type testTimeoutError bool
763764

764-
func (t tempError) Temporary() bool { return bool(t) }
765-
func (t tempError) Error() string { return fmt.Sprintf("Temporary: %t", t) }
765+
func (t testTimeoutError) Timeout() bool { return bool(t) }
766+
func (t testTimeoutError) Error() string { return fmt.Sprintf("Timeout: %t", t) }
766767

767768
// rotateFailureExchanger is a dns.Exchange implementation that tracks a count
768769
// of the number of calls to `Exchange` for a given address in the `lookups`
@@ -775,7 +776,7 @@ type rotateFailureExchanger struct {
775776
}
776777

777778
// Exchange for rotateFailureExchanger tracks the `a` argument in `lookups` and
778-
// if present in `brokenAddresses`, returns a temporary error.
779+
// if present in `brokenAddresses`, returns a timeout error.
779780
func (e *rotateFailureExchanger) Exchange(m *dns.Msg, a string) (*dns.Msg, time.Duration, error) {
780781
e.Lock()
781782
defer e.Unlock()
@@ -785,8 +786,8 @@ func (e *rotateFailureExchanger) Exchange(m *dns.Msg, a string) (*dns.Msg, time.
785786

786787
// If its a broken server, return a retryable error
787788
if e.brokenAddresses[a] {
788-
isTempErr := &url.Error{Op: "read", Err: tempError(true)}
789-
return nil, 2 * time.Millisecond, isTempErr
789+
isTimeoutErr := &url.Error{Op: "read", Err: testTimeoutError(true)}
790+
return nil, 2 * time.Millisecond, isTimeoutErr
790791
}
791792

792793
return m, 2 * time.Millisecond, nil
@@ -848,11 +849,10 @@ func TestRotateServerOnErr(t *testing.T) {
848849

849850
}
850851

851-
type mockTempURLError struct{}
852+
type mockTimeoutURLError struct{}
852853

853-
func (m *mockTempURLError) Error() string { return "whoops, oh gosh" }
854-
func (m *mockTempURLError) Timeout() bool { return false }
855-
func (m *mockTempURLError) Temporary() bool { return true }
854+
func (m *mockTimeoutURLError) Error() string { return "whoops, oh gosh" }
855+
func (m *mockTimeoutURLError) Timeout() bool { return true }
856856

857857
type dohAlwaysRetryExchanger struct {
858858
sync.Mutex
@@ -863,13 +863,13 @@ func (dohE *dohAlwaysRetryExchanger) Exchange(m *dns.Msg, a string) (*dns.Msg, t
863863
dohE.Lock()
864864
defer dohE.Unlock()
865865

866-
tempURLerror := &url.Error{
866+
timeoutURLerror := &url.Error{
867867
Op: "GET",
868868
URL: "https://example.com",
869-
Err: &mockTempURLError{},
869+
Err: &mockTimeoutURLError{},
870870
}
871871

872-
return nil, time.Second, tempURLerror
872+
return nil, time.Second, timeoutURLerror
873873
}
874874

875875
func TestDOHMetric(t *testing.T) {
@@ -878,7 +878,7 @@ func TestDOHMetric(t *testing.T) {
878878

879879
testClient := New(time.Second*11, staticProvider, metrics.NoopRegisterer, clock.NewFake(), 0, "", blog.UseMock(), tlsConfig)
880880
resolver := testClient.(*impl)
881-
resolver.dnsClient = &dohAlwaysRetryExchanger{err: &url.Error{Op: "read", Err: tempError(true)}}
881+
resolver.dnsClient = &dohAlwaysRetryExchanger{err: &url.Error{Op: "read", Err: testTimeoutError(true)}}
882882

883883
// Starting out, we should count 0 "out of retries" errors.
884884
test.AssertMetricWithLabelsEquals(t, resolver.timeoutCounter, prometheus.Labels{"qtype": "None", "type": "out of retries", "resolver": "127.0.0.1", "isTLD": "false"}, 0)

0 commit comments

Comments
 (0)