From e54c76b70f6a42643f5ed131affef300ba69e349 Mon Sep 17 00:00:00 2001 From: Tom Limoncelli Date: Mon, 26 Oct 2026 08:22:19 -0700 Subject: [PATCH] fix(LUADNS): bug: occasional "Server returned bad status code (500)" fails a push The LuaDNS API occasionally answers a valid request with a 5xx. Repeat requests that cannot create anything twice (updates, deletes, reads) after a 500/502/503/504, with a doubling pause. Repeat every request after a 429. Creates (POST) are not repeated after a 5xx, since the first attempt may have succeeded. Co-Authored-By: Claude Opus 5.5 --- providers/luadns/luadnsProvider.go | 47 ++++++++++++++------------ providers/luadns/retry.go | 54 ++++++++++++++++++++++++++++++ providers/luadns/retry_test.go | 52 ++++++++++++++++++++++++++++ 3 files changed, 131 insertions(+), 22 deletions(-) create mode 100644 providers/luadns/retry.go create mode 100644 providers/luadns/retry_test.go diff --git a/providers/luadns/luadnsProvider.go b/providers/luadns/luadnsProvider.go index 3885078d8e..a6bc1f2082 100644 --- a/providers/luadns/luadnsProvider.go +++ b/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 } diff --git a/providers/luadns/retry.go b/providers/luadns/retry.go new file mode 100644 index 0000000000..4ac13066e1 --- /dev/null +++ b/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 +} diff --git a/providers/luadns/retry_test.go b/providers/luadns/retry_test.go new file mode 100644 index 0000000000..3028b253b9 --- /dev/null +++ b/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) + } + }) + } +}