fix(p/LUADNS): bug: occasional "Server returned bad status code (500)" fails a push

providers/luadns/luadnsProvider.go

@@ -160,11 +160,10 @@ func (l *luadnsProvider) GetZoneRecordsCorrections(dc *models.DomainConfig, reco
 			providers.EndToNative(l.observer, "recordsToNative", before, change.New, req, nil)
 			corrections = append(corrections, &models.Correction{
 				F: func() error {
-					if err := l.rateLimiter.Wait(l.ctx); err != nil {
+					return l.call("CreateManyRecords", false, func() error {
+						_, err := l.provider.CreateManyRecords(l.ctx, zone, req)
 						return err
-					}
-					_, err := l.provider.CreateManyRecords(l.ctx, zone, req)
-					return err
+					})
 				},
 				Msg: change.MsgsJoined,
 			})
@@ -174,11 +173,10 @@ func (l *luadnsProvider) GetZoneRecordsCorrections(dc *models.DomainConfig, reco
 			providers.EndToNative(l.observer, "recordsToNative", before, change.New, req, nil)
 			corrections = append(corrections, &models.Correction{
 				F: func() error {
-					if err := l.rateLimiter.Wait(l.ctx); err != nil {
+					return l.call("UpdateManyRecords", true, func() error {
+						_, err := l.provider.UpdateManyRecords(l.ctx, zone, req)
 						return err
-					}
-					_, err := l.provider.UpdateManyRecords(l.ctx, zone, req)
-					return err
+					})
 				},
 				Msg: change.MsgsJoined,
 			})
@@ -188,11 +186,10 @@ func (l *luadnsProvider) GetZoneRecordsCorrections(dc *models.DomainConfig, reco
 			providers.EndToNative(l.observer, "recordsToNative", before, change.Old, req, nil)
 			corrections = append(corrections, &models.Correction{
 				F: func() error {
-					if err := l.rateLimiter.Wait(l.ctx); err != nil {
+					return l.call("DeleteManyRecords", true, func() error {
+						_, err := l.provider.DeleteManyRecords(l.ctx, zone, req)
 						return err
-					}
-					_, err := l.provider.DeleteManyRecords(l.ctx, zone, req)
-					return err
+					})
 				},
 				Msg: change.MsgsJoined,
 			})
@@ -211,10 +208,12 @@ func (l *luadnsProvider) EnsureZoneExists(dc *models.DomainConfig) error {
 			return err
 		}
 	}
-	if err := l.rateLimiter.Wait(l.ctx); err != nil {
+	var zone *api.Zone
+	err := l.call("CreateZone", false, func() error {
+		var err error
+		zone, err = l.provider.CreateZone(l.ctx, &api.Zone{Name: domain})
 		return err
-	}
-	zone, err := l.provider.CreateZone(l.ctx, &api.Zone{Name: domain})
+	})
 	if err != nil {
 		return err
 	}
@@ -223,10 +222,12 @@ func (l *luadnsProvider) EnsureZoneExists(dc *models.DomainConfig) error {
 }
 
 func (l *luadnsProvider) fetchDomainList() error {
-	if err := l.rateLimiter.Wait(l.ctx); err != nil {
+	var zones []*api.Zone
+	err := l.call("ListZones", true, func() error {
+		var err error
+		zones, err = l.provider.ListZones(l.ctx, &api.ListParams{})
 		return err
-	}
-	zones, err := l.provider.ListZones(l.ctx, &api.ListParams{})
+	})
 	if err != nil {
 		return err
 	}
@@ -249,10 +250,12 @@ func (l *luadnsProvider) getZone(name string) (*api.Zone, error) {
 }
 
 func (l *luadnsProvider) getRecords(zone *api.Zone) ([]*api.Record, error) {
-	if err := l.rateLimiter.Wait(l.ctx); err != nil {
-		return nil, err
-	}
-	records, err := l.provider.ListRecords(l.ctx, zone, &api.ListParams{})
+	var records []*api.Record
+	err := l.call("ListRecords", true, func() error {
+		var err error
+		records, err = l.provider.ListRecords(l.ctx, zone, &api.ListParams{})
+		return err
+	})
 	if err != nil {
 		return nil, err
 	}

providers/luadns/retry.go

@@ -0,0 +1,54 @@
+package luadns
+
+import (
+	"errors"
+	"net/http"
+	"time"
+
+	"github.com/DNSControl/dnscontrol/v5/pkg/printer"
+	api "github.com/luadns/luadns-go"
+)
+
+// The API occasionally answers a valid request with a 5xx. A request that
+// cannot create anything twice (anything but a POST) is repeated after a 500,
+// 502, 503 or 504, with a pause that doubles up to maxRetryPause. A 429 means
+// the request was not processed, so every request is repeated after one.
+const (
+	firstRetryPause = 2 * time.Second
+	maxRetryPause   = 30 * time.Second
+	maxRetries      = 6
+)
+
+// sleep is a variable so that tests can skip the pauses.
+var sleep = time.Sleep
+
+// call waits for the rate limiter and runs fn, repeating it as described above.
+// idempotent reports whether fn may be repeated after a 5xx.
+func (l *luadnsProvider) call(what string, idempotent bool, fn func() error) error {
+	pause := firstRetryPause
+	for attempt := 0; ; attempt++ {
+		if err := l.rateLimiter.Wait(l.ctx); err != nil {
+			return err
+		}
+		err := fn()
+		if err == nil || attempt >= maxRetries || !retryable(err, idempotent) {
+			return err
+		}
+		printer.Warnf("LUADNS: %s: %v, waiting %s\n", what, err, pause)
+		sleep(pause)
+		pause = min(2*pause, maxRetryPause)
+	}
+}
+
+func retryable(err error, idempotent bool) bool {
+	if _, ok := errors.AsType[*api.ErrTooManyRequests](err); ok {
+		return true
+	}
+	if e, ok := errors.AsType[*api.ErrBadStatusCode](err); ok && idempotent {
+		switch e.StatusCode {
+		case http.StatusInternalServerError, http.StatusBadGateway, http.StatusServiceUnavailable, http.StatusGatewayTimeout:
+			return true
+		}
+	}
+	return false
+}

providers/luadns/retry_test.go

@@ -0,0 +1,52 @@
+package luadns
+
+import (
+	"context"
+	"errors"
+	"testing"
+	"time"
+
+	api "github.com/luadns/luadns-go"
+	"golang.org/x/time/rate"
+)
+
+func TestCall(t *testing.T) {
+	sleep = func(time.Duration) {}
+	defer func() { sleep = time.Sleep }()
+
+	err500 := &api.ErrBadStatusCode{StatusCode: 500}
+	tests := []struct {
+		name       string
+		idempotent bool
+		errs       []error // returned by successive attempts; nil after the list ends
+		wantCalls  int
+		wantErr    bool
+	}{
+		{"success", true, nil, 1, false},
+		{"500 then success", true, []error{err500, err500}, 3, false},
+		{"500 on POST is final", false, []error{err500}, 1, true},
+		{"429 on POST is repeated", false, []error{&api.ErrTooManyRequests{}}, 2, false},
+		{"404 is final", true, []error{&api.ErrBadStatusCode{StatusCode: 404}}, 1, true},
+		{"other error is final", true, []error{errors.New("boom")}, 1, true},
+		{"gives up", true, []error{err500, err500, err500, err500, err500, err500, err500, err500}, maxRetries + 1, true},
+	}
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			l := &luadnsProvider{ctx: context.Background(), rateLimiter: rate.NewLimiter(rate.Inf, 1)}
+			calls := 0
+			err := l.call("test", tc.idempotent, func() error {
+				calls++
+				if calls <= len(tc.errs) {
+					return tc.errs[calls-1]
+				}
+				return nil
+			})
+			if calls != tc.wantCalls {
+				t.Errorf("calls = %d, want %d", calls, tc.wantCalls)
+			}
+			if (err != nil) != tc.wantErr {
+				t.Errorf("err = %v, wantErr %v", err, tc.wantErr)
+			}
+		})
+	}
+}